fetchgit{,hub}: add tag argument#355973
Conversation
github-actions
bot
added
the
6.topic: fetch
github-actions
bot
added
the
8.has: documentation
ofborg
bot
added
10.rebuild-darwin: 1-10
It's become a common pattern to use `rev = "refs/tags/${version}"` rather than
just `rev = version` to ensure that the tag gets fetched rather than a branch
that has the same name. This has so far been done using boilerplate though, so
let's add a simple abstraction to fetch a tag instead.
|
#355685 is now in master. |
Atemu
added
9.needs: port to stable
|
This will not apply to 24.05 cleanly because of the docs but I will backport the functionality if no breakage is detected on unstable after this is merged. There really shouldn't be any but just to be sure. |
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
I really like this direction, but the implementation (done with defaults) means it's possible for a user to specify both a tag and a rev, and the derivation will silently pick the rev.
Instead, how about implementing this with a let block, so that the derivation-creating function is in charge of exactly how and in what circumstances rev and tag interact?
|
I feel like that is a contrived example, do you expect that to actually happen? This seems to be a theoretical concern to me, not a practical one. |
wegank
added
the
12.approvals: 1
|
I think I'd like to go ahead with this and self-merge unless someone objects within the next week or so. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
Exposed an issue with using |
|
It seems like it would be best for this to expose the appropriate |
|
We could do that (PRs welcome) but you really shouldn't be depending on |
|
I think |
You couldn't use |
|
Sure you could; https://github.com/NixOS/nixpkgs/blob/refs/tags/24.11-pre/README.md works fine and is fact better than the alternative for precisely the same reason we want to use |
mdaniels5757
added
8.has: port to stable
15 participants
It's become a common pattern to use
rev = "refs/tags/${version}"rather than justrev = versionto ensure that the tag gets fetched rather than a branch that has the same name. This is done using boilerplate though, so let's add a simple abstraction for fetching tags instead.The first two commits are from #355685 which should be merged first. Please review those separately.This should be backported as it's a non-breaking addition and because people will likely want to make use of this in changes which they want to backport.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.