From bb7c63c052b47db7b4c36931c7f81ac7826cda8a Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Wed, 21 Nov 2018 15:19:19 -0500 Subject: [PATCH] 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 --- .circleci/config.yml | 4 ++- build/args/native_mksnapshot.gn | 2 -- build/zip.py | 13 +++++-- patches/common/v8/.patches | 1 + ...ot_run_arm_arm64_mksnapshot_binaries.patch | 36 +++++++++++++++++++ 5 files changed, 50 insertions(+), 6 deletions(-) delete mode 100644 build/args/native_mksnapshot.gn create mode 100644 patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch diff --git a/.circleci/config.yml b/.circleci/config.yml index 7b6fa61ea4eb..ff4a4361a58d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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 diff --git a/build/args/native_mksnapshot.gn b/build/args/native_mksnapshot.gn deleted file mode 100644 index ab0f41008bdd..000000000000 --- a/build/args/native_mksnapshot.gn +++ /dev/null @@ -1,2 +0,0 @@ -import("release.gn") -v8_enable_i18n_support = false diff --git a/build/zip.py b/build/zip.py index 0302f800ecfe..5518322e0159 100644 --- a/build/zip.py +++ b/build/zip.py @@ -19,10 +19,17 @@ PATHS_TO_SKIP = [ '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): diff --git a/patches/common/v8/.patches b/patches/common/v8/.patches index df872b2d7e85..9cebca405be1 100644 --- a/patches/common/v8/.patches +++ b/patches/common/v8/.patches @@ -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 diff --git a/patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch b/patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch new file mode 100644 index 000000000000..a9e58cd7f3b1 --- /dev/null +++ b/patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch @@ -0,0 +1,36 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: John Kleinschmidt +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" ] + }