cmd/go: support sha1 repos when git default is sha256

When git is recent enough (beyond 2.29), always set the --object-format
flag.

This fixes repo cloning when users have set the git configuration
init.defaultObjectFormat to sha256.

Git is planning[1] to switch the default hash function to sha256 with
the 3.0 release sometime in late 2026. (that may slip, but it's still
worth being ahead of the curve)

This change moves the version-check function from cl/698835 into
codehost/git.go so we can use it to condition setting
--object-format=sha1.

Adjust the regexp parsing git version output to handle more cases.

[1]: https://lore.kernel.org/lkml/xmqqikikk1hr.fsf@gitster.g/T/#u

Updates #68359
Change-Id: I7d59eb4e116b8afb47d3d1ca068d75eb5047d5c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/720500
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>

diff --git a/src/cmd/go/internal/modfetch/codehost/git.go b/src/cmd/go/internal/modfetch/codehost/git.go
index b615fc0f13bd8a0627a17fa931fcd003b8c16aae..3d50dca3c442558a8a22a9b542aad94b2b909d25 100644 (file)
--- a/src/cmd/go/internal/modfetch/codehost/git.go
+++ b/src/cmd/go/internal/modfetch/codehost/git.go
@@ -17,6 +17,7 @@ import (
        "os"
        "os/exec"
        "path/filepath"
+       "regexp"
        "runtime"
        "slices"
        "sort"
@@ -91,9 +92,21 @@ func newGitRepo(ctx context.Context, remote string, local bool) (Repo, error) {
                                break
                        }
                }
+               gitSupportsSHA256, gitVersErr := gitSupportsSHA256()
+               if gitVersErr != nil {
+                       return nil, fmt.Errorf("unable to resolve git version: %w", gitVersErr)
+               }
                objFormatFlag := []string{}
+               // If git is sufficiently recent to support sha256,
+               // always initialize with an explicit object-format.
                if repoSha256Hash {
+                       // We always set --object-format=sha256 if the repo
+                       // we're cloning uses sha256 hashes because if the git
+                       // version is too old, it'll fail either way, so we
+                       // might as well give it one last chance.
                        objFormatFlag = []string{"--object-format=sha256"}
+               } else if gitSupportsSHA256 {
+                       objFormatFlag = []string{"--object-format=sha1"}
                }
                if _, err := Run(ctx, r.dir, "git", "init", "--bare", objFormatFlag); err != nil {
                        os.RemoveAll(r.dir)
@@ -389,7 +402,7 @@ func (r *gitRepo) Latest(ctx context.Context) (*RevInfo, error) {
 
 func (r *gitRepo) checkConfigSHA256(ctx context.Context) bool {
        if hashType, sha256CfgErr := r.runGit(ctx, "git", "config", "extensions.objectformat"); sha256CfgErr == nil {
-               return "sha256" == strings.TrimSpace(string(hashType))
+               return strings.TrimSpace(string(hashType)) == "sha256"
        }
        return false
 }
@@ -972,3 +985,36 @@ func (r *gitRepo) runGit(ctx context.Context, cmdline ...any) ([]byte, error) {
        }
        return RunWithArgs(ctx, args)
 }
+
+// Capture the major, minor and (optionally) patch version, but ignore anything later
+var gitVersLineExtract = regexp.MustCompile(`git version\s+(\d+\.\d+(?:\.\d+)?)`)
+
+func gitVersion() (string, error) {
+       gitOut, runErr := exec.Command("git", "version").CombinedOutput()
+       if runErr != nil {
+               return "v0", fmt.Errorf("failed to execute git version: %w", runErr)
+       }
+       return extractGitVersion(gitOut)
+}
+
+func extractGitVersion(gitOut []byte) (string, error) {
+       matches := gitVersLineExtract.FindSubmatch(gitOut)
+       if len(matches) < 2 {
+               return "v0", fmt.Errorf("git version extraction regexp did not match version line: %q", gitOut)
+       }
+       return "v" + string(matches[1]), nil
+}
+
+func hasAtLeastGitVersion(minVers string) (bool, error) {
+       gitVers, gitVersErr := gitVersion()
+       if gitVersErr != nil {
+               return false, gitVersErr
+       }
+       return semver.Compare(minVers, gitVers) <= 0, nil
+}
+
+const minGitSHA256Vers = "v2.29"
+
+func gitSupportsSHA256() (bool, error) {
+       return hasAtLeastGitVersion(minGitSHA256Vers)
+}

diff --git a/src/cmd/go/internal/modfetch/codehost/git_test.go b/src/cmd/go/internal/modfetch/codehost/git_test.go
index e032a14e124e9a2c9df7af6c4a6d53dc3880f3ab..1c950686bf057909a94ee68c9d928ab782239199 100644 (file)
--- a/src/cmd/go/internal/modfetch/codehost/git_test.go
+++ b/src/cmd/go/internal/modfetch/codehost/git_test.go
@@ -16,11 +16,9 @@ import (
        "io/fs"
        "log"
        "os"
-       "os/exec"
        "path"
        "path/filepath"
        "reflect"
-       "regexp"
        "runtime"
        "strings"
        "sync"
@@ -196,42 +194,56 @@ func testRepo(ctx context.Context, t *testing.T, remote string) (Repo, error) {
        return NewRepo(ctx, vcsName, remote, false)
 }
 
-var gitVersLineExtract = regexp.MustCompile(`git version\s+([\d.]+)`)
-
-func gitVersion(t testing.TB) string {
-       gitOut, runErr := exec.Command("git", "version").CombinedOutput()
-       if runErr != nil {
-               t.Logf("failed to execute git version: %s", runErr)
-               return "v0"
-       }
-       matches := gitVersLineExtract.FindSubmatch(gitOut)
-       if len(matches) < 2 {
-               t.Logf("git version extraction regexp did not match version line: %q", gitOut)
-               return "v0"
+func TestExtractGitVersion(t *testing.T) {
+       t.Parallel()
+       for _, tbl := range []struct {
+               in, exp string
+       }{
+               {in: "git version 2.52.0.rc2", exp: "v2.52.0"},
+               {in: "git version 2.52.0.38.g5e6e4854e0", exp: "v2.52.0"},
+               {in: "git version 2.51.2", exp: "v2.51.2"},
+               {in: "git version 1.5.0.5.GIT", exp: "v1.5.0"},
+               {in: "git version 1.5.1-rc3.GIT", exp: "v1.5.1"},
+               {in: "git version 1.5.2.GIT", exp: "v1.5.2"},
+               {in: "git version 2.43.0.rc2.23.gc3cc3e1da7", exp: "v2.43.0"},
+       } {
+               t.Run(tbl.exp, func(t *testing.T) {
+                       out, extrErr := extractGitVersion([]byte(tbl.in))
+                       if extrErr != nil {
+                               t.Errorf("failed to extract git version from %q: %s", tbl.in, extrErr)
+                       }
+                       if out != tbl.exp {
+                               t.Errorf("unexpected git version extractGitVersion(%q) = %q; want %q", tbl.in, out, tbl.exp)
+                       }
+               })
        }
-       return "v" + string(matches[1])
 }
 
-const minGitSHA256Vers = "v2.29"
-
 func TestTags(t *testing.T) {
-       t.Parallel()
 
-       gitVers := gitVersion(t)
+       gitVers, gitVersErr := gitVersion()
+       if gitVersErr != nil {
+               t.Logf("git version check failed: %s", gitVersErr)
+       }
 
        type tagsTest struct {
                repo   string
                prefix string
                tags   []Tag
+               // Override the git default hash for a few cases to make sure
+               // we handle all 3 reasonable states.
+               defGitHash string
        }
 
        runTest := func(tt tagsTest) func(*testing.T) {
                return func(t *testing.T) {
-                       t.Parallel()
                        if tt.repo == gitsha256repo && semver.Compare(gitVers, minGitSHA256Vers) < 0 {
                                t.Skipf("git version is too old (%+v); skipping git sha256 test", gitVers)
                        }
                        ctx := testContext(t)
+                       if tt.defGitHash != "" {
+                               t.Setenv("GIT_DEFAULT_HASH", tt.defGitHash)
+                       }
 
                        r, err := testRepo(ctx, t, tt.repo)
                        if err != nil {
@@ -248,27 +260,37 @@ func TestTags(t *testing.T) {
        }
 
        for _, tt := range []tagsTest{
-               {gitrepo1, "xxx", []Tag{}},
+               {gitrepo1, "xxx", []Tag{}, ""},
+               {gitrepo1, "xxx", []Tag{}, "sha256"},
+               {gitrepo1, "xxx", []Tag{}, "sha1"},
                {gitrepo1, "", []Tag{
                        {"v1.2.3", "ede458df7cd0fdca520df19a33158086a8a68e81"},
                        {"v1.2.4-annotated", "ede458df7cd0fdca520df19a33158086a8a68e81"},
                        {"v2.0.1", "76a00fb249b7f93091bc2c89a789dab1fc1bc26f"},
                        {"v2.0.2", "9d02800338b8a55be062c838d1f02e0c5780b9eb"},
                        {"v2.3", "76a00fb249b7f93091bc2c89a789dab1fc1bc26f"},
-               }},
+               }, ""},
                {gitrepo1, "v", []Tag{
                        {"v1.2.3", "ede458df7cd0fdca520df19a33158086a8a68e81"},
                        {"v1.2.4-annotated", "ede458df7cd0fdca520df19a33158086a8a68e81"},
                        {"v2.0.1", "76a00fb249b7f93091bc2c89a789dab1fc1bc26f"},
                        {"v2.0.2", "9d02800338b8a55be062c838d1f02e0c5780b9eb"},
                        {"v2.3", "76a00fb249b7f93091bc2c89a789dab1fc1bc26f"},
-               }},
+               }, ""},
                {gitrepo1, "v1", []Tag{
                        {"v1.2.3", "ede458df7cd0fdca520df19a33158086a8a68e81"},
                        {"v1.2.4-annotated", "ede458df7cd0fdca520df19a33158086a8a68e81"},
-               }},
-               {gitrepo1, "2", []Tag{}},
-               {gitsha256repo, "xxx", []Tag{}},
+               }, ""},
+               {gitrepo1, "v1", []Tag{
+                       {"v1.2.3", "ede458df7cd0fdca520df19a33158086a8a68e81"},
+                       {"v1.2.4-annotated", "ede458df7cd0fdca520df19a33158086a8a68e81"},
+               }, "sha256"},
+               {gitrepo1, "v1", []Tag{
+                       {"v1.2.3", "ede458df7cd0fdca520df19a33158086a8a68e81"},
+                       {"v1.2.4-annotated", "ede458df7cd0fdca520df19a33158086a8a68e81"},
+               }, "sha1"},
+               {gitrepo1, "2", []Tag{}, ""},
+               {gitsha256repo, "xxx", []Tag{}, ""},
                {gitsha256repo, "", []Tag{
                        {"v1.2.3", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
                        {"v1.2.4-annotated", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
@@ -276,7 +298,7 @@ func TestTags(t *testing.T) {
                        {"v2.0.1", "b7550fd9d2129c724c39ae0536e8b2fae4364d8c82bb8b0880c9b71f67295d09"},
                        {"v2.0.2", "1401e4e1fdb4169b51d44a1ff62af63ccc708bf5c12d15051268b51bbb6cbd82"},
                        {"v2.3", "b7550fd9d2129c724c39ae0536e8b2fae4364d8c82bb8b0880c9b71f67295d09"},
-               }},
+               }, ""},
                {gitsha256repo, "v", []Tag{
                        {"v1.2.3", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
                        {"v1.2.4-annotated", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
@@ -284,13 +306,23 @@ func TestTags(t *testing.T) {
                        {"v2.0.1", "b7550fd9d2129c724c39ae0536e8b2fae4364d8c82bb8b0880c9b71f67295d09"},
                        {"v2.0.2", "1401e4e1fdb4169b51d44a1ff62af63ccc708bf5c12d15051268b51bbb6cbd82"},
                        {"v2.3", "b7550fd9d2129c724c39ae0536e8b2fae4364d8c82bb8b0880c9b71f67295d09"},
-               }},
+               }, ""},
+               {gitsha256repo, "v1", []Tag{
+                       {"v1.2.3", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
+                       {"v1.2.4-annotated", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
+                       {"v1.3.0", "a9157cad2aa6dc2f78aa31fced5887f04e758afa8703f04d0178702ebf04ee17"},
+               }, ""},
+               {gitsha256repo, "v1", []Tag{
+                       {"v1.2.3", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
+                       {"v1.2.4-annotated", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
+                       {"v1.3.0", "a9157cad2aa6dc2f78aa31fced5887f04e758afa8703f04d0178702ebf04ee17"},
+               }, "sha1"},
                {gitsha256repo, "v1", []Tag{
                        {"v1.2.3", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
                        {"v1.2.4-annotated", "47b8b51b2a2d9d5caa3d460096c4e01f05700ce3a9390deb54400bd508214c5c"},
                        {"v1.3.0", "a9157cad2aa6dc2f78aa31fced5887f04e758afa8703f04d0178702ebf04ee17"},
-               }},
-               {gitsha256repo, "2", []Tag{}},
+               }, "sha256"},
+               {gitsha256repo, "2", []Tag{}, ""},
        } {
                t.Run(path.Base(tt.repo)+"/"+tt.prefix, runTest(tt))
                if tt.repo == gitrepo1 {
@@ -315,7 +347,10 @@ func TestTags(t *testing.T) {
 func TestLatest(t *testing.T) {
        t.Parallel()
 
-       gitVers := gitVersion(t)
+       gitVers, gitVersErr := gitVersion()
+       if gitVersErr != nil {
+               t.Logf("git version check failed: %s", gitVersErr)
+       }
 
        type latestTest struct {
                repo string
@@ -409,7 +444,10 @@ func TestLatest(t *testing.T) {
 func TestReadFile(t *testing.T) {
        t.Parallel()
 
-       gitVers := gitVersion(t)
+       gitVers, gitVersErr := gitVersion()
+       if gitVersErr != nil {
+               t.Logf("git version check failed: %s", gitVersErr)
+       }
 
        type readFileTest struct {
                repo string
@@ -508,7 +546,10 @@ type zipFile struct {
 func TestReadZip(t *testing.T) {
        t.Parallel()
 
-       gitVers := gitVersion(t)
+       gitVers, gitVersErr := gitVersion()
+       if gitVersErr != nil {
+               t.Logf("git version check failed: %s", gitVersErr)
+       }
 
        type readZipTest struct {
                repo   string
@@ -798,7 +839,10 @@ var hgmap = map[string]string{
 func TestStat(t *testing.T) {
        t.Parallel()
 
-       gitVers := gitVersion(t)
+       gitVers, gitVersErr := gitVersion()
+       if gitVersErr != nil {
+               t.Logf("git version check failed: %s", gitVersErr)
+       }
 
        type statTest struct {
                repo string

diff --git a/src/cmd/go/internal/vcweb/script.go b/src/cmd/go/internal/vcweb/script.go
index 8fa00b277503d7443eb6e1e6255b112bacb14859..99f001254b54c7a08e191a01ce86c0e8fbb26662 100644 (file)
--- a/src/cmd/go/internal/vcweb/script.go
+++ b/src/cmd/go/internal/vcweb/script.go
@@ -398,7 +398,8 @@ func hasWorkingBzr() bool {
        return err == nil
 }
 
-var gitVersLineExtract = regexp.MustCompile(`git version\s+([\d.]+)`)
+// Capture the major, minor and (optionally) patch version, but ignore anything later
+var gitVersLineExtract = regexp.MustCompile(`git version\s+(\d+\.\d+(?:\.\d+)?)`)
 
 func gitVersion() (string, error) {
        gitOut, runErr := exec.Command("git", "version").CombinedOutput()

diff --git a/src/cmd/go/scriptconds_test.go b/src/cmd/go/scriptconds_test.go
index 4d7a9ac54bb663429e7d8a90f06379820d9b8450..f24b74bfda3dd058a91fbe2069bd361ffc567365 100644 (file)
--- a/src/cmd/go/scriptconds_test.go
+++ b/src/cmd/go/scriptconds_test.go
@@ -157,7 +157,8 @@ func hasWorkingGit() bool {
        return err == nil
 }
 
-var gitVersLineExtract = regexp.MustCompile(`git version\s+([\d.]+)`)
+// Capture the major, minor and (optionally) patch version, but ignore anything later
+var gitVersLineExtract = regexp.MustCompile(`git version\s+(\d+\.\d+(?:\.\d+)?)`)
 
 func gitVersion() (string, error) {
        gitOut, runErr := exec.Command("git", "version").CombinedOutput()