Fix nix-prefetch-git for syntax variations. #570
Conversation
This fixes several issues for nix-prefetch-git:
1. Variances in `git submodule status` output.
Here are some examples of various output from this command:
-20007c90b9c5e49c241df0494077f44d4559af45 submodules/foo
20007c90b9c5e49c241df0494077f44d4559af45 submodules/foo (remotes/origin/stable-3-g20007c9)
20007c90b9c5e49c241df0494077f44d4559af45 submodules/foo ((null))
The original regex processing would only match the first of these.
The observable effect of the other forms of output was complaints
about "too many parameters to 'cd'" because the entire line was
used as the "dir" and "hash" variable values.
2. Remote URL determination.
The original form attempted to parse the directory from the
`git submodule status` output above and look that up in the
.git/config file to determine the URL. Unfortunately, the
latter file uses the config-ini *section name* as the index,
as does the .gitmodules file, so if the section name differs
from the subdirectory, this failed and returned a blank URL.
Sample (valid!) .gitmodule contents that would exhibit
this error:
.gitmodules:
[submodule "x86/tests/submodules/dwarf"]
path = deps/dwarf
url = https://github.com/myteam/dwarf.git
This patch resolves both of the above by using the the builtin
`git submodule foreach` to iterate through the submodules, where
this command explicitly sets shell variables to represent most
of the items, and the submodule lookup in .git/config is now done
using the submodule name and not the submodule path.
|
@kquick thanks for your PR, it works as expected! |
|
This did not work for me. I ended up with:
|
|
@ElvishJerricco is your top-level repo public such that I could reproduce this locally? Or if not, can you provide a minimal example that reproduces the issue? [Sorry for the delay in responding to your feedback: I just returned from vacation.] |
|
Hmm, actually it did not work for me, too. @kquick have a look @ https://hydra.microlab.ntua.gr/jobset/joko-nixos/unstable-boxen#tabs-configuration The error: |
|
Thanks for the info @jokogr. I think I see the issue and will update this patch today or tomorrow when I get this resolved. |
|
@jokogr @ElvishJerricco The latest version just uses the standard git functionality, which should be more reliable and addresses the issues you two reported. Thank you! |
|
|
||
| clone "$dir" "$url" "$hash" ""; | ||
| done; | ||
| git submodule update --recursive |
There was a problem hiding this comment.
You could even fold this into the above line:
git submodule update --init --recursive
There was a problem hiding this comment.
It could, although I think the two specific steps provide more readability into the sequence of operations being performed. Your suggestion is definitely valid git but I always feel like the init is a command and it's being hidden behind an option flag when doing it that way. That's purely a stylistic perspective though: if I'm missing a technical reason why there's a difference I could easily be convinced to change my thinking here.
|
Can we close this? Since it's already implemented in 8bffbb7 |
|
That commit doesn't pass |
|
Are you guys still interested into merging this PR? |
|
@RaitoBezarius yes, I think this is an improvement and should be merged. The 8bffbb7 change includes an extra command that doesn't assist the process, and leaves in a misleading comment in addition to lacking the |
|
Can we do something to help @kquick ? |
|
I submitted the PR and tried to keep it up to date for a while, but it appears there has been no interest by the maintainers in merging it. I'm not sure there's anything else to be done at this point other than closing it. |
[Note: this pull request is suggested as a more comprehensive solution than https://github.com//pull/552
and also resolves https://github.com//issues/482 which is probably still an issue as well as https://github.com//issues/543 although not in the manner suggested therein.]
This fixes several issues for nix-prefetch-git:
Variances in
git submodule statusoutput.Here are some examples of various output from this command:
-20007c90b9c5e49c241df0494077f44d4559af45 submodules/foo
20007c90b9c5e49c241df0494077f44d4559af45 submodules/foo (remotes/origin/stable-3-g20007c9)
20007c90b9c5e49c241df0494077f44d4559af45 submodules/foo ((null))
The original regex processing would only match the first of these.
The observable effect of the other forms of output was complaints
about "too many parameters to 'cd'" because the entire line was
used as the "dir" and "hash" variable values.
Remote URL determination.
The original form attempted to parse the directory from the
git submodule statusoutput above and look that up in the.git/config file to determine the URL. Unfortunately, the
latter file uses the config-ini section name as the index,
as does the .gitmodules file, so if the section name differs
from the subdirectory, this failed and returned a blank URL.
Sample (valid!) .gitmodule contents that would exhibit
this error:
.gitmodules:
[submodule "x86/tests/submodules/dwarf"]
path = deps/dwarf
url = https://github.com/myteam/dwarf.git
This patch resolves both of the above by using the the builtin
git submodule foreachto iterate through the submodules, wherethis command explicitly sets shell variables to represent most
of the items, and the submodule lookup in .git/config is now done
using the submodule name and not the submodule path.