fetchYarnBerryDeps: init

[yuyuyureka]

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• 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
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[SuperSandro2000]

Noise!

Isn't there a yarn command to add missing hashes or am I imagining things? I would prefer that over maintaining an extra hashes file.

[TomaSajt]

Another thing that should be addressed IMO:

There are certain node modules that cannot be rebuilt without the network, but have an install or rebuild script that runs automatically when running yarn install.
Some, like @electron/get have an env var to disable it, (e.g. ELECTRON_SKIP_BINARY_DOWNLOAD,) but other packages might not.

yarn install can take the --mode=skip-build flag to not run the scripts automatically.

There should be a way to access this behaviour, maybe there should be a yarnInstallFlags array.

---

for reference, npmHooks.npmConfigHook does something like this:

npm ci --ignore-scripts $npmInstallFlags $npmFlags
patchShebangs node_modules
npm rebuild $npmRebuildFlags $npmFlags

(To solve the problem mentioned above, we could add --ignore-scripts to npmFlags or npmRebuildFlags to make the last line essentially a NOOP)

[yuyuyureka]

Yes, the packages attempting to download stuff in prepack (for git deps) or postInstall are very annoying, and have to be handled on a package by package basis. But IMO every one of them we encounter is a win for this type of packaging, because it exposes something that was potentially unsafe with the old method.
Disabling the build scripts is one way to handle it, but needs to be decided on a case-by-case basis and might unexpectedly break things (also at runtime).

I noticed the issue of patching shebangs before running rebuilds, and was planning something like this (would have been included in the push to your ytmdesktop branch once you responded): 02e3b55

@Kranzes solved the extra flags in openbao using YARN_ENABLE_SCRIPTS environment variable, but I'd accept a merge request adding yarnInstallFlags.

[TomaSajt]

I noticed the issue of patching shebangs before running rebuilds, and was planning something like this (would have been included in the push to your ytmdesktop branch once you responded): 02e3b55

You can push to that branch.
I have not encountered anything on that PR that required me to patch the shebangs, so I'm interested to hear where it was needed.

[TomaSajt]

Okay, another thing I found:

Try using the fetcher on mx-puppet-discord's lockfile.
(do note that it has been marked as broken because it wanted to use the now removed nodejs_18)

The lockfile has some lines with the following resolution value:

  resolution: "@mx-puppet/better-discord.js@npm:12.5.1::__archiveUrl=https%3A%2F%2Fgitlab.com%2Fapi%2Fv4%2Fprojects%2F35796068%2Fpackages%2Fnpm%2F%40mx-puppet%2Fbetter-discord.js%2F-%2F%40mx-puppet%2Fbetter-discord.js-12.5.1.tgz"

which the fetcher cannot handle

[yuyuyureka]

I implemented the __archiveUrl binding and tagged a new release: #401813

Also: happy to report that nix-update does not require any changes to be able to update the offlineCache hashes of fetchYarnBerryDeps.

[euank]

Thanks for the work to get this merged, @yuyuyureka, very much appreciated!
I've been fighting with a lock file over in #392737 which this will make way easier.

Unfortunately, one of anki's deps (license-checker-rseidelsohn) hits a pathological case in the yarn-fetcher code, so I can't actually get the fetcher to complete.

I've filed an issue upstream, happy to instead file it over here on the nixpkgs repo if that seems better

[yuyuyureka]

The fix for the performance issue has also been included in #401813

[Ma27]

At least you could have waited for @Ma27 to finish the review.

Sorry, I guess I should've been more explicit here: I don't know much about the current state & internals of JS packaging. I looked at this from the perspective of a package maintainer who takes care of a package (grafana) that's affected by this change.

node-gyp still needs to run, but this is done in yarnBerryConfigHook automatically by setting npm_config_nodedir and npm_config_node_gyp, (inspired by how npmConfigHook does it)

@yuyuyureka if you don't mind, just a question because I'm not sure I fully understand this: the node-gyp is needed for some dependencies, but not for the things in grafana's missing-hashes file (i.e. @rollup/rollup-..., @parcel/watcher-... etc) since we fetch the native versions here already. And this file is needed because these native things aren't locked because of

Their reasoning for this is that the local cache files (which are meant to be checked in and distributed over git) for these dependencies are ignored and the platform-specific dependencies are re-fetched on each user's computer

right?

[yuyuyureka]

There is no change in which dependencies are installed into node_modules, including native dependencies whose hashes are missing in the yarn.lock file.
They would have been downloaded by the yarn install in the FOD before this change too, just without verifying any hashes.

I don't know which of grafana's dependencies use node-gyp, but it's the same dependencies that would have used it before. A quick grep on the build logs reveals that @tree-sitter-grammars/tree-sitter-yaml seems to be one of them.

The missing-hashes.json file is needed, because yarn requires some cache files whose hashes are missing from yarn.lock to perform a normal yarn install. This has nothing to do with node-gyp.

