Skip to content

fetchgit: take postCheckout to allow gathering revision and commit metadata without leaving .git#465497

Merged
philiptaron merged 5 commits intoNixOS:masterfrom
ShamrockLee:fetchgit-postCheckout
Dec 17, 2025
Merged

fetchgit: take postCheckout to allow gathering revision and commit metadata without leaving .git#465497
philiptaron merged 5 commits intoNixOS:masterfrom
ShamrockLee:fetchgit-postCheckout

Conversation

@ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Nov 27, 2025

Description

This PR introduces the postCheckout argument for fetchgit to allow running shell commands (especially git describe) before the .git directory is removed or sanitized. It uses nix-prefetch-git's existing support to environment variable NIX_PREFETCH_GIT_CHECKOUT_HOOK, while giving it a more friendly name.

To make git describe work when specifying fetchgit { tag = "my_tag"; ... }, this PR also creates a reference of the same name (path) as the --rev <ref>'s <ref> operand.

Partially addresses PR #8567

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) labels Nov 27, 2025
@ShamrockLee
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 465497
Commit: 143961bc02f6973460b825bc065d65e44ad304c9 (subsequent changes)
Merge: 9ed9b097d571f324f48fea1c84c80ecab055fa47

Logs: https://github.com/ShamrockLee/nixpkgs-review-gha/actions/runs/19728343354


x86_64-linux

✅ 30 packages built:
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.git-describe-without-dot-git
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchurl.header
  • tests.haskell.cabalSdist.assumptionLocalHasDirectReference
  • tests.haskell.cabalSdist.localHasNoDirectReference
  • tests.testers.runCommand.bork

aarch64-linux

✅ 28 packages built:
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.git-describe-without-dot-git
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchurl.header
  • tests.testers.runCommand.bork

x86_64-darwin (sandbox = relaxed)

❌ 1 package failed to build:
  • tests.testers.runCommand.bork
✅ 27 packages built:
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.git-describe-without-dot-git
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchurl.header

Error logs: `x86_64-darwin`
tests.testers.runCommand.bork
                    exc.stdout, exc.stderr = process.communicate()
                else:
                    # POSIX _communicate already populated the output so
                    # far into the TimeoutExpired exception.
                    process.wait()
                raise
            except:  # Including KeyboardInterrupt, communicate handled that.
                process.kill()
                # We don't call process.wait() as .__exit__ does that for us.
                raise
            retcode = process.poll()
            if check and retcode:
>               raise CalledProcessError(retcode, process.args,
                                         output=stdout, stderr=stderr)
E               subprocess.CalledProcessError: Command '['/nix/store/84s01mzl74vh69aphpl3rf07pp49pkz0-python3-3.13.9/bin/python3.13', '-m', 'bork', 'download', 'gh:duckinator/emanate', 'v7.0.0']' returned non-zero exit status 1.

/nix/store/84s01mzl74vh69aphpl3rf07pp49pkz0-python3-3.13.9/lib/python3.13/subprocess.py:577: CalledProcessError
=========================== short test summary info ============================
FAILED bork/tests/test_cmd_download.py::test_download - subprocess.CalledProcessError: Command '['/nix/store/84s01mzl74vh69aphpl3rf...
============= 1 failed, 3 passed, 5 deselected in 81.45s (0:01:21) =============


aarch64-darwin (sandbox = relaxed)

✅ 28 packages built:
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.git-describe-without-dot-git
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchtorrent.http-link (tests.fetchtorrent.http-link-transmission)
  • tests.fetchtorrent.magnet-link (tests.fetchtorrent.magnet-link-transmission)
  • tests.fetchurl.header
  • tests.testers.runCommand.bork

@ShamrockLee ShamrockLee marked this pull request as ready for review November 27, 2025 08:14
@nixpkgs-ci nixpkgs-ci bot requested a review from philiptaron November 27, 2025 08:14
@ShamrockLee
Copy link
Contributor Author

tests.testers.runCommand.bork is known to be flaky, and is unrelated to fetchgit.

