NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.02k stars 14.03k forks source link

fetchYarnDeps doesn't support v2 (YAML) lockfiles #254369

Open delroth opened 1 year ago

delroth commented 1 year ago

Describe the bug

When trying to get fetchYarnDeps to consume the yarn.lock from Grafana (https://github.com/grafana/grafana/blob/main/yarn.lock), it dies with a parsing error:

SyntaxError: Unknown token: { line: 3, col: 2, type: 'INVALID', value: undefined } 3:2 in lockfile
    at Parser.unexpected (/nix/store/b1wnrddjgg85i1w4kzgsp8mksa344abx-prefetch-yarn-deps/libexec/yarnpkg-lockfile.js:5064:11)
    at Parser.parse (/nix/store/b1wnrddjgg85i1w4kzgsp8mksa344abx-prefetch-yarn-deps/libexec/yarnpkg-lockfile.js:5193:14)
    at parse (/nix/store/b1wnrddjgg85i1w4kzgsp8mksa344abx-prefetch-yarn-deps/libexec/yarnpkg-lockfile.js:5262:17)
    at module.exports.exports.default (/nix/store/b1wnrddjgg85i1w4kzgsp8mksa344abx-prefetch-yarn-deps/libexec/yarnpkg-lockfile.js:4835:96)
    at prefetchYarnDeps (/nix/store/b1wnrddjgg85i1w4kzgsp8mksa344abx-prefetch-yarn-deps/libexec/index.js:133:28)
    at main (/nix/store/b1wnrddjgg85i1w4kzgsp8mksa344abx-prefetch-yarn-deps/libexec/index.js:187:9)

I suspect that this is because it's a new-style yarn.lock file using the YAML syntax, and not the old "custom" syntax. The yarnpkg-lockfile npm package can't parse it for that reason.

Notify maintainers

cc @lilyinstarlight (not technically maintainer I think, but you touched it last :p)

lilyinstarlight commented 1 year ago

Our tooling still won't actually work with yarn-berry yet, even if it did parse the lockfile. I do want proper yarn-berry support soon though, given a lot of things are starting to use it now

not technically maintainer I think, but you touched it last :p

No I'm not technically maintainer, but unless someone else steps up I'm probably going to be the one to add yarn-berry support to our tooling anyway 🙈

delroth commented 1 year ago

Yeah, I hacked around to add YAML parsing support and found out that there's (unsurprisingly) a lot more things to fix, and I'm very out of my depth with 0 knowledge of that ecosystem unfortunately :) Thank you!

ashkitten commented 9 months ago

i looked into this more with some digging into the yarn codebase and it seems like we've got a few issues:

this is possible to fix but imo the better solution would be to find some way to use yarn cli to do the actual fetching and validation because this is much more complicated

ashkitten commented 9 months ago

UPDATE: the checksum thing is ridiculous. i did some testing and yes, changing the compression level in the yarn config will change the hashes of every single lockfile entry, or fail if --immutable is passed.

the default compress level is 0, so it's not like this will always break...

i'm leaning towards just running yarn install --immutable --mode skip-scripts instead of reimplementing their own logic but worse. i'm not entirely sure why we don't always just do that, i think it's probably because we operate off of only the lockfile and yarn itself requires the presence of a package.json as well.

EDIT: bug report submitted upstream here: https://github.com/yarnpkg/berry/issues/6068

dotlambda commented 9 months ago

i'm not entirely sure why we don't always just do that

We need to use a format that's guaranteed to be stable across different versions of Yarn.

ShamrockLee commented 8 months ago

