Skip to content

[24.05 backport] nodejs changes#336570

Merged
dasJ merged 12 commits intoNixOS:staging-24.05from
aduh95:backport-nodejs-changes-24.05
Sep 13, 2024
Merged

[24.05 backport] nodejs changes#336570
dasJ merged 12 commits intoNixOS:staging-24.05from
aduh95:backport-nodejs-changes-24.05

Conversation

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Aug 22, 2024 •
edited
Loading

Description of changes

Generated with

for commit in $(git log HEAD...origin/staging --reverse --pretty=format:"%H" -- pkgs/development/web/nodejs/); do
             echo "About to cherry-pick $commit";
             git cherry-pick -x "$commit";
             read -p "Press any key to resume ...";
done

I only picked the commits that did not break the builds.

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.11 Release Notes (or backporting 23.11 and 24.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 Aug 22, 2024
@ofborg ofborg bot requested a review from cko August 22, 2024 17:56
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally 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: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. labels Aug 22, 2024
@tie tie self-assigned this Aug 28, 2024
@tie
Copy link
Member

tie commented Aug 28, 2024

Hm, did the commit that switches the build system to ninja fail for some reason?
(note that we’d also need #337360 as a followup fix)

@aduh95 aduh95 mentioned this pull request Aug 29, 2024
Closed
13 tasks
Ma27
Ma27 reviewed Sep 1, 2024
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

note that we’d also need #337360 as a followup fix

Given the significant changes to the build-system and the fact that there is in fact fall-out already, I'd like to raise the question whether it's actually a good idea to backport all of that.

Would it be an option to just backport the bumps and bugfixes (if any?)

@aduh95
Copy link
Contributor Author

aduh95 commented Sep 1, 2024

I agree with Maximilian, not backporting that ninja change minimizes the risk of breakage. I don't remember exactly if I dropped it because of a build failure, I think I didn't even try to resolve the conflicts and just skipped it. FWIW I'm not against backporting it ever, but I'd rather not be the one doing it.

Would it be an option to just backport the bumps and bugfixes (if any?)

Unless I'm missing something, that's currently the case. The changes are either version bump, bug fixes, or "ineffective" changes included just to minimize the amount of conflicts (i.e. all test-related changes don't have any impact on the 24.05 branches because we don't run the tests there, see 21e3782).

@Ma27
Copy link
Member

Ma27 commented Sep 1, 2024

FWIW I'm not against backporting it ever, but I'd rather not be the one doing it.

I don't think this needs to be backported at all: yes, further version bumps and all of that will be more painful, but I'd argue it's less of a risk than backporting all the build-system changes. Also, the situation will resolve itself by the end of the year when 24.05 gets retired.

Unless I'm missing something, that's currently the case

I thought this wasn't the case before since the build-system changes were included? But on a second glance, this isn't the case anymore (I guess the NINJA env change was a red herring and I fell for it 🙈 ).

In that case it's fine I'd say.

@tie
Copy link
Member

tie commented Sep 2, 2024 •
edited
Loading

I guess the NINJA env change was a red herring and I fell for it

Yes, I was a bit confused by this as well and was half-expecting other ninja-related changes 😅

Would it be an option to just backport the bumps and bugfixes (if any?)

Perhaps we shouldn’t backport 4b90b5e since it is a non-trivial change to the Node.js build process (i.e. using an emulator instead of building for multiple platforms)? Otherwise the commits in this PR look good to me.

@aduh95
Copy link
Contributor Author

aduh95 commented Sep 3, 2024

Marking as draft until #339252 can be included (because of #336556 (comment)), and 4b90b5e is removed from this PR.

@aduh95 aduh95 marked this pull request as draft September 3, 2024 14:07
K900 and others added 12 commits September 6, 2024 11:01
@K900 @aduh95
nodejs_18: pin to Python 3.11, fix tests with OpenSSL 3.0.14
20c7134 
(cherry picked from commit 7c1177c)
@aduh95
nodejs: use upstream patch instead of custom one
46a28e8 
(cherry picked from commit f75d503)
@aduh95
nodejs_22: 22.4.1 -> 22.5.1
7104914 
(cherry picked from commit 070791f)
@bryango @aduh95
nodejs: use .finalPackage for ease of overriding
08b1da9 
Previously, the nodejs `passthru.pkgs` attribute refers to the package
fixed point derived in the current context, without considering
potential overrides to the main nodejs package.

This is fixed here with the `.finalPackage` attribute, which refers to
the final package fixed point, after potential overrides.

For example, previously, after overriding nodejs with `nodejs.override`,
`nodejs.pkgs` would still refer to the original package, instead of the
overridden one. After this fix, `nodejs.pkgs` would be correctly based
on the overridden `nodejs` package.

(cherry picked from commit ee9a623)
@tie @aduh95
nodejs: fix tests and disable parallel checking
d51b4b9 
(cherry picked from commit d7f46fb)
@tie @aduh95
nodejs: disable failing tests on x86_64-darwin
d6e6de8 
(cherry picked from commit 31d2381)
@aduh95
nodejs: skip tests failing in sandbox in all platforms
0f89c05 
ce685a8 added list of tests failing on sandbox, but for some reason
only for Darwin. We should skip those tests on all platforms otherwise
OfBorg builds are failing.

(cherry picked from commit f5846d2)
@aduh95
nodejs_20: 20.15.1 -> 20.16.0
bac5858 
(cherry picked from commit 35b556d)
@aduh95
nodejs: 22.5.1 -> 22.6.0 (NixOS#333210)
252d986 
(cherry picked from commit 4c38d8a)
@aduh95
nodejs_22: 22.6.0 -> 22.7.0 (NixOS#336556)
87790b4 
(cherry picked from commit c455cc3)
@aduh95
nodejs_20: 20.16.0 -> 20.17.0 (NixOS#336388)
e554d97 
(cherry picked from commit 45db1ff)
@aduh95
nodejs_22: 22.7.0 -> 22.8.0
4682120 
(cherry picked from commit 235ae9c)
@aduh95 aduh95 force-pushed the backport-nodejs-changes-24.05 branch from 15ab207 to 4682120 Compare September 6, 2024 09:14
@aduh95 aduh95 marked this pull request as ready for review September 6, 2024 09:14
@aduh95 aduh95 force-pushed the backport-nodejs-changes-24.05 branch from 4682120 to 64c2100 Compare September 6, 2024 09:22
@aduh95
Copy link
Contributor Author

aduh95 commented Sep 6, 2024 •
edited
Loading

7f3e4cc not found in any pickable branch

I guess I should wait for staging-next to be merged into staging. EDIT: that's done, the cherry-pick workflow no longer reports it as unpickable.

@aduh95 aduh95 force-pushed the backport-nodejs-changes-24.05 branch from 64c2100 to 4682120 Compare September 6, 2024 22:33
@dasJ dasJ merged commit c23b03c into NixOS:staging-24.05 Sep 13, 2024
@aduh95 aduh95 deleted the backport-nodejs-changes-24.05 branch September 16, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ma27 Ma27 Ma27 left review comments

@cko cko Awaiting requested review from cko

+1 more reviewer

@Conni2461 Conni2461 Conni2461 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tie tie

Labels

6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally 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: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@aduh95 @tie @Ma27 @Conni2461 @dasJ @K900 @bryango