-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
nodejs: use ninja for build #327653
nodejs: use ninja for build #327653
Changes from all commits
e5906dd
d162d8e
3cf3f6a
d7f46fb
31d2381
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,42 @@ | ||
Avoids needing xcrun or xcodebuild in PATH for native package builds | ||
|
||
diff --git a/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py b/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py | ||
index a75d8ee..476440d 100644 | ||
--- a/tools/gyp/pylib/gyp/xcode_emulation.py | ||
+++ b/tools/gyp/pylib/gyp/xcode_emulation.py | ||
@@ -522,7 +522,13 @@ class XcodeSettings: | ||
# Since the CLT has no SDK paths anyway, returning None is the | ||
# most sensible route and should still do the right thing. | ||
try: | ||
- return GetStdoutQuiet(["xcrun", "--sdk", sdk, infoitem]) | ||
+ #return GetStdoutQuiet(["xcrun", "--sdk", sdk, infoitem]) | ||
+ return { | ||
+ "--show-sdk-platform-path": "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform", | ||
+ "--show-sdk-path": "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk", | ||
Comment on lines
+12
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this work with the build sandbox? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node.js has two copies of GYP and this change just extends the existing patch to the second GYP copy. This works fine with sandbox, but I'm not really sure how that's supposed to work without sandbox since technically nothing stops the build from accessing these paths. Anyway, this allows us to drop xcbuild from Node.js native build inputs with minor copy-paste modification to an existing patch. I'm leaving refactoring or documenting this behavior to my future self (e.g. if we add split dev output, we can use fake xcbuild in propagatedNativeBuildInputs instead of patching GYP). |
||
+ "--show-sdk-build-version": "19A547", | ||
+ "--show-sdk-version": "10.15" | ||
+ }[infoitem] | ||
except GypError: | ||
pass | ||
|
||
@@ -1499,7 +1505,8 @@ def XcodeVersion(): | ||
version = "" | ||
build = "" | ||
try: | ||
- version_list = GetStdoutQuiet(["xcodebuild", "-version"]).splitlines() | ||
+ #version_list = GetStdoutQuiet(["xcodebuild", "-version"]).splitlines() | ||
+ version_list = [] | ||
# In some circumstances xcodebuild exits 0 but doesn't return | ||
# the right results; for example, a user on 10.7 or 10.8 with | ||
# a bogus path set via xcode-select | ||
@@ -1510,7 +1517,8 @@ def XcodeVersion(): | ||
version = version_list[0].split()[-1] # Last word on first line | ||
build = version_list[-1].split()[-1] # Last word on last line | ||
except GypError: # Xcode not installed so look for XCode Command Line Tools | ||
- version = CLTVersion() # macOS Catalina returns 11.0.0.0.1.1567737322 | ||
+ #version = CLTVersion() # macOS Catalina returns 11.0.0.0.1.1567737322 | ||
+ version = "11.0.0.0.1.1567737322" | ||
if not version: | ||
raise GypError("No Xcode or CLT version detected!") | ||
# Be careful to convert "4.2.3" to "0423" and "11.0.0" to "1100": | ||
--- a/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py | ||
+++ b/deps/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py | ||
@@ -522,7 +522,13 @@ class XcodeSettings: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
Allows ARM FPU to be set to vfpv2, e.g. for Raspberry Pi. | ||
|
||
See https://github.com/nodejs/node/issues/44357#issuecomment-1235821878 | ||
|
||
--- a/configure.py | ||
+++ b/configure.py | ||
@@ -50,7 +50,7 @@ | ||
valid_arch = ('arm', 'arm64', 'ia32', 'mips', 'mipsel', 'mips64el', 'ppc', | ||
'ppc64', 'x64', 'x86', 'x86_64', 's390x', 'riscv64', 'loong64') | ||
valid_arm_float_abi = ('soft', 'softfp', 'hard') | ||
-valid_arm_fpu = ('vfp', 'vfpv3', 'vfpv3-d16', 'neon') | ||
+valid_arm_fpu = ('vfp', 'vfpv2', 'vfpv3', 'vfpv3-d16', 'neon') | ||
valid_mips_arch = ('loongson', 'r1', 'r2', 'r6', 'rx') | ||
valid_mips_fpu = ('fp32', 'fp64', 'fpxx') | ||
valid_mips_float_abi = ('soft', 'hard') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
From 4b83f714c821d6d4d2306673ee3a87907cfec80e Mon Sep 17 00:00:00 2001 | ||
From: Ivan Trubach <mr.trubach@icloud.com> | ||
Date: Fri, 19 Jul 2024 10:45:13 +0300 | ||
Subject: [PATCH] build: support setting an emulator from configure script | ||
MIME-Version: 1.0 | ||
Content-Type: text/plain; charset=UTF-8 | ||
Content-Transfer-Encoding: 8bit | ||
|
||
V8’s JIT infrastructure requires binaries such as mksnapshot to be run | ||
during the build. However, these binaries must have the same bit-width | ||
as the host platform (e.g. a x86_64 build platform targeting ARMv6 needs | ||
to produce a 32-bit binary). | ||
|
||
To work around this issue, allow building the binaries for the host | ||
platform and running them on the build platform with an emulator. | ||
|
||
Based on Buildroot’s nodejs-src 0001-add-qemu-wrapper-support.patch. | ||
https://gitlab.com/buildroot.org/buildroot/-/blob/c1d5eada4d4db9eeaa1c44dd1dea95a67c8a70ca/package/nodejs/nodejs-src/0001-add-qemu-wrapper-support.patch | ||
|
||
Upstream: https://github.com/nodejs/node/pull/53899 | ||
--- | ||
common.gypi | 1 + | ||
configure.py | 14 ++++++++++++++ | ||
node.gyp | 3 +++ | ||
tools/v8_gypfiles/v8.gyp | 4 ++++ | ||
4 files changed, 22 insertions(+) | ||
|
||
diff --git a/common.gypi b/common.gypi | ||
index ec92c9df4c..6474495ab6 100644 | ||
--- a/common.gypi | ||
+++ b/common.gypi | ||
@@ -13,6 +13,7 @@ | ||
'enable_pgo_generate%': '0', | ||
'enable_pgo_use%': '0', | ||
'python%': 'python', | ||
+ 'emulator%': [], | ||
|
||
'node_shared%': 'false', | ||
'force_dynamic_crt%': 0, | ||
diff --git a/configure.py b/configure.py | ||
index 82916748fd..10dc0becbb 100755 | ||
--- a/configure.py | ||
+++ b/configure.py | ||
@@ -112,6 +112,12 @@ parser.add_argument('--dest-cpu', | ||
choices=valid_arch, | ||
help=f"CPU architecture to build for ({', '.join(valid_arch)})") | ||
|
||
+parser.add_argument('--emulator', | ||
+ action='store', | ||
+ dest='emulator', | ||
+ default=None, | ||
+ help='emulator command that can run executables built for the target system') | ||
+ | ||
parser.add_argument('--cross-compiling', | ||
action='store_true', | ||
dest='cross_compiling', | ||
@@ -2160,6 +2166,14 @@ write('config.mk', do_not_edit + config_str) | ||
gyp_args = ['--no-parallel', '-Dconfiguring_node=1'] | ||
gyp_args += ['-Dbuild_type=' + config['BUILDTYPE']] | ||
|
||
+if options.emulator is not None: | ||
+ if not options.cross_compiling: | ||
+ # Note that emulator is a list so we have to quote the variable. | ||
+ gyp_args += ['-Demulator=' + shlex.quote(options.emulator)] | ||
+ else: | ||
+ # TODO: perhaps use emulator for tests? | ||
+ warn('The `--emulator` option has no effect when cross-compiling.') | ||
+ | ||
if options.use_ninja: | ||
gyp_args += ['-f', 'ninja-' + flavor] | ||
elif flavor == 'win' and sys.platform != 'msys': | ||
diff --git a/node.gyp b/node.gyp | ||
index 08cb3f38e8..515b305933 100644 | ||
--- a/node.gyp | ||
+++ b/node.gyp | ||
@@ -332,6 +332,7 @@ | ||
'<(SHARED_INTERMEDIATE_DIR)/node_snapshot.cc', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(node_mksnapshot_exec)', | ||
'--build-snapshot', | ||
'<(node_snapshot_main)', | ||
@@ -351,6 +352,7 @@ | ||
'<(SHARED_INTERMEDIATE_DIR)/node_snapshot.cc', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
], | ||
@@ -1520,6 +1522,7 @@ | ||
'<(PRODUCT_DIR)/<(node_core_target_name).def', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(PRODUCT_DIR)/gen_node_def.exe', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp | ||
index ba8b161f0f..d5c90dad50 100644 | ||
--- a/tools/v8_gypfiles/v8.gyp | ||
+++ b/tools/v8_gypfiles/v8.gyp | ||
@@ -99,6 +99,7 @@ | ||
'<@(torque_outputs_inc)', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)torque<(EXECUTABLE_SUFFIX)', | ||
'-o', '<(SHARED_INTERMEDIATE_DIR)/torque-generated', | ||
'-v8-root', '<(V8_ROOT)', | ||
@@ -219,6 +220,7 @@ | ||
'action': [ | ||
'<(python)', | ||
'<(V8_ROOT)/tools/run.py', | ||
+ '<@(emulator)', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
], | ||
@@ -442,6 +444,7 @@ | ||
}], | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'>@(_inputs)', | ||
'>@(mksnapshot_flags)', | ||
], | ||
@@ -1577,6 +1580,7 @@ | ||
'action': [ | ||
'<(python)', | ||
'<(V8_ROOT)/tools/run.py', | ||
+ '<@(emulator)', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
], | ||
-- | ||
2.44.1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
From 999d918bc8fefec1752243743a47c0ce5380bcec Mon Sep 17 00:00:00 2001 | ||
From: Ivan Trubach <mr.trubach@icloud.com> | ||
Date: Wed, 17 Jul 2024 10:16:02 +0300 | ||
Subject: [PATCH] build: support setting an emulator from configure script | ||
MIME-Version: 1.0 | ||
Content-Type: text/plain; charset=UTF-8 | ||
Content-Transfer-Encoding: 8bit | ||
|
||
V8’s JIT infrastructure requires binaries such as mksnapshot to be run | ||
during the build. However, these binaries must have the same bit-width | ||
as the host platform (e.g. a x86_64 build platform targeting ARMv6 needs | ||
to produce a 32-bit binary). | ||
|
||
To work around this issue, allow building the binaries for the host | ||
platform and running them on the build platform with an emulator. | ||
|
||
Based on Buildroot’s nodejs-src 0001-add-qemu-wrapper-support.patch. | ||
https://gitlab.com/buildroot.org/buildroot/-/blob/c1d5eada4d4db9eeaa1c44dd1dea95a67c8a70ca/package/nodejs/nodejs-src/0001-add-qemu-wrapper-support.patch | ||
|
||
Upstream: https://github.com/nodejs/node/pull/53899 | ||
--- | ||
common.gypi | 1 + | ||
configure.py | 14 ++++++++++++++ | ||
node.gyp | 4 ++++ | ||
tools/v8_gypfiles/v8.gyp | 4 ++++ | ||
4 files changed, 23 insertions(+) | ||
|
||
diff --git a/common.gypi b/common.gypi | ||
index 154bbf2a0d..54d2afe3b3 100644 | ||
--- a/common.gypi | ||
+++ b/common.gypi | ||
@@ -13,6 +13,7 @@ | ||
'enable_pgo_generate%': '0', | ||
'enable_pgo_use%': '0', | ||
'python%': 'python', | ||
+ 'emulator%': [], | ||
|
||
'node_shared%': 'false', | ||
'force_dynamic_crt%': 0, | ||
diff --git a/configure.py b/configure.py | ||
index f7e3310723..f7c7acdf4f 100755 | ||
--- a/configure.py | ||
+++ b/configure.py | ||
@@ -112,6 +112,12 @@ parser.add_argument('--dest-cpu', | ||
choices=valid_arch, | ||
help=f"CPU architecture to build for ({', '.join(valid_arch)})") | ||
|
||
+parser.add_argument('--emulator', | ||
+ action='store', | ||
+ dest='emulator', | ||
+ default=None, | ||
+ help='emulator command that can run executables built for the target system') | ||
+ | ||
parser.add_argument('--cross-compiling', | ||
action='store_true', | ||
dest='cross_compiling', | ||
@@ -2276,6 +2282,14 @@ if flavor == 'win' and python.lower().endswith('.exe'): | ||
# will fail to run python scripts. | ||
gyp_args += ['-Dpython=' + python] | ||
|
||
+if options.emulator is not None: | ||
+ if not options.cross_compiling: | ||
+ # Note that emulator is a list so we have to quote the variable. | ||
+ gyp_args += ['-Demulator=' + shlex.quote(options.emulator)] | ||
+ else: | ||
+ # TODO: perhaps use emulator for tests? | ||
+ warn('The `--emulator` option has no effect when cross-compiling.') | ||
+ | ||
if options.use_ninja: | ||
gyp_args += ['-f', 'ninja-' + flavor] | ||
elif flavor == 'win' and sys.platform != 'msys': | ||
diff --git a/node.gyp b/node.gyp | ||
index 9617596760..439c76aca6 100644 | ||
--- a/node.gyp | ||
+++ b/node.gyp | ||
@@ -703,6 +703,7 @@ | ||
'<(SHARED_INTERMEDIATE_DIR)/node_snapshot.cc', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(node_mksnapshot_exec)', | ||
'--build-snapshot', | ||
'<(node_snapshot_main)', | ||
@@ -722,6 +723,7 @@ | ||
'<(SHARED_INTERMEDIATE_DIR)/node_snapshot.cc', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
], | ||
@@ -1010,6 +1012,7 @@ | ||
'<(SHARED_INTERMEDIATE_DIR)/node_javascript.cc', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(node_js2c_exec)', | ||
'<@(_outputs)', | ||
'lib', | ||
@@ -1477,6 +1480,7 @@ | ||
'<(PRODUCT_DIR)/<(node_core_target_name).def', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(PRODUCT_DIR)/gen_node_def.exe', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp | ||
index d65a5c268e..5cd6c36b86 100644 | ||
--- a/tools/v8_gypfiles/v8.gyp | ||
+++ b/tools/v8_gypfiles/v8.gyp | ||
@@ -112,6 +112,7 @@ | ||
'<@(torque_outputs_inc)', | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)torque<(EXECUTABLE_SUFFIX)', | ||
'-o', '<(SHARED_INTERMEDIATE_DIR)/torque-generated', | ||
'-v8-root', '<(V8_ROOT)', | ||
@@ -232,6 +233,7 @@ | ||
'action': [ | ||
'<(python)', | ||
'<(V8_ROOT)/tools/run.py', | ||
+ '<@(emulator)', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
], | ||
@@ -453,6 +455,7 @@ | ||
}], | ||
], | ||
'action': [ | ||
+ '<@(emulator)', | ||
'>@(_inputs)', | ||
'>@(mksnapshot_flags)', | ||
], | ||
@@ -1842,6 +1845,7 @@ | ||
'action': [ | ||
'<(python)', | ||
'<(V8_ROOT)/tools/run.py', | ||
+ '<@(emulator)', | ||
'<@(_inputs)', | ||
'<@(_outputs)', | ||
], | ||
-- | ||
2.44.1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please delete the old code instead of commenting it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be inconsistent with the part of the patch that applies to the other copy of this code, which was already in‐tree. Is it important enough to re‐do the whole patch over?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #327653 (comment)
I don’t think it’s worth redoing this patch in the context of this PR.