search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.3k stars 13.54k forks source link

Migrate existing usages of fixup_yarn_lock with fixup-yarn-lock from prefetch-yarn-deps #240174

Open lilyinstarlight opened 1 year ago

lilyinstarlight commented 1 year ago

Issue description

This is a follow-up to #214062 to migrate existing packages which use fixup_yarn_lock (from yarn2nix) to fixup-yarn-lock (from prefetch-yarn-deps)

It should be a drop-in replacement, just by adding prefetch-yarn-deps to nativeBuildInputs and changing the fixup_yarn_lock invocation to fixup-yarn-lock

cc @NixOS/node

Packages to update

Stunkymonkey commented 9 months ago

I started doing the package docuseal which used fixup_yarn_lock (worked). When using the new "drop-in-method" (fixup-yarn-lock) i am getting:

/nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:2325
    throw error;
    ^

Invariant Violation: Commit hash required
    at invariant (/nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:2318:15)
    at Extract.<anonymous> (/nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:89267:11)
    at Extract.emit (node:events:529:35)
    at finishMaybe (/nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:74117:14)
    at /nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:74095:5
    at module.exports.Extract._final (/nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:151032:3)
    at callFinal (/nix/store/ypig039xrfd6p355v3diy9avbs58zywj-yarn-1.22.19/libexec/yarn/lib/cli.js:74088:10)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  framesToPop: 1
}

I did it the same way the other two PRs were done. Any idea?

lilyinstarlight commented 9 months ago

It seems to be due to yarn not liking missing a commit hash in both the resolved URL and version spec here: https://github.com/docusealco/docuseal/blob/3c69430b947c1c1ea61e25ad9275b14df005a3c7/package.json#L10

I've got a fix for fixup-yarn-lock that I'll PR imminently

Stunkymonkey commented 9 months ago

@lilyinstarlight what about the following files? they use the fixup_yarn_lock as well:

should these be added at the top?

lilyinstarlight commented 9 months ago

should these be added at the top?

Yeah, I imagine they didn't use it at the time I made this tracking issue (also wow are there a lot of open PRs now!)

Feel free to edit them in yourself or I'll get around to it when I have some time to get through some of these, thank you so much! :)

TomaSajt commented 9 months ago

I'll try to rework the redisinsight derivation, 'cuz I think it was broken to begin with.

SuperSandro2000 commented 7 months ago

It should be a drop-in replacement, just by adding prefetch-yarn-deps to nativeBuildInputs and changing the fixup_yarn_lock invocation to fixup-yarn-lock

I have encountered one issue with that. fixup-yarn-lock has an extra dependency on nix-prefetch-git and through that git-lfs which busted a cache for me because I update my standard golang compiler. Is there a possibility to reduce the dependencies of prefetch-yarn-deps/nix-prefetch-git?

SuperSandro2000 commented 7 months ago

I did a POC with that in #281902 which I still need to validate.

Stunkymonkey commented 4 months ago

currently only redisinsight is missing (currently failing on master). The rest is migrated. Do we keep this issue open?

lilyinstarlight commented 4 months ago

currently only redisinsight is missing (currently failing on master). The rest is migrated. Do we keep this issue open?

If it's all done, I'm happy for it to be closed :)

Thank you for all of the work <3

codingCoffee commented 4 months ago

Hey, came across this issue while trying to install the redisinsight package. The installation still seems to be broken, is there any temporary workaround for installing it?

pyrox0 commented 4 months ago

Packages that haven't been migrated:

Would there be a way/an incentive to disallow new packages from using fixup_yarn_lock? Alternatively, once all usages of it are removed from the tree, is there any reason to keep it around? If there is a reason, should there be a deprecation notice added when fixup_yarn_lock is used?

Stunkymonkey commented 2 months ago

@pyrox0 thanks. added them to the list with the MRs.