-
Notifications
You must be signed in to change notification settings - Fork 29.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: fix V8 compilation on GCC 12 #53728
Conversation
Review requested:
|
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? |
0a9e556
to
c896192
Compare
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). |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Extended the range check to include 12.1 which is also broken. Also changed the referenced link to the GCC fix. |
@richardlau Thanks for catching it, I fixed it in the CL, the commit queue request should be cancelled. |
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
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 |
Landed in 8027a7b |
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>
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>
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>
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>
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>
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>
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>
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>
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