From fab92ee8a1d068ab89bd0579a37ef27fef0a4967 Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Fri, 11 Oct 2019 12:42:17 -0400 Subject: [PATCH 1/8] ci: skip build on doc only changes --- .circleci/config.yml | 105 ++++++++++++++++++++++++++++++++++++++ appveyor.yml | 8 +++ script/doc-only-change.js | 58 +++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 script/doc-only-change.js diff --git a/.circleci/config.yml b/.circleci/config.yml index 74357441127d0..a6f1a4303c74e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -704,6 +704,50 @@ step-save-src-cache: &step-save-src-cache key: v5-src-cache-{{ arch }}-{{ checksum "src/electron/.depshash" }} name: Persisting src cache +# Check for doc only change +step-check-for-doc-only-change: &step-check-for-doc-only-change + run: + name: Check if commit is doc only change + command: | + cd src/electron + node script/yarn install --frozen-lockfile + export DOC_ONLY_CHANGE="`node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST`" + if [ "$DOC_ONLY_CHANGE" = "true" ]; then + echo $DOC_ONLY_CHANGE > .skip-ci-build + else + touch .skip-ci-build + fi + +step-persist-doc-only-change: &step-persist-doc-only-change + persist_to_workspace: + root: . + paths: + - src/electron/.skip-ci-build + +step-maybe-early-exit-doc-only-change: &step-maybe-early-exit-doc-only-change + run: + name: Shortcircuit build if doc only change + command: | + if [ -s src/electron/.skip-ci-build ]; then + circleci-agent step halt + fi + +step-maybe-early-exit-no-doc-change: &step-maybe-early-exit-no-doc-change + run: + name: Shortcircuit job if change is not doc only + command: | + if [ ! -s src/electron/.skip-ci-build ]; then + circleci-agent step halt + fi + +step-ts-compile: &step-ts-compile + run: + name: Run TS/JS compile on doc only change + command: | + cd src + ninja -C out/Default electron:default_app_js -j $NUMBER_OF_NINJA_PROCESSES + ninja -C out/Default electron:atom_js2c -j $NUMBER_OF_NINJA_PROCESSES + # Lists of steps. steps-lint: &steps-lint steps: @@ -752,6 +796,9 @@ steps-lint: &steps-lint steps-checkout-fast: &steps-checkout-fast steps: - *step-checkout-electron + - *step-check-for-doc-only-change + - *step-persist-doc-only-change + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-get - *step-depot-tools-add-to-path - *step-restore-brew-cache @@ -783,6 +830,9 @@ steps-checkout-fast: &steps-checkout-fast steps-checkout-and-save-cache: &steps-checkout-and-save-cache steps: - *step-checkout-electron + - *step-check-for-doc-only-change + - *step-persist-doc-only-change + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-get - *step-depot-tools-add-to-path - *step-restore-brew-cache @@ -813,6 +863,7 @@ steps-electron-gn-check: &steps-electron-gn-check steps: - attach_workspace: at: . + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-add-to-path - *step-setup-env-for-build - *step-gn-gen-default @@ -822,6 +873,7 @@ steps-electron-build: &steps-electron-build steps: - attach_workspace: at: . + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-add-to-path - *step-setup-env-for-build - *step-restore-brew-cache @@ -875,6 +927,9 @@ steps-electron-build-with-inline-checkout-for-tests: &steps-electron-build-with- steps: # Checkout - Copied ffrom steps-checkout - *step-checkout-electron + - *step-check-for-doc-only-change + - *step-persist-doc-only-change + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-get - *step-depot-tools-add-to-path - *step-restore-brew-cache @@ -946,6 +1001,45 @@ steps-electron-build-with-inline-checkout-for-tests: &steps-electron-build-with- - *step-maybe-notify-slack-failure +steps-electron-ts-compile-for-doc-change: &steps-electron-ts-compile-for-doc-change + steps: + # Checkout - Copied ffrom steps-checkout + - *step-checkout-electron + - *step-check-for-doc-only-change + - *step-maybe-early-exit-no-doc-change + - *step-depot-tools-get + - *step-depot-tools-add-to-path + - *step-restore-brew-cache + - *step-get-more-space-on-mac + - *step-install-gnutar-on-mac + - *step-generate-deps-hash + - *step-touch-sync-done + - *step-maybe-restore-src-cache + - *step-maybe-restore-git-cache + - *step-set-git-cache-path + # This sync call only runs if .circle-sync-done is an EMPTY file + - *step-gclient-sync + # These next few steps reset Electron to the correct commit regardless of which cache was restored + - run: + name: Wipe Electron + command: rm -rf src/electron + - *step-checkout-electron + - *step-run-electron-only-hooks + - *step-generate-deps-hash-cleanly + - *step-mark-sync-done + - *step-minimize-workspace-size-from-checkout + + - *step-depot-tools-add-to-path + - *step-setup-env-for-build + - *step-restore-brew-cache + - *step-get-more-space-on-mac + - *step-install-npm-deps-on-mac + - *step-fix-sync-on-mac + - *step-gn-gen-default + + #Compile ts/js to verify doc change didn't break anything + - *step-ts-compile + steps-electron-build-for-publish: &steps-electron-build-for-publish steps: - *step-checkout-electron @@ -1064,6 +1158,7 @@ steps-tests: &steps-tests steps: - attach_workspace: at: . + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-add-to-path - *step-electron-dist-unzip - *step-mksnapshot-unzip @@ -1106,6 +1201,7 @@ steps-test-nan: &steps-test-nan steps: - attach_workspace: at: . + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-add-to-path - *step-electron-dist-unzip - *step-setup-linux-for-headless-testing @@ -1121,6 +1217,7 @@ steps-test-node: &steps-test-node steps: - attach_workspace: at: . + - *step-maybe-early-exit-doc-only-change - *step-depot-tools-add-to-path - *step-electron-dist-unzip - *step-setup-linux-for-headless-testing @@ -1146,6 +1243,13 @@ jobs: <<: *env-linux-medium <<: *steps-lint + ts-compile-doc-change: + <<: *machine-linux-medium + environment: + <<: *env-linux-medium + <<: *env-testing-build + <<: *steps-electron-ts-compile-for-doc-change + # Layer 1: Checkout. linux-checkout-fast: <<: *machine-linux-2xlarge @@ -1917,6 +2021,7 @@ workflows: - linux-arm64-testing-gn-check: requires: - linux-checkout-fast + - ts-compile-doc-change build-mac: when: << pipeline.parameters.run-build-mac >> diff --git a/appveyor.yml b/appveyor.yml index 1ab650fd36641..46f4b63aca24c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -50,6 +50,14 @@ build_script: - ps: >- if(($env:APPVEYOR_PULL_REQUEST_HEAD_REPO_NAME -split "/")[0] -eq ($env:APPVEYOR_REPO_NAME -split "/")[0]) { Write-warning "Skipping PR build for branch"; Exit-AppveyorBuild + } else { + npx.cmd yarn@1.15.2 install --frozen-lockfile + + node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH + + if($DOC_ONLY_CHANGE -eq "true" ) { + Write-warning "Skipping build for doc only change"; Exit-AppveyorBuild + } } - echo "Building $env:GN_CONFIG build" - git config --global core.longpaths true diff --git a/script/doc-only-change.js b/script/doc-only-change.js new file mode 100644 index 0000000000000..9ce27b3c5d2f9 --- /dev/null +++ b/script/doc-only-change.js @@ -0,0 +1,58 @@ +const args = require('minimist')(process.argv.slice(2)) +const octokit = require('@octokit/rest')() +const path = require('path') + +const SOURCE_ROOT = path.normalize(path.dirname(__dirname)) + +async function checkIfDocOnlyChange () { + if (args.prNumber || args.prBranch || args.prURL) { + try { + let pullRequestNumber = args.prNumber + if (!pullRequestNumber || isNaN(pullRequestNumber)) { + if (args.prBranch) { + // AppVeyor doesn't provide a PR number for branch builds - figure it out from the branch + const prsForBranch = await octokit.pulls.list({ + owner: 'electron', + repo: 'electron', + state: 'open', + head: `electron:${args.prBranch}` + }) + if (prsForBranch.data.length === 1) { + pullRequestNumber = prsForBranch.data[0].number + } else { + // If there is more than one PR on a branch, just assume that this is more than a doc change + console.log('false') + } + } else if (args.prURL) { + // CircleCI doesn't provide the PR number for branch builds, but it does provide the PR URL + const pullRequestParts = args.prURL.split('/') + pullRequestNumber = pullRequestParts[pullRequestParts.length - 1] + } + } + const filesChanged = await octokit.pulls.listFiles({ + owner: 'electron', repo: 'electron', pull_number: pullRequestNumber + }) + + const nonDocChange = filesChanged.data.find((fileInfo) => { + const fileDirs = fileInfo.filename.split('/') + if (fileDirs[0] !== 'docs') { + return true + } + }) + if (nonDocChange) { + console.log('false') + } else { + console.log('true') + } + } catch (ex) { + console.error('Error getting list of files changed: ', ex) + process.exit(-1) + } + } else { + console.error(`Check if only the docs were changed for a commit. + Usage: doc-only-change.js --prNumber=PR_NUMBER || --prBranch=PR_BRANCH || --prURL=PR_URL`) + process.exit(-1) + } +} + +checkIfDocOnlyChange() From ff5e566e95c4e46c3c2422e2c4de6e503bace1cc Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Fri, 18 Oct 2019 16:59:28 -0400 Subject: [PATCH 2/8] Try using exit codes on doc-only-change --- .circleci/config.yml | 5 ++--- appveyor.yml | 8 ++++---- script/doc-only-change.js | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a6f1a4303c74e..3263effae6150 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -711,9 +711,8 @@ step-check-for-doc-only-change: &step-check-for-doc-only-change command: | cd src/electron node script/yarn install --frozen-lockfile - export DOC_ONLY_CHANGE="`node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST`" - if [ "$DOC_ONLY_CHANGE" = "true" ]; then - echo $DOC_ONLY_CHANGE > .skip-ci-build + if [ node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST`" -eq 1 ]; then + echo "true" > .skip-ci-build else touch .skip-ci-build fi diff --git a/appveyor.yml b/appveyor.yml index 46f4b63aca24c..123e9d2dee0c4 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -53,10 +53,10 @@ build_script: } else { npx.cmd yarn@1.15.2 install --frozen-lockfile - node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH - - if($DOC_ONLY_CHANGE -eq "true" ) { - Write-warning "Skipping build for doc only change"; Exit-AppveyorBuild + if (node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH) { + Write-warning "Skipping build for doc only change1"; Exit-AppveyorBuild + } else { + Write-warning "Skipping build for doc only change2"; Exit-AppveyorBuild } } - echo "Building $env:GN_CONFIG build" diff --git a/script/doc-only-change.js b/script/doc-only-change.js index 9ce27b3c5d2f9..7bd656a55f888 100644 --- a/script/doc-only-change.js +++ b/script/doc-only-change.js @@ -21,7 +21,7 @@ async function checkIfDocOnlyChange () { pullRequestNumber = prsForBranch.data[0].number } else { // If there is more than one PR on a branch, just assume that this is more than a doc change - console.log('false') + process.exit(0) } } else if (args.prURL) { // CircleCI doesn't provide the PR number for branch builds, but it does provide the PR URL @@ -40,9 +40,9 @@ async function checkIfDocOnlyChange () { } }) if (nonDocChange) { - console.log('false') + process.exit(0) } else { - console.log('true') + process.exit(1) } } catch (ex) { console.error('Error getting list of files changed: ', ex) From d4430cc440ebbf542fb1a843f52fc16d942e1095 Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Fri, 18 Oct 2019 17:10:35 -0400 Subject: [PATCH 3/8] Fixup --- .circleci/config.yml | 2 +- appveyor.yml | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3263effae6150..cd31d32d08647 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -711,7 +711,7 @@ step-check-for-doc-only-change: &step-check-for-doc-only-change command: | cd src/electron node script/yarn install --frozen-lockfile - if [ node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST`" -eq 1 ]; then + if [ node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST -eq 1 ]; then echo "true" > .skip-ci-build else touch .skip-ci-build diff --git a/appveyor.yml b/appveyor.yml index 123e9d2dee0c4..04be24c2a2ff0 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -54,9 +54,7 @@ build_script: npx.cmd yarn@1.15.2 install --frozen-lockfile if (node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH) { - Write-warning "Skipping build for doc only change1"; Exit-AppveyorBuild - } else { - Write-warning "Skipping build for doc only change2"; Exit-AppveyorBuild + Write-warning "Skipping build for doc only change"; Exit-AppveyorBuild } } - echo "Building $env:GN_CONFIG build" From 0294515db217609afcded903bdbab11228714b7e Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Mon, 21 Oct 2019 11:58:15 -0400 Subject: [PATCH 4/8] Fixup circleci doc-only check --- .circleci/config.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index cd31d32d08647..341c7848ed03c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -711,10 +711,12 @@ step-check-for-doc-only-change: &step-check-for-doc-only-change command: | cd src/electron node script/yarn install --frozen-lockfile - if [ node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST -eq 1 ]; then - echo "true" > .skip-ci-build - else + if node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST; then + #PR is not a doc only change; create empty file to indicate check has been done touch .skip-ci-build + else + #PR is doc only change; save file with value true to indicate doc only change + echo "true" > .skip-ci-build fi step-persist-doc-only-change: &step-persist-doc-only-change From 24825f4419f1f68d106baa5fcb2c8a5f73239ba3 Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Mon, 21 Oct 2019 16:12:39 -0400 Subject: [PATCH 5/8] Update appveyor.yml Co-Authored-By: Samuel Attard --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 04be24c2a2ff0..f27aee93ee334 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -51,7 +51,7 @@ build_script: if(($env:APPVEYOR_PULL_REQUEST_HEAD_REPO_NAME -split "/")[0] -eq ($env:APPVEYOR_REPO_NAME -split "/")[0]) { Write-warning "Skipping PR build for branch"; Exit-AppveyorBuild } else { - npx.cmd yarn@1.15.2 install --frozen-lockfile + node script/yarn.js install --frozen-lockfile if (node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH) { Write-warning "Skipping build for doc only change"; Exit-AppveyorBuild From 1b9063faeeada10e9d29cddffe3c0c1f980fde1e Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Mon, 21 Oct 2019 16:14:20 -0400 Subject: [PATCH 6/8] Properly detect doc only change on Windows --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index f27aee93ee334..832d15bc54bc1 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -53,7 +53,7 @@ build_script: } else { node script/yarn.js install --frozen-lockfile - if (node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH) { + if ($(node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH;$LASTEXITCODE)) { Write-warning "Skipping build for doc only change"; Exit-AppveyorBuild } } From 9161d9f73ba919fb95f336cb1a99c9293cd79b9f Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Wed, 23 Oct 2019 16:05:19 -0400 Subject: [PATCH 7/8] Flip exit code per review --- .circleci/config.yml | 6 +++--- appveyor.yml | 2 +- script/doc-only-change.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 341c7848ed03c..7b9e57624bda8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -712,11 +712,11 @@ step-check-for-doc-only-change: &step-check-for-doc-only-change cd src/electron node script/yarn install --frozen-lockfile if node script/doc-only-change.js --prNumber=$CIRCLE_PR_NUMBER --prURL=$CIRCLE_PULL_REQUEST; then - #PR is not a doc only change; create empty file to indicate check has been done - touch .skip-ci-build - else #PR is doc only change; save file with value true to indicate doc only change echo "true" > .skip-ci-build + else + #PR is not a doc only change; create empty file to indicate check has been done + touch .skip-ci-build fi step-persist-doc-only-change: &step-persist-doc-only-change diff --git a/appveyor.yml b/appveyor.yml index 832d15bc54bc1..8b53f943b7543 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -53,7 +53,7 @@ build_script: } else { node script/yarn.js install --frozen-lockfile - if ($(node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH;$LASTEXITCODE)) { + if ($(node script/doc-only-change.js --prNumber=$env:APPVEYOR_PULL_REQUEST_NUMBER --prBranch=$env:APPVEYOR_REPO_BRANCH;$LASTEXITCODE -eq 0)) { Write-warning "Skipping build for doc only change"; Exit-AppveyorBuild } } diff --git a/script/doc-only-change.js b/script/doc-only-change.js index 7bd656a55f888..498f1a3428225 100644 --- a/script/doc-only-change.js +++ b/script/doc-only-change.js @@ -40,9 +40,9 @@ async function checkIfDocOnlyChange () { } }) if (nonDocChange) { - process.exit(0) - } else { process.exit(1) + } else { + process.exit(0) } } catch (ex) { console.error('Error getting list of files changed: ', ex) From 2b841ce9c372be9c916cfcb82be1ed9053bfc7ca Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Mon, 28 Oct 2019 09:40:21 -0400 Subject: [PATCH 8/8] build: fix doc only change when there isn't a PR (#20749) * build: fix doc only change when there isn't a PR Fixes issue where CI was mistakenly marking a PR as a doc only change because the CI was kicked off before the PR was created. (cherry picked from commit 73da4b721513f3d947746371e20e782603b1eaab) --- script/doc-only-change.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/doc-only-change.js b/script/doc-only-change.js index 498f1a3428225..a5569dfea8833 100644 --- a/script/doc-only-change.js +++ b/script/doc-only-change.js @@ -20,8 +20,8 @@ async function checkIfDocOnlyChange () { if (prsForBranch.data.length === 1) { pullRequestNumber = prsForBranch.data[0].number } else { - // If there is more than one PR on a branch, just assume that this is more than a doc change - process.exit(0) + // If there are 0 PRs or more than one PR on a branch, just assume that this is more than a doc change + process.exit(1) } } else if (args.prURL) { // CircleCI doesn't provide the PR number for branch builds, but it does provide the PR URL