Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to git dependencies in private/ssh repositories #128

Merged
merged 13 commits into from
Sep 6, 2024

Conversation

bendlas
Copy link
Contributor

@bendlas bendlas commented Aug 6, 2024

fix #70

implements Option 2: Only builtins.fetchGit

I've been running with this option since a year ago, to access my private git repositories. I feel that this is the best option:

Despite using it only with ssh urls myself, I feel like it's also a good bet for http urls, since those may also have credential integration, that builtins.fetchGit can access.

pkgs/mkDepsCache.nix Outdated Show resolved Hide resolved
@jlesquembre
Copy link
Owner

If you've been running it for a year without noticing any performance issues, I think we're good to go with builtins.fetchGit. My only concern was about performance, but it seems that builtins.fetchGit is fast enough.

this allows us to do the same secure hash checking, as fetchgit
to avoid patching shebangs, which would mess with fixed-output
pkgs/mkDepsCache.nix Outdated Show resolved Hide resolved
@bendlas
Copy link
Contributor Author

bendlas commented Sep 4, 2024

I think the builtins.fetchTree variant is super cool.

One issue that I've found with both bultins is, that they are already downloaded during eval (i.e. --dry-run), which could bring back performance concerns, especially in conjunction with the shallow = false;, we need ... none of the private repos git deps I work with are that big.

There is a related issue in nix NixOS/nix#9077 (comment), so maybe we'll benefit from a resolution eventually ...

Your call now. As far as I'm concerned, I'm happy I now also lean more towards having both fetchers. Feel free to sqash if you prefer :-)

P.S. if you are concerned about big repos and that nix issue, then I can also rework this, so that a dependency can take e.g. :clj-nix.git.fetch/builtin true, as noted in #70 .. just tell me what you'd like to do with this ..
P.P.S. seems like I'm just catching up to your comment from a year ago #70 (comment) 🤣

docs/lock-file.md Outdated Show resolved Hide resolved
now that this test uses pkgs.fetchgit again

This reverts commit 690f5c9.
@bendlas
Copy link
Contributor Author

bendlas commented Sep 5, 2024

eh .. I think that test is non deterministic. the same commit on my machine r/n:

± nix develop --command kaocha
integration:   100% [==================================================] 15/15
       unit:   100% [==================================================] 13/13
28 tests, 62 assertions, 0 failures.
± git show HEAD
commit 2f75dde9069a4154d6ecd95abb4197835b54cab4 (HEAD -> builtin-fetchgit, origin/builtin-fetchgit)

@jlesquembre
Copy link
Owner

Thanks a lot, I'm happy with the current version, merging it as it is :)

@jlesquembre jlesquembre changed the title use builtins.fetchGit for git dependencies Add support to git dependencies in private/ssh repositories Sep 6, 2024
@jlesquembre jlesquembre merged commit 07ee3cc into jlesquembre:main Sep 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider builtins.fetchGit for private repositories
2 participants