Fix a segfault in the git fetcher

[thufschmitt]

The git fetcher code used to dereference the (potentially empty) ref
input attribute. This was magically working, probably because the
compiler somehow outsmarted us, but is now blowing up with newer nixpkgs
versions.

Fix that by not trying to access this field while we don't know for sure
that it has been defined.

Fix #6554

@harrisonthorne if you have a way to reproduce the original issue, can you confirm that the fix is correct?

Related to #6509

[edolstra]

I fixed this differently in #6530 by making sure that we always have a ref at that point:

nix/src/libfetchers/git.cc

Line 494 in dfbb9a9

{"ref", ref},

[fogti]

why don't we move this part above the "is-local" check? the code duplication seems unnecessary, especially because the isLocal condition appears to be a simple boolean branch...
e.g.

        if (!input.getRef()) {
            auto head = isLocal ? readHead(actualUrl) : readHeadCached(actualUrl);
            if (!head) {
                warn("could not read HEAD ref from repo at '%s', using 'master'", actualUrl);
                head = "master";
            }
            input.attrs.insert_or_assign("ref", *head);
            unlockedAttrs.insert_or_assign("ref", *head);
        }

Also, what's going on with the following (although probably unrelated to this PR):

nix/src/libfetchers/git.cc

Line 445 in ec07a70

if (!input.getRef() && !input.getRev() && isLocal) {

The following also doesn't really make any sense... (when getRef() returned nullopt or such, we explicitly dereference it afterwards, instantly UB)

nix/src/libfetchers/git.cc

Lines 471 to 473 in ec07a70

	if (!input.getRev())
	input.attrs.insert_or_assign("rev",
	Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "--git-dir", gitDir, "rev-parse", *input.getRef() })), htSHA1).gitRev());

the meaning/usage of getRef() and what it returns are really confusing... like what's this?

nix/src/libfetchers/git.cc

Line 518 in ec07a70

runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "cat-file", "-e", input.getRev()->gitRev() });

[FlorianFranzen]

I ran into this issue with nixUnstable on nixos-unstable, e.g. with the following expression:

nix eval --expr 'builtins.fetchGit { url = "https://github.com/mozilla/nixpkgs-mozilla.git"; rev = "15b7a05f20aab51c4ffbefddb1b448e862dccb7d"; }'

[fogti]

I guess this should be merged because the breakage on nixpkgs master/unstable of nixUnstable is extremely annoying... we can improve the code afterwards if necessary; it appears good enough for now, the more problematic parts of git fetcher are pretty much unrelated to this.

[thufschmitt]

Yup' I agree with @zseri, #6530 doesn't look like it can be merged in a matter of days, so it seems important to have this fix land since it's pretty bad

[edolstra]

I can't reproduce this on current master (2.10.0pre20220531_04a699b).

./result/bin/nix eval --expr 'builtins.fetchGit { url = "https://github.com/mozilla/nixpkgs-mozilla.git"; rev = "15b7a05f20aab51c4ffbefddb1b448e862dccb7d"; }'
{ lastModified = 1645464064; lastModifiedDate = "20220221172104"; narHash = "sha256-YeN4bpPvHkVOpQzb8APTAfE7/R+MFMwJUMkqmfvytSk="; outPath = "/nix/store/5snmi8nb601c90swp17b96g3a1622366-source"; rev = "15b7a05f20aab51c4ffbefddb1b448e862dccb7d"; revCount = 242; shortRev = "15b7a05"; submodules = false; }

[thufschmitt]

I can't reproduce this on current master (2.10.0pre20220531_04a699b).

Yes, as stated in the issue, this only shows for newer nixpkgs versions. nix build nix --override-input nixpkgs nixpkgs/nixos-unstable will fail the fetchGit test because of that segfault

[github-actions]

Successfully created backport PR #6597 for 2.9-maintenance.