Nixpkgs: System git version suffix is not computed correctly when nixpkgs is a git worktree

[colemickens]

Issue description

When you build the system configuration from a nixpkgs that is a git worktree, you don't get a nice git short hash of the nixpkgs revision.

Steps to reproduce

1. Clone nixpkgs somewhere. (/etc/nixpkgs)
2. Navigate to the checkout.
3. git worktree add ... checkout another branch somewhere else (/etc/nixpkgs2)
4. Set NIX_PATH=nixpkg=/etc/nixpkgs2:nixos-config=/etc/nixos/configuration.nix
5. nixos-rebuild build

Note that the output at the end lacks the git version suffix.

I was told to look here: https://github.com/NixOS/nixpkgs/blob/master/lib/trivial.nix#L95-L118

It seems obvious what the problem is, I just don't have time to try to fix it now.

I assume we'd want to do something like adding another branch to the conditional, "isGitWorktree" that checks to see if there's a .git _file_ and if so, use the path inside the file to use with lib.commitIdFromGitRepo.

[colemickens]

Well, on second thought, getting the commit id from the directory pointed to in the file will be for the wrong branch. It would be nice if it could just call git and let git do the work of figuring out the worktree.

[LnL7]

The set of impure operations allowed during evaluation is very limited, executing commands isn't one of those (and shouldn't be IMHO).

[colemickens]

Agreed. I looked at the lib code a bit more last night, as well as how git worktrees work. It should be fairly straightforward to support this scenario without invoking git. I'll try to get to this in the next few days.

[colemickens]

It seems like this is still not working for me despite the changes in #77473 and #77704.

When I point my machine default.nix's "pinned" nixpkgs to ../nixpkgs/master it correctly picks up the git revision from master and tells me it's going to build: /nix/store/v9k6crxi3rdax7m2i6ik2cz94gyazi2b-nixos-system-xeep-20.09.git.9c76aacaf2c.drv

but if I point it at a worktree sibling dir ../nixpkgs/cmpkgs (my default branch), then it winds up building toplevel=/nix/store/816rcx4pbjf643bgpw9nlsynmyg05ra5-nixos-system-xeep-20.09pre-git.

And "pre-git" is the default version suffix, as I understand it.

[colemickens]

cc: @elseym Do you have any ideas why I'd be seeing this behavior?

[colemickens]

It looks like the only place this is called checks to see if the .git is a git repository first, and seems to fail that check. I'm not sure how this altered code is ever called unless someone is manually called commitIdFromGitRepo in their own nix code.

https://github.com/NixOS/nixpkgs/blob/3bbd074217cd11b6e14abec24655091b83aacc6f/lib/trivial.nix#L183-L198

[elseym]

f0f040c3f7f07fa4dc28b32d44e1db78fa3a0cc1 removed parsing of the git revision from the git-repository for both config.system.nixos.revision and config.system.nixos.versionSuffix. afaics revisionWithDefault is now only used when building the docs.

maybe your 'cmpkgs' is newer than this commit and therefore doesn't provide this functionality anymore? or maybe your git-worktree-checkout is broken (on a git-worktree-level, so that the now 'dead-code' in lib.commitIdFromGitRepo can't parse it).

[colemickens]

Hm. I'm still pretty confused but your comment helped, I think. I finally deleted .version-suffix from the master branch directory and now they both seem to behave similarly and not have the git version as part of the system derivation name. And to confirm, the git rev in the build output I showed above is not actually the git commit at that point in time! It was possibly from the last time before I rebased on top of f0f040c! Anyway, that's what I'm telling myself because it makes sense.

Thanks for the reply @elseym !

[davidak]

This seem to be fixed: 20.09.git.8a894fe9d00 (Nightingale)

But you need git installed. https://github.com/NixOS/nixpkgs/issues/91199

[jerith666]

I've been annoyed by this for a while as well, and finally took some time to look into it, which eventually led me here.

There is another piece of the puzzle that took me a while to discover: at https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/installer/tools/nixos-rebuild.sh#L392, the .version-suffix file is only written if $canRun; that in turn is only true for nixos-rebuild boot, nixos-rebuild switch, or nixos-rebuild test (see https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/installer/tools/nixos-rebuild.sh#L213).

My workflow leads me to almost always run switch or boot from a non-worktree, but frequently run build from a worktree. So I was confused for a while into thinking that the problem was related to worktrees, when now AFAICT it's not.

If we really believe the reasoning in the commit message of f0f040c, then I think we should remove the commit hash from the system name entirely. This way, one can nixos-rebuild build and then later on be sure that a nixos-rebuild switch from that same commit will not need to rebuild anything.

[jerith666]

But on the other hand, it is _really useful_ to have the nixpkgs commit hash baked into the built system!

[jerith666]

FWIW b0d9a1ac7d1fd6e7e2fba1c32f5ac350795024bb is how I've addressed this locally.

[colemickens]

Just a heads up - this problem is automatically resolved in flakes.