git checkout of non-branch refs doesn't work

[torarnv]

It looks like buildkite has problems with non-branch refs: https://buildkite.com/qt/qtbase/builds/6#48adf902-41c7-42fb-87e9-6a073b237b5c/1

$ git fetch -v origin refs/changes/54/249154/1
--
  | POST git-upload-pack (919 bytes)
  | remote: Counting objects: 26, done
  | remote: Finding sources: 100% (14/14)
  | remote: Total 14 (delta 6), reused 7 (delta 6)
  | Unpacking objects: 100% (14/14), done.
  | From https://codereview.qt-project.org/qt/qtbase
  | * branch                  refs/changes/54/249154/1 -> FETCH_HEAD
  | $ git checkout -f refs/changes/54/249154/1
  | error: pathspec 'refs/changes/54/249154/1' did not match any file(s) known to git.

For a normal branch-ref, buildkite seems to do the right thing:

https://buildkite.com/qt/qtbase/builds/4#a2df90d6-fea2-45b0-a1d7-4b7dc16f4a95/1-18

$ git fetch -v --prune origin 5.12
  | From https://codereview.qt-project.org/qt/qtbase
  | * branch                  5.12       -> FETCH_HEAD
  | = [up to date]            5.12       -> origin/5.12
  | $ git checkout -f FETCH_HEAD

[torarnv]

refs/heads/5.12 seems to follow a similar logic of checking out the ref, instead of FETCH_HEAD, but for branch refs, it works since it's a remote branch:

https://buildkite.com/qt/qtbase/builds/7#21a46902-33af-43de-b784-9b1d7ec41f1e/1-18

[lox]

Hi @torarnv 👋 Sorry about that, I believe you might have to use an pre-checkout hook to include refs/changes in $BUILDKITE_REFSPEC. See https://buildkite.com/docs/pipelines/environment-variables for some more details.

Our of interest, what are you using ref/changes for?

[lox]

Feel free to drop us an email at support@buildkite.com if we can assist further, happy to help! I'm going to close this one for now.

[torarnv]

Thanks Lachlan!

I’ll try the environment variable.

refs/changes is the namespace used by Gerrit for its “pull requests”

On 10 Jan 2019, at 11:23, Lachlan Donald notifications@github.com wrote:

Feel free to drop us an email at support@buildkite.com if we can assist further, happy to help! I'm going to close this one for now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[lox]

For reference, the checkout logic that the agent uses is at https://github.com/buildkite/agent/blob/3f8f6022ab76da5a3503161868435aa42a1889c4/bootstrap/bootstrap.go#L933-L993.

We'd certainly consider making some changes to make Gerrit integration easier!

[torarnv]

Ah, great, I’ll have a look!

It would be great if the pull-request-number and other env variables would be filled out based on the gerrit change, but perhaps that’s more easily done using custom hooks and env vars? Where is the existing logic for that part?

[lox]

Typically that happens via a webhook from the source control provider to us. How are you triggering builds currently?

[lox]

Something like this looks like it might work: https://github.com/jtolds/gerrit-webhooks

[torarnv]

So far I've just been testing by triggering manual builds :)

[lox]

Perhaps we could adapt https://github.com/jtolds/gerrit-webhooks to create builds via our API? Happy to assist.

[lox]

That way you could set the refspec and also set the correct pull request envs.

[lox]

This is the API endpoint you would use: https://buildkite.com/docs/apis/rest-api/builds#create-a-build

[torarnv]

Ah, nice, yeah that would be great once I get the manually triggered builds working!

[lox]

You will end up with something like this:

BUILDKITE_REFSPEC = 'refs/changes/47/11/2'
BUILDKITE_COMMIT = 'c72380cbb771fb97dd11404354834201a4d17a56'

[torarnv]

All right, so the "problem" here is that when I triggered a new build manually, I interpreted the "Commit" field as supporting extended SHA1-1 syntax, based on it defaulting to HEAD, and that it would subsequently be passed through git-rev-parse to resolve the actually commit SHA1.

But it seems the only magic "commit" supported is HEAD https://github.com/buildkite/agent/blob/3f8f6022ab76da5a3503161868435aa42a1889c4/bootstrap/bootstrap.go#L966

And any other commit ends up in the code path that expects a raw SHA1, so it of course fails to check out directly:

https://github.com/buildkite/agent/blob/3f8f6022ab76da5a3503161868435aa42a1889c4/bootstrap/bootstrap.go#L979

Some other non-sha1 "commits" work, but this is incidental and the code doesn't account for that and expects a raw sha1.

One workaround for this situation is as you said to pass both BUILDKITE_REFSPEC and BUILDKITE_COMMIT explicitly from the outside. Ideally I would only need the first, and the agent would resolve the commit using rev-parse.

But even better would be for the agent to treat the commit as a extended SHA1-1 syntax, giving a similar experience to git's own tools, that e.g. support HEAD~4, refs/remotes/foo/bar, master@{yesterday}and all the other goodies. Is this something you've considered?

[torarnv]

I guess one way to support this is to ensure that all the git commands and arguments used in the checkout logic support the extended SHA1 syntax. That way it would magically work by propagating the logic to git itself