Adds a process entry for all of the binaries #142
Conversation
- Every binary that is produced as part of the build process is now added to the process list with their type being thier base binary name.
go_build_process.go
Outdated
| @@ -94,22 +93,22 @@ func (p GoBuildProcess) Execute(config GoBuildConfiguration) (string, error) { | |||
| p.logs.Action("Failed after %s", duration.Round(time.Millisecond)) | |||
| p.logs.Detail(buffer.String()) | |||
|
|
|||
| return "", fmt.Errorf("failed to execute 'go build': %w", err) | |||
| return nil, fmt.Errorf("failed to execute 'go build': %w", err) | |||
| } | |||
|
|
|||
| p.logs.Action("Completed in %s", duration.Round(time.Millisecond)) | |||
| p.logs.Break() | |||
|
|
|||
| paths, err := filepath.Glob(fmt.Sprintf("%s/*", config.Output)) | |||
There was a problem hiding this comment.
Now that the binaries are exposed as launch processes I think we need to provide the ability for buildpack consumers to control which one is the default process. I think the default process needs to be the first target listed in the BP_GO_TARGETS rather than just the first binary in the glob search. This decision is likely a breaking change since the buildpack today will select the first alphabetical globbed binary.
There was a problem hiding this comment.
If I have
├── cmd
│ ├── one
│ │ └── main.go
│ └── two
│ └── main.go
├── go.mod
├── main.go
Then
BP_GO_TARGETS="./cmd/two:.:./cmd/one"
BP_GO_TARGETS=".:./cmd/one:./cmd/two"
BP_GO_TARGETS="./cmd/one:./cmd/two:."
should all provide a different default launch process
There was a problem hiding this comment.
Hmm I see what you mean. I will try and fix this up.
There was a problem hiding this comment.
@wburningham Alright after some fettling, I have managed to get all of the paths in order of appearance meaning the first target's binary should be the default process type. Take a look and see if that works for you.
| "command": command, | ||
| } | ||
|
|
||
| command, ok := targetsLayer.Metadata["command"].(string) |
There was a problem hiding this comment.
@ForestEckhardt do you know why this code existed before or when you could ever get into the failure case where command would not be defined as a string?
There was a problem hiding this comment.
We use to reuse the binary layer from cache if nothing changed workspace directory. We no longer do that and always rebuild from source. This was metadata that we used to rewrite the start command on a rebuild where we reused the binary layer. That functionality has been removed so this is just dead code.
- This is so that we can obtain the binaries names in the order of the targets that were passed to the build command. This is to ensure that we set the correct default start command that is for the binary produced from the first target.
| return nil, fmt.Errorf("failed to parse 'go list' output: %w", err) | ||
| } | ||
|
|
||
| paths = append(paths, filepath.Join(config.Output, filepath.Base(list.ImportPath))) |
There was a problem hiding this comment.
Small optional microoptimization: Since the length of paths will be the length of config.Target, you can initialize path with a length of len(config.Targets and use path[i] = filepath.Join(config.Output, filepath.Base(list.ImportPath)) rather than append (which might have to grow).
|
@ForestEckhardt LGTM 👍 |
added to the process list with their type being their base binary name.
Resolves #139
Checklist