[c3n21]

Does this support pnp?

I am trying to package verdaccio 6.1.2 which uses yarn berry and pnp for node modules resolution but it seems to break.

You can reproduce it like this:

git apply verdaccio.PATCH
git add pkgs/by-name/ve/verdaccio/package.nix
nix build  .#legacyPackages.x86_64-linux.verdaccio
./result/bin/verdaccio

The last command will throw an error about requiring a non-existent dependency

verdaccio.PATCH

[TomaSajt]

I don't think pnp is supported ATM. Seems like the packageLocation fields are all pointing to the yarn cache, but the cache is discarded after the build.

You should somehow configure it to use node-modules nodeLinker. Not sure where/how you can do that, but I'll experiment a bit.

If you still want to use the .pnp.cjs method, you'd probably have to copy the cache as well. I have not tried this method yet.

[TomaSajt]

I made it use node_modules by editing .yarnrc.yml via yarn config set
and then I used npmHooks.npmInstalHook to automatically install the files into $out
I also had to add dontNpmPrune = true, because it seemed to get stuck at that step. IDK why. Maybe just very very slow.

  preConfigure = ''
    yarn config set nodeLinker node-modules
  '';

  nativeBuildInputs = [
    nodejs
    yarn-berry
    yarn-berry.yarnBerryConfigHook
    npmHooks.npmInstallHook
  ];

  offlineCache = yarn-berry.fetchYarnBerryDeps {
    inherit (finalAttrs) src;
    hash = "sha256-jzkmDxQtIFMa1LIPcvKKsXIItPntgXTavoWhd5eZWyQ=";
  };

  buildPhase = ''
    runHook preBuild
    yarn run build
    runHook postBuild
  '';

  # this would run npm-specific logic that seems to get stuck
  dontNpmPrune = true;

npmHooks.npmInstallHook is pretty useful for packaging CLIs otherwise installed via npm install -g

[TomaSajt]

After checking 5 random apps that use fetchYarnBerryDeps it seems like each one has nodeLinker: node-modules in their .yarnrc.yml file.

The current tooling assumes we have node_modules, so I guess we should probably add yarn config set nodeLinker node-modules to the config hook, no?

[yuyuyureka]

I don't know the install process for applications using the pnp linker, but if it gets up to the yarn install, then I would say the current code supports it and it just needs more tooling to make the installation work?

[TomaSajt]

Okay, I came up with this solution which doesn't even use the yarnBerryConfigHook:

{
  lib,
  stdenv,
  fetchFromGitHub,
  makeWrapper,
  yarn-berry_3,
}:

let
  yarn-berry = yarn-berry_3;
in
stdenv.mkDerivation (finalAttrs: {
  pname = "verdaccio";
  version = "6.1.2";

  src = fetchFromGitHub {
    owner = "verdaccio";
    repo = "verdaccio";
    tag = "v${finalAttrs.version}";
    hash = "sha256-EssvN5HtGI5Hmw4EXetj5nzrkBZAAJGgOx09dlYJzhI=";
  };

  strictDeps = true;

  nativeBuildInputs = [
    makeWrapper
    yarn-berry
  ];

  configurePhase = ''
    runHook preConfigure

    export HOME=$(mktemp -d)
    yarn config set enableTelemetry false

    # all dependencies are vendored in-repo in .yarn/cache
    # we still need to run this because "unplugged packages must be fully materialized on disk to work"
    yarn install

    runHook postConfigure
  '';

  buildPhase = ''
    runHook preBuild
    yarn run build
    runHook postBuild
  '';

  installPhase = ''
    runHook preInstall

    mkdir -p "$out/share"
    cp -r . "$out/share/verdaccio"

    # PnP based pacakges need `yarn node` instead of `node` to run binaries
    makeWrapper ${lib.getExe yarn-berry} "$out/bin/verdaccio" \
      --chdir "$out/share/verdaccio" \
      --add-flags "node ./bin/verdaccio"

    runHook postInstall
  '';
})

seems like the main issue was trying to run the binary using node instead of yarn node, because node doesn't respect the .pnp.cjs file.

In this, I just copied everything into share/verdaccio, but there may be unnecessary files this way.

[TomaSajt]

What if we update the config hook so that it detects if you're using PnP via yarn config get nodeLinker (it even works if it's unset in the .yarnrc.yml) and it just doesn't replace the local yarn cache + it doesn't run the patchShebangs on the non-existant node_modules directory (assuming we merge that other PR which runs patchShebang node_modules).

So basically it would only do what I did manually in configurePhase.

This code-path wouldn't even require the offlineCache variable.

We could also make it toggleable via a variable instead of using the yarn config, but IMO the config method is cleaner.

[SomeoneSerge]

Hi! Just to make this explicit, because the manual currently doesn't, are missing-hashes.json expected to be non-reproducible/quick-to-rot, and is that the reason we don't wrap them into another FOD?