From 7eaa57b1162e542159673a20ac8a3ced46e78f70 Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Thu, 28 Mar 2019 11:05:43 -0400 Subject: [PATCH] build: remove native mksnapshot for arm/arm64 (#17561) * build: remove native mksnapshot for arm/arm64 --- .circleci/config.yml | 104 ++++-------------- patches/common/v8/.patches | 2 - ...ild-torque-with-x64-toolchain-on-arm.patch | 31 ------ ...ot_run_arm_arm64_mksnapshot_binaries.patch | 36 ------ patches/common/v8/expose_mksnapshot.patch | 2 +- script/upload.py | 4 - script/verify-mksnapshot.py | 44 +++++--- vsts-arm-test-steps.yml | 26 +++-- 8 files changed, 65 insertions(+), 184 deletions(-) delete mode 100644 patches/common/v8/build-torque-with-x64-toolchain-on-arm.patch delete mode 100644 patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch diff --git a/.circleci/config.yml b/.circleci/config.yml index 636ae20449..707a2684e1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -429,44 +429,29 @@ step-maybe-zip-symbols: &step-maybe-zip-symbols export BUILD_PATH="$PWD/out/Default" electron/script/zip-symbols.py -b $BUILD_PATH -step-maybe-native-mksnapshot-gn-gen: &step-maybe-native-mksnapshot-gn-gen +step-maybe-cross-arch-snapshot: &step-maybe-cross-arch-snapshot run: - name: Native mksnapshot GN gen (arm/arm64) + name: Generate cross arch snapshot (arm/arm64) command: | - if [ "$BUILD_NATIVE_MKSNAPSHOT" == "1" ]; then + if [ "$TRIGGER_ARM_TEST" == "true" ] && [ -z "$CIRCLE_PR_NUMBER" ]; then cd src - gn gen out/native_mksnapshot --args='import("'$GN_CONFIG'") cc_wrapper="'"$SCCACHE_PATH"'" v8_snapshot_toolchain="'"$MKSNAPSHOT_TOOLCHAIN"'"'" $GN_EXTRA_ARGS v8_enable_embedded_builtins = false" - else - echo 'Skipping native mksnapshot GN gen for non arm build' + if [ "$TARGET_ARCH" == "arm" ]; then + export MKSNAPSHOT_PATH="clang_x86_v8_arm" + elif [ "$TARGET_ARCH" == "arm64" ]; then + export MKSNAPSHOT_PATH="clang_x64_v8_arm64" + fi + cp "out/Default/$MKSNAPSHOT_PATH/mksnapshot" out/Default + cp "out/Default/$MKSNAPSHOT_PATH/libffmpeg.so" out/Default + cp "out/Default/$MKSNAPSHOT_PATH/v8_context_snapshot_generator" out/Default + python electron/script/verify-mksnapshot.py --source-root "$PWD" --build-dir out/Default --create-snapshot-only + mkdir cross-arch-snapshots + cp out/Default-mksnapshot-test/*.bin cross-arch-snapshots fi -step-maybe-native-mksnapshot-build: &step-maybe-native-mksnapshot-build - run: - name: Native mksnapshot build (arm/arm64) - no_output_timeout: 30m - command: | - if [ "$BUILD_NATIVE_MKSNAPSHOT" == "1" ]; then - cd src - ninja -C out/native_mksnapshot electron:electron_mksnapshot_zip -j $NUMBER_OF_NINJA_PROCESSES - else - echo 'Skipping native mksnapshot build for non arm build' - fi - -step-maybe-native-mksnapshot-strip: &step-maybe-native-mksnapshot-strip - run: - name: Native mksnapshot binary strip (arm/arm64) - command: | - if [ "$BUILD_NATIVE_MKSNAPSHOT" == "1" ]; then - cd src - electron/script/strip-binaries.py --file $PWD/out/native_mksnapshot/mksnapshot --target-cpu="$TARGET_ARCH" - else - echo 'Skipping native mksnapshot binary strip' - fi - -step-maybe-native-mksnapshot-store: &step-maybe-native-mksnapshot-store +step-maybe-cross-arch-snapshot-store: &step-maybe-cross-arch-snapshot-store store_artifacts: - path: src/out/native_mksnapshot/mksnapshot.zip - destination: native_mksnapshot.zip + path: src/cross-arch-snapshots + destination: cross-arch-snapshots step-maybe-trigger-arm-test: &step-maybe-trigger-arm-test run: @@ -626,12 +611,8 @@ steps-electron-build-for-tests: &steps-electron-build-for-tests # mksnapshot - *step-mksnapshot-build - *step-mksnapshot-store - - # native_mksnapshot - - *step-maybe-native-mksnapshot-gn-gen - - *step-maybe-native-mksnapshot-build - - *step-maybe-native-mksnapshot-strip - - *step-maybe-native-mksnapshot-store + - *step-maybe-cross-arch-snapshot + - *step-maybe-cross-arch-snapshot-store # ffmpeg - *step-ffmpeg-gn-gen @@ -673,12 +654,6 @@ steps-electron-build-for-publish: &steps-electron-build-for-publish - *step-mksnapshot-build - *step-mksnapshot-store - # native_mksnapshot - - *step-maybe-native-mksnapshot-gn-gen - - *step-maybe-native-mksnapshot-build - - *step-maybe-native-mksnapshot-strip - - *step-maybe-native-mksnapshot-store - # chromedriver - *step-electron-chromedriver-build - *step-electron-chromedriver-store @@ -712,18 +687,6 @@ steps-chromedriver-build: &steps-chromedriver-build - *step-maybe-notify-slack-failure -steps-native-mksnapshot-build: &steps-native-mksnapshot-build - steps: - - attach_workspace: - at: . - - *step-depot-tools-add-to-path - - *step-setup-env-for-build - - *step-maybe-native-mksnapshot-gn-gen - - *step-maybe-native-mksnapshot-build - - *step-maybe-native-mksnapshot-store - - - *step-maybe-notify-slack-failure - steps-native-tests: &steps-native-tests steps: - attach_workspace: @@ -1017,16 +980,6 @@ jobs: GCLIENT_EXTRA_ARGS: '--custom-var=checkout_arm=True --custom-var=checkout_boto=True --custom-var=checkout_requests=True' <<: *steps-electron-build-for-publish - linux-arm-native-mksnapshot: - <<: *machine-linux-2xlarge - environment: - <<: *env-linux-medium - <<: *env-arm - <<: *env-release-build - <<: *env-enable-sccache - <<: *env-send-slack-notifications - <<: *steps-native-mksnapshot-build - linux-arm64-debug: <<: *machine-linux-2xlarge environment: @@ -1091,16 +1044,6 @@ jobs: GCLIENT_EXTRA_ARGS: '--custom-var=checkout_arm64=True --custom-var=checkout_boto=True --custom-var=checkout_requests=True' <<: *steps-electron-build-for-publish - linux-arm64-native-mksnapshot: - <<: *machine-linux-2xlarge - environment: - <<: *env-linux-medium - <<: *env-arm64 - <<: *env-release-build - <<: *env-enable-sccache - <<: *env-send-slack-notifications - <<: *steps-native-mksnapshot-build - osx-testing: <<: *machine-mac-large environment: @@ -1565,9 +1508,6 @@ workflows: - linux-arm-release: requires: - linux-checkout - - linux-arm-native-mksnapshot: - requires: - - linux-checkout - linux-arm-chromedriver: requires: - linux-checkout @@ -1575,14 +1515,11 @@ workflows: requires: - linux-arm-release - linux-arm-chromedriver - - linux-arm-native-mksnapshot + - linux-arm64-release: requires: - linux-checkout - - linux-arm64-native-mksnapshot: - requires: - - linux-checkout - linux-arm64-chromedriver: requires: - linux-checkout @@ -1590,7 +1527,6 @@ workflows: requires: - linux-arm64-release - linux-arm64-chromedriver - - linux-arm64-native-mksnapshot nightly-mac-release-test: triggers: diff --git a/patches/common/v8/.patches b/patches/common/v8/.patches index 023d49d6f1..af2396167f 100644 --- a/patches/common/v8/.patches +++ b/patches/common/v8/.patches @@ -2,7 +2,5 @@ deps_backport_detailed_line_info_for_cpu_profiler.patch add_realloc.patch build_gn.patch expose_mksnapshot.patch -build-torque-with-x64-toolchain-on-arm.patch -do_not_run_arm_arm64_mksnapshot_binaries.patch deps_provide_more_v8_backwards_compatibility.patch dcheck.patch diff --git a/patches/common/v8/build-torque-with-x64-toolchain-on-arm.patch b/patches/common/v8/build-torque-with-x64-toolchain-on-arm.patch deleted file mode 100644 index 1b4b001289..0000000000 --- a/patches/common/v8/build-torque-with-x64-toolchain-on-arm.patch +++ /dev/null @@ -1,31 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: deepak1556 -Date: Thu, 21 Feb 2019 00:06:32 +0530 -Subject: build-torque-with-x64-toolchain-on-arm.patch - -torque binary has to be run during the build. - -diff --git a/BUILD.gn b/BUILD.gn -index d3dbe37d0a145921dddaea72e394c87826d6d5fe..61f952f1a577b17c6063996fd1e17a195714bf5a 100644 ---- a/BUILD.gn -+++ b/BUILD.gn -@@ -178,7 +178,9 @@ declare_args() { - # the snapshot toolchain is the target toolchain and, hence, can't be used. - v8_generator_toolchain = v8_snapshot_toolchain - if (host_cpu == "x64" && -- (v8_current_cpu == "mips" || v8_current_cpu == "mips64")) { -+ (v8_current_cpu == "mips" || v8_current_cpu == "mips64" || -+ v8_snapshot_toolchain == "//build/toolchain/linux:clang_arm" || -+ v8_snapshot_toolchain == "//build/toolchain/linux:clang_arm64")) { - v8_generator_toolchain = "//build/toolchain/linux:clang_x64" - } - -@@ -3545,7 +3547,7 @@ if (v8_use_snapshot && current_toolchain == v8_snapshot_toolchain) { - } - } - --if (current_toolchain == v8_snapshot_toolchain) { -+if (current_toolchain == v8_generator_toolchain) { - v8_executable("torque") { - visibility = [ ":*" ] # Only targets in this file can depend on this. - 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 deleted file mode 100644 index 585c3dc0c0..0000000000 --- a/patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch +++ /dev/null @@ -1,36 +0,0 @@ -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 61f952f1a577b17c6063996fd1e17a195714bf5a..2a1c7d0348ace504c9b26850de7340d981cbb440 100644 ---- a/BUILD.gn -+++ b/BUILD.gn -@@ -1322,9 +1322,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" ] - } diff --git a/patches/common/v8/expose_mksnapshot.patch b/patches/common/v8/expose_mksnapshot.patch index 6a08657339..5a4b67754b 100644 --- a/patches/common/v8/expose_mksnapshot.patch +++ b/patches/common/v8/expose_mksnapshot.patch @@ -3,7 +3,7 @@ From: Shelley Vohr Date: Mon, 22 Oct 2018 10:47:13 -0700 Subject: expose_mksnapshot.patch -Needed in order to build mksnapshot on arm. +Needed in order to target mksnapshot for mksnapshot zip. diff --git a/BUILD.gn b/BUILD.gn index d8a2b8e11a3e0e9820dca061a00dbf1a6859bcf4..d3dbe37d0a145921dddaea72e394c87826d6d5fe 100644 diff --git a/script/upload.py b/script/upload.py index 6dbdc9fcca..179c2a9177 100755 --- a/script/upload.py +++ b/script/upload.py @@ -96,10 +96,6 @@ def main(): mksnapshot = get_zip_name('mksnapshot', ELECTRON_VERSION) mksnapshot_zip = os.path.join(OUT_DIR, mksnapshot) if get_target_arch().startswith('arm'): - # Upload the native mksnapshot as mksnapshot.zip - shutil.copy2(os.path.join(SRC_DIR, 'out', 'native_mksnapshot', - 'mksnapshot.zip'), mksnapshot_zip) - upload_electron(release, mksnapshot_zip, args) # Upload the x64 binary for arm/arm64 mksnapshot mksnapshot = get_zip_name('mksnapshot', ELECTRON_VERSION, 'x64') mksnapshot_zip = os.path.join(OUT_DIR, mksnapshot) diff --git a/script/verify-mksnapshot.py b/script/verify-mksnapshot.py index 6bb9a3bd22..51e71cb385 100755 --- a/script/verify-mksnapshot.py +++ b/script/verify-mksnapshot.py @@ -23,20 +23,28 @@ def main(): returncode = 0 try: with scoped_cwd(app_path): - mkargs = [ get_binary_path('mksnapshot', app_path), \ - SNAPSHOT_SOURCE, '--startup_blob', 'snapshot_blob.bin', \ - '--turbo_instruction_scheduling' ] - subprocess.check_call(mkargs) - print 'ok mksnapshot successfully created snapshot_blob.bin.' - context_snapshot = 'v8_context_snapshot.bin' - context_snapshot_path = os.path.join(app_path, context_snapshot) - gen_binary = get_binary_path('v8_context_snapshot_generator', \ - app_path) - genargs = [ gen_binary, \ - '--output_file={0}'.format(context_snapshot_path) ] - subprocess.check_call(genargs) - print 'ok v8_context_snapshot_generator successfully created ' \ - + context_snapshot + if args.snapshot_files_dir is None: + mkargs = [ get_binary_path('mksnapshot', app_path), \ + SNAPSHOT_SOURCE, '--startup_blob', 'snapshot_blob.bin', \ + '--turbo_instruction_scheduling' ] + subprocess.check_call(mkargs) + print 'ok mksnapshot successfully created snapshot_blob.bin.' + context_snapshot = 'v8_context_snapshot.bin' + context_snapshot_path = os.path.join(app_path, context_snapshot) + gen_binary = get_binary_path('v8_context_snapshot_generator', \ + app_path) + genargs = [ gen_binary, \ + '--output_file={0}'.format(context_snapshot_path) ] + subprocess.check_call(genargs) + print 'ok v8_context_snapshot_generator successfully created ' \ + + context_snapshot + if args.create_snapshot_only: + return 0 + else: + gen_bin_path = os.path.join(args.snapshot_files_dir, '*.bin') + generated_bin_files = glob.glob(gen_bin_path) + for bin_file in generated_bin_files: + shutil.copy2(bin_file, app_path) test_path = os.path.join(SOURCE_ROOT, 'spec', 'fixtures', \ 'snapshot-items-available') @@ -93,6 +101,14 @@ def parse_args(): Relative to the --source-root.', default=None, required=True) + parser.add_argument('--create-snapshot-only', + help='Just create snapshot files, but do not run test', + action='store_true') + parser.add_argument('--snapshot-files-dir', + help='Directory containing snapshot files to use \ + for testing', + default=None, + required=False) parser.add_argument('--source-root', default=SOURCE_ROOT, required=False) diff --git a/vsts-arm-test-steps.yml b/vsts-arm-test-steps.yml index a27723a964..d405e45269 100644 --- a/vsts-arm-test-steps.yml +++ b/vsts-arm-test-steps.yml @@ -32,15 +32,6 @@ steps: env: CIRCLE_TOKEN: $(CIRCLECI_TOKEN) -- bash: | - cd src/electron - node script/download-circleci-artifacts.js --buildNum=$CIRCLE_BUILD_NUM --name=native_mksnapshot.zip --dest=$ZIP_DEST - cd $ZIP_DEST - unzip -o native_mksnapshot.zip - displayName: 'Download and unzip native_mksnapshot.zip for test' - env: - CIRCLE_TOKEN: $(CIRCLECI_TOKEN) - - bash: | export NODE_HEADERS_DEST=$PWD/src/out/Default/gen mkdir -p $NODE_HEADERS_DEST @@ -52,6 +43,17 @@ steps: env: CIRCLE_TOKEN: $(CIRCLECI_TOKEN) +- bash: | + export CROSS_ARCH_SNAPSHOTS=$PWD/src/out/Default/cross-arch-snapshots + mkdir -p $CROSS_ARCH_SNAPSHOTS + cd src/electron + node script/download-circleci-artifacts.js --buildNum=$CIRCLE_BUILD_NUM --name=cross-arch-snapshots/snapshot_blob.bin --dest=$CROSS_ARCH_SNAPSHOTS + node script/download-circleci-artifacts.js --buildNum=$CIRCLE_BUILD_NUM --name=cross-arch-snapshots/v8_context_snapshot.bin --dest=$CROSS_ARCH_SNAPSHOTS + displayName: 'Download cross arch snapshot files' + env: + CIRCLE_TOKEN: $(CIRCLECI_TOKEN) + + - bash: | cd src export npm_config_nodedir=$PWD/out/Default/gen/node_headers @@ -83,9 +85,9 @@ steps: - bash: | cd src - echo Verify mksnapshot temporarily disabled due to M74 upgrade - #python electron/script/verify-mksnapshot.py --source-root "$PWD" --build-dir out/Default - displayName: Verify mksnapshot + echo Verify cross arch snapshot + python electron/script/verify-mksnapshot.py --source-root "$PWD" --build-dir out/Default --snapshot-files-dir $PWD/out/Default/cross-arch-snapshots + displayName: Verify cross arch snapshot timeoutInMinutes: 5 env: ELECTRON_DISABLE_SANDBOX: 1