addFlagsFromPrBody

.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

.circleci/scripts/git-diff-develop.ts

@@ -1,29 +1,42 @@
-import { hasProperty } from '@metamask/utils';
 import { exec as execCallback } from 'child_process';
 import fs from 'fs';
 import path from 'path';
 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<string | null> {
-  if (!process.env.CIRCLE_PULL_REQUEST) {
+async function getPrInfo(): Promise<PRInfo | null> {
+  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<string | null> {
  */
 async function fetchWithDepth(depth: number): Promise<boolean> {
   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,55 +99,70 @@ async function fetchUntilMergeBaseFound() {
  */
 async function gitDiff(): Promise<string> {
   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);
     process.exit(1);
   }
 }
 
-storeGitDiffOutput();
+storeGitDiffOutputAndPrBody();

test/e2e/set-manifest-flags.ts

@@ -10,29 +10,26 @@ 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 {
       // Get 1st capturing group from regex
       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(