Skip to content

nodejs: fix libv8 object files list#337360

Merged
Ma27 merged 1 commit intoNixOS:stagingfrom
tie:pg-nodejs-libv8
Sep 2, 2024
Merged

nodejs: fix libv8 object files list#337360
Ma27 merged 1 commit intoNixOS:stagingfrom
tie:pg-nodejs-libv8

Conversation

@tie
Copy link
Member

@tie tie commented Aug 25, 2024 •
edited
Loading

Description of changes

See #327653 (comment)

Well, we have an outdated v8 package in Nixpkgs and #147506 introduced a libv8 output for Node.js package with v8 static library.

[…] looks like I’ve missed that these object files are actually under completely different paths when built with Ninja (line 349 in particular does not match any files).

Manually verified with ar t that nodejs_20.libv8 has the same list of object files as before and tested with

nix build --file . postgresql.pkgs.plv8.tests.smoke

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 25, 2024
@tie tie requested a review from wolfgangwalther August 25, 2024 23:55
@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: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Aug 26, 2024
@tie tie force-pushed the pg-nodejs-libv8 branch from 6777c9a to b3dd92e Compare August 26, 2024 11:00
@tie tie marked this pull request as ready for review August 26, 2024 11:04
@ofborg ofborg bot added 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. and removed 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Aug 26, 2024
wolfgangwalther
wolfgangwalther approved these changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

I confirmed this fixed postgresql.pkgs.plv8.tests for me.

@tie tie mentioned this pull request Aug 28, 2024
Merged
13 tasks
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 31, 2024
Ma27
Ma27 approved these changes 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.

LGTM.

But I'm not a nodejs maintainer, so I'll refrain from merging until a nodejs maintainer has taken a look at this.

@Ma27 Ma27 merged commit efbf70d into NixOS:staging Sep 2, 2024
@tie tie deleted the pg-nodejs-libv8 branch September 2, 2024 07:10
@aduh95 aduh95 mentioned this pull request Sep 6, 2024
Closed
@jackwilsdon jackwilsdon mentioned this pull request Oct 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ma27 Ma27 Ma27 approved these changes

@wolfgangwalther wolfgangwalther wolfgangwalther approved these changes

@aduh95 aduh95 aduh95 approved these changes

@danbst danbst Awaiting requested review from danbst

@ivan ivan Awaiting requested review from ivan

@globin globin Awaiting requested review from globin

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

Assignees

No one assigned

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. 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

@tie @Ma27 @wolfgangwalther @aduh95 @wegank