build: remove native mksnapshot for arm/arm64 (#17561)

* build: remove native mksnapshot for arm/arm64
2019-03-28 11:05:43 -04:00 · 2019-03-28 11:05:43 -04:00 · 7eaa57b116
commit 7eaa57b116
parent 8cf15cc931
8 changed files with 65 additions and 184 deletions
--- 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:
--- 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
--- a/patches/common/v8/build-torque-with-x64-toolchain-on-arm.patch
+++ b/patches/common/v8/build-torque-with-x64-toolchain-on-arm.patch
@ -1,31 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: deepak1556 <hop2deep@gmail.com>
-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.
- 
--- a/patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch
+++ b/patches/common/v8/do_not_run_arm_arm64_mksnapshot_binaries.patch
@ -1,36 +0,0 @@
-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 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" ]
-     }
--- a/patches/common/v8/expose_mksnapshot.patch
+++ b/patches/common/v8/expose_mksnapshot.patch
@ -3,7 +3,7 @@ From: Shelley Vohr <shelley.vohr@gmail.com>
 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
--- 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)
--- 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)
--- 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