@ShamrockLee ShamrockLee force-pushed the fetchgit-postCheckout branch from 143961b to 08c189f Compare November 27, 2025 10:18
@ShamrockLee
Copy link
Contributor Author

Test cases improved.

@bjornfor
Copy link
Contributor

Be aware that git describe is not a reproducible command. Cloning a repo at two different times can produce different git describe results even if the working tree is checked out at the same commit. So there is a risk associated with caching the command output.

@ShamrockLee ShamrockLee changed the title fetchgit: take postCheckout to allow git-describe without leaving .git fetchgit: take postCheckout to allow gathering commit messages without leaving .git Nov 27, 2025
@ShamrockLee ShamrockLee changed the title fetchgit: take postCheckout to allow gathering commit messages without leaving .git fetchgit: take postCheckout to allow gathering revision and commit metadata without leaving .git Nov 27, 2025
Copy link
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

This sounds and looks sensible to me

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Nov 27, 2025

Be aware that git describe is not a reproducible command. Cloning a repo at two different times can produce different git describe results even if the working tree is checked out at the same commit.

Is there a reproducer to demonstrate this non-reproducibility?

@Atemu
Copy link
Member

Atemu commented Nov 28, 2025

Also, wouldn't this only occur when upstream changes tags retroactively? That's not a case our regular fetchgit can handle gracefully either.

@bjornfor
Copy link
Contributor

Be aware that git describe is not a reproducible command. Cloning a repo at two different times can produce different git describe results even if the working tree is checked out at the same commit.

Is there a reproducer to demonstrate this non-reproducibility?

  1. Create repo with commit abcd123 and tag 1.0-rc1 at that commit.
  2. Fetch repo at commit abc123. Capture the hash of it (and the git describe output) in Nix.
  3. The next day, someone pushes a new tag on the same commit: 1.0
  4. Fetches since then will be rejected by Nix, since git describe returns a different output and the hash does not match anymore.

@bjornfor
Copy link
Contributor

bjornfor commented Nov 28, 2025

Also, wouldn't this only occur when upstream changes tags retroactively?

Not only change. It can happen if they add a tag anywhere that git describe finds quicker than the previous tag (EDIT: s/commit/tag/).

That's not a case our regular fetchgit can handle gracefully either.

Right, if you specify fetchgit { tag = ...; } instead of {fetchgit { rev = ...; }. (Hm, I haven't though about that before.)

@me-and
Copy link
Member

me-and commented Nov 28, 2025

I'm not worried about the non-determinism of git describe here: fundamentally we're always accessing something from a third party, so the cast-iron guarantee that things haven't changed unexpectedly is the FOD hash not changing.

It's on the user to make sure they use the tools in a way that isn't inherently fragile. Someone relying on a source package marked "development latest" would be a poor idea, as would relying on a Git repository where tags aren't fixed (as GitHub recommends with GitHub Actions, for example).

I'd expect using git describe to be a bad idea in some circumstances, and a good idea in others. As a key example, I use it in https://github.com/me-and/nix-git-ci, which is testing the Git source code itself, where (a) unexpected changes to the describe output wouldn't be a problem and (b) the Git source repository has clear policies that make that unlikely anyway.

@ShamrockLee ShamrockLee force-pushed the fetchgit-postCheckout branch 3 times, most recently from 8d97c10 to 65f011e Compare November 30, 2025 07:38
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Nov 30, 2025

Be aware that git describe is not a reproducible command. Cloning a repo at two different times can produce different git describe results even if the working tree is checked out at the same commit.

Is there a reproducer to demonstrate this non-reproducibility?

1. Create repo with commit abcd123 and tag 1.0-rc1 at that commit.

2. Fetch repo at commit abc123. Capture the hash of it (and the git describe output) in Nix.

3. The next day, someone pushes a new tag on the same commit: 1.0

4. Fetches since then will be rejected by Nix, since `git describe` returns a different output and the hash does not match anymore.

So the non-reproducibility is brought by fetchTags or deepClone, and such non-reproducibility might affect hash stability by the use of postFetch or postCheckout.

Considering this, let's avoid the use of fetchTags, but name the tag reference as its original name (refs/tags/*) instead of just FETCH_HEAD. If applied, git describe will work deterministically inside postCheckout when specifying fetchgit { tags = ...; }.

As for unstable versions that specifies the Git revision directly but still want git describe to describe the last tag, we could introduce a new attribute extraTags or make fetchTags to optionally take a list. (The latter requires __structuredAttrs = true that PR #464475 introduces.)

Cc: @bjornfor @me-and

@ShamrockLee ShamrockLee requested a review from me-and November 30, 2025 07:47
@ShamrockLee
Copy link
Contributor Author

All the tests.fetchgit tests now build on my x86_64 machine.

@ShamrockLee ShamrockLee force-pushed the fetchgit-postCheckout branch from eb3c6ad to 2945886 Compare December 10, 2025 14:24
@philiptaron
Copy link
Contributor

I'd like to merge in a day or two. Hopefully @me-and has a chance to review.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 10, 2025
@me-and
Copy link
Member

me-and commented Dec 14, 2025

I'd like to merge in a day or two. Hopefully @me-and has a chance to review.

I should be able to take a look (and, more to the point, a think) in the next couple of days. If you want to merge before then, that's absolutely fine; if I have any feedback that really needs addressing we can iterate from that point.

Copy link
Member

@me-and me-and left a comment

Choose a reason for hiding this comment

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

This looks fantastically useful, thank you! I think it wants a few fixes before it's quite ready to be merged, but it's all just tidying to improve long-term maintainability; the fundamental changes are spot on.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 465497
Commit: 29458868f15aa1d1227e931f2c9f07690ba2fc5e


x86_64-linux

✅ 92 packages built:
  • bundix
  • cabal2nix
  • chirpstack-concentratord
  • corto
  • crate2nix
  • crush
  • crystal2nix
  • dub-to-nix
  • gcalcli
  • gcalcli.dist
  • git-unroll
  • haskellPackages.cli-nix
  • haskellPackages.cli-nix.doc
  • haskellPackages.nix-thunk
  • haskellPackages.nix-thunk.doc
  • haskellPackages.nvfetcher
  • haskellPackages.nvfetcher.doc
  • haskellPackages.update-nix-fetchgit
  • haskellPackages.update-nix-fetchgit.doc
  • kcl
  • lixPackageSets.git.nix-update
  • lixPackageSets.git.nix-update.dist
  • lixPackageSets.latest.nix-update (lixPackageSets.lix_2_94.nix-update)
  • lixPackageSets.latest.nix-update.dist (lixPackageSets.lix_2_94.nix-update.dist)
  • lixPackageSets.stable.nix-update (lixPackageSets.lix_2_93.nix-update)
  • lixPackageSets.stable.nix-update.dist (lixPackageSets.lix_2_93.nix-update.dist)
  • localtunnel
  • lon
  • luarocks-packages-updater
  • luarocks-packages-updater.dist
  • mdwatch
  • meshlab
  • mlv-app
  • nim_lk
  • nix-prefetch-git
  • nix-prefetch-scripts
  • nix-update
  • nix-update-source
  • nix-update-source.dist
  • nix-update.dist
  • npins
  • nvfetcher
  • outline
  • prefetch-yarn-deps
  • prowlarr
  • python312Packages.pymeshlab
  • python313Packages.nixpkgs-updaters-library
  • python313Packages.nixpkgs-updaters-library.dist
  • python313Packages.pymeshlab
  • radarr
  • sonarr
  • sus-compiler
  • tests.fetchFromBitbucket.withEncodedWhitespaceGit
  • tests.fetchFromGitHub.fetchTags
  • tests.fetchFromGitHub.leave-git
  • tests.fetchFromGitHub.rootDir
  • tests.fetchFromGitHub.sparseCheckout
  • tests.fetchFromGitHub.sparseCheckoutNonConeMode
  • tests.fetchFromGitHub.submodule-deep
  • tests.fetchFromGitHub.submodule-leave-git
  • tests.fetchFromGitHub.submodule-leave-git-deep
  • tests.fetchFromGitHub.submodule-simple
  • tests.fetchgit.cached-prefetch-avoids-fetch
  • tests.fetchgit.collect-rev
  • tests.fetchgit.describe-tag
  • tests.fetchgit.dumb-http-signed-tag
  • tests.fetchgit.fetchTags
  • tests.fetchgit.leave-git
  • tests.fetchgit.prefetch-git-no-add-path
  • tests.fetchgit.rootDir
  • tests.fetchgit.simple
  • tests.fetchgit.simple-tag
  • tests.fetchgit.sparseCheckout
  • tests.fetchgit.sparseCheckoutNonConeMode
  • tests.fetchgit.submodule-deep
  • tests.fetchgit.submodule-leave-git
  • tests.fetchgit.submodule-leave-git-deep
  • tests.fetchgit.submodule-revision-count
  • tests.fetchgit.submodule-simple
  • tests.fetchgit.withGitConfig
  • tests.fetchurl.header
  • tests.haskell.cabalSdist.assumptionLocalHasDirectReference
  • tests.haskell.cabalSdist.localHasNoDirectReference
  • tests.testers.runCommand.bork
  • typescript-language-server
  • update-nix-fetchgit
  • update-python-libraries
  • vclab-nexus
  • vclab-nexus.bin
  • vimPluginsUpdater
  • xosd-xft
  • yarn2nix

…T_HOOK

Co-authored-by: Adam Dinwoodie <adam@dinwoodie.org>
@ShamrockLee ShamrockLee force-pushed the fetchgit-postCheckout branch from 2945886 to e348c7b Compare December 16, 2025 17:16
@ShamrockLee ShamrockLee force-pushed the fetchgit-postCheckout branch from e348c7b to 8974669 Compare December 16, 2025 17:19
@ShamrockLee ShamrockLee requested a review from me-and December 16, 2025 17:20
Copy link
Member

@me-and me-and left a comment

Choose a reason for hiding this comment

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

Fantastic. Thank you!

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Dec 17, 2025
@philiptaron philiptaron added this pull request to the merge queue Dec 17, 2025
Merged via the queue into NixOS:master with commit cca4ec8 Dec 17, 2025
31 of 34 checks passed
@Aleksanaa Aleksanaa mentioned this pull request Dec 21, 2025
13 tasks
@RaitoBezarius
Copy link
Member

Commit 7e08567 broke nix build git+https://git.afnix.fr/afnix/nix-gerrit#gerrit.src --rebuild --override-input nixpkgs "github:nixos/nixpkgs?rev=7e085677f9961b1bf06f292198614e2bdeede9d4, was it intended?

It seems like this commit is not achieving its intended effect as tags are indeed not fetched where it was expected before.

This should probably be reverted as it's breaking backward compatibility?

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Dec 24, 2025

Commit 7e08567 broke nix build git+https://git.afnix.fr/afnix/nix-gerrit#gerrit.src --rebuild --override-input nixpkgs "github:nixos/nixpkgs?rev=7e085677f9961b1bf06f292198614e2bdeede9d4, was it intended

This is not intended.

@RaitoBezarius Could you help test if PR #473411 now restores the hash compatibility?

marcin-serwin added a commit to marcin-serwin/nixpkgs that referenced this pull request Jan 5, 2026
Following NixOS#465497 it's possible
to specify `postCheckout` to perform actions after repo is fetched but
before `.git` is deleted.

Signed-off-by: Marcin Serwin <marcin@serwin.dev>
marcin-serwin added a commit to marcin-serwin/nixpkgs that referenced this pull request Jan 5, 2026
Following NixOS#465497 it's possible
to specify `postCheckout` to perform actions after repo is fetched but
before `.git` is deleted.

Signed-off-by: Marcin Serwin <marcin@serwin.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants