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

pnpm.fetchDeps: init #290715

Merged
merged 12 commits into from
Jun 6, 2024
Merged

Conversation

Scrumplex
Copy link
Member

@Scrumplex Scrumplex commented Feb 22, 2024

Description of changes

Nowadays pnpm is almost unavoidable when working with any larger Node.js project. In an effort to support this package manager in Nixpkgs, this PR introduces pnpm.fetchDeps which produces a FOD containing a pnpm store that can be used for a reproducible offline build.

Closes #231513

To-dos

  • Disable withNode for pnpm when used in a derivation
  • Add a pnpm server start script to pnpmDeps FODs
  • Support pnpm-workspace.yaml projects

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:
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@getchoo
Copy link
Member

getchoo commented Feb 22, 2024

this could be a near drop-in replacement for methods used in geph, kiwitalk, pot, vesktop, and youtube-music

edit: can also be used in gitbutler and modrinth-app

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Feb 22, 2024
@ofborg ofborg bot requested review from getchoo, vgskye and pluiedev February 22, 2024 22:49
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Feb 22, 2024
@Scrumplex Scrumplex force-pushed the pkgs/build-support/fetchPnpmDeps branch from d9e613d to c4cc9f6 Compare February 23, 2024 21:19
@Scrumplex
Copy link
Member Author

I got rid of the supportedArchitectures code, as --force achieves the same without having to specify all platforms

@hacker1024 hacker1024 mentioned this pull request Feb 25, 2024
13 tasks
@Septias

This comment was marked as resolved.

@dotlambda

This comment was marked as resolved.

@Septias

This comment was marked as resolved.

@gepbird gepbird mentioned this pull request Mar 23, 2024
13 tasks
@nyabinary
Copy link
Contributor

Needs a rebased :3

@Scrumplex Scrumplex force-pushed the pkgs/build-support/fetchPnpmDeps branch from c4cc9f6 to 85a46a9 Compare March 26, 2024 20:20
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 26, 2024
@wegank wegank force-pushed the pkgs/build-support/fetchPnpmDeps branch from 85a46a9 to d39acd5 Compare April 27, 2024 05:07
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jun 6, 2024

We should have put the overrides for pnpm for the consuming packages inside the packages. pnpm is really bitchy about a wrong version and that needs to be closely tracked.

Also maybe we can add some small script, to detect this and catch this error early.

youtube-music-pnpm-deps> Running phase: installPhase
youtube-music-pnpm-deps>  WARN  using --force I sure hope you know what you are doing
youtube-music-pnpm-deps>  WARN  Ignoring not compatible lockfile at /build/source/pnpm-lock.yaml
youtube-music-pnpm-deps>  ERR_PNPM_LOCKFILE_CONFIG_MISMATCH  Cannot proceed with the frozen installation. The current "overrides" configuration doesn't match the value found in the lockfile
youtube-music-pnpm-deps>
youtube-music-pnpm-deps> Update your lockfile using "pnpm install --no-frozen-lockfile"

@doronbehar
Copy link
Contributor

We should have put the overrides for pnpm for the consuming packages inside the packages.

I don't understand this sentence...

@SuperSandro2000
Copy link
Member

They are right now in all-packages where you can easily miss them and wonder why your pnpmDeps build is failing for no apparent reason, since the error message is so bad.

see #317739 for my idea on how to improve that

@NyCodeGHG
Copy link
Member

I'm trying to package renovate using pnpm.fetchDeps, but I get the error ERR_PNPM_NO_OFFLINE_META when executing pnpm deploy, looks like this is the same problem as in #309100

I can only reproduce it inside the nix sandbox, outside of it (and in a network namespace without internet access), it works fine, not sure how to debug this further.

build log:
https://gist.github.com/NyCodeGHG/073f40768d21796fac69cd98b8e0a8df

@NyCodeGHG NyCodeGHG mentioned this pull request Jun 6, 2024
13 tasks
@GetPsyched
Copy link
Member

Dropping in here to ask about monorepo/workspaces support. I'm packaging a monorepo project and noticed from the discussion here that it isn't supported yet.

Is there an issue or, better yet, a draft PR that I can track for this?

@pluiedev
Copy link
Contributor

pluiedev commented Jun 6, 2024

Dropping in here to ask about monorepo/workspaces support. I'm packaging a monorepo project and noticed from the discussion here that it isn't supported yet.

Is there an issue or, better yet, a draft PR that I can track for this?

I believe you're looking for #316908

@doronbehar
Copy link
Contributor

I'm trying to package renovate using pnpm.fetchDeps, but I get the error ERR_PNPM_NO_OFFLINE_META when executing pnpm deploy, looks like this is the same problem as in #309100

@NyCodeGHG since the issue #309100 was diagnosed as a workspaces issue, perhaps your issue with renovate is the same? See:

@NyCodeGHG
Copy link
Member

NyCodeGHG commented Jun 7, 2024

I'm not sure if thats the problem, renovate has a pnpm-workspace.yaml file without a single member
don't ask me why

@doronbehar
Copy link
Contributor

I'm not sure if thats the problem, renovate has a pnpm-workspace.yaml file without a single member don't ask me why

Does it help to simply remove it? :)

@NyCodeGHG
Copy link
Member

i just tried, no.
looks like they have that file to prevent pnpm from also installing dependencies from test fixtures

tomhoule added a commit to grafbase/grafbase that referenced this pull request Jun 11, 2024
Since c896aa3, we haven't been able to
build the Grafbase CLI as a nix derivation, due to the breaking upgrade
to pnpm 9 and the lack of support for pnpm 9 in pnpm2nix. Fortunately,
NixOS/nixpkgs#290715 was recently merging into
nixpkgs, so we can use pnpm_9.fetchDeps to re-instate the cli-app
derivation, and as a cascading effect build the cli derivation again.
tomhoule added a commit to grafbase/grafbase that referenced this pull request Jun 11, 2024
Since
c896aa3,
we haven't been able to
build the Grafbase CLI as a nix derivation, due to the breaking upgrade
to pnpm 9 and the lack of support for pnpm 9 in pnpm2nix. Fortunately,
NixOS/nixpkgs#290715 was recently merging into
nixpkgs, so we can use pnpm_9.fetchDeps to re-instate the cli-app
derivation, and as a cascading effect build the cli derivation again.
@wizardlink wizardlink mentioned this pull request Jun 15, 2024
13 tasks
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/help-wanted-planning-for-summer-of-nix-2025/47556/1

@Scrumplex Scrumplex deleted the pkgs/build-support/fetchPnpmDeps branch September 29, 2024 09:01
@Artturin
Copy link
Member

This has to be applied to node2nix otherwise it wont stick

https://github.com/orgs/nix-community/discussions/1523

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nodejs 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add nixpkgs tooling for pnpm-lock.yaml