fetchGit can't handle annotated tags

[jakubgs]

Description

Currently builtins.fetchGit does not seem to handle annotated tags and fails with:

error: cannot update ref 'refs/heads/xyz': trying to write non-commit object 098abc123 to branch 'refs/heads/xyz'

Reproduction

You can see me reproduce this in status-im/status-mobile#10825.

Docs

This was reported once in #2385 and a fix for docs was applied in ae244af by adding:

Due to a bug #2385 only non-annotated tags can be fetched.

To documentation of builtins.fetchGit, but that change does not seem to be there in current docs:
https://nixos.org/nix/manual/#ssec-builtins
And there doesn't appear to be any fix in sight.

Solutions

If the fix is difficult to implement for builtins.fetchGit could we at least update the documentation with this information and make fetchGit return an error that actually makes sense instead of this trying to write non-commit object nonsense?

[justinlovinger]

It looks like you can fix this error by using ref = "refs/tags/my-tag" instead of ref = "my-tag". Is there a good reason Nix uses refs/heads/ instead of refs/tags/ for tags? Even if branches are not found under refs/tags/, Nix could try both refs/heads/ and refs/tags/. Or perhaps fetchGit should have separate branch = and tag = arguments, instead of a single ref =.

[jakubgs]

Separate arguments would be a pain considering how developers use this. For example in our repo we have a JSON file that specifies the version of a library for our application that is bundled into it at build time. Developers usually use branches or sometimes specific commits, while stable builds use tags. If there were separate arguments we'd need automation to detect which is which. Pretty sure we're not the only ones that do this.

[jakubgs]

Using separate arguments could also be a royal pain for something like yarn2nix which we use.

[FRidh]

Earlier issue #2385 which this one duplicates but which was closed with ae244af. The issue has indeed not really been fixed. Maybe @Ma27 knows more about this issue, having worked on fetchTree?

[Ma27]

To documentation of builtins.fetchGit, but that change does not seem to be there in current docs:
https://nixos.org/nix/manual/#ssec-builtins

I assume this isn't backported to Nix 2.3? IIRC nixos.org always provides the docs of the latest stable release.

Maybe @Ma27 knows more about this issue, having worked on fetchTree?

Well, one could change the logic of the git-support in libfetchers:

nix/src/libfetchers/git.cc

Lines 353 to 356 in 839f0fe

	auto ref = input.getRef();
	auto fetchRef = ref->compare(0, 5, "refs/") == 0
	? *ref
	: "refs/heads/" + *ref;

However I think that theremarks @jakubgs brought up are quite reasonable. If we add those arguments those should be "translated" to "refs/{heads,tags}/foo" in the fetchGit primop[1]

[1]

nix/src/libexpr/primops/fetchTree.cc

Lines 234 to 237 in 839f0fe

	static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v)
	{
	fetchTree(state, pos, args, v, "git", true);
	}

[stale]

I marked this as stale due to inactivity. → More info

[jakubgs]

As far as I know this is still an issue.

[stale]

I marked this as stale due to inactivity. → More info

[jakubgs]

Nope.

[stale]

I marked this as stale due to inactivity. → More info

[jakubgs]

Still a regression.

[balsoft]

See #6766

[nixos-discourse]

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

https://discourse.nixos.org/t/git-tags-in-flakes-inputs/25511/2

[dpc]

This is a painful paper-cut, especially combined with the proclivity of Nix to ignore any problems with inputs. It will silently fail to do the weird "shallow-semi-automated-update" and just move on with the old lock file content.