Skip to content

nodejs_{18,20}: fix build with LLVM 19#356408

Merged
emilazy merged 3 commits intoNixOS:llvm-19from
emilazy:push-kqzsvnurmvxv
Nov 20, 2024
Merged

nodejs_{18,20}: fix build with LLVM 19#356408
emilazy merged 3 commits intoNixOS:llvm-19from
emilazy:push-kqzsvnurmvxv

Conversation

@emilazy
Copy link
Member

@emilazy emilazy commented Nov 16, 2024 •
edited
Loading

First commit cherry‐picked from Randy’s branch. I’ve omitted the return address signing commit as it seems there was upstream churn.

@aduh95 It seems like we try to use shared versions of these libraries? Does that not work on older versions, or not work on Darwin, or something?

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/)
  • 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.

@github-actions github-actions bot added the 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment label Nov 16, 2024
@emilazy emilazy changed the title nodejs_{18,20}: fix build with clang 18 on Darwin nodejs_{18,20}: fix build with LLVM 19 Nov 16, 2024
@emilazy emilazy marked this pull request as draft November 16, 2024 16:39
@emilazy emilazy marked this pull request as ready for review November 16, 2024 17:15
@emilazy
Copy link
Member Author

emilazy commented Nov 16, 2024

They both build successfully, although I’d still like to understand the Zlib situation better and hopefully de‐vendor it instead if possible.

@aduh95 The V8 patches here might be worth considering for backporting to these Node releases.

@ofborg ofborg bot requested a review from aduh95 November 16, 2024 23:32
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Nov 16, 2024
@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: qt/kde Object-oriented framework for GUI creation 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: printing Drivers, CUPS & Co. 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: stdenv Standard environment 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 6.topic: lib The Nixpkgs function library 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Nov 17, 2024
@github-actions github-actions bot removed 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 6.topic: lib The Nixpkgs function library 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 6.topic: k3s Kubernates distribution (https://k3s.io/) 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions labels Nov 17, 2024
@emilazy
Copy link
Member Author

emilazy commented Nov 17, 2024

Fixed the rebase mishap.

@emilazy emilazy force-pushed the llvm-19 branch 3 times, most recently from 44a914d to c106c88 Compare November 18, 2024 00:15
@emilazy emilazy mentioned this pull request Nov 19, 2024
Merged
15 tasks
paparodeo
paparodeo approved these changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@paparodeo paparodeo left a comment

Choose a reason for hiding this comment

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

patches look straightforward.

@emilazy
Copy link
Member Author

emilazy commented Nov 19, 2024

@ofborg eval

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 19, 2024
@emilazy
Copy link
Member Author

emilazy commented Nov 19, 2024

@aduh95 gentle ping, if you don’t mind taking a look at this :) I feel pretty confident about the V8 backports but would like to understand why the vendored Zlib is getting used at all.

@aduh95
Copy link
Contributor

aduh95 commented Nov 19, 2024

I'm not sure why it's the case, it could be that the vendored one is still compiled without being included in the binary, and I'm not familiar enough with the build tooling to help.

@aduh95
Copy link
Contributor

aduh95 commented Nov 19, 2024

Actually I see that V8 has its own vendored version, separate from the one Node.js uses. I think --shared-zlib only affects Node.js own code, not V8's. There's an open issue for it, but it's quite old: nodejs/node#33848

@emilazy
Copy link
Member Author

emilazy commented Nov 20, 2024

Thanks for the insight and review. Perhaps we can devendor it one day, but it can wait.

@emilazy emilazy merged commit 57269e5 into NixOS:llvm-19 Nov 20, 2024
@emilazy emilazy deleted the push-kqzsvnurmvxv branch November 20, 2024 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 approved these changes

+1 more reviewer

@paparodeo paparodeo paparodeo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@emilazy @aduh95 @paparodeo @wegank @reckenrode