build: compile Node.js with C++20 support #43555

codebytere · 2024-09-04T15:01:03Z

Description of Change

Refs nodejs/node#45427
Refs nodejs/node#52870

V8 updated their required version of c++ in https://chromium-review.googlesource.com/c/v8/v8/+/55
87859 and we did not accordingly follow suit.

Checklist

PR description included and stakeholders cc'd
npm test passes
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: none

deepak1556 · 2024-09-04T15:17:53Z

script/spec-runner.js

  // but don't clobber any other CXXFLAGS that were passed into spec-runner.js
-  const CXXFLAGS = ['-std=c++17', process.env.CXXFLAGS].filter(x => !!x).join(' ');
+  const CXXFLAGS = ['-std=c++20', process.env.CXXFLAGS].filter(x => !!x).join(' ');


Suggested change

const CXXFLAGS = ['-std=c++20', process.env.CXXFLAGS].filter(x => !!x).join(' ');

const CXXFLAGS = ['-std=gnu++20', process.env.CXXFLAGS].filter(x => !!x).join(' ');

if the CI uses gcc <= 9 then it would be -std=gnu++2a but no need to make the condition if it passes with the above.

Not new to this PR -- for example, I see we had gnu++17 before -- but does anyone know if the GNU extensions are actually needed here?

Unless we need the GNU extensions, we'd be better off using the standard, i.e. -std=c++20 here and in the patch above

We don't use it but the headers from Node.js could, but I missed this before but we should actually remove the -std argument from this list, it will get set by common.gypi when building the native modules. We were overriding previously when headers had c++14 and we wanted to use c++17.

@deepak1556 done in dc1e1bb

deepak1556

Thanks!

deepak1556 · 2024-09-04T15:20:29Z

V8 change landed first in Chromium 127, removing 31-x-y label.

deepak1556

LGTM

codebytere · 2024-09-05T09:59:04Z

g++: error: unrecognized command line option '-std=gnu++20'; did you mean '-std=gnu++2a'?

some failures - investigating!

deepak1556 · 2024-09-05T10:05:03Z

@codebytere that is the one I mentioned in #43555 (comment). If the CI uses gcc <= 9 which in this case seems to be we would need to override the CXXFLAGS. You can get the version from g++ -dumpversion.

codebytere · 2024-09-05T10:09:59Z

@deepak1556 ah understood - i read #43555 (comment) as meaning we should just remove it entirely.

deepak1556 · 2024-09-05T10:33:23Z

i read #43555 (comment) as meaning we should just remove it entirely.

You are correct, I did mean we could remove it completely if CI passed by using newer gcc which I was not sure when the comment was made. Given the current failure we had to bring this back.

release-clerk · 2024-09-11T13:01:49Z

No Release Notes

trop · 2024-09-11T13:02:00Z

I was unable to backport this PR to "32-x-y" cleanly;
you will need to perform this backport manually.

trop · 2024-09-11T13:02:12Z

I have automatically backported this PR to "33-x-y", please check out #43684

gdavidkov · 2024-09-12T12:01:48Z

Is it possible to be merged into 32-x-y branch?

amitparida · 2024-09-13T10:05:30Z

When is this change expected to be released in an electron version?

* build: compile with C++20 support * build: update build-image-sha for gcc 10

trop · 2024-09-18T18:54:19Z

@ckerr has manually backported this PR to "32-x-y", please check out #43785

