fetchFromGitiles: init #74862
fetchFromGitiles: init #74862
Conversation
This has the same motivation as fetchFromGitHub/fetchFromGitLab -- it's cheaper to download a tarball of a single revision than it is to download a whole history. I could have gone with domain/group/repo, like fetchFromGitLab, but it would have made implementation more difficult, and this syntax means it's a drop-in replacement for fetchgit, so I decided it wasn't worth it.
This is only the easy cases -- some fetchgit uses that point to Gitiles instances are in generated code, where the generating code would have to know in advance if it was fetching from Gitiles or not. I don't think this is worth it.
|
What can be rev? Literal commit ID or a tag name? |
|
rev can be anything that git understands. so a commitish, tag, or ref. https://git-scm.com/docs/git-rev-parse#_specifying_revisions |
|
Then was «refs/tags/version» replacement to «version» necessary when switching to Gitiles? |
|
I think what you’re talking about is actually a change from |
|
unless a repo is doing something like using branches and tags with the same names, but different commits, then it wont matter. |
|
I am just an annoying literalist: |
|
not familiar with gitiles, 🤷♂️ |
|
I am just an annoying literalist: `fetchFromGitiles` says it accepts
arguments similar to `fetchgit`, and the latter says it prefers full
`refs/tags/` form so I wonder whether Gitiles fully supports the full
form.
It does support it, but I still don't want to use it here, because
whatever fetchgit says, refs/tags/ is not the norm in NixOS. Either
there should be a treewide change to refs/tags/, or the documentation
should be changed, and both of those things are out of scope for this
PR.
The documentation shouldn't say something different to what actual
practice is.
|
|
> I am just an annoying literalist: `fetchFromGitiles` says it accepts
> arguments similar to `fetchgit`, and the latter says it prefers full
> `refs/tags/` form so I wonder whether Gitiles fully supports the full
> form.
It does support it, but I still don't want to use it here, because
Ah that's fine, I agree about scope, I was just checking that the level
of consistency doesn't go down when inheriting the full-reference
support promise (not all web frontends like slash-separated arguments).
|
|
Cool :)
Otherwise happy with the PR?
|
Motivation for this change
This has the same motivation as fetchFromGitHub/fetchFromGitLab --
it's cheaper to download a tarball of a single revision than it is to
download a whole history.
I could have gone with domain/group/repo, like fetchFromGitLab, but it
would have made implementation more difficult, and this syntax means
it's a drop-in replacement for fetchgit, so I decided it wasn't worth
it.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)Notify maintainers
cc @