From dbe83e0a23f2a69501c6624e82185d08ccf90dca Mon Sep 17 00:00:00 2001 From: aminya Date: Thu, 16 Jul 2020 06:09:01 -0500 Subject: [PATCH 1/7] more restore keys --- script/vsts/platforms/templates/cache.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/script/vsts/platforms/templates/cache.yml b/script/vsts/platforms/templates/cache.yml index 37683f403a4..df78cd47965 100644 --- a/script/vsts/platforms/templates/cache.yml +++ b/script/vsts/platforms/templates/cache.yml @@ -12,6 +12,9 @@ steps: displayName: Cache node_modules inputs: key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json, script/vsts/platforms/${{ parameters.OS }}.yml' + restoreKeys: | + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" path: 'node_modules' cacheHitVar: MainNodeModulesRestored @@ -19,6 +22,9 @@ steps: displayName: Cache script/node_modules inputs: key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json, script/vsts/platforms/${{ parameters.OS }}.yml' + restoreKeys: | + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" path: 'script/node_modules' cacheHitVar: ScriptNodeModulesRestored @@ -26,5 +32,8 @@ steps: displayName: Cache apm/node_modules inputs: key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json, script/vsts/platforms/${{ parameters.OS }}.yml' + restoreKeys: | + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" path: 'apm/node_modules' cacheHitVar: ApmNodeModulesRestored From 8fac66295e3a12ae93775b31a99fcd7ab2dc4152 Mon Sep 17 00:00:00 2001 From: Amin Yahyaabadi Date: Wed, 29 Jul 2020 13:31:37 -0500 Subject: [PATCH 2/7] Add preparation.yml to the keys for cache restoration Co-authored-by: DeeDeeG --- script/vsts/platforms/templates/cache.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/vsts/platforms/templates/cache.yml b/script/vsts/platforms/templates/cache.yml index df78cd47965..ca4c8a4b415 100644 --- a/script/vsts/platforms/templates/cache.yml +++ b/script/vsts/platforms/templates/cache.yml @@ -11,7 +11,7 @@ steps: - task: Cache@2 displayName: Cache node_modules inputs: - key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json, script/vsts/platforms/${{ parameters.OS }}.yml' + key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json, script/vsts/platforms/templates/preparation.yml' restoreKeys: | npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json npm | "$(Agent.OS)" | "$(BUILD_ARCH)" @@ -21,7 +21,7 @@ steps: - task: Cache@2 displayName: Cache script/node_modules inputs: - key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json, script/vsts/platforms/${{ parameters.OS }}.yml' + key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json, script/vsts/platforms/templates/preparation.yml' restoreKeys: | npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json npm | "$(Agent.OS)" | "$(BUILD_ARCH)" @@ -31,7 +31,7 @@ steps: - task: Cache@2 displayName: Cache apm/node_modules inputs: - key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json, script/vsts/platforms/${{ parameters.OS }}.yml' + key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json, script/vsts/platforms/templates/preparation.yml' restoreKeys: | npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json npm | "$(Agent.OS)" | "$(BUILD_ARCH)" From c9cf513e27cc9133d12469214d9d9b711df38ad2 Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Thu, 30 Jul 2020 14:37:15 -0400 Subject: [PATCH 3/7] bootstrap: Always use `npm install` (not `npm ci`) This change only affects the CI environment. Allows better re-use of existing installed packages (such as those restored from a cache). `npm ci` and `apm ci` forced a fresh reinstall of packages, including rebuilding all native code. Rebuilding native code accounts for the majority of install time. By not rebuilding native code unless totally necessary, `npm install` and `apm install` can save a lot of time versus `npm ci` and `apm ci`. --- script/lib/install-script-dependencies.js | 4 ++-- script/lib/run-apm-install.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/script/lib/install-script-dependencies.js b/script/lib/install-script-dependencies.js index 6c69c360fe4..fe7bf6eab04 100644 --- a/script/lib/install-script-dependencies.js +++ b/script/lib/install-script-dependencies.js @@ -9,8 +9,8 @@ process.env.ELECTRON_CUSTOM_VERSION = CONFIG.appMetadata.electronVersion; module.exports = function(ci) { console.log('Installing script dependencies'); childProcess.execFileSync( - CONFIG.getNpmBinPath(ci), - ['--loglevel=error', ci ? 'ci' : 'install'], + CONFIG.getNpmBinPath(), + ['--loglevel=error', 'install'], { env: process.env, cwd: CONFIG.scriptRootPath } ); }; diff --git a/script/lib/run-apm-install.js b/script/lib/run-apm-install.js index b5ce0869163..1bcd5a34179 100644 --- a/script/lib/run-apm-install.js +++ b/script/lib/run-apm-install.js @@ -11,7 +11,7 @@ module.exports = function(packagePath, ci, stdioOptions) { // Set our target (Electron) version so that node-pre-gyp can download the // proper binaries. installEnv.npm_config_target = CONFIG.appMetadata.electronVersion; - childProcess.execFileSync(CONFIG.getApmBinPath(), [ci ? 'ci' : 'install'], { + childProcess.execFileSync(CONFIG.getApmBinPath(), ['install'], { env: installEnv, cwd: packagePath, stdio: stdioOptions || 'inherit' From 74f6ca20b7f0cca3f6c85c73eaebceec2b54e95c Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Thu, 30 Jul 2020 15:37:01 -0400 Subject: [PATCH 4/7] CI: Add `platforms/[OS]` back to cache key This should be included, to catch OS version upgrades. --- script/vsts/platforms/templates/cache.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/vsts/platforms/templates/cache.yml b/script/vsts/platforms/templates/cache.yml index ca4c8a4b415..cd358b27352 100644 --- a/script/vsts/platforms/templates/cache.yml +++ b/script/vsts/platforms/templates/cache.yml @@ -11,7 +11,7 @@ steps: - task: Cache@2 displayName: Cache node_modules inputs: - key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json, script/vsts/platforms/templates/preparation.yml' + key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml' restoreKeys: | npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json npm | "$(Agent.OS)" | "$(BUILD_ARCH)" @@ -21,7 +21,7 @@ steps: - task: Cache@2 displayName: Cache script/node_modules inputs: - key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json, script/vsts/platforms/templates/preparation.yml' + key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml' restoreKeys: | npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json npm | "$(Agent.OS)" | "$(BUILD_ARCH)" @@ -31,7 +31,7 @@ steps: - task: Cache@2 displayName: Cache apm/node_modules inputs: - key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json, script/vsts/platforms/templates/preparation.yml' + key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml' restoreKeys: | npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json npm | "$(Agent.OS)" | "$(BUILD_ARCH)" From a2c211d48afb66fca87c55f800dd3d1148ee52ee Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Thu, 30 Jul 2020 15:48:52 -0400 Subject: [PATCH 5/7] CI: Better cache restoreKeys Allow restoring cache even if package.json or package-lock.json have changed. `npm install` should be able to sort out the updated packages. --- script/vsts/platforms/templates/cache.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/script/vsts/platforms/templates/cache.yml b/script/vsts/platforms/templates/cache.yml index cd358b27352..a3953289b18 100644 --- a/script/vsts/platforms/templates/cache.yml +++ b/script/vsts/platforms/templates/cache.yml @@ -13,8 +13,8 @@ steps: inputs: key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml' restoreKeys: | - npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, package-lock.json - npm | "$(Agent.OS)" | "$(BUILD_ARCH)" + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | package.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml path: 'node_modules' cacheHitVar: MainNodeModulesRestored @@ -23,8 +23,8 @@ steps: inputs: key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml' restoreKeys: | - npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/package-lock.json - npm | "$(Agent.OS)" | "$(BUILD_ARCH)" + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/package.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml path: 'script/node_modules' cacheHitVar: ScriptNodeModulesRestored @@ -33,7 +33,7 @@ steps: inputs: key: 'npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml' restoreKeys: | - npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, apm/package-lock.json - npm | "$(Agent.OS)" | "$(BUILD_ARCH)" + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | apm/package.json, script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml + npm | "$(Agent.OS)" | "$(BUILD_ARCH)" | script/vsts/platforms/templates/preparation.yml, script/vsts/platforms/${{ parameters.OS }}.yml path: 'apm/node_modules' cacheHitVar: ApmNodeModulesRestored From 035531a7e6c9e71ad41154cacb0e5e87941f6b6c Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Thu, 30 Jul 2020 17:05:44 -0400 Subject: [PATCH 6/7] bootstrap: Use the `--no-save` flag in CI Prevents the cache from becoming outdated when e.g. The "version" field of `package.json` is updated and checked in, but not updated or checked in for `package-lock.json`. --- script/lib/install-apm.js | 2 +- script/lib/run-apm-install.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/lib/install-apm.js b/script/lib/install-apm.js index a233f611951..e2d9c61befb 100644 --- a/script/lib/install-apm.js +++ b/script/lib/install-apm.js @@ -9,7 +9,7 @@ module.exports = function(ci) { // npm ci leaves apm with a bunch of unmet dependencies childProcess.execFileSync( CONFIG.getNpmBinPath(), - ['--global-style', '--loglevel=error', 'install'], + ['--global-style', '--loglevel=error', 'install', ci ? '--no-save' : ''], { env: process.env, cwd: CONFIG.apmRootPath } ); }; diff --git a/script/lib/run-apm-install.js b/script/lib/run-apm-install.js index 1bcd5a34179..6e5eb7264dd 100644 --- a/script/lib/run-apm-install.js +++ b/script/lib/run-apm-install.js @@ -11,7 +11,7 @@ module.exports = function(packagePath, ci, stdioOptions) { // Set our target (Electron) version so that node-pre-gyp can download the // proper binaries. installEnv.npm_config_target = CONFIG.appMetadata.electronVersion; - childProcess.execFileSync(CONFIG.getApmBinPath(), ['install'], { + childProcess.execFileSync(CONFIG.getApmBinPath(), ['install', ci ? '--no-save' : ''], { env: installEnv, cwd: packagePath, stdio: stdioOptions || 'inherit' From 72c1d798a8cd76b9bffdf02d369ec0d6dc27beeb Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Thu, 30 Jul 2020 19:04:03 -0400 Subject: [PATCH 7/7] :shirt: script/lib/run-apm-install.js: Fix lints --- script/lib/run-apm-install.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/script/lib/run-apm-install.js b/script/lib/run-apm-install.js index 6e5eb7264dd..14b1c5d9905 100644 --- a/script/lib/run-apm-install.js +++ b/script/lib/run-apm-install.js @@ -11,9 +11,13 @@ module.exports = function(packagePath, ci, stdioOptions) { // Set our target (Electron) version so that node-pre-gyp can download the // proper binaries. installEnv.npm_config_target = CONFIG.appMetadata.electronVersion; - childProcess.execFileSync(CONFIG.getApmBinPath(), ['install', ci ? '--no-save' : ''], { - env: installEnv, - cwd: packagePath, - stdio: stdioOptions || 'inherit' - }); + childProcess.execFileSync( + CONFIG.getApmBinPath(), + ['install', ci ? '--no-save' : ''], + { + env: installEnv, + cwd: packagePath, + stdio: stdioOptions || 'inherit' + } + ); };