build: compile Node.js with C++20 support (#43555) * build: compile with C++20 support * build: update build-image-sha for gcc 10 Co-authored-by: Shelley Vohr <[email protected]>

Spartan-Hex-Shadow · 2024-09-19T20:43:38Z

Will there be a new 32-x-y release with this issue fixed?

codebytere added semver/patch backwards-compatible bug fixes target/31-x-y PR should also be added to the "31-x-y" branch. target/32-x-y PR should also be added to the "32-x-y" branch. target/33-x-y PR should also be added to the "33-x-y" branch. labels Sep 4, 2024

codebytere requested review from ckerr and deepak1556 September 4, 2024 15:01

codebytere requested a review from a team as a code owner September 4, 2024 15:01

codebytere changed the title ~~build: compile with C++20 support~~ build: compile Node.js with C++20 support Sep 4, 2024

deepak1556 reviewed Sep 4, 2024

View reviewed changes

deepak1556 approved these changes Sep 4, 2024

View reviewed changes

deepak1556 removed the target/31-x-y PR should also be added to the "31-x-y" branch. label Sep 4, 2024

ckerr approved these changes Sep 4, 2024

View reviewed changes

deepak1556 approved these changes Sep 4, 2024

View reviewed changes

codebytere force-pushed the v8-cpp20 branch from dc1e1bb to bdb9ba0 Compare September 5, 2024 10:13

build: compile with C++20 support

54268fb

codebytere force-pushed the v8-cpp20 branch from bdb9ba0 to 54268fb Compare September 10, 2024 17:37

codebytere mentioned this pull request Sep 10, 2024

chore: force gcc/g++ minimum to 10 electron/build-images#37

Merged

codebytere requested a review from a team as a code owner September 10, 2024 21:02

build: update build-image-sha for gcc 10

898b260

erickzhao added the needs-docs An API or larger change that needs expanded documentation label Sep 10, 2024

codebytere force-pushed the v8-cpp20 branch from a721ea8 to 898b260 Compare September 11, 2024 07:53

jkleinsc approved these changes Sep 11, 2024

View reviewed changes

jkleinsc merged commit 90fbf30 into main Sep 11, 2024
50 of 51 checks passed

jkleinsc deleted the v8-cpp20 branch September 11, 2024 13:01

trop bot added needs-manual-bp/32-x-y and removed target/32-x-y PR should also be added to the "32-x-y" branch. labels Sep 11, 2024

trop bot mentioned this pull request Sep 11, 2024

build: compile Node.js with C++20 support #43684

Merged

trop bot added in-flight/33-x-y merged/33-x-y PR was merged to the "33-x-y" branch. and removed target/33-x-y PR should also be added to the "33-x-y" branch. in-flight/33-x-y labels Sep 11, 2024

amitparida mentioned this pull request Sep 13, 2024

[Bug]: /Users/amitpari/.electron-gyp/32.0.2/include/node/v8config.h:13:2: error: "C++20 or later required." #error "C++20 or later required." #43580

Closed

3 tasks

ckerr pushed a commit that referenced this pull request Sep 18, 2024

build: compile Node.js with C++20 support (#43555)

17cc450

* build: compile with C++20 support * build: update build-image-sha for gcc 10

ckerr mentioned this pull request Sep 18, 2024

build: compile Node.js with C++20 support (32-x-y) #43785

Merged

trop bot added in-flight/32-x-y and removed needs-manual-bp/32-x-y labels Sep 18, 2024

richardMSFT mentioned this pull request Sep 19, 2024

Update to Electron 33 microsoft/AzureStorageExplorer#8109

Closed

trop bot added merged/32-x-y PR was merged to the "32-x-y" branch. and removed in-flight/32-x-y labels Sep 19, 2024

serggl mentioned this pull request Oct 11, 2024

"C++20 or later required." when building with Electron 32 nodegit/nodegit#2006

Closed

codebytere mentioned this pull request Nov 1, 2024

chore: bump Node.js to v22.9.0 #44281

Merged

5 tasks

	const CXXFLAGS = ['-std=c++20', process.env.CXXFLAGS].filter(x => !!x).join(' ');
	const CXXFLAGS = ['-std=gnu++20', process.env.CXXFLAGS].filter(x => !!x).join(' ');

build: compile Node.js with C++20 support #43555

build: compile Node.js with C++20 support #43555

Uh oh!

Conversation

codebytere commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

Release Notes

Uh oh!

deepak1556 Sep 4, 2024

Choose a reason for hiding this comment

Uh oh!

ckerr Sep 4, 2024

Choose a reason for hiding this comment

Uh oh!

deepak1556 Sep 4, 2024

Choose a reason for hiding this comment

Uh oh!

codebytere Sep 4, 2024

Choose a reason for hiding this comment

Uh oh!

deepak1556 left a comment

Choose a reason for hiding this comment

Uh oh!

deepak1556 commented Sep 4, 2024

Uh oh!

deepak1556 left a comment

Choose a reason for hiding this comment

Uh oh!

codebytere commented Sep 5, 2024

Uh oh!

deepak1556 commented Sep 5, 2024

Uh oh!

codebytere commented Sep 5, 2024

Uh oh!

deepak1556 commented Sep 5, 2024

Uh oh!

Uh oh!

release-clerk bot commented Sep 11, 2024

Uh oh!

trop bot commented Sep 11, 2024

Uh oh!

trop bot commented Sep 11, 2024

Uh oh!

gdavidkov commented Sep 12, 2024

Uh oh!

amitparida commented Sep 13, 2024

Uh oh!

trop bot commented Sep 18, 2024

Uh oh!

Spartan-Hex-Shadow commented Sep 19, 2024

Uh oh!

Uh oh!

codebytere commented Sep 4, 2024 •

edited

Loading