build: remove dead symlink from MAS build (#24158) (#24238)

* build: remove dead symlink from MAS build * chore: new out cache * build: fixup gn check * Update node_main.cc * chore: fix lint
electron · Jun 22, 2020 · 31e8b9f · 31e8b9f
1 parent 06902de
commit 31e8b9f
Show file tree

Hide file tree

Showing 9 changed files with 121 additions and 28 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -409,6 +409,7 @@ step-electron-build: &step-electron-build
       fi
       cd src
       ninja -C out/Default electron -j $NUMBER_OF_NINJA_PROCESSES
+      node electron/script/check-symlinks.js
 
 step-native-unittests-build: &step-native-unittests-build
   run:
@@ -821,7 +822,7 @@ step-restore-out-cache: &step-restore-out-cache
     paths:
       - ./src/out/Default
     keys:
-      - v7-out-cache-{{ checksum "src/electron/.depshash" }}-{{ checksum "src/electron/.depshash-target" }}
+      - v8-out-cache-{{ checksum "src/electron/.depshash" }}-{{ checksum "src/electron/.depshash-target" }}
     name: Restoring out cache
 
 step-set-git-cache-path: &step-set-git-cache-path
@@ -845,7 +846,7 @@ step-save-out-cache: &step-save-out-cache
   save_cache:
     paths:
       - ./src/out/Default
-    key: v7-out-cache-{{ checksum "src/electron/.depshash" }}-{{ checksum "src/electron/.depshash-target" }}
+    key: v8-out-cache-{{ checksum "src/electron/.depshash" }}-{{ checksum "src/electron/.depshash-target" }}
     name: Persisting out cache
 
 step-run-electron-only-hooks: &step-run-electron-only-hooks

diff --git a/BUILD.gn b/BUILD.gn
@@ -337,7 +337,6 @@ source_set("electron_lib") {
     "//chrome/app/resources:platform_locale_settings",
     "//chrome/services/printing/public/mojom",
     "//components/certificate_transparency",
-    "//components/crash/core/app",
     "//components/language/core/browser",
     "//components/net_log",
     "//components/network_hints/browser",
@@ -443,6 +442,9 @@ source_set("electron_lib") {
       "*\bviews/*",
     ]
   }
+  if (!is_mas_build) {
+    deps += [ "//components/crash/core/app" ]
+  }
 
   set_sources_assignment_filter(
       sources_assignment_filter + extra_source_filters)
@@ -468,10 +470,13 @@ source_set("electron_lib") {
     deps += [
       "//components/remote_cocoa/app_shim",
       "//content/common:mac_helpers",
-      "//third_party/crashpad/crashpad/client",
       "//ui/accelerated_widget_mac",
     ]
 
+    if (!is_mas_build) {
+      deps += [ "//third_party/crashpad/crashpad/client" ]
+    }
+
     libs = [
       "AVFoundation.framework",
       "Carbon.framework",
@@ -492,6 +497,12 @@ source_set("electron_lib") {
       sources += [ "shell/browser/api/electron_api_app_mas.mm" ]
       sources -= [ "shell/browser/auto_updater_mac.mm" ]
       defines += [ "MAS_BUILD" ]
+      sources -= [
+        "shell/app/electron_crash_reporter_client.cc",
+        "shell/app/electron_crash_reporter_client.h",
+        "shell/common/crash_keys.cc",
+        "shell/common/crash_keys.h",
+      ]
     } else {
       libs += [
         "Squirrel.framework",
@@ -796,8 +807,10 @@ if (is_mac) {
     framework_contents = [
       "Resources",
       "Libraries",
-      "Helpers",
     ]
+    if (!is_mas_build) {
+      framework_contents += [ "Helpers" ]
+    }
     public_deps = [
       ":electron_framework_libraries",
       ":electron_lib",
@@ -1028,13 +1041,16 @@ if (is_mac) {
 
     group("electron_symbols") {
       deps = [
-        ":crashpad_handler_syms",
         ":electron_app_syms",
         ":electron_framework_syms",
         ":swiftshader_egl_syms",
         ":swiftshader_gles_syms",
       ]
 
+      if (!is_mas_build) {
+        deps += [ ":crashpad_handler_syms" ]
+      }
+
       foreach(helper_params, content_mac_helpers) {
         _helper_target = helper_params[0]
         deps += [ ":electron_helper_syms_${_helper_target}" ]

diff --git a/script/check-symlinks.js b/script/check-symlinks.js
@@ -0,0 +1,42 @@
+const fs = require('fs');
+const path = require('path');
+
+const utils = require('./lib/utils');
+
+if (process.platform !== 'darwin') {
+  console.log('Not checking symlinks on non-darwin platform');
+  process.exit(0);
+}
+
+const appPath = path.resolve(__dirname, '..', '..', 'out', utils.getOutDir(), 'Electron.app');
+const visited = new Set();
+const traverse = (p) => {
+  if (visited.has(p)) return;
+
+  visited.add(p);
+  if (!fs.statSync(p).isDirectory()) return;
+
+  for (const child of fs.readdirSync(p)) {
+    const childPath = path.resolve(p, child);
+    let realPath;
+    try {
+      realPath = fs.realpathSync(childPath);
+    } catch (err) {
+      if (err.path) {
+        console.error('Detected an invalid symlink');
+        console.error('Source:', childPath);
+        let link = fs.readlinkSync(childPath);
+        if (!link.startsWith('.')) {
+          link = `../${link}`;
+        }
+        console.error('Target:', path.resolve(childPath, link));
+        process.exit(1);
+      } else {
+        throw err;
+      }
+    }
+    traverse(realPath);
+  }
+};
+
+traverse(appPath);
diff --git a/script/zip_manifests/dist_zip.mac_mas.x64.manifest b/script/zip_manifests/dist_zip.mac_mas.x64.manifest
@@ -3,7 +3,6 @@ Electron.app/Contents/
 Electron.app/Contents/Frameworks/
 Electron.app/Contents/Frameworks/Electron Framework.framework/
 Electron.app/Contents/Frameworks/Electron Framework.framework/Electron Framework
-Electron.app/Contents/Frameworks/Electron Framework.framework/Helpers
 Electron.app/Contents/Frameworks/Electron Framework.framework/Libraries
 Electron.app/Contents/Frameworks/Electron Framework.framework/Resources
 Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/

diff --git a/shell/app/electron_main_delegate.cc b/shell/app/electron_main_delegate.cc
@@ -22,9 +22,6 @@
 #include "base/strings/string_split.h"
 #include "chrome/common/chrome_paths.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
-#include "components/crash/core/app/crashpad.h"
-#include "components/crash/core/common/crash_key.h"
-#include "components/crash/core/common/crash_keys.h"
 #include "content/public/common/content_switches.h"
 #include "electron/buildflags/buildflags.h"
 #include "extensions/common/constants.h"
@@ -33,13 +30,10 @@
 #include "services/service_manager/sandbox/switches.h"
 #include "services/tracing/public/cpp/stack_sampling/tracing_sampler_profiler.h"
 #include "shell/app/electron_content_client.h"
-#include "shell/app/electron_crash_reporter_client.h"
-#include "shell/browser/api/electron_api_crash_reporter.h"
 #include "shell/browser/electron_browser_client.h"
 #include "shell/browser/electron_gpu_client.h"
 #include "shell/browser/feature_list.h"
 #include "shell/browser/relauncher.h"
-#include "shell/common/crash_keys.h"
 #include "shell/common/electron_paths.h"
 #include "shell/common/options_switches.h"
 #include "shell/renderer/electron_renderer_client.h"
@@ -64,6 +58,15 @@
 #include "v8/include/v8.h"
 #endif
 
+#if !defined(MAS_BUILD)
+#include "components/crash/core/app/crashpad.h"  // nogncheck
+#include "components/crash/core/common/crash_key.h"
+#include "components/crash/core/common/crash_keys.h"
+#include "shell/app/electron_crash_reporter_client.h"
+#include "shell/browser/api/electron_api_crash_reporter.h"
+#include "shell/common/crash_keys.h"
+#endif
+
 namespace electron {
 
 namespace {

diff --git a/shell/app/node_main.cc b/shell/app/node_main.cc
@@ -14,19 +14,15 @@
 #include "base/feature_list.h"
 #include "base/task/thread_pool/thread_pool_instance.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "components/crash/core/app/crashpad.h"
 #include "content/public/common/content_switches.h"
 #include "electron/electron_version.h"
 #include "gin/array_buffer.h"
 #include "gin/public/isolate_holder.h"
 #include "gin/v8_initializer.h"
-#include "shell/app/electron_crash_reporter_client.h"
 #include "shell/app/uv_task_runner.h"
-#include "shell/browser/api/electron_api_crash_reporter.h"
 #include "shell/browser/javascript_environment.h"
 #include "shell/browser/node_debugger.h"
 #include "shell/common/api/electron_bindings.h"
-#include "shell/common/crash_keys.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
@@ -39,6 +35,13 @@
 #include "chrome/child/v8_crashpad_support_win.h"
 #endif
 
+#if !defined(MAS_BUILD)
+#include "components/crash/core/app/crashpad.h"  // nogncheck
+#include "shell/app/electron_crash_reporter_client.h"
+#include "shell/browser/api/electron_api_crash_reporter.h"
+#include "shell/common/crash_keys.h"
+#endif
+
 namespace {
 
 bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
@@ -48,6 +51,11 @@ bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
   return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
 }
 
+#if defined(MAS_BUILD)
+void SetCrashKeyStub(const std::string& key, const std::string& value) {}
+void ClearCrashKeyStub(const std::string& key) {}
+#endif
+
 }  // namespace
 
 namespace electron {
@@ -175,9 +183,14 @@ int NodeMain(int argc, char* argv[]) {
 #endif
 
     reporter.SetMethod("getParameters", &GetParameters);
+#if defined(MAS_BUILD)
+    reporter.SetMethod("addExtraParameter", &SetCrashKeyStub);
+    reporter.SetMethod("removeExtraParameter", &ClearCrashKeyStub);
+#else
     reporter.SetMethod("addExtraParameter", &electron::crash_keys::SetCrashKey);
     reporter.SetMethod("removeExtraParameter",
                        &electron::crash_keys::ClearCrashKey);
+#endif
 
     process.Set("crashReporter", reporter);
 

diff --git a/shell/browser/api/electron_api_crash_reporter.cc b/shell/browser/api/electron_api_crash_reporter.cc
@@ -16,25 +16,28 @@
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
-#include "chrome/browser/crash_upload_list/crash_upload_list_crashpad.h"
 #include "chrome/common/chrome_paths.h"
-#include "components/crash/core/app/crashpad.h"
-#include "components/crash/core/common/crash_key.h"
 #include "components/upload_list/crash_upload_list.h"
 #include "components/upload_list/text_log_upload_list.h"
 #include "content/public/common/content_switches.h"
 #include "gin/arguments.h"
 #include "gin/data_object_builder.h"
 #include "services/service_manager/embedder/switches.h"
-#include "shell/app/electron_crash_reporter_client.h"
-#include "shell/common/crash_keys.h"
 #include "shell/common/electron_paths.h"
 #include "shell/common/gin_converters/callback_converter.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_converters/time_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
-#include "third_party/crashpad/crashpad/client/crashpad_info.h"
+
+#if !defined(MAS_BUILD)
+#include "chrome/browser/crash_upload_list/crash_upload_list_crashpad.h"
+#include "components/crash/core/app/crashpad.h"  // nogncheck
+#include "components/crash/core/common/crash_key.h"
+#include "shell/app/electron_crash_reporter_client.h"
+#include "shell/common/crash_keys.h"
+#include "third_party/crashpad/crashpad/client/crashpad_info.h"  // nogncheck
+#endif
 
 #if defined(OS_LINUX)
 #include "components/crash/core/app/breakpad_linux.h"
@@ -62,6 +65,14 @@ namespace api {
 
 namespace crash_reporter {
 
+#if defined(MAS_BUILD)
+namespace {
+
+void NoOp() {}
+
+}  // namespace
+#endif
+
 bool IsCrashReporterEnabled() {
   return g_crash_reporter_initialized;
 }
@@ -203,8 +214,13 @@ void Initialize(v8::Local<v8::Object> exports,
                 void* priv) {
   gin_helper::Dictionary dict(context->GetIsolate(), exports);
   dict.SetMethod("start", &electron::api::crash_reporter::Start);
+#if defined(MAS_BUILD)
+  dict.SetMethod("addExtraParameter", &electron::api::crash_reporter::NoOp);
+  dict.SetMethod("removeExtraParameter", &electron::api::crash_reporter::NoOp);
+#else
   dict.SetMethod("addExtraParameter", &electron::crash_keys::SetCrashKey);
   dict.SetMethod("removeExtraParameter", &electron::crash_keys::ClearCrashKey);
+#endif
   dict.SetMethod("getParameters", &GetParameters);
   dict.SetMethod("getUploadedReports", &GetUploadedReports);
   dict.SetMethod("setUploadToServer", &SetUploadToServer);

diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc
@@ -168,12 +168,12 @@
 #include "content/public/common/child_process_host.h"
 #endif
 
-#if defined(OS_LINUX)
+#if defined(OS_LINUX) && !defined(MAS_BUILD)
 #include "base/debug/leak_annotations.h"
 #include "components/crash/content/browser/crash_handler_host_linux.h"
-#include "components/crash/core/app/breakpad_linux.h"
-#include "components/crash/core/app/crash_switches.h"
-#include "components/crash/core/app/crashpad.h"
+#include "components/crash/core/app/breakpad_linux.h"  // nogncheck
+#include "components/crash/core/app/crash_switches.h"  // nogncheck
+#include "components/crash/core/app/crashpad.h"        // nogncheck
 #endif
 
 using content::BrowserThread;

diff --git a/shell/renderer/api/electron_api_crash_reporter_renderer.cc b/shell/renderer/api/electron_api_crash_reporter_renderer.cc
@@ -3,10 +3,13 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
-#include "shell/common/crash_keys.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
 
+#if !defined(MAS_BUILD)
+#include "shell/common/crash_keys.h"
+#endif
+
 namespace {
 
 v8::Local<v8::Value> GetParameters(v8::Isolate* isolate) {