Skip to content

Comments

nodejs: fix cross compilation for non-emulatable platforms#384413

Merged
rhelmot merged 1 commit intoNixOS:stagingfrom
rhelmot:freebsd-nodejs
Mar 20, 2025
Merged

nodejs: fix cross compilation for non-emulatable platforms#384413
rhelmot merged 1 commit intoNixOS:stagingfrom
rhelmot:freebsd-nodejs

Conversation

@rhelmot
Copy link
Contributor

@rhelmot rhelmot commented Feb 23, 2025

This was tested x86_64-linux -> x86_64-freebsd. It works by injecting the tools that would otherwise be run at build time for the same executables extracted from a native build.

Without this, there are multiple issues:
a) It will fail to start building since there are naming conflicts
between the "host" (build system) and "normal" (host system) object
files. This is resolved with a patch from buildroot.
b) It will then still fail to build since it will still try to build the
"host" objects, with a host compiler, but it will use the
configuration flags for the "target" OS. This is resolved by
importing the executables that would otherwise be run on the build
system from the intermediate stage of a native build, saved in a new
"dev" output. We also fake the "host" compiler as a tool which simply
touches its outputs.
c) Finally, there is a clang bug which causes a static assert that
something is trivially copyable to fire as a false positive. We
remove this check with a patch from rubyjs.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • x86_64-freebsd (cross)
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

@ofborg ofborg bot added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Feb 23, 2025
@github-actions github-actions bot added the 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment label Feb 23, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin 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. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels Feb 23, 2025
@nix-owners nix-owners bot requested a review from aduh95 February 23, 2025 03:02
Comment on lines +17 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

For non-upstreamable patches, I think it makes more sense to vendor them in the repo rather than fetching them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes more sense for us to use another distro's patches if we're not super clear on the patch's long term situation because we can refer back to the other distro in order to see how they adjust the patch moving forward. If you think this is a silly/counterproductive/antisocial/etc argument I will gladly vendor it though!

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

I haven't tested the cross compilation, but the change LGTM

@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 Feb 24, 2025
@rhelmot
Copy link
Contributor Author

rhelmot commented Mar 5, 2025

An additional data point: I finally deployed a real application (immich) with node built cross for FreeBSD, and it works perfectly. There are some more patches coming in a separate PR for npm stuff, but it is definitely working without issue.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 16, 2025
This was tested x86_64-linux -> x86_64-freebsd. It works by injecting
the tools that would otherwise be run at build time for the same
executables extracted from a native build.

Without this, there are multiple issues:
a) It will fail to start building since there are naming conflicts
   between the "host" (build system) and "normal" (host system) object
   files. This is resolved with a patch from buildroot.
b) It will then still fail to build since it will still try to build the
   "host" objects, with a host compiler, but it will use the
   configuration flags for the "target" OS. This is resolved by
   importing the executables that would otherwise be run on the build
   system from the intermediate stage of a native build, saved in a new
   "dev" output. We also fake the "host" compiler as a tool which simply
   touches its outputs.
c) Finally, there is a clang bug which causes a static assert that
   something is trivially copyable to fire as a false positive. We
   remove this check with a patch from rubyjs.
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 19, 2025
@github-actions github-actions bot added 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. and removed 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Mar 19, 2025
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Mar 19, 2025
@rhelmot rhelmot merged commit 6a8334d into NixOS:staging Mar 20, 2025
27 checks passed
@rhelmot rhelmot deleted the freebsd-nodejs branch March 20, 2025 06:53
@cyclic-pentane cyclic-pentane mentioned this pull request May 18, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: cross-compilation Building packages on a different platform than they will be used on 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: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants