ci: make git-diff-develop work for PRs from foreign repos (#27268)

The `gh` CLI requires authentication, even if the API endpoints do not. We can just fetch the PR URL directly without shelling out. Fixes [CI error](https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/101231/workflows/7b91edba-0c97-4075-934c-5db83a71a2c0/jobs/3769045). Also removes dependency on `prep-deps` step by removing dependency on external module. --------- Co-authored-by: Howard Braham <howrad@gmail.com>
MetaMask · Oct 8, 2024 · b5b8b8f · b5b8b8f
1 parent 74378eb
commit b5b8b8f
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 41 deletions.
diff --git 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
@@ -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);
@@ -59,18 +74,16 @@ async function fetchUntilMergeBaseFound() {
       await exec(`git merge-base origin/HEAD HEAD`);
       return;
     } catch (error: unknown) {
-      if (
-        error instanceof Error &&
-        hasProperty(error, 'code') &&
-        error.code === 1
-      ) {
-        console.error(`Error 'no merge base' encountered with depth ${depth}. Incrementing depth...`);
+      if (error instanceof Error && 'code' in error) {
+        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,9 +95,11 @@ 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;
 }
@@ -99,30 +114,33 @@ 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}`)
-    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`);
       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}`);