FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: fix V8 compilation on GCC 12 by joyeecheung · Pull Request #53728 · nodejs/node · GitHub

/ node Public

deps: fix V8 compilation on GCC 12 - #53728

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:fix-gcc-12
Jul 25, 2024
ConversationCommits (1)ChecksFiles changed
Merged

deps: fix V8 compilation on GCC 12#53728
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:fix-gcc-12

Conversation

joyeecheung commented Jul 4, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

I am not sure if this is acceptable in the upstream considering they are dropping support for MSVC and GCC support would also be limited in the near future. But this should address the problem of not being able to build Node.js with GCC 12 on certain Linux distros.

EDIT: it has been merged in the upstream in https://chromium-review.googlesource.com/c/v8/v8/+/5679182

Refs: #45427
Refs: nodejs/help#4406
Refs: #53633
Refs: nodejs/help#4430
Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

cc @targos

nodejs-github-bot commented Jul 4, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update

nodejs-github-bot added needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Jul 4, 2024

joyeecheung commented Jul 4, 2024

Copy link
Copy Markdown
Member Author

Upstreamed https://chromium-review.googlesource.com/c/v8/v8/+/5679182 to see if it's acceptable in the upstream...if not I think we should float it to prevent a hole in our GCC >= 10.1 on Linux support matrix?

joyeecheung force-pushed the fix-gcc-12 branch 3 times, most recently from 0a9e556 to c896192 Compare July 4, 2024 18:07

richardlau commented Jul 4, 2024

Copy link
Copy Markdown
Member

I can confirm this fixes building Node.js with the 12.2.0 gcc Docker container (main fails without this in that container, succeeds in the gcc:12.3.0 container).

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 4, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 4, 2024

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view
richardlau mentioned this pull request Jul 5, 2024
Closed
legendecas reviewed Jul 5, 2024
View reviewed changes
Comment thread deps/v8/src/base/macros.h Outdated

joyeecheung commented Jul 5, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Extended the range check to include 12.1 which is also broken. Also changed the referenced link to the GCC fix.

richardlau reviewed Jul 5, 2024
View reviewed changes
Comment thread deps/v8/src/base/macros.h Outdated
joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2024
deps: V8: cherry-pick 35888fee7bba
23faf6a 
Original commit message:

    [base] fix builds with GCC 12 on certain Linux distributions

    With GCC 12 on certain Linux distributions (at least Debian 12,
    Alpine 3.18, Fedora 37, that ships GCC 12.2),
    std::is_trivially_copyable is broken
    and as a result, V8 fails to compile. This patch uses the
    same polyfill on MSVC to make it compile with GCC 12.2.

    See nodejs#45427 for more context.
    Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

    Change-Id: Ie0ab1bb1ec105bacbd80b341adf7dbd8569f031f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5679182
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#95181}

Refs: v8/v8@35888fe

joyeecheung commented Jul 24, 2024

Copy link
Copy Markdown
Member Author

The V8 upstream patch has landed, so updated this PR to be a proper V8 backport.

Can you take a look again? Thanks! @jasnell @santigimeno @lpinca @gengjiawen @richardlau @daeyeon @legendecas

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 24, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 24, 2024

nodejs-github-bot commented Jul 24, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60599/

nodejs-github-bot commented Jul 24, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6094/

nodejs-github-bot commented Jul 24, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6094/

nodejs-github-bot commented Jul 24, 2024

Copy link
Copy Markdown
Collaborator

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6094/

joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 25, 2024
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 25, 2024
nodejs-github-bot merged commit 8027a7b into nodejs:main Jul 25, 2024

nodejs-github-bot commented Jul 25, 2024

Copy link
Copy Markdown
Collaborator

Landed in 8027a7b

targos pushed a commit that referenced this pull request Jul 28, 2024
deps: V8: cherry-pick 35888fee7bba
17dba31 
Original commit message:

    [base] fix builds with GCC 12 on certain Linux distributions

    With GCC 12 on certain Linux distributions (at least Debian 12,
    Alpine 3.18, Fedora 37, that ships GCC 12.2),
    std::is_trivially_copyable is broken
    and as a result, V8 fails to compile. This patch uses the
    same polyfill on MSVC to make it compile with GCC 12.2.

    See #45427 for more context.
    Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

    Change-Id: Ie0ab1bb1ec105bacbd80b341adf7dbd8569f031f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5679182
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#95181}

