Merge pull request #2465 from buildkite/pdp-1878-fix-checkout-of-shor…

…t-commit-hashes

internal/job/checkout.go

@@ -532,6 +532,8 @@ func (e *Executor) defaultCheckoutPhase(ctx context.Context) error {
 			switch gerr.Type {
 			case gitErrorFetchRetryClean, gitErrorFetchBadObject:
 				return fmt.Errorf("fetching commit %q: %w", e.Commit, err)
+			case gitErrorFetchBadReference:
+				// fallback to fetching all heads and tags
 			}
 		}
 

internal/job/git.go

@@ -25,6 +25,7 @@ const (
 	gitErrorFetch
 	gitErrorFetchRetryClean
 	gitErrorFetchBadObject
+	gitErrorFetchBadReference
 	gitErrorClean
 	gitErrorCleanSubmodules
 )
@@ -158,7 +159,13 @@ func gitFetch(
 	}
 
 	const badObject = "fatal: bad object"
-	if o, err := sh.RunWithOlfactor(ctx, []string{badObject}, "git", commandArgs...); err != nil {
+	const badReference = "fatal: couldn't find remote ref"
+	if o, err := sh.RunWithOlfactor(
+		ctx,
+		[]string{badObject, badReference},
+		"git",
+		commandArgs...,
+	); err != nil {
 		// "fatal: bad object" can happen when the local repo in the checkout
 		// directory is corrupted, not just the remote or the mirror.
 		// When using git mirrors, the existing checkout directory might have a
@@ -169,6 +176,11 @@ func gitFetch(
 			return &gitError{error: err, Type: gitErrorFetchBadObject}
 		}
 
+		// "fatal: couldn't find remote ref" can happen when the just the short commit hash is given.
+		if o.Smelt(badReference) {
+			return &gitError{error: err, Type: gitErrorFetchBadReference}
+		}
+
 		// 128 is extremely broad, but it seems permissions errors, network unreachable errors etc,
 		// don't result in it
 		if exitErr := new(exec.ExitError); errors.As(err, &exitErr) && exitErr.ExitCode() == 128 {

internal/job/integration/checkout_integration_test.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/buildkite/agent/v3/internal/experiments"
 	"github.com/buildkite/bintest/v3"
+	"gotest.tools/v3/assert"
 )
 
 // Example commit info:
@@ -274,6 +275,61 @@ func TestCheckingOutShallowCloneOfLocalGitProject(t *testing.T) {
 	tester.RunAndCheck(t, env...)
 }
 
+func TestCheckingOutLocalGitProjectWithShortCommitHash(t *testing.T) {
+	t.Parallel()
+
+	tester, err := NewBootstrapTester(mainCtx)
+	assert.NilError(t, err)
+	defer tester.Close()
+
+	// Do one checkout
+	tester.RunAndCheck(t)
+
+	// Create another commit on the same branch in the remote repo
+	err = tester.Repo.ExecuteAll([][]string{
+		{"commit", "--allow-empty", "-m", "Another commit"},
+	})
+	assert.NilError(t, err)
+
+	commitHash, err := tester.Repo.RevParse("HEAD")
+	assert.NilError(t, err)
+	shortCommitHash := commitHash[:7]
+
+	git := tester.
+		MustMock(t, "git").
+		PassthroughToLocalCommand()
+
+	// Git should attempt to fetch the shortHash, but fail. Then fallback to fetching
+	// all the heads and tags and checking out the short hash.
+	git.ExpectAll([][]any{
+		{"remote", "get-url", "origin"},
+		{"clean", "-ffxdq"},
+		{"fetch", "--", "origin", shortCommitHash},
+		{"config", "remote.origin.fetch"},
+		{"fetch", "--", "origin", "+refs/heads/*:refs/remotes/origin/*", "+refs/tags/*:refs/tags/*"},
+		{"checkout", "-f", shortCommitHash},
+		{"clean", "-ffxdq"},
+		{"--no-pager", "log", "-1", "HEAD", "-s", "--no-color", gitShowFormatArg},
+	})
+
+	// Mock out the meta-data calls to the agent after checkout
+	agent := tester.MockAgent(t)
+	agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
+	agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)
+
+	env := []string{
+		fmt.Sprintf("BUILDKITE_COMMIT=%s", shortCommitHash),
+	}
+	tester.RunAndCheck(t, env...)
+
+	// Check state of the checkout directory
+	checkoutRepo := &gitRepository{Path: tester.CheckoutDir()}
+	checkoutRepoCommit, err := checkoutRepo.RevParse("HEAD")
+	assert.NilError(t, err)
+
+	assert.Equal(t, checkoutRepoCommit, commitHash)
+}
+
 func TestCheckoutErrorIsRetried(t *testing.T) {
 	t.Parallel()
 

internal/job/integration/git.go

@@ -9,6 +9,10 @@ import (
 	"strings"
 )
 
+type gitRepository struct {
+	Path string
+}
+
 func createTestGitRespository() (*gitRepository, error) {
 	repo, err := newGitRepository()
 	if err != nil {
@@ -66,10 +70,6 @@ func createTestGitRespository() (*gitRepository, error) {
 	return repo, nil
 }
 
-type gitRepository struct {
-	Path string
-}
-
 func newGitRepository() (*gitRepository, error) {
 	tempDirRaw, err := os.MkdirTemp("", "git-repo")
 	if err != nil {