From 14fc6f308106c644c2e59527b51bb9c295b30b04 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 3 Oct 2018 16:03:26 -0700 Subject: [PATCH] chore: add GN linting (#14678) * chore: add GN linter * chore: fix GN lint errors * try some crazy bash to get a gn exe * base64 on linux is different * cloning build_tools doesn't download GN * download_from_google_storage needs depot_tools in the path * fixup! chore: add GN linter --- .vsts/lint.yml | 22 ++++++++++++++++++--- brightray/BUILD.gn | 46 +++++++++++++++++++++---------------------- build/config/BUILD.gn | 1 + build/npm.gni | 20 ++++++++++++------- electron_paks.gni | 4 ++-- filenames.gni | 8 ++------ native_mate/BUILD.gn | 2 +- package.json | 1 + script/lint.js | 29 ++++++++++++++++++++++++--- 9 files changed, 88 insertions(+), 45 deletions(-) diff --git a/.vsts/lint.yml b/.vsts/lint.yml index be009fe035..780c96cad7 100644 --- a/.vsts/lint.yml +++ b/.vsts/lint.yml @@ -3,13 +3,29 @@ pool: steps: - bash: | + # "depot_tools" has to be checkout into "//third_party/depot_tools" so pylint.py can a "pylintrc" file. git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git "${AGENT_BUILDDIRECTORY}/third_party/depot_tools" - echo "##vso[task.setvariable variable=PATH]$PATH:${AGENT_BUILDDIRECTORY}/depot_tools" + echo "##vso[task.setvariable variable=PATH]$PATH:${AGENT_BUILDDIRECTORY}/third_party/depot_tools" displayName: Setup Depot Tools - bash: | - export PATH="$PATH:${AGENT_BUILDDIRECTORY}/third_party/depot_tools" - echo "##vso[task.setvariable variable=PATH]$PATH" + chromium_revision="$(grep -A1 chromium_version DEPS | tr -d '\n' | cut -d\' -f4)" + buildtools_revision="$(curl -sL "https://chromium.googlesource.com/chromium/src/+/${chromium_revision}/DEPS?format=TEXT" | base64 -d | grep buildtools_revision -A1 | tr -d '\n' | cut -d\' -f4)" + + git clone https://chromium.googlesource.com/chromium/buildtools "${AGENT_TEMPDIRECTORY}/buildtools" + (cd "${AGENT_TEMPDIRECTORY}/buildtools" && git checkout "$buildtools_revision") + echo "##vso[task.setvariable variable=CHROMIUM_BUILDTOOLS_PATH]$AGENT_TEMPDIRECTORY/buildtools" + + download_from_google_storage --bucket chromium-gn -s "${AGENT_TEMPDIRECTORY}/buildtools/linux64/gn.sha1" + displayName: Download gn binary + +- bash: | + # gn.py tries to find a gclient root folder starting from the current dir. + # When it fails and returns "None" path, the whole script fails. Let's "fix" it. + touch .gclient + # Another option would be to checkout "buildtools" inside the Electron checkout, + # but then we would lint its contents (at least gn format), and it doesn't pass it. + npm install npm run lint displayName: Run Lint diff --git a/brightray/BUILD.gn b/brightray/BUILD.gn index 2441ba8052..f057954bb7 100644 --- a/brightray/BUILD.gn +++ b/brightray/BUILD.gn @@ -47,15 +47,21 @@ static_library("brightray") { "browser/inspectable_web_contents.cc", "browser/inspectable_web_contents.h", "browser/inspectable_web_contents_delegate.h", - "browser/inspectable_web_contents_view_delegate.cc", - "browser/inspectable_web_contents_view_delegate.h", "browser/inspectable_web_contents_impl.cc", "browser/inspectable_web_contents_impl.h", "browser/inspectable_web_contents_view.h", + "browser/inspectable_web_contents_view_delegate.cc", + "browser/inspectable_web_contents_view_delegate.h", "browser/inspectable_web_contents_view_mac.h", "browser/inspectable_web_contents_view_mac.mm", "browser/io_thread.cc", "browser/io_thread.h", + "browser/linux/libnotify_loader.cc", + "browser/linux/libnotify_loader.h", + "browser/linux/libnotify_notification.cc", + "browser/linux/libnotify_notification.h", + "browser/linux/notification_presenter_linux.cc", + "browser/linux/notification_presenter_linux.h", "browser/mac/bry_inspectable_web_contents_view.h", "browser/mac/bry_inspectable_web_contents_view.mm", "browser/mac/cocoa_notification.h", @@ -76,19 +82,21 @@ static_library("brightray") { "browser/net/require_ct_delegate.h", "browser/net_log.cc", "browser/net_log.h", + "browser/notification.cc", + "browser/notification.h", "browser/notification_delegate.h", "browser/notification_presenter.cc", "browser/notification_presenter.h", - "browser/notification.cc", - "browser/notification.h", "browser/platform_notification_service.cc", "browser/platform_notification_service.h", - "browser/linux/libnotify_loader.h", - "browser/linux/libnotify_loader.cc", - "browser/linux/libnotify_notification.h", - "browser/linux/libnotify_notification.cc", - "browser/linux/notification_presenter_linux.h", - "browser/linux/notification_presenter_linux.cc", + "browser/url_request_context_getter.cc", + "browser/url_request_context_getter.h", + "browser/views/inspectable_web_contents_view_views.cc", + "browser/views/inspectable_web_contents_view_views.h", + "browser/views/views_delegate.cc", + "browser/views/views_delegate.h", + "browser/web_ui_controller_factory.cc", + "browser/web_ui_controller_factory.h", "browser/win/notification_presenter_win.cc", "browser/win/notification_presenter_win.h", "browser/win/notification_presenter_win7.cc", @@ -98,26 +106,18 @@ static_library("brightray") { "browser/win/win32_desktop_notifications/common.h", "browser/win/win32_desktop_notifications/desktop_notification_controller.cc", "browser/win/win32_desktop_notifications/desktop_notification_controller.h", - "browser/win/win32_desktop_notifications/toast_uia.cc", - "browser/win/win32_desktop_notifications/toast_uia.h", "browser/win/win32_desktop_notifications/toast.cc", "browser/win/win32_desktop_notifications/toast.h", + "browser/win/win32_desktop_notifications/toast_uia.cc", + "browser/win/win32_desktop_notifications/toast_uia.h", "browser/win/win32_notification.cc", "browser/win/win32_notification.h", "browser/win/windows_toast_notification.cc", "browser/win/windows_toast_notification.h", - "browser/url_request_context_getter.cc", - "browser/url_request_context_getter.h", - "browser/views/inspectable_web_contents_view_views.h", - "browser/views/inspectable_web_contents_view_views.cc", - "browser/views/views_delegate.cc", - "browser/views/views_delegate.h", - "browser/web_ui_controller_factory.cc", - "browser/web_ui_controller_factory.h", "browser/zoom_level_delegate.cc", "browser/zoom_level_delegate.h", - "common/application_info.h", "common/application_info.cc", + "common/application_info.h", "common/application_info_mac.mm", "common/application_info_win.cc", "common/content_client.cc", @@ -127,10 +127,10 @@ static_library("brightray") { "common/main_delegate.cc", "common/main_delegate.h", "common/main_delegate_mac.mm", + "common/platform_util.h", + "common/platform_util_linux.cc", "common/switches.cc", "common/switches.h", - "common/platform_util_linux.cc", - "common/platform_util.h", ] set_sources_assignment_filter(sources_assignment_filter) diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn index 677ac8b8fa..e805478b21 100644 --- a/build/config/BUILD.gn +++ b/build/config/BUILD.gn @@ -17,6 +17,7 @@ config("build_time_executable") { } } } + # For MAS build, we force defining "MAS_BUILD". config("mas_build") { if (is_mas_build) { diff --git a/build/npm.gni b/build/npm.gni index 2c903f0143..e98afc1621 100644 --- a/build/npm.gni +++ b/build/npm.gni @@ -1,16 +1,22 @@ template("npm_action") { assert(defined(invoker.script), "Need script name to run (must be defined in package.json)") - assert(defined(invoker.args), - "Need script argumets") + assert(defined(invoker.args), "Need script argumets") action(target_name) { - forward_variables_from(invoker, ["deps", "public_deps", "sources", "inputs", "outputs"]) + forward_variables_from(invoker, + [ + "deps", + "public_deps", + "sources", + "inputs", + "outputs", + ]) script = "//electron/build/npm-run.py" args = [ - "--silent", - invoker.script, - "--" - ] + invoker.args + "--silent", + invoker.script, + "--", + ] + invoker.args } } diff --git a/electron_paks.gni b/electron_paks.gni index 3f7d67b94f..c540fdc894 100644 --- a/electron_paks.gni +++ b/electron_paks.gni @@ -17,9 +17,9 @@ template("electron_repack_percent") { # All sources should also have deps for completeness. sources = [ - "$root_gen_dir/third_party/blink/public/resources/blink_scaled_resources_${percent}_percent.pak", "$root_gen_dir/components/components_resources_${percent}_percent.pak", "$root_gen_dir/content/app/resources/content_resources_${percent}_percent.pak", + "$root_gen_dir/third_party/blink/public/resources/blink_scaled_resources_${percent}_percent.pak", "$root_gen_dir/ui/resources/ui_resources_${percent}_percent.pak", ] @@ -53,12 +53,12 @@ template("electron_extra_paks") { ]) output = "${invoker.output_dir}/resources.pak" sources = [ - "$root_gen_dir/third_party/blink/public/resources/blink_resources.pak", "$root_gen_dir/components/components_resources.pak", "$root_gen_dir/content/browser/tracing/tracing_resources.pak", "$root_gen_dir/content/content_resources.pak", "$root_gen_dir/mojo/public/js/mojo_bindings_resources.pak", "$root_gen_dir/net/net_resources.pak", + "$root_gen_dir/third_party/blink/public/resources/blink_resources.pak", ] deps = [ "//components/resources", diff --git a/filenames.gni b/filenames.gni index 19d914ceb9..55353382bd 100644 --- a/filenames.gni +++ b/filenames.gni @@ -644,9 +644,7 @@ filenames = { "chromium_src/library_loaders/libspeechd.h", ] - lib_sources_linux = [ - "chromium_src/chrome/browser/icon_loader_auralinux.cc", - ] + lib_sources_linux = [ "chromium_src/chrome/browser/icon_loader_auralinux.cc" ] lib_sources_nss = [ "chromium_src/chrome/browser/certificate_manager_model.cc", "chromium_src/chrome/browser/certificate_manager_model.h", @@ -672,7 +670,5 @@ filenames = { "atom/app/atom_library_main.mm", ] - login_helper_sources = [ - "atom/app/atom_login_helper.mm", - ] + login_helper_sources = [ "atom/app/atom_login_helper.mm" ] } diff --git a/native_mate/BUILD.gn b/native_mate/BUILD.gn index c723f0e96a..453aa37a5d 100644 --- a/native_mate/BUILD.gn +++ b/native_mate/BUILD.gn @@ -6,9 +6,9 @@ config("native_mate_config") { source_set("native_mate") { deps = [ - "//third_party/electron_node:node_lib", "//base", "//net", + "//third_party/electron_node:node_lib", "//v8:v8_headers", ] public_configs = [ ":native_mate_config" ] diff --git a/package.json b/package.json index f65e44c817..888dea29ad 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "lint:clang-format": "python script/run-clang-format.py -r -c atom/ chromium_src/ brightray/ || (echo \"\\nCode not formatted correctly.\" && exit 1)", "lint:cpp": "./script/lint.js --cc", "lint:py": "./script/lint.js --py", + "lint:gn": "./script/lint.js --gn", "lint:docs": "remark docs -qf && npm run lint:js-in-markdown && npm run create-typescript-definitions && npm run lint:docs-relative-links", "lint:docs-relative-links": "python ./script/check-relative-doc-links.py", "lint:js-in-markdown": "standard-markdown docs", diff --git a/script/lint.js b/script/lint.js index 88a5b1e81b..e135a9a9c4 100755 --- a/script/lint.js +++ b/script/lint.js @@ -86,12 +86,35 @@ const LINTERS = [ { if (opts.fix) args.unshift('--fix') spawnAndCheckExitCode(cmd, args, { cwd: SOURCE_ROOT }) } +}, { + key: 'gn', + roots: ['.'], + test: filename => filename.endsWith('.gn') || filename.endsWith('.gni'), + run: (opts, filenames) => { + const allOk = filenames.map(filename => { + const args = ['format', filename] + if (!opts.fix) args.push('--dry-run') + const result = childProcess.spawnSync('gn', args, { stdio: 'inherit' }) + if (result.status === 0) { + return true + } else if (result.status === 2) { + console.log(`GN format errors in "${filename}". Run 'gn format "${filename}"' or rerun with --fix to fix them.`) + return false + } else { + console.log(`Error running 'gn format --dry-run "${filename}"': exit code ${result.status}`) + return false + } + }).every(x => x) + if (!allOk) { + process.exit(1) + } + } }] function parseCommandLine () { let help const opts = minimist(process.argv.slice(2), { - boolean: [ 'c++', 'javascript', 'python', 'help', 'changed', 'fix', 'verbose' ], + boolean: [ 'c++', 'javascript', 'python', 'gn', 'help', 'changed', 'fix', 'verbose' ], alias: { 'c++': ['cc', 'cpp', 'cxx'], javascript: ['js', 'es'], python: 'py', changed: 'c', help: 'h', verbose: 'v' }, unknown: arg => { help = true } }) @@ -167,8 +190,8 @@ async function main () { const opts = parseCommandLine() // no mode specified? run 'em all - if (!opts['c++'] && !opts.javascript && !opts.python) { - opts['c++'] = opts.javascript = opts.python = true + if (!opts['c++'] && !opts.javascript && !opts.python && !opts.gn) { + opts['c++'] = opts.javascript = opts.python = opts.gn = true } const linters = LINTERS.filter(x => opts[x.key])