If builtins.fromYaml (NixOS/nix#7340) gets merged, would it be possible to use the hashes from yarn.lock directly and get rid of other sort of vendor hashs?

Atemu commented 8 months ago

I don't see us using fromYAML any time soon even if it was merged tomorrow because it'd break eval with old versions of Nix.

ShamrockLee commented 8 months ago

I don't see us using fromYAML any time soon even if it was merged tomorrow because it'd break eval with old versions of Nix.

Not sure about the process to determine when to use a built-in function, but it's sensible to wait at least until the stable branch-off after it hits Nixpkgs.

https://discourse.nixos.org/t/when-is-a-nix-builtin-function-builtins-considered-ready-to-be-used-inside-nixpkgs/35369/2

eclairevoyant commented 8 months ago

There's a sizable chunk of users still on nix 2.3. I don't see a good reason to break eval for them. And if it has to be polyfilled anyway, then we don't even need to wait for the PR merge to do so.

szlend commented 6 months ago

I'm not sure if this checks off all the stability requirements for nixpkgs, but if you need to package something for personal use, you can do something like this:

Expand source ```nix { fetchFromGitHub , stdenvNoCC , yarn-berry , nodejs , cacert }: stdenvNoCC.mkDerivation (finalAttrs: { pname = "spectral"; version = "6.11.1"; src = fetchFromGitHub { owner = "stoplightio"; repo = "spectral"; rev = "v${finalAttrs.version}"; sha256 = "sha256-ivuu7kpVJIn2RiFAe/slMffcisVeq7yeG1TwoXMOMeA="; }; nativeBuildInputs = [ yarn-berry nodejs ]; yarnOfflineCache = stdenvNoCC.mkDerivation { name = "spectral-deps"; nativeBuildInputs = [ yarn-berry ]; inherit (finalAttrs) src; NODE_EXTRA_CA_CERTS = "${cacert}/etc/ssl/certs/ca-bundle.crt"; supportedArchitectures = builtins.toJSON { os = [ "darwin" "linux" ]; cpu = [ "arm" "arm64" "ia32" "x64" ]; libc = [ "glibc" "musl" ]; }; configurePhase = '' runHook preConfigure export HOME="$NIX_BUILD_TOP" export YARN_ENABLE_TELEMETRY=0 yarn config set enableGlobalCache false yarn config set cacheFolder $out yarn config set supportedArchitectures --json "$supportedArchitectures" runHook postConfigure ''; buildPhase = '' runHook preBuild mkdir -p $out yarn install --immutable --mode skip-build runHook postBuild ''; dontInstall = true; outputHashAlgo = "sha256"; outputHash = "sha256-yxOOt665fyNjayRic9nlWdwk9RXeQ4oN3e91dOUvjLg="; outputHashMode = "recursive"; }; configurePhase = '' runHook preConfigure export HOME="$NIX_BUILD_TOP" export YARN_ENABLE_TELEMETRY=0 yarn config set enableGlobalCache false yarn config set cacheFolder $yarnOfflineCache runHook postConfigure ''; buildPhase = '' runHook preBuild yarn install --immutable --immutable-cache yarn build yarn workspaces focus --all --production runHook postBuild ''; installPhase = '' runHook preInstall mkdir -p $out/{bin,lib} cp -r node_modules $out/lib/ for package in packages/*; do mkdir -p $out/lib/$package cp -r $package/{dist,package.json} $out/lib/$package/ done ln -s $out/lib/node_modules/.bin/spectral $out/bin/spectral runHook postInstall ''; fixupPhase = '' runHook preFixup patchShebangs $out/lib runHook postFixup ''; doInstallCheck = true; installCheckPhase = '' $out/bin/spectral --version ''; }) ```
dotlambda commented 6 months ago

@delroth Why was this closed?

szlend commented 6 months ago

Still an issue.

Turns out you can make the deps reproducible between (non-windows) platforms by predefining the list of supported architecture like so:

supportedArchitectures = builtins.toJSON {
  os = [ "darwin" "linux" ];
  cpu = [ "arm" "arm64" "ia32" "x64" ];
  libc = [ "glibc" "musl" ];
};

configurePhase = ''
  # ...
  yarn config set supportedArchitectures --json "$supportedArchitectures"
'';

This makes it download all platform/architecture specific dependencies. I updated the example in my previous comment.