From b202ad1e249d168875560b73375abca56b2ca3ab Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 5 Feb 2019 12:10:15 -0800 Subject: [PATCH] refactor: remove js2asar.py and port logic to JS in more readable / GN-style way (#16718) * refactor: remove js2asar.py and port logic to JS in more readable / GN-style way * refactor: further clean up ASAR impl, add new node_action GN template --- BUILD.gn | 53 +++++++++++++++++++++++++++++------ build/asar.gni | 34 +++++++++++++++-------- build/node.gni | 21 ++++++++++++++ build/run-node.py | 14 ++++++++++ default_app/index.html | 2 +- default_app/renderer.js | 2 +- filenames.gni | 3 ++ script/gn-asar.js | 61 +++++++++++++++++++++++++++++++++++++++++ tools/js2asar.py | 51 ---------------------------------- 9 files changed, 168 insertions(+), 73 deletions(-) create mode 100644 build/node.gni create mode 100644 build/run-node.py create mode 100644 script/gn-asar.js delete mode 100755 tools/js2asar.py diff --git a/BUILD.gn b/BUILD.gn index 8d8000dc28..959c811dad 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -137,7 +137,12 @@ action("atom_js2c") { rebase_path(sources, root_build_dir) } -asar("js2asar") { +target_gen_electron_js = "$target_gen_dir/js/electron" +target_gen_default_app_js = "$target_gen_dir/js/default_app" + +# TODO(MarshallOfSound) +# This copy will be replaced by a call to tsc in the future +copy("lib_js") { sources = filenames.js_sources if (enable_desktop_capturer) { sources += [ @@ -156,18 +161,50 @@ asar("js2asar") { "lib/browser/api/views/text-field.js", ] } + + outputs = [ + "$target_gen_electron_js/{{source}}", + ] +} + +asar("electron_asar") { + deps = [ + ":lib_js", + ] + + root = "$target_gen_electron_js/electron/lib" + sources = get_target_outputs(":lib_js") outputs = [ "$root_out_dir/resources/electron.asar", ] - root = "lib" } -asar("app2asar") { +copy("default_app_js") { sources = filenames.default_app_sources + outputs = [ + "$target_gen_default_app_js/{{source}}", + ] +} + +copy("default_app_octicon_deps") { + sources = filenames.default_app_octicon_sources + outputs = [ + "$target_gen_default_app_js/electron/default_app/octicon/{{source_file_part}}", + ] +} + +asar("default_app_asar") { + deps = [ + ":default_app_js", + ":default_app_octicon_deps", + ] + + root = "$target_gen_default_app_js/electron/default_app" + sources = get_target_outputs(":default_app_js") + + get_target_outputs(":default_app_octicon_deps") outputs = [ "$root_out_dir/resources/default_app.asar", ] - root = "default_app" } grit("resources") { @@ -712,9 +749,9 @@ if (is_mac) { bundle_data("electron_app_resources") { public_deps = [ - ":app2asar", + ":default_app_asar", ":electron_app_strings_bundle_data", - ":js2asar", + ":electron_asar", ] sources = [ "$root_out_dir/resources/default_app.asar", @@ -760,10 +797,10 @@ if (is_mac) { sources = filenames.app_sources include_dirs = [ "." ] deps = [ - ":app2asar", + ":default_app_asar", ":electron_app_manifest", + ":electron_asar", ":electron_lib", - ":js2asar", ":packed_resources", "//content:sandbox_helper_win", "//ui/strings", diff --git a/build/asar.gni b/build/asar.gni index 110a1d8928..4df8ea34dd 100644 --- a/build/asar.gni +++ b/build/asar.gni @@ -1,5 +1,6 @@ -import("npm.gni") +import("node.gni") +# TODO(MarshallOfSound): Move to electron/node, this is the only place it is used now # Run an action with a given working directory. Behaves identically to the # action() target type, with the exception that it changes directory before # running the script. @@ -32,19 +33,28 @@ template("chdir_action") { template("asar") { assert(defined(invoker.sources), - "Need sources in $target_name listing the JS files.") + "Need sources in $target_name listing the source files") assert(defined(invoker.outputs), "Need asar name (as 1-element array, e.g. \$root_out_dir/foo.asar)") - assert(defined(invoker.root), "Need asar root directory") - asar_root = invoker.root + assert(defined(invoker.root), "Need the base dir for generating the ASAR") - # js2asar.py expects relative paths to its inputs, so we must run it in a - # working directory in which those relative paths make sense. - chdir_action(target_name) { - sources = invoker.sources - outputs = invoker.outputs - script = "//electron/tools/js2asar.py" - cwd = rebase_path(get_path_info(".", "abspath")) - args = rebase_path(outputs, cwd) + [ asar_root ] + rebase_path(sources, ".") + node_action(target_name) { + forward_variables_from(invoker, + "*", + [ + "script", + "args", + ]) + + script = "//electron/script/gn-asar.js" + args = [ + "--base", + rebase_path(root), + "--files", + ] + rebase_path(sources) + + [ + "--out", + rebase_path(outputs[0]), + ] } } diff --git a/build/node.gni b/build/node.gni new file mode 100644 index 0000000000..a65f718e63 --- /dev/null +++ b/build/node.gni @@ -0,0 +1,21 @@ +template("node_action") { + assert(defined(invoker.script), "Need script path to run") + assert(defined(invoker.args), "Need script argumets") + + action(target_name) { + forward_variables_from(invoker, + [ + "deps", + "public_deps", + "sources", + "inputs", + "outputs", + ]) + if (!defined(inputs)) { + inputs = [] + } + inputs += [ invoker.script ] + script = "//electron/build/run-node.py" + args = [ rebase_path(invoker.script) ] + invoker.args + } +} diff --git a/build/run-node.py b/build/run-node.py new file mode 100644 index 0000000000..0e0c30d671 --- /dev/null +++ b/build/run-node.py @@ -0,0 +1,14 @@ +import os +import subprocess +import sys + + +SOURCE_ROOT = os.path.dirname(os.path.dirname(__file__)) + +def main(): + # Proxy all args to node script + script = os.path.join(SOURCE_ROOT, sys.argv[1]) + subprocess.check_call(['node', script] + [str(x) for x in sys.argv[2:]]) + +if __name__ == '__main__': + sys.exit(main()) diff --git a/default_app/index.html b/default_app/index.html index c2a796291d..cba87e5eef 100644 --- a/default_app/index.html +++ b/default_app/index.html @@ -4,7 +4,7 @@ Electron - + diff --git a/default_app/renderer.js b/default_app/renderer.js index 6d26007b7a..785e27fa5a 100644 --- a/default_app/renderer.js +++ b/default_app/renderer.js @@ -35,7 +35,7 @@ function initialize () { document.querySelector('.command-example').innerText = `${electronPath} path-to-app` function getOcticonSvg (name) { - const octiconPath = path.resolve(__dirname, 'node_modules', 'octicons', 'build', 'svg', `${name}.svg`) + const octiconPath = path.resolve(__dirname, 'octicon', `${name}.svg`) if (fs.existsSync(octiconPath)) { const content = fs.readFileSync(octiconPath, 'utf8') const div = document.createElement('div') diff --git a/filenames.gni b/filenames.gni index 48cb824972..23ab276fa0 100644 --- a/filenames.gni +++ b/filenames.gni @@ -99,6 +99,9 @@ filenames = { "default_app/package.json", "default_app/renderer.js", "default_app/styles.css", + ] + + default_app_octicon_sources = [ "node_modules/octicons/build/build.css", "node_modules/octicons/build/svg/gist.svg", "node_modules/octicons/build/svg/mark-github.svg", diff --git a/script/gn-asar.js b/script/gn-asar.js new file mode 100644 index 0000000000..a68b141deb --- /dev/null +++ b/script/gn-asar.js @@ -0,0 +1,61 @@ +const asar = require('asar') +const assert = require('assert') +const fs = require('fs-extra') +const os = require('os') +const path = require('path') + +const getArgGroup = (name) => { + const group = [] + let inGroup = false + for (const arg of process.argv) { + // At the next flag we stop being in the current group + if (arg.startsWith('--')) inGroup = false + // Push all args in the group + if (inGroup) group.push(arg) + // If we find the start flag, start pushing + if (arg === `--${name}`) inGroup = true + } + + return group +} + +const base = getArgGroup('base') +const files = getArgGroup('files') +const out = getArgGroup('out') + +assert(base.length === 1, 'should have a single base dir') +assert(files.length >= 1, 'should have at least one input file') +assert(out.length === 1, 'should have a single out path') + +// Ensure all files are inside the base dir +for (const file of files) { + if (!file.startsWith(base[0])) { + console.error(`Expected all files to be inside the base dir but "${file}" was not in "${base[0]}"`) + process.exit(1) + } +} + +const tmpPath = fs.mkdtempSync(path.resolve(os.tmpdir(), 'electron-gn-asar-')) + +try { + // Copy all files to a tmp dir to avoid including scrap files in the ASAR + for (const file of files) { + const newLocation = path.resolve(tmpPath, path.relative(base[0], file)) + fs.mkdirsSync(path.dirname(newLocation)) + fs.writeFileSync(newLocation, fs.readFileSync(file)) + } +} catch (err) { + console.error('Unexpected error while generating ASAR', err) + fs.removeSync(tmpPath) + process.exit(1) +} + +// Create the ASAR archive +asar.createPackageWithOptions(tmpPath, out[0], {}, (err) => { + fs.removeSync(tmpPath) + + if (err) { + console.error('Unexpected error while generating ASAR', err) + process.exit(1) + } +}) diff --git a/tools/js2asar.py b/tools/js2asar.py deleted file mode 100755 index 145c1298c6..0000000000 --- a/tools/js2asar.py +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env python - -import errno -import os -import shutil -import subprocess -import sys -import tempfile - -SOURCE_ROOT = os.path.dirname(os.path.dirname(__file__)) - - -def main(): - archive = sys.argv[1] - folder_name = sys.argv[2] - source_files = sys.argv[3:] - - output_dir = tempfile.mkdtemp() - copy_files(source_files, output_dir, folder_name) - call_asar(archive, os.path.join(output_dir, folder_name)) - shutil.rmtree(output_dir) - - -def copy_files(source_files, output_dir, folder_name): - for source_file in source_files: - output_path = os.path.join(output_dir, source_file) - # Files that aren't in the default_app folder need to be put inside - # the temp one we are making so they end up in the ASAR - if not os.path.normpath(source_file).startswith(folder_name + os.sep): - output_path = os.path.join(output_dir, folder_name, source_file) - safe_mkdir(os.path.dirname(output_path)) - shutil.copy2(source_file, output_path) - - -def call_asar(archive, output_dir): - asar = os.path.join(SOURCE_ROOT, 'node_modules', '.bin', 'asar') - if sys.platform in ['win32', 'cygwin']: - asar += '.cmd' - subprocess.check_call([asar, 'pack', output_dir, archive]) - - -def safe_mkdir(path): - try: - os.makedirs(path) - except OSError as e: - if e.errno != errno.EEXIST: - raise - - -if __name__ == '__main__': - sys.exit(main())