Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
39 tasks done
lilyinstarlight opened this issue Jun 27, 2023 · 12 comments
Closed
39 tasks done
Labels
3.skill: sprintable A larger issue which is split into distinct actionable tasks 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems 6.topic: nodejs

Comments

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Jun 27, 2023

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
Copy link
Contributor

Stunkymonkey commented Nov 15, 2023

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
Copy link
Member Author

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
Copy link
Contributor

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

  • pkgs/applications/networking/cluster/tilt/assets.nix
  • pkgs/applications/networking/instant-messengers/hydrogen-web/unwrapped.nix
  • pkgs/development/python-modules/dash/default.nix
  • pkgs/development/tools/electron/common.nix
  • pkgs/development/tools/redisinsight/default.nix
  • pkgs/servers/gotify/ui.nix
  • pkgs/servers/monitoring/grafana-agent/default.nix

should these be added at the top?

@lilyinstarlight
Copy link
Member Author

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
Copy link
Contributor

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

@SuperSandro2000
Copy link
Member

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?

@yu-re-ka yu-re-ka mentioned this issue Jan 18, 2024
13 tasks
@SuperSandro2000
Copy link
Member

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

@Stunkymonkey
Copy link
Contributor

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

@lilyinstarlight
Copy link
Member Author

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

@samueldr samueldr added the 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems label Apr 23, 2024
@codingCoffee
Copy link
Contributor

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
Copy link
Member

pyrox0 commented May 4, 2024

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
Copy link
Contributor

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

@github-project-automation github-project-automation bot moved this from Tracking Issue to Done in Node.js Team Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.skill: sprintable A larger issue which is split into distinct actionable tasks 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems 6.topic: nodejs
Projects
Status: Done
Development

No branches or pull requests

7 participants