search

svanderburg / node2nix

Generate Nix expressions to build NPM packages
MIT License
527 stars 100 forks source link

fetchgit modernisation #298

Closed ghost closed 1 year ago

ghost commented 2 years ago

These patches implement some upgrades the project's usage a fetchgit function. pkgs.fetchGitPrivate has been removed from master since apparently 2019, and pkgs.fetchGit has some bad behaviour like ignoring the user's ssh_config. This defaults the fetchgit function to builtins.fetchgit to allow fetching from public and private repositories without thinking about it, retaining backwards compatibility with --use-fetchgit-private and even allowing sha256 in case of a user with some niche use-case with their own fancy fetchGit replacement.

svanderburg commented 2 years ago

The changes look ok, but there's one concern I have with making builtins.fetchGit the default. The major difference between builtins.fetchGit and the ordinary fetchgit is that the former runs at instantiation time whereas the latter at realization time. Moreover, the main purpose builtins.fetchGit is to fetch a collection of Nix expressions at evaluation time, although it can also be used for other purposes.

builtins.fetchGit is definitely more convenient and powerful. For example, it does not require anyone to specify an output hash of a checkout and it also respects the user's ssh_config like you have described.

However, when it is desired to integrate generated packages into Nixpkgs or to build Node.js projects on Hydra: the Nix-based continuous integration service, then the evaluation phase becomes much more expensive and Hydra does not show any jobs until the evaluation phase completes.

As far as I know using builtins.fetchGit in Nixpkgs is not a standard practice yet.

I'm not against integrating this as a feature, but for the reasons mentioned earlier we should also still retain the ability to use the Nixpkgs fetchgit function.