From 06e6b34ba7edbcc62ae2c8b9d1206e7df5c0c213 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 16 Jun 2020 14:19:57 -0700 Subject: [PATCH] build: remove dead symlink from MAS build (#24158) * build: remove dead symlink from MAS build * chore: new out cache * build: fixup gn check * Update node_main.cc * chore: fix lint --- .circleci/config.yml | 5 ++- BUILD.gn | 24 +++++++++-- script/check-symlinks.js | 42 +++++++++++++++++++ .../dist_zip.mac_mas.x64.manifest | 1 - shell/app/electron_main_delegate.cc | 15 ++++--- shell/app/node_main.cc | 21 ++++++++-- .../api/electron_api_crash_reporter.cc | 28 ++++++++++--- shell/browser/electron_browser_client.cc | 8 ++-- .../electron_api_crash_reporter_renderer.cc | 5 ++- 9 files changed, 121 insertions(+), 28 deletions(-) create mode 100644 script/check-symlinks.js diff --git a/.circleci/config.yml b/.circleci/config.yml index 706c19b8048be..fc3c6c5dd8bc9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -418,6 +418,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: @@ -830,7 +831,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 @@ -854,7 +855,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 index 4cd73d5f6243d..841a7f0d863c4 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -326,7 +326,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", @@ -434,6 +433,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) @@ -459,10 +461,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", @@ -483,6 +488,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", @@ -773,8 +784,10 @@ if (is_mac) { framework_contents = [ "Resources", "Libraries", - "Helpers", ] + if (!is_mas_build) { + framework_contents += [ "Helpers" ] + } public_deps = [ ":electron_framework_libraries", ":electron_lib", @@ -1005,13 +1018,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 new file mode 100644 index 0000000000000..db422bb6dc7e6 --- /dev/null +++ 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 index 19fbaebd8551b..df05fa1caeebb 100644 --- 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 index a9515c068a816..c174594750853 100644 --- 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 index bc1257758b13f..2e42ee18e99c6 100644 --- a/shell/app/node_main.cc +++ b/shell/app/node_main.cc @@ -18,19 +18,15 @@ #include "base/strings/utf_string_conversions.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" @@ -43,6 +39,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 { // Initialize Node.js cli options to pass to Node.js @@ -84,6 +87,11 @@ void SetNodeCliFlags() { ProcessGlobalArgs(&args, nullptr, &errors, node::kDisallowedInEnvironment); } +#if defined(MAS_BUILD) +void SetCrashKeyStub(const std::string& key, const std::string& value) {} +void ClearCrashKeyStub(const std::string& key) {} +#endif + } // namespace namespace electron { @@ -217,10 +225,15 @@ 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 index cc09c82da8c1b..650fe65cfb564 100644 --- 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 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 index c5a941868668c..cca9c0eeaef64 100644 --- a/shell/browser/electron_browser_client.cc +++ b/shell/browser/electron_browser_client.cc @@ -171,12 +171,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 index 624a28e4330e1..b011a806315b0 100644 --- 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 GetParameters(v8::Isolate* isolate) {