Skip to content

nodejs_22: re-add patch to fix sandboxed builds using gyp on Darwin#416077

Merged
FliegendeWurst merged 1 commit intoNixOS:stagingfrom
fugidev:nodejs_22-darwin-fix
Jun 17, 2025
Merged

nodejs_22: re-add patch to fix sandboxed builds using gyp on Darwin#416077
FliegendeWurst merged 1 commit intoNixOS:stagingfrom
fugidev:nodejs_22-darwin-fix

Conversation

@fugidev
Copy link
Member

@fugidev fugidev commented Jun 12, 2025

Fixes a regression introduced by #349157, which removed gypPatches for nodejs_22.

As was noted in the past, for some reason Node.js v22 vendors two different gyp versions:

  • tools/gyp/
  • deps/npm/node_modules/node-gyp/gyp/

v22.10.0 updated gyp in the former directory, making the patch in question obsolete. However, the latter vendored gyp still needs to be patched.

Apart from some flaky checks that seem unrelated to me, I've successfully tested building nodejs_22 and a yarn-berry package I'm working on, which failed before.

Maintainer ping: @aduh95

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jun 12, 2025
@github-actions github-actions bot added 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Jun 12, 2025
@nix-owners nix-owners bot requested a review from aduh95 June 12, 2025 08:39
@aduh95
Copy link
Contributor

aduh95 commented Jun 12, 2025

nit: it might be better to refactor gyp-patches.nix to get the patch from there, but the current approach also LGTM.

I'm able to build this when rebased on top of staging-next.

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 13, 2025
@fugidev fugidev mentioned this pull request Jun 15, 2025
13 tasks
@aduh95 aduh95 requested a review from FliegendeWurst June 16, 2025 21:38
@github-actions github-actions bot removed the 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. label Jun 16, 2025
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
url = "https://github.com/nodejs/gyp-next/commit/706d04aba5bd18f311dc56f84720e99f64c73466.patch";
url = "https://github.com/nodejs/gyp-next/commit/706d04aba5bd18f311dc56f84720e99f64c73466.patch?full_index=1";

Or use the old fetchpatch. #257446

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW it's unlikely to actually cause problems given the low activity on nodejs/gyp-next – but sure, let's fix it. gyp-patches.nix has the same issue btw:

url = "https://github.com/nodejs/gyp-next/commit/706d04aba5bd18f311dc56f84720e99f64c73466.patch";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix gyp-patches.nix.

FWIW it's unlikely to actually cause problems given the low activity on nodejs/gyp-next

It is possible to break the hash with some targeted commits in a fork, I think. (Though I really doubt anyone would try to ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I committed the fix here

@fugidev fugidev force-pushed the nodejs_22-darwin-fix branch from a150388 to f0b3061 Compare June 17, 2025 06:53
@FliegendeWurst FliegendeWurst merged commit 01a999f into NixOS:staging Jun 17, 2025
12 of 16 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jun 17, 2025

Successfully created backport PR for staging-25.05:

@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Jun 17, 2025
@fugidev fugidev deleted the nodejs_22-darwin-fix branch June 28, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants