Prevent fetchGit from using incorrect cached rev for different refs

[agbrooks]

Background

This is a fix for #7146.

When using fetchGit on a non-local repository and using a ref (but no rev), fetchGit will decide that any cached rev for that repository is usable (if not too stale).

This is potentially really disruptive. If a user uses fetchGit to fetch both the nixos-22.05 and nixos-21.11 branches of nixpkgs back-to-back, and they don't specify a rev, they'll wind up with nixos-22.05 sources in both cases (!!):

nix-repl> builtins.fetchGit { url = "git@github.com:nixos/nixpkgs"; ref = "refs/heads/nixos-22.05"; }
{ lastModified = 1665613119; lastModifiedDate = "20221012221839"; narHash = "sha256-VTutbv5YKeBGWou6ladtgfx11h6et+Wlkdyh4jPJ3p0="; outPath = "/nix/store/xc6s7iccz5nkm8vl9nybz698nq2ngxlc-source"; rev = "e06bd4b64bbfda91d74f13cb5eca89485d47528f"; revCount = 383579; shortRev = "e06bd4b"; submodules = false; }

nix-repl> builtins.fetchGit { url = "git@github.com:nixos/nixpkgs"; ref = "refs/heads/nixos-21.11"; } 
{ lastModified = 1665613119; lastModifiedDate = "20221012221839"; narHash = "sha256-VTutbv5YKeBGWou6ladtgfx11h6et+Wlkdyh4jPJ3p0="; outPath = "/nix/store/xc6s7iccz5nkm8vl9nybz698nq2ngxlc-source"; rev = "e06bd4b64bbfda91d74f13cb5eca89485d47528f"; revCount = 383579; shortRev = "e06bd4b"; submodules = false; }

The fix

The fix I'm suggesting is pretty straightforward: when no rev is provided for a remote repo fetched with fetchGit, incorporate the ref into the input attrs to differentiate cached revs between different refs.

To illustrate, here's the fetcher cache after running the example above in Nix 2.9.1:

sqlite3 ~/.cache/nix/fetcher-cache-v1.sqlite 'SELECT * FROM Cache;'
input                                                                            info                                                                                            path                                                immutable  timestamp 
-------------------------------------------------------------------------------  ----------------------------------------------------------------------------------------------  --------------------------------------------------  ---------  ----------
{"name":"source","type":"git","url":"ssh://git@github.com/nixos/nixpkgs"}        {"lastModified":1665613119,"rev":"e06bd4b64bbfda91d74f13cb5eca89485d47528f","revCount":383579}  /nix/store/xc6s7iccz5nkm8vl9nybz698nq2ngxlc-source  0          1665789700
{"name":"source","rev":"e06bd4b64bbfda91d74f13cb5eca89485d47528f","type":"git"}  {"lastModified":1665613119,"rev":"e06bd4b64bbfda91d74f13cb5eca89485d47528f","revCount":383579}  /nix/store/xc6s7iccz5nkm8vl9nybz698nq2ngxlc-source  1          1665789700

and this is how it looks on my bugfix branch:

sqlite3 ~/.cache/nix/fetcher-cache-v1.sqlite 'SELECT * FROM Cache;'
input                                                                                                     info                                                                                            path                                                immutable  timestamp 
--------------------------------------------------------------------------------------------------------  ----------------------------------------------------------------------------------------------  --------------------------------------------------  ---------  ----------
{"name":"source","ref":"refs/heads/nixos-22.05","type":"git","url":"ssh://git@github.com/nixos/nixpkgs"}  {"lastModified":1665613119,"rev":"e06bd4b64bbfda91d74f13cb5eca89485d47528f","revCount":383579}  /nix/store/xc6s7iccz5nkm8vl9nybz698nq2ngxlc-source  0          1665789958
{"name":"source","rev":"e06bd4b64bbfda91d74f13cb5eca89485d47528f","type":"git"}                           {"lastModified":1665613119,"rev":"e06bd4b64bbfda91d74f13cb5eca89485d47528f","revCount":383579}  /nix/store/xc6s7iccz5nkm8vl9nybz698nq2ngxlc-source  1          1665789958
{"name":"source","ref":"refs/heads/nixos-21.11","type":"git","url":"ssh://git@github.com/nixos/nixpkgs"}  {"lastModified":1659446231,"rev":"eabc38219184cc3e04a974fe31857d8e0eac098d","revCount":337975}  /nix/store/9ks8076lanzwp09dmwy5r3racihsc1ip-source  0          1665790070
{"name":"source","rev":"eabc38219184cc3e04a974fe31857d8e0eac098d","type":"git"}                           {"lastModified":1659446231,"rev":"eabc38219184cc3e04a974fe31857d8e0eac098d","revCount":337975}  /nix/store/9ks8076lanzwp09dmwy5r3racihsc1ip-source  1          1665790070

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/55

[thufschmitt]

Thanks for the fix. It's indeed much more correct to include the ref in the cache key 👍

[agbrooks]

@thufschmitt: Is there anything else to address before this can be merged?

I have a few co-workers who have been affected by #7146 , so I'd be happy to make any changes needed.

[thufschmitt]

Is there anything else to address before this can be merged?

Nope sorry, I guess I just forgot to press the merge button 😬

Thanks @edolstra for the merge :)

[edolstra]

I merged this into the lazy-trees PR by deleting the changes to git.cc, and the test kept working, so I guess this bug was already fixed on that branch...

[agbrooks]

Just manually tested the example in the description with your lazy-trees branch to rule out an issue with the test -- your branch did indeed fix it. :)

[Yuhanun]

Heyo we're seeing some weird behavior happening within our production landscape and we feel like it relates to this PR.

getFlake on a remote flake using a ref= in the URL causes it to ignore the ref= and pull master instead.

We do not specify a rev=, we just specify git+https://.../?ref=...

It works again if we rm -rf ~/.cache/nix

This issue started occuring when we upgraded our engineers from 2.4 to 2.11.1

[Radvendii]

This seems like a major issue. Should it be back-ported to earlier versions of nix? it seems to have started in nix 2.9