search

jlesquembre / clj-nix

Nix helpers for Clojure projects
https://jlesquembre.github.io/clj-nix/
Eclipse Public License 2.0
146 stars 22 forks source link

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

Closed bendlas closed 1 month ago

bendlas commented 2 months ago

fix https://github.com/jlesquembre/clj-nix/issues/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.

jlesquembre commented 1 month ago

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.

bendlas commented 1 month ago

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 https://github.com/NixOS/nix/issues/9077#issuecomment-1971216202, 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 https://github.com/jlesquembre/clj-nix/issues/70#issuecomment-1646877557 🤣

bendlas commented 1 month ago

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 commented 1 month ago

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