Create yarnBuildHook and yarnConfigHook

[doronbehar]

Motivation for this change

Many packages that use fetchYarnDeps (104 according to git grep), need to run the commands introduced by these new hooks, or variants of them. Any project with a yarn.lock a contributor may wish to package, essentially will need to run some variant of these commands. It'd be better to share this functionality, document it, and hence expose it.

Further more, as discussed with @yu-re-ka at #296856 , mkYarnPackage has issues, and should be replaced by hooks.

Other people who may be interested in this:

@SuperSandro2000 @lilyinstarlight @Scrumplex ( ❤️ )

Things done / TODO

• Fix TODOs in hooks.
• Documentation.
• Release notes.
  external source.
• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

[SuperSandro2000]

Just wanted to say that I am very thankful that you are working on this and trying to get this into shape. 🎉

[SuperSandro2000]

While we are reworking things. We cannot limit this to node_modules/.bin?

[doronbehar]

While we are reworking things. We cannot limit this to node_modules/.bin?

What would be the value in trying that out? Here's an example of a patchShebangs log:

node_modules/acorn/bin/acorn: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/cross-spawn/node_modules/which/bin/node-which: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/findup/bin/findup.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/browserslist/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/envinfo/dist/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/errno/build.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/errno/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/esprima/bin/esvalidate.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/esprima/bin/esparse.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/flat/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/import-local/fixtures/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/he/bin/he: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/js-yaml/bin/js-yaml.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/json5/lib/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/mkdirp/bin/cmd.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/mocha/bin/_mocha: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/mocha/bin/mocha: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/mocha/lib/cli/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/semver/bin/semver: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/terser/bin/terser: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/terser/bin/uglifyjs: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/ts-loader/node_modules/semver/bin/semver.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/tslint/bin/tslint: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/ts-node/dist/bin-script-deprecated.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/ts-node/dist/bin-script.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/ts-node/dist/bin-transpile.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/ts-node/dist/bin.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/typescript/bin/tsc: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/typescript/bin/tsserver: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/webpack/bin/webpack.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/webpack-cli/bin/cli.js: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"
node_modules/which/bin/which: interpreter directive changed from "#!/usr/bin/env node" to "/nix/store/6g9n96qf1yx139xklnmy3v4xhjvjgsji-nodejs-20.12.2/bin/node"

None of the paths above includes node_modules/.bin...

[SuperSandro2000]

or a pattern like node_modules/*/bin. They seem to be all over the place...

[doronbehar]

or a pattern like node_modules/*/bin. They seem to be all over the place...

It's worth checking in anycase whether this patchShebangs is really needed. I'll run a nixpkgs-review on a strong machine with and without it when I finish iterating all packages.

[SuperSandro2000]

Uh, that would be super awesome if we can avoid carrying this forward with us.

[doronbehar]

Similarly to my comment to @viraptor here, I think it should be out of scope for this PR to remove that patchShebangs command, since this is also the behavior of all of these packages before this PR.

This also helps me demonstrate in the little diff --recursive shell loop below how the packages barely change due to this PR. I still kept the TODO comment near there though - maybe someone will be bothered by this in the future and provide a clean solution.

[SuperSandro2000]

Just to confirm: did you check for potential update scripts where we can drop dowloading the package.json file fron upstream?

[doronbehar]

Just to confirm: did you check for potential update scripts where we can drop dowloading the package.json file fron upstream?

Good comment. The files I touched in the meantime don't have an updateScript, but those who do are:

pkgs/applications/office/micropad/update.sh
pkgs/servers/home-assistant/custom-lovelace-modules/zigbee2mqtt-networkmap/update.sh
pkgs/servers/jellyseerr/update.sh
pkgs/servers/matrix-appservice-discord/update.sh
pkgs/tools/admin/meshcentral/update.sh

And most of them do download the package.json file and sometimes even yarn.lock. I noted this list in the TODO of the first comment of the PR.

[doronbehar]

Thanks @pinpox for running nixpkgs-review! Many of the new packages are built again because electron is changed in this PR due to the above commented change. Other then that, the packages that I directly changed to use the hooks (those that appear in the commit log) have been tested by myself.

That redisinsight seems suspicious, I'll test shortly if that is failed on master as well. The Hydra web interface (and hydra-check as well) don't show this job... ( https://hydra.nixos.org/eval/1807565?filter=redisinsight )

I also fixed the merge conflict.

[doronbehar]

redisinsight also fails to build on master...

[doronbehar]

Now with the additional approval, I'll merge this in a few days if no one will comment on this.