Refs: v8/v8@35888fe
PR-URL: #53728
Refs: #45427
Refs: nodejs/help#4406
Refs: #53633
Refs: nodejs/help#4430
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit to targos/nodejs-build that referenced this pull request Jul 28, 2024
ansible,jenkins: revert to default gcc-12 on debian12
7faae1f 
Refs: nodejs#3806
Refs: nodejs/node#53728
targos mentioned this pull request Jul 28, 2024
Merged
RafaelGSS mentioned this pull request Jul 30, 2024
Merged
RafaelGSS pushed a commit that referenced this pull request Aug 5, 2024
deps: V8: cherry-pick 35888fee7bba
f4a7ac5 
Original commit message:

    [base] fix builds with GCC 12 on certain Linux distributions

    With GCC 12 on certain Linux distributions (at least Debian 12,
    Alpine 3.18, Fedora 37, that ships GCC 12.2),
    std::is_trivially_copyable is broken
    and as a result, V8 fails to compile. This patch uses the
    same polyfill on MSVC to make it compile with GCC 12.2.

    See #45427 for more context.
    Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

    Change-Id: Ie0ab1bb1ec105bacbd80b341adf7dbd8569f031f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5679182
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#95181}

Refs: v8/v8@35888fe
PR-URL: #53728
Refs: #45427
Refs: nodejs/help#4406
Refs: #53633
Refs: nodejs/help#4430
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit to nodejs/build that referenced this pull request Aug 6, 2024
ansible,jenkins: revert to default gcc-12 on debian12 (#3845)
c806876 
Refs: #3806
Refs: nodejs/node#53728
targos pushed a commit to targos/node that referenced this pull request Aug 9, 2024
deps: V8: cherry-pick 35888fee7bba
e60deab 
Original commit message:

    [base] fix builds with GCC 12 on certain Linux distributions

    With GCC 12 on certain Linux distributions (at least Debian 12,
    Alpine 3.18, Fedora 37, that ships GCC 12.2),
    std::is_trivially_copyable is broken
    and as a result, V8 fails to compile. This patch uses the
    same polyfill on MSVC to make it compile with GCC 12.2.

    See nodejs#45427 for more context.
    Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

    Change-Id: Ie0ab1bb1ec105bacbd80b341adf7dbd8569f031f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5679182
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#95181}

Refs: v8/v8@35888fe
PR-URL: nodejs#53728
Refs: nodejs#45427
Refs: nodejs/help#4406
Refs: nodejs#53633
Refs: nodejs/help#4430
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Aug 14, 2024
deps: V8: cherry-pick 35888fee7bba
9a6b3c4 
Original commit message:

    [base] fix builds with GCC 12 on certain Linux distributions

    With GCC 12 on certain Linux distributions (at least Debian 12,
    Alpine 3.18, Fedora 37, that ships GCC 12.2),
    std::is_trivially_copyable is broken
    and as a result, V8 fails to compile. This patch uses the
    same polyfill on MSVC to make it compile with GCC 12.2.

    See nodejs#45427 for more context.
    Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

    Change-Id: Ie0ab1bb1ec105bacbd80b341adf7dbd8569f031f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5679182
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#95181}

Refs: v8/v8@35888fe
PR-URL: nodejs#53728
Refs: nodejs#45427
Refs: nodejs/help#4406
Refs: nodejs#53633
Refs: nodejs/help#4430
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Aug 16, 2024
deps: V8: cherry-pick 35888fee7bba
415bc75 
Original commit message:

    [base] fix builds with GCC 12 on certain Linux distributions

    With GCC 12 on certain Linux distributions (at least Debian 12,
    Alpine 3.18, Fedora 37, that ships GCC 12.2),
    std::is_trivially_copyable is broken
    and as a result, V8 fails to compile. This patch uses the
    same polyfill on MSVC to make it compile with GCC 12.2.

    See #45427 for more context.
    Refs: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc

    Change-Id: Ie0ab1bb1ec105bacbd80b341adf7dbd8569f031f
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5679182
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#95181}

Refs: v8/v8@35888fe
PR-URL: #53728
Refs: #45427
Refs: nodejs/help#4406
Refs: #53633
Refs: nodejs/help#4430
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #54077
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
wdconinc mentioned this pull request Dec 8, 2024
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
santigimeno santigimeno approved these changes
lpinca lpinca approved these changes
gengjiawen gengjiawen approved these changes
richardlau richardlau approved these changes
daeyeon daeyeon approved these changes
legendecas legendecas approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL