Skip to content

util/gitutil: Full/ShortCommit(): replace git show with git rev-parse#3747

Open
jegorbunov wants to merge 1 commit intodocker:masterfrom
jegorbunov:gitutil-use-rev-parse-instead-of-show
Open

util/gitutil: Full/ShortCommit(): replace git show with git rev-parse#3747
jegorbunov wants to merge 1 commit intodocker:masterfrom
jegorbunov:gitutil-use-rev-parse-instead-of-show

Conversation

@jegorbunov
Copy link
Copy Markdown

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])?

@jegorbunov jegorbunov force-pushed the gitutil-use-rev-parse-instead-of-show branch from 200f023 to ba6eae3 Compare March 27, 2026 10:50
@jegorbunov jegorbunov force-pushed the gitutil-use-rev-parse-instead-of-show branch from 2f19caf to 758cdba Compare March 30, 2026 14:14
@jegorbunov jegorbunov force-pushed the gitutil-use-rev-parse-instead-of-show branch from 758cdba to 28c48d8 Compare April 7, 2026 14:27
Comment on lines 61 to 62
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like your PR was opened from an outdated branch and pulled in a commit from #3750 - can you try rebasing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed.

@jegorbunov jegorbunov force-pushed the gitutil-use-rev-parse-instead-of-show branch from 28c48d8 to 93b562c Compare April 7, 2026 15:04
@github-actions github-actions bot removed the area/ci label Apr 7, 2026
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>
@jegorbunov jegorbunov force-pushed the gitutil-use-rev-parse-instead-of-show branch from 93b562c to 9b56d03 Compare April 7, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants