diff --git a/.circleci/config.yml b/.circleci/config.yml index 095650aae02d..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 @@ -360,11 +358,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: @@ -501,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 3cf5022d4e12..e7009145b7a8 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'; @@ -6,24 +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/${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_PULL_REQUEST) { +async function getPrInfo(): Promise { + if (!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; + return await ( + await fetch( + `https://api.github.com/repos/${process.env.CIRCLE_PROJECT_USERNAME}/${process.env.CIRCLE_PROJECT_REPONAME}/pulls/${PR_NUMBER}`, + ) + ).json(); } /** @@ -34,8 +47,10 @@ 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); @@ -61,16 +76,18 @@ async function fetchUntilMergeBaseFound() { } catch (error: unknown) { if ( error instanceof Error && - hasProperty(error, 'code') && + Object.hasOwnProperty.call(error, 'code') && 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; } } } - await exec(`git fetch --unshallow origin develop`); + await exec(`git fetch --unshallow origin ${MAIN_BRANCH}`); } /** @@ -82,50 +99,65 @@ 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.'); + throw new Error('Unable to get diff after full checkout.'); } return diffResult; } +function writePrBodyToFile(prBody: string) { + const prBodyPath = path.resolve(CHANGED_FILES_DIR, 'pr-body.txt'); + fs.writeFileSync(prBodyPath, prBody.trim()); + console.log(`PR body saved to ${prBodyPath}`); +} + /** * Stores the output of git diff to a file. * * @returns Returns a promise that resolves when the git diff output is successfully stored. */ -async function storeGitDiffOutput() { +async function storeGitDiffOutputAndPrBody() { try { // 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}`) - if (!process.env.CIRCLE_PULL_REQUEST) { - console.log("Not a PR, skipping git diff"); + console.log( + `Determining whether this run is for a PR targeting ${MAIN_BRANCH}`, + ); + if (!PR_NUMBER) { + console.log('Not a PR, skipping git diff'); return; } - const baseRef = await getBaseRef(); - if (baseRef === null) { - console.log("Not a PR, skipping git diff"); + 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) { console.log(`This is for a PR targeting '${baseRef}', skipping git diff`); + writePrBodyToFile(prInfo.body); return; } - console.log("Attempting to get git diff..."); + console.log('Attempting to get git diff...'); const diffOutput = await gitDiff(); 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}`); + + writePrBodyToFile(prInfo.body); + process.exit(0); } catch (error: any) { console.error('An error occurred:', error.message); @@ -133,4 +165,4 @@ async function storeGitDiffOutput() { } } -storeGitDiffOutput(); +storeGitDiffOutputAndPrBody(); diff --git a/test/e2e/set-manifest-flags.ts b/test/e2e/set-manifest-flags.ts index c09d69cf677e..df8d57bd58fe 100644 --- a/test/e2e/set-manifest-flags.ts +++ b/test/e2e/set-manifest-flags.ts @@ -10,21 +10,18 @@ function parseIntOrUndefined(value: string | undefined): number | undefined { } /** - * Grab flags from the Git message if they are set + * Search a string for `flags = {...}` and return ManifestFlags if it exists * - * To use this feature, add a line to your commit message like: - * `flags = {"sentry": {"tracesSampleRate": 0.1}}` - * (must be valid JSON) - * - * @returns flags object if found, undefined otherwise + * @param str - The string to search + * @param errorType - The type of error to log if parsing fails + * @returns The ManifestFlags object if valid, otherwise undefined */ -function getFlagsFromGitMessage(): object | undefined { - const gitMessage = execSync( - `git show --format='%B' --no-patch "HEAD"`, - ).toString(); - - // Search gitMessage for `flags = {...}` - const flagsMatch = gitMessage.match(/flags\s*=\s*(\{.*\})/u); +function regexSearchForFlags( + str: string, + errorType: string, +): ManifestFlags | undefined { + // Search str for `flags = {...}` + const flagsMatch = str.match(/flags\s*=\s*(\{.*\})/u); if (flagsMatch) { try { @@ -32,7 +29,7 @@ function getFlagsFromGitMessage(): object | undefined { return JSON.parse(flagsMatch[1]); } catch (error) { console.error( - 'Error parsing flags from git message, ignoring flags\n', + `Error parsing flags from ${errorType}, ignoring flags\n`, error, ); } @@ -41,6 +38,55 @@ function getFlagsFromGitMessage(): object | undefined { return undefined; } +/** + * Add flags from the GitHub PR body if they are set + * + * To use this feature, add a line to your PR body like: + * `flags = {"sentry": {"tracesSampleRate": 0.1}}` + * (must be valid JSON) + * + * @param flags - The flags object to add to + */ +function addFlagsFromPrBody(flags: ManifestFlags) { + let body; + + try { + body = fs.readFileSync('changed-files/pr-body.txt', 'utf8'); + } catch (error) { + console.debug('No pr-body.txt, ignoring flags'); + return; + } + + const newFlags = regexSearchForFlags(body, 'PR body'); + + if (newFlags) { + // Use lodash merge to do a deep merge (spread operator is shallow) + merge(flags, newFlags); + } +} + +/** + * Add flags from the Git message if they are set + * + * To use this feature, add a line to your commit message like: + * `flags = {"sentry": {"tracesSampleRate": 0.1}}` + * (must be valid JSON) + * + * @param flags - The flags object to add to + */ +function addFlagsFromGitMessage(flags: ManifestFlags) { + const gitMessage = execSync( + `git show --format='%B' --no-patch "HEAD"`, + ).toString(); + + const newFlags = regexSearchForFlags(gitMessage, 'git message'); + + if (newFlags) { + // Use lodash merge to do a deep merge (spread operator is shallow) + merge(flags, newFlags); + } +} + // Alter the manifest with CircleCI environment variables and custom flags export function setManifestFlags(flags: ManifestFlags = {}) { if (process.env.CIRCLECI) { @@ -55,12 +101,10 @@ export function setManifestFlags(flags: ManifestFlags = {}) { ), }; - const gitMessageFlags = getFlagsFromGitMessage(); + addFlagsFromPrBody(flags); + addFlagsFromGitMessage(flags); - if (gitMessageFlags) { - // Use lodash merge to do a deep merge (spread operator is shallow) - flags = merge(flags, gitMessageFlags); - } + console.log('flags at end', flags); } const manifest = JSON.parse(