Skip to content

Commit 28c48d8

Browse files
committed
util/gitutil: Full/ShortCommit(): replace git show with git rev-parse
The FullCommit and ShortCommit methods are expected to simply get the hash of HEAD. Using `git show` for this purpose is overkill because `git show` can have side effects that are annoying when `docker build` runs in a CI environment: 1) CI environments (e.g. CircleCI) may use blobless clones, where the target repository is checked out using `git clone --filter=blob:none {url}`. 2) `git show` does more than just discover the hash of a specified revision: in general, it needs parent commits and blobs to compute the diff. When a blobless clone is used, `git show` may try to download these additional blobs from the remote. From my experiments, this happens even when `--quiet` or `--no-patch` is specified. 3) If the `docker build` step runs in an environment where Git is not configured properly to download from the remote, the step may hang indefinitely, as it did in my case with the following prompt: The authenticity of host 'github.com (140.82.112.3)' can't be established. ED25519 key fingerprint is SHA256:+.... This key is not known by any other names. Are you sure you want to continue connecting (yes/no/[fingerprint])? Notes on test fixes: git rev-parse HEAD doesn't need any `--`, however, when it is called on an empty repo git returns an error: ``` fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' ``` So, it is both IsUnknownRevision and IsAmbiguousArgument. Lets simply fix the test by flipping the check: I don't see any dependency on this behavior in non-test code. Moreover, `git rev-parse --short HEAD` fails with another error: ``` fatal: Needed a single revision ``` Lets handle it as IsUnknownRevision Signed-off-by: Jegor Gorbunov <jegor.gorbunov@point-devel.com>
1 parent 8419911 commit 28c48d8

File tree

2 files changed

+9
-6
lines changed

2 files changed

+9
-6
lines changed

util/gitutil/gitutil.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,11 @@ func (c *Git) RemoteURL() (string, error) {
101101
}
102102

103103
func (c *Git) FullCommit() (string, error) {
104-
return c.clean(c.run("show", "--format=%H", "HEAD", "--quiet", "--"))
104+
return c.clean(c.run("rev-parse", "HEAD"))
105105
}
106106

107107
func (c *Git) ShortCommit() (string, error) {
108-
return c.clean(c.run("show", "--format=%h", "HEAD", "--quiet", "--"))
108+
return c.clean(c.run("rev-parse", "--short", "HEAD"))
109109
}
110110

111111
func (c *Git) Tag() (string, error) {
@@ -185,8 +185,11 @@ func IsUnknownRevision(err error) bool {
185185
return false
186186
}
187187
// https://github.com/git/git/blob/a6a323b31e2bcbac2518bddec71ea7ad558870eb/setup.c#L204
188+
// https://github.com/git/git/blob/1adf5bca8c3cf778103548b9355777cf2d12efdd/builtin/rev-parse.c#L585
188189
errMsg := strings.ToLower(err.Error())
189-
return strings.Contains(errMsg, "unknown revision or path not in the working tree") || strings.Contains(errMsg, "bad revision")
190+
return strings.Contains(errMsg, "unknown revision or path not in the working tree") ||
191+
strings.Contains(errMsg, "bad revision") ||
192+
strings.Contains(errMsg, "needed a single revision")
190193
}
191194

192195
// stripCredentials takes a URL and strips username and password from it.

util/gitutil/gitutil_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestGitFullCommitErr(t *testing.T) {
6262
_, err = c.FullCommit()
6363
require.Error(t, err)
6464
require.True(t, gitutil.IsUnknownRevision(err))
65-
require.False(t, gittestutil.IsAmbiguousArgument(err))
65+
require.True(t, gittestutil.IsAmbiguousArgument(err))
6666
}
6767

6868
func TestGitShortCommitErr(t *testing.T) {
@@ -74,8 +74,8 @@ func TestGitShortCommitErr(t *testing.T) {
7474

7575
_, err = c.ShortCommit()
7676
require.Error(t, err)
77-
require.True(t, gitutil.IsUnknownRevision(err))
78-
require.False(t, gittestutil.IsAmbiguousArgument(err))
77+
require.True(t, gitutil.IsUnknownRevision(err), err.Error())
78+
require.False(t, gittestutil.IsAmbiguousArgument(err), err.Error())
7979
}
8080

8181
func TestGitTagsPointsAt(t *testing.T) {

0 commit comments

Comments
 (0)