`fetchGit` should fail if `rev` and `ref` don't match

[l0b0]

Describe the bug

It seems that ref is ignored when rev is specified, which could lead to problematic situations. If someone thinks they are using version X.Y.Z, when in reality someone forgot to update the ref when updating the rev, they might spend a lot of time investigating why the software isn't behaving as expected for version X.Y.Z. Even worse would be if someone updated a ref to a new version of a broken or compromised package, but forgot to update rev, leading users to believe they are running a fixed/safe version.

Steps To Reproduce

Example shell session, downloading Nix with a non-existing ref:

$ nix store delete /nix/store/g1pkhqamigraik4f1fvd34g7ss0xqrv7-source
1 store paths deleted, 6.13 MiB freed
$ nix eval --expr 'builtins.fetchGit {url = "https://github.com/NixOS/nix"; rev = "e76bbe413e86e3208bb9824e339d59af25327101"; ref = "no-such-ref";}'
{ lastModified = 1744095711; lastModifiedDate = "20250408070151"; narHash = "sha256-Aqnj5+sA7B4ZRympuyfWPPK83iomKHEHMYhlwslI8iA="; outPath = "/nix/store/g1pkhqamigraik4f1fvd34g7ss0xqrv7-source"; rev = "e76bbe413e86e3208bb9824e339d59af25327101"; revCount = 19748; shortRev = "e76bbe4"; submodules = false; }

Expected behavior

Some alternatives:

• Treat it like most linters treat unused variables: if the ref is meant to be ignored when rev is specified, Nix should emit a warning about this. That way Nix authors can choose whether to keep or remove a ref which is not actually used by Nix, and which may at any time be out of sync with the rev. This has the advantage of avoiding any kind of slow-down because of expensive checks.
• Treat it like deadnix, such that specifying both ref and rev is an error. This also avoids expensive checks at build time, with the cost of losing useful metadata for developers.
• Treat it like an additional safety measure: Nix should verify that ref points to rev when downloading, and emit an error message and return with a non-zero exit code if not. This has the advantage of safety, but could be an expensive check, so it might be necessary to include a flag to disable it in performance-sensitive situations.

Metadata

nix-env (Nix) 2.24.13

Additional context

Checklist

• checked latest Nix manual (source)
• checked open bug issues and pull requests for possible duplicates

---

Add 👍 to issues you find important.

[ajlekcahdp4]

Hi! I encountered the same issue with updating "ref" and forgetting to update "rev" this week. After some digging in the source code I see that "ref" is clearly ignored if "rev" is specified in the following lines: https://github.com/NixOS/nix/blob/master/src/libfetchers/git.cc#L624. Considering this I have a question what do the following lines in the documentation mean:

> **Note**
>
> It is nice to always specify the branch which a revision
> belongs to. Without the branch being specified, the fetcher
> might fail if the default branch changes. Additionally, it can
> be confusing to try a commit from a non-default branch and see
> the fetch fail. If the branch is specified the fault is much
> more obvious.

(at https://github.com/NixOS/nix/blob/master/src/libexpr/primops/fetchTree.cc#L790)

I understand these lines as "fetching might fail if ref is invalid". However I couldn't make it fail and I don't understand how can it fail from looking at the source code. Does that mean that this code meant to check that reference is consistent with revision (as described in the third solution proposed in the issue)? If so it seems broken (probably since commit e4066c04442f86a5a12d492d588e3e82b533053d). @edolstra Can you comment on this please, it seems to be your code.

In my opinion both forbidding to use "rev" and "ref" at the same time or checking for their consistency are valid options. I'd like to hear input from the code authors as I am not very familiar with the code base. Happy to implement a patch if we decide on something.

[fzakaria]

Okay I spent some time looking into this.

I think this has changed slightly or is more confusing since now git supports (as of 2.5+) allowReachableSHA1InWant or maybe even allowAnySHA1InWant (tough to get confirmation of which on GitHub and GitLab). These settings allow you to fetch commits from refs which are not advertised.

However even without these settings, any commit that is reachable from a reference that is advertised by the server git ls-remote will work.

All of this is to say, I don't even see why rev and ref would ever be necessary together.
There is already some friendly message to let you know to use allRefs is the commit is not reachable.

nix/src/libfetchers/git.cc

Line 655 in 59c7dac

"Cannot find Git revision '%s' in ref '%s' of repository '%s'! "

[fzakaria]

@l0b0 let me know what you think of #13440
I think from your description this might be the intended behavior that "makes sense" --

I think though it might be a "breaking change" by catching cases where the ref did not in fact match the rev.
(Is it a breaking change if it's a "bug" ?....)