Always set git commit #2676
Always set git commit #2676
Conversation
This will ensure that it runs even after customised checkout hooks
internal/job/checkout.go
Outdated
| @@ -217,6 +217,11 @@ func (e *Executor) CheckoutPhase(ctx context.Context) error { | |||
| } | |||
| } | |||
|
|
|||
| err = e.sendCommitToBuildkite(ctx) | |||
| if err != nil { | |||
| return err | |||
There was a problem hiding this comment.
Are you satisfied that this won't be a breaking change for anyone? I wonder if there are customers out there who have modified their checkout phase to such an extent that this won't resolve – in which case logging this as a warning rather than erroring out might be a good idea? I'm not sure - just want to make sure we cover the bases
internal/job/checkout.go
Outdated
| @@ -217,6 +217,11 @@ func (e *Executor) CheckoutPhase(ctx context.Context) error { | |||
| } | |||
| } | |||
|
|
|||
| err = e.sendCommitToBuildkite(ctx) | |||
| if err != nil { | |||
| e.shell.OptionalWarningf("commit-metadata", err.Error()) | |||
There was a problem hiding this comment.
Maybe don't use an error string in the format arg (e.g. imagine if err.Error() returns a string with %s in it)...
| e.shell.OptionalWarningf("commit-metadata", err.Error()) | |
| e.shell.OptionalWarningf("commit-metadata", "Couldn't %v", err) |
(Adjust format string or error strings as necessary.)
There was a problem hiding this comment.
oooh yeah good call. it felt yucky doing it, i'll fix it up
There was a problem hiding this comment.
ah shoot, i had this committed locally, but forgot to push it 🤦 i'll make a new PR.
Co-authored-by: David Barrell <david@barrell.me>
|
FYI there is a forum post about this functionality that has broken it. I believe its probably broken for the customer in a way that highlights the exact point this is trying to fix. But looping it in here in case other things come out as well https://forum.buildkite.community/t/agent-3-67-0-has-incorrect-pipeline-headings/3651 |
Description
As a part of the checkout process, the agent sets a special metadata key called
buildkite:git:committo a string containing information about the commit that the agent has checked out. This metadata is used by the buildkite backend to populate VCS-related information on the build, like the commit message, author and commit hash - highlighted below:(note that in some cases this information can come from webhooks when builds are triggered that way, but not all builds are triggered by webhooks)
this information is also used set the commit used for the rest of the build; often a build will get triggered at the HEAD of a specific branch, but the HEAD of a branch will change if extra commits are added over time, so the agent needs to know the "canonical" commit for the build, which is stored in this metadata value.
However, this metadata is only set during the default checkout phase - if a user overrides the checkout phase using agent hooks or plugins, this special metadata value will never get set, and it can lead to a build running across multiple commits by accident if the checkout hook is overridden.
This PR hoists the commit metadata setting process up a level in the checkout process, and it will now happen after any checkout hook, not just the default one.
Context
Coda link
Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go fmt ./...)