Merge pull request #1860 from github/aeisenberg/better-error-messages

Add better error messages when determining merge-base

Author: aeisenberg

CHANGELOG.md

@@ -11,6 +11,7 @@ No user facing changes.
 - Update default CodeQL bundle version to 2.14.3. [#1845](https://github.com/github/codeql-action/pull/1845)
 - Fixed a bug in CodeQL Action 2.21.3 onwards that affected beta support for [Project Lombok](https://projectlombok.org/) when analyzing Java. The environment variable `CODEQL_EXTRACTOR_JAVA_RUN_ANNOTATION_PROCESSORS` will now be respected if it was manually configured in the workflow. [#1844](https://github.com/github/codeql-action/pull/1844)
 - Enable support for Kotlin 1.9.20 when running with CodeQL CLI v2.13.4 through v2.14.3. [#1853](https://github.com/github/codeql-action/pull/1853)
+- Better error message when there is a failure to determine the merge base of the code to analysis. [#1860](https://github.com/github/codeql-action/pull/1860)
 
 ## 2.21.4 - 14 Aug 2023
 

lib/actions-util.js

@@ -1,6 +1,7 @@
 import * as fs from "fs";
 import * as path from "path";
 
+import * as core from "@actions/core";
 import test from "ava";
 import * as sinon from "sinon";
 
@@ -267,3 +268,50 @@ test("isAnalyzingDefaultBranch()", async (t) => {
     getAdditionalInputStub.restore();
   });
 });
+
+test("determineMergeBaseCommitOid non-pullrequest", async (t) => {
+  const infoStub = sinon.stub(core, "info");
+
+  process.env["GITHUB_EVENT_NAME"] = "hucairz";
+  process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+  const result = await actionsUtil.determineMergeBaseCommitOid(__dirname);
+  t.deepEqual(result, undefined);
+  t.deepEqual(0, infoStub.callCount);
+
+  infoStub.restore();
+});
+
+test("determineMergeBaseCommitOid no error", async (t) => {
+  const infoStub = sinon.stub(core, "info");
+
+  process.env["GITHUB_EVENT_NAME"] = "pull_request";
+  process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+  await actionsUtil.determineMergeBaseCommitOid(path.join(__dirname, "../.."));
+  t.deepEqual(1, infoStub.callCount);
+  t.assert(
+    infoStub.firstCall.args[0].startsWith(
+      "The checkout path provided to the action does not appear to be a git repository.",
+    ),
+  );
+
+  infoStub.restore();
+});
+
+test("determineMergeBaseCommitOid other error", async (t) => {
+  const infoStub = sinon.stub(core, "info");
+
+  process.env["GITHUB_EVENT_NAME"] = "pull_request";
+  process.env["GITHUB_SHA"] = "100912429fab4cb230e66ffb11e738ac5194e73a";
+  const result = await actionsUtil.determineMergeBaseCommitOid(
+    path.join(__dirname, "../../i-dont-exist"),
+  );
+  t.deepEqual(result, undefined);
+  t.deepEqual(1, infoStub.callCount);
+  t.assert(
+    infoStub.firstCall.args[0].startsWith(
+      "Failed to call git to determine merge base.",
+    ),
+  );
+
+  infoStub.restore();
+});

lib/actions-util.js.map

@@ -63,6 +63,7 @@ export const getCommitOid = async function (
   // the merge commit, which must mean that git is available.
   // Even if this does go wrong, it's not a huge problem for the alerts to
   // reported on the merge commit.
+  let stderr = "";
   try {
     let commitOid = "";
     await new toolrunner.ToolRunner(
@@ -75,19 +76,25 @@ export const getCommitOid = async function (
             commitOid += data.toString();
           },
           stderr: (data) => {
-            process.stderr.write(data);
+            stderr += data.toString();
           },
         },
         cwd: checkoutPath,
       },
     ).exec();
     return commitOid.trim();
   } catch (e) {
-    core.info(
-      "Could not determine current commit SHA using git. Continuing with data from user input or environment.",
-    );
-    core.debug(`Reason: ${(e as Error).message}`);
-    core.debug((e as Error).stack || "NO STACK");
+    if (stderr.includes("not a git repository")) {
+      core.info(
+        "Could not determine current commit SHA using git. Continuing with data from user input or environment. " +
+          "The checkout path provided to the action does not appear to be a git repository.",
+      );
+    } else {
+      core.info(
+        `Could not determine current commit SHA using git. Continuing with data from user input or environment. ${stderr}`,
+      );
+    }
+
     return getOptionalInput("sha") || getRequiredEnvParam("GITHUB_SHA");
   }
 };
@@ -96,15 +103,17 @@ export const getCommitOid = async function (
  * If the action was triggered by a pull request, determine the commit sha of the merge base.
  * Returns undefined if run by other triggers or the merge base cannot be determined.
  */
-export const determineMergeBaseCommitOid = async function (): Promise<
-  string | undefined
-> {
+export const determineMergeBaseCommitOid = async function (
+  checkoutPathOverride?: string,
+): Promise<string | undefined> {
   if (getWorkflowEventName() !== "pull_request") {
     return undefined;
   }
 
   const mergeSha = getRequiredEnvParam("GITHUB_SHA");
-  const checkoutPath = getOptionalInput("checkout_path");
+  const checkoutPath =
+    checkoutPathOverride ?? getOptionalInput("checkout_path");
+  let stderr = "";
 
   try {
     let commitOid = "";
@@ -129,7 +138,7 @@ export const determineMergeBaseCommitOid = async function (): Promise<
             }
           },
           stderr: (data) => {
-            process.stderr.write(data);
+            stderr += data.toString();
           },
         },
         cwd: checkoutPath,
@@ -146,10 +155,17 @@ export const determineMergeBaseCommitOid = async function (): Promise<
     }
     return undefined;
   } catch (e) {
-    core.info(
-      `Failed to call git to determine merge base. Continuing with data from environment: ${e}`,
-    );
-    core.info((e as Error).stack || "NO STACK");
+    if (stderr.includes("not a git repository")) {
+      core.info(
+        "The checkout path provided to the action does not appear to be a git repository. " +
+          "Will calculate the merge base on the server.",
+      );
+    } else {
+      core.info(
+        `Failed to call git to determine merge base. Will calculate the merge base on ` +
+          `the server. Reason: ${stderr}`,
+      );
+    }
     return undefined;
   }
 };