From ef03847867450c7d3b395c6fc8393c8837b0d2f5 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 19 Sep 2024 03:59:55 +0000 Subject: [PATCH 1/5] ci: make git-diff-develop work for PRs from foreign repos The gh API requires authentication, even if the API endpoints do not. We can just fetch the PR URL directly without shelling out. --- .circleci/scripts/git-diff-develop.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/.circleci/scripts/git-diff-develop.ts b/.circleci/scripts/git-diff-develop.ts index 3cf5022d4e12..93da1dec7d13 100644 --- a/.circleci/scripts/git-diff-develop.ts +++ b/.circleci/scripts/git-diff-develop.ts @@ -14,16 +14,12 @@ const MAIN_BRANCH = 'develop'; * @returns The name of the branch targeted by the PR. */ async function getBaseRef(): Promise { - if (!process.env.CIRCLE_PULL_REQUEST) { + if (!process.env.CIRCLE_PR_NUMBER) { return null; } - // We're referencing the CIRCLE_PULL_REQUEST environment variable within the script rather than - // passing it in because this makes it easier to use Bash parameter expansion to extract the - // PR number from the URL. - const result = await exec(`gh pr view --json baseRefName "\${CIRCLE_PULL_REQUEST##*/}" --jq '.baseRefName'`); - const baseRef = result.stdout.trim(); - return baseRef; + const pull = await (await fetch(`https://api.github.com/repos/${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PR_REPONAME}/pulls/${process.env.CIRCLE_PR_NUMBER}`)).json(); + return pull.base.ref; } /** From 91ce41d29687917fd314692a55c52687ffe37d14 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 19 Sep 2024 05:21:08 +0000 Subject: [PATCH 2/5] ci: git-diff-develop does not depend on prep-deps --- .circleci/config.yml | 5 ++--- .circleci/scripts/git-diff-develop.ts | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 095650aae02d..f384ae756c61 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -360,11 +360,10 @@ workflows: value: << pipeline.git.branch >> jobs: - prep-deps - - get-changed-files-with-git-diff: - requires: - - prep-deps + - get-changed-files-with-git-diff - validate-locales-only: requires: + - prep-deps - get-changed-files-with-git-diff - test-lint: requires: diff --git a/.circleci/scripts/git-diff-develop.ts b/.circleci/scripts/git-diff-develop.ts index 93da1dec7d13..421bec42919b 100644 --- a/.circleci/scripts/git-diff-develop.ts +++ b/.circleci/scripts/git-diff-develop.ts @@ -1,4 +1,3 @@ -import { hasProperty } from '@metamask/utils'; import { exec as execCallback } from 'child_process'; import fs from 'fs'; import path from 'path'; @@ -57,7 +56,8 @@ async function fetchUntilMergeBaseFound() { } catch (error: unknown) { if ( error instanceof Error && - hasProperty(error, 'code') && + Object.hasOwnProperty.call(error, 'code') && + // @ts-expect-error error.code === 1 ) { console.error(`Error 'no merge base' encountered with depth ${depth}. Incrementing depth...`); From 978eae7a96912c57c89f77b937445cc7b58dd605 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 19 Sep 2024 05:12:00 +0000 Subject: [PATCH 3/5] ci: get-changed-files-with-git-diff: stable branch names --- .circleci/scripts/git-diff-develop.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.circleci/scripts/git-diff-develop.ts b/.circleci/scripts/git-diff-develop.ts index 421bec42919b..42adff728174 100644 --- a/.circleci/scripts/git-diff-develop.ts +++ b/.circleci/scripts/git-diff-develop.ts @@ -6,6 +6,7 @@ import { promisify } from 'util'; const exec = promisify(execCallback); const MAIN_BRANCH = 'develop'; +const SOURCE_BRANCH = `refs/pull/${process.env.CIRCLE_PR_NUMBER}/head`; /** * Get the target branch for the given pull request. @@ -29,8 +30,8 @@ async function getBaseRef(): Promise { */ async function fetchWithDepth(depth: number): Promise { try { - await exec(`git fetch --depth ${depth} origin develop`); - await exec(`git fetch --depth ${depth} origin ${process.env.CIRCLE_BRANCH}`); + await exec(`git fetch --depth ${depth} origin ${MAIN_BRANCH}`); + await exec(`git fetch --depth ${depth} origin ${SOURCE_BRANCH}:${SOURCE_BRANCH}`); return true; } catch (error: unknown) { console.error(`Failed to fetch with depth ${depth}:`, error); @@ -66,7 +67,7 @@ async function fetchUntilMergeBaseFound() { } } } - await exec(`git fetch --unshallow origin develop`); + await exec(`git fetch --unshallow origin ${MAIN_BRANCH}`); } /** @@ -78,7 +79,7 @@ async function fetchUntilMergeBaseFound() { */ async function gitDiff(): Promise { await fetchUntilMergeBaseFound(); - const { stdout: diffResult } = await exec(`git diff --name-only origin/HEAD...${process.env.CIRCLE_BRANCH}`); + const { stdout: diffResult } = await exec(`git diff --name-only origin/HEAD...${SOURCE_BRANCH}`); if (!diffResult) { throw new Error('Unable to get diff after full checkout.'); } From 04e5f480e62907509091379825a615e3bbb5ec12 Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 19 Sep 2024 05:16:32 +0000 Subject: [PATCH 4/5] prettier --- .circleci/config.yml | 9 +++----- .circleci/scripts/git-diff-develop.ts | 30 +++++++++++++++++++-------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f384ae756c61..b2c5ab712973 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -118,11 +118,9 @@ workflows: - prep-deps - get-changed-files-with-git-diff: filters: - branches: - ignore: - - master - requires: - - prep-deps + branches: + ignore: + - master - test-deps-audit: requires: - prep-deps @@ -500,7 +498,6 @@ jobs: - run: sudo corepack enable - attach_workspace: at: . - - gh/install - run: name: Get changed files with git diff command: npx tsx .circleci/scripts/git-diff-develop.ts diff --git a/.circleci/scripts/git-diff-develop.ts b/.circleci/scripts/git-diff-develop.ts index 42adff728174..7f631ec47500 100644 --- a/.circleci/scripts/git-diff-develop.ts +++ b/.circleci/scripts/git-diff-develop.ts @@ -18,7 +18,11 @@ async function getBaseRef(): Promise { return null; } - const pull = await (await fetch(`https://api.github.com/repos/${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PR_REPONAME}/pulls/${process.env.CIRCLE_PR_NUMBER}`)).json(); + const pull = await ( + await fetch( + `https://api.github.com/repos/${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PR_REPONAME}/pulls/${process.env.CIRCLE_PR_NUMBER}`, + ) + ).json(); return pull.base.ref; } @@ -31,7 +35,9 @@ async function getBaseRef(): Promise { async function fetchWithDepth(depth: number): Promise { try { await exec(`git fetch --depth ${depth} origin ${MAIN_BRANCH}`); - await exec(`git fetch --depth ${depth} origin ${SOURCE_BRANCH}:${SOURCE_BRANCH}`); + await exec( + `git fetch --depth ${depth} origin ${SOURCE_BRANCH}:${SOURCE_BRANCH}`, + ); return true; } catch (error: unknown) { console.error(`Failed to fetch with depth ${depth}:`, error); @@ -61,7 +67,9 @@ async function fetchUntilMergeBaseFound() { // @ts-expect-error error.code === 1 ) { - console.error(`Error 'no merge base' encountered with depth ${depth}. Incrementing depth...`); + console.error( + `Error 'no merge base' encountered with depth ${depth}. Incrementing depth...`, + ); } else { throw error; } @@ -79,9 +87,11 @@ async function fetchUntilMergeBaseFound() { */ async function gitDiff(): Promise { await fetchUntilMergeBaseFound(); - const { stdout: diffResult } = await exec(`git diff --name-only origin/HEAD...${SOURCE_BRANCH}`); + const { stdout: diffResult } = await exec( + `git diff --name-only origin/HEAD...${SOURCE_BRANCH}`, + ); if (!diffResult) { - throw new Error('Unable to get diff after full checkout.'); + throw new Error('Unable to get diff after full checkout.'); } return diffResult; } @@ -99,22 +109,24 @@ async function storeGitDiffOutput() { const outputDir = 'changed-files'; fs.mkdirSync(outputDir, { recursive: true }); - console.log(`Determining whether this run is for a PR targetting ${MAIN_BRANCH}`) + console.log( + `Determining whether this run is for a PR targetting ${MAIN_BRANCH}`, + ); if (!process.env.CIRCLE_PULL_REQUEST) { - console.log("Not a PR, skipping git diff"); + console.log('Not a PR, skipping git diff'); return; } const baseRef = await getBaseRef(); if (baseRef === null) { - console.log("Not a PR, skipping git diff"); + console.log('Not a PR, skipping git diff'); return; } else if (baseRef !== MAIN_BRANCH) { console.log(`This is for a PR targeting '${baseRef}', skipping git diff`); return; } - console.log("Attempting to get git diff..."); + console.log('Attempting to get git diff...'); const diffOutput = await gitDiff(); console.log(diffOutput); From c554b953027d20e34fed736307bd3438acc16ac9 Mon Sep 17 00:00:00 2001 From: Howard Braham Date: Tue, 8 Oct 2024 13:13:00 -0700 Subject: [PATCH 5/5] back-ported changes from #27548 --- .circleci/scripts/git-diff-develop.ts | 59 +++++++++++++++------------ 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/.circleci/scripts/git-diff-develop.ts b/.circleci/scripts/git-diff-develop.ts index 7f631ec47500..9f6c8f0ae4df 100644 --- a/.circleci/scripts/git-diff-develop.ts +++ b/.circleci/scripts/git-diff-develop.ts @@ -5,25 +5,38 @@ import { promisify } from 'util'; const exec = promisify(execCallback); +// The CIRCLE_PR_NUMBER variable is only available on forked Pull Requests +const PR_NUMBER = + process.env.CIRCLE_PR_NUMBER || + process.env.CIRCLE_PULL_REQUEST?.split('/').pop(); + const MAIN_BRANCH = 'develop'; -const SOURCE_BRANCH = `refs/pull/${process.env.CIRCLE_PR_NUMBER}/head`; +const SOURCE_BRANCH = `refs/pull/${PR_NUMBER}/head`; + +const CHANGED_FILES_DIR = 'changed-files'; + +type PRInfo = { + base: { + ref: string; + }; + body: string; +}; /** - * Get the target branch for the given pull request. + * Get JSON info about the given pull request * - * @returns The name of the branch targeted by the PR. + * @returns JSON info from GitHub */ -async function getBaseRef(): Promise { - if (!process.env.CIRCLE_PR_NUMBER) { +async function getPrInfo(): Promise { + if (!PR_NUMBER) { return null; } - const pull = await ( + return await ( await fetch( - `https://api.github.com/repos/${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PR_REPONAME}/pulls/${process.env.CIRCLE_PR_NUMBER}`, + `https://api.github.com/repos/${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PROJECT_REPONAME}/pulls/${PR_NUMBER}`, ) ).json(); - return pull.base.ref; } /** @@ -34,9 +47,9 @@ async function getBaseRef(): Promise { */ async function fetchWithDepth(depth: number): Promise { try { - await exec(`git fetch --depth ${depth} origin ${MAIN_BRANCH}`); + await exec(`git fetch --depth ${depth} origin "${MAIN_BRANCH}"`); await exec( - `git fetch --depth ${depth} origin ${SOURCE_BRANCH}:${SOURCE_BRANCH}`, + `git fetch --depth ${depth} origin "${SOURCE_BRANCH}:${SOURCE_BRANCH}"`, ); return true; } catch (error: unknown) { @@ -61,12 +74,7 @@ async function fetchUntilMergeBaseFound() { await exec(`git merge-base origin/HEAD HEAD`); return; } catch (error: unknown) { - if ( - error instanceof Error && - Object.hasOwnProperty.call(error, 'code') && - // @ts-expect-error - error.code === 1 - ) { + if (error instanceof Error && 'code' in error) { console.error( `Error 'no merge base' encountered with depth ${depth}. Incrementing depth...`, ); @@ -75,7 +83,7 @@ async function fetchUntilMergeBaseFound() { } } } - await exec(`git fetch --unshallow origin ${MAIN_BRANCH}`); + await exec(`git fetch --unshallow origin "${MAIN_BRANCH}"`); } /** @@ -88,7 +96,7 @@ async function fetchUntilMergeBaseFound() { async function gitDiff(): Promise { await fetchUntilMergeBaseFound(); const { stdout: diffResult } = await exec( - `git diff --name-only origin/HEAD...${SOURCE_BRANCH}`, + `git diff --name-only "origin/HEAD...${SOURCE_BRANCH}"`, ); if (!diffResult) { throw new Error('Unable to get diff after full checkout.'); @@ -106,19 +114,20 @@ async function storeGitDiffOutput() { // Create the directory // This is done first because our CirleCI config requires that this directory is present, // even if we want to skip this step. - const outputDir = 'changed-files'; - fs.mkdirSync(outputDir, { recursive: true }); + fs.mkdirSync(CHANGED_FILES_DIR, { recursive: true }); console.log( - `Determining whether this run is for a PR targetting ${MAIN_BRANCH}`, + `Determining whether this run is for a PR targeting ${MAIN_BRANCH}`, ); - if (!process.env.CIRCLE_PULL_REQUEST) { + if (!PR_NUMBER) { console.log('Not a PR, skipping git diff'); return; } - const baseRef = await getBaseRef(); - if (baseRef === null) { + const prInfo = await getPrInfo(); + + const baseRef = prInfo?.base.ref; + if (!baseRef) { console.log('Not a PR, skipping git diff'); return; } else if (baseRef !== MAIN_BRANCH) { @@ -131,7 +140,7 @@ async function storeGitDiffOutput() { console.log(diffOutput); // Store the output of git diff - const outputPath = path.resolve(outputDir, 'changed-files.txt'); + const outputPath = path.resolve(CHANGED_FILES_DIR, 'changed-files.txt'); fs.writeFileSync(outputPath, diffOutput.trim()); console.log(`Git diff results saved to ${outputPath}`);