Permalink
Browse files

build: fix native_mksnapshot build (#15770)

* build: fix native_mksnapshot build

When we changed our electron_mksnapshot_zip target to include the v8_context_snapshot_generator, this dependency made the `run_mksnapshot` target run which was trying to run an arm/arm64 binary on x64 hardware.

Don't use custom build args for native_mksnapshot as they are not needed

* Added comment on why snapshot_blob.bin is skipped on arm/arm64
  • Loading branch information...
jkleinsc committed Nov 21, 2018
1 parent 9e8b26c commit bb7c63c052b47db7b4c36931c7f81ac7826cda8a
@@ -370,7 +370,7 @@ step-maybe-native-mksnapshot-gn-gen: &step-maybe-native-mksnapshot-gn-gen
command: |
if [ "$BUILD_NATIVE_MKSNAPSHOT" == "1" ]; then
cd src
gn gen out/native_mksnapshot --args='import("//electron/build/args/native_mksnapshot.gn") cc_wrapper="'"$SCCACHE_PATH"'" v8_snapshot_toolchain="'"$MKSNAPSHOT_TOOLCHAIN"'"'" $GN_EXTRA_ARGS"
gn gen out/native_mksnapshot --args='import("'$GN_CONFIG'") cc_wrapper="'"$SCCACHE_PATH"'" v8_snapshot_toolchain="'"$MKSNAPSHOT_TOOLCHAIN"'"'" $GN_EXTRA_ARGS"
else
echo 'Skipping native mksnapshot GN gen for non arm build'
fi
@@ -919,6 +919,7 @@ jobs:
executor: linux-medium
environment:
<<: *env-arm
<<: *env-release-build
<<: *env-enable-sccache
<<: *env-send-slack-notifications
<<: *steps-native-mksnapshot-build
@@ -978,6 +979,7 @@ jobs:
executor: linux-medium
environment:
<<: *env-arm64
<<: *env-release-build
<<: *env-enable-sccache
<<: *env-send-slack-notifications
<<: *steps-native-mksnapshot-build

This file was deleted.

Oops, something went wrong.
@@ -19,10 +19,17 @@
'pyproto',
]
def skip_path(dep):
def skip_path(dep, dist_zip, target_cpu):
# Skip specific paths and extensions as well as the following special case:
# snapshot_blob.bin is a dependency of mksnapshot.zip because
# v8_context_generator needs it, but this file does not get generated for arm
# and arm 64 binaries of mksnapshot since they are built on x64 hardware.
# Consumers of arm and arm64 mksnapshot can generate snapshot_blob.bin
# themselves by running mksnapshot.
should_skip = (
any(dep.startswith(path) for path in PATHS_TO_SKIP) or
any(dep.endswith(ext) for ext in EXTENSIONS_TO_SKIP))
any(dep.endswith(ext) for ext in EXTENSIONS_TO_SKIP) or
('arm' in target_cpu and dist_zip == 'mksnapshot.zip' and dep == 'snapshot_blob.bin'))
if should_skip:
print("Skipping {}".format(dep))
return should_skip
@@ -47,7 +54,7 @@ def main(argv):
else:
with zipfile.ZipFile(dist_zip, 'w', zipfile.ZIP_DEFLATED) as z:
for dep in dist_files:
if skip_path(dep):
if skip_path(dep, dist_zip, target_cpu):
continue
if os.path.isdir(dep):
for root, dirs, files in os.walk(dep):
@@ -18,3 +18,4 @@ dcheck.patch
disable-warning-win.patch
expose_mksnapshot.patch
build-torque-with-x64-toolchain-on-arm.patch
do_not_run_arm_arm64_mksnapshot_binaries.patch
@@ -0,0 +1,36 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: John Kleinschmidt <jkleinsc@github.com>
Date: Mon, 19 Nov 2018 18:33:56 -0500
Subject: Do not run arm/arm64 mksnapshot binaries
For arm and arm64 target_arches, Chromium builds mksnapshot as an x64 binary and
as part of that build mksnapshot is executed to produce snapshot_blob.bin.
Chromium does not build native arm and arm64 binaries of mksnapshot, but
Electron does, so this patch makes sure that the build doesn't try to run
the mksnapshot binary if it was built for arm or arm64.
diff --git a/BUILD.gn b/BUILD.gn
index b1194e4b828e66d8d09fac57481efe313031f3ac..c256945650c24324c28a2e17fb299a1d0c2dcd19 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1247,9 +1247,19 @@ if (v8_use_snapshot && v8_use_external_startup_data) {
]
public_deps = [
":natives_blob",
- ":run_mksnapshot_default",
]
+ if (v8_snapshot_toolchain == "//build/toolchain/linux:clang_arm" ||
+ v8_snapshot_toolchain == "//build/toolchain/linux:clang_arm64") {
+ public_deps += [
+ ":mksnapshot($v8_snapshot_toolchain)",
+ ]
+ } else {
+ public_deps += [
+ ":run_mksnapshot_default",
+ ]
+ }
+
if (v8_use_multi_snapshots) {
public_deps += [ ":run_mksnapshot_trusted" ]
}

0 comments on commit bb7c63c

Please sign in to comment.