Merge pull request #2638 from github/cklin/diff-informed-graph-fetching-tweak

Improve Git subgraph fetching for diff-informed queries

Author: cklin

lib/actions-util.js

@@ -163,7 +163,7 @@ export const determineBaseBranchHeadCommitOid = async function (
 };
 
 /**
- * Deepen the git history of the given ref by one level. Errors are logged.
+ * Deepen the git history of HEAD by one level. Errors are logged.
  *
  * This function uses the `checkout_path` to determine the repository path and
  * works only when called from `analyze` or `upload-sarif`.
@@ -172,7 +172,14 @@ export const deepenGitHistory = async function () {
   try {
     await runGitCommand(
       getOptionalInput("checkout_path"),
-      ["fetch", "--no-tags", "--deepen=1"],
+      [
+        "fetch",
+        "origin",
+        "HEAD",
+        "--no-tags",
+        "--no-recurse-submodules",
+        "--deepen=1",
+      ],
       "Cannot deepen the shallow repository.",
     );
   } catch {
@@ -198,6 +205,24 @@ export const gitFetch = async function (branch: string, extraFlags: string[]) {
   }
 };
 
+/**
+ * Repack the git repository, using with the given flags. Errors are logged.
+ *
+ * This function uses the `checkout_path` to determine the repository path and
+ * works only when called from `analyze` or `upload-sarif`.
+ */
+export const gitRepack = async function (flags: string[]) {
+  try {
+    await runGitCommand(
+      getOptionalInput("checkout_path"),
+      ["repack", ...flags],
+      "Cannot repack the repository.",
+    );
+  } catch {
+    // Errors are already logged by runGitCommand()
+  }
+};
+
 /**
  * Compute the all merge bases between the given refs. Returns an empty array
  * if no merge base is found, or if there is an error.

lib/actions-util.js.map

@@ -18,7 +18,7 @@ import { addDiagnostic, makeDiagnostic } from "./diagnostics";
 import { EnvVar } from "./environment";
 import { FeatureEnablement, Feature } from "./feature-flags";
 import { isScannedLanguage, Language } from "./languages";
-import { Logger, withGroup } from "./logging";
+import { Logger, withGroupAsync } from "./logging";
 import { DatabaseCreationTimings, EventReport } from "./status-report";
 import { ToolsFeature } from "./tools-features";
 import { endTracingForCluster } from "./tracer-config";
@@ -256,14 +256,17 @@ export async function setupDiffInformedQueryRun(
   if (!(await features.getValue(Feature.DiffInformedQueries, codeql))) {
     return undefined;
   }
-  return await withGroup("Generating diff range extension pack", async () => {
-    const diffRanges = await getPullRequestEditedDiffRanges(
-      baseRef,
-      headRef,
-      logger,
-    );
-    return writeDiffRangeDataExtensionPack(logger, diffRanges);
-  });
+  return await withGroupAsync(
+    "Generating diff range extension pack",
+    async () => {
+      const diffRanges = await getPullRequestEditedDiffRanges(
+        baseRef,
+        headRef,
+        logger,
+      );
+      return writeDiffRangeDataExtensionPack(logger, diffRanges);
+    },
+  );
 }
 
 interface DiffThunkRange {
@@ -295,7 +298,7 @@ async function getPullRequestEditedDiffRanges(
 
   // To compute the merge bases between the base branch and the PR topic branch,
   // we need to fetch the commit graph from the branch heads to those merge
-  // babes. The following 4-step procedure does so while limiting the amount of
+  // babes. The following 6-step procedure does so while limiting the amount of
   // history fetched.
 
   // Step 1: Deepen from the PR merge commit to the base branch head and the PR
@@ -314,7 +317,12 @@ async function getPullRequestEditedDiffRanges(
   // Step 4: Fetch the base branch history, stopping when we reach commits that
   // are reachable from the PR topic branch head.
   await actionsUtil.gitFetch(baseRef, [`--shallow-exclude=${headRef}`]);
-  // Step 5: Deepen the history so that we have the merge bases between the base
+  // Step 5: Repack the history to remove the shallow grafts that were added by
+  // the previous fetches. This step works around a bug that causes subsequent
+  // deepening fetches to fail with "fatal: error in object: unshallow <SHA>".
+  // See https://stackoverflow.com/q/63878612
+  await actionsUtil.gitRepack(["-d"]);
+  // Step 6: Deepen the history so that we have the merge bases between the base
   // branch and the PR topic branch.
   await actionsUtil.deepenGitHistory();
 

lib/analyze.js

@@ -41,6 +41,18 @@ export function withGroup<T>(groupName: string, f: () => T): T {
   }
 }
 
+export async function withGroupAsync<T>(
+  groupName: string,
+  f: () => Promise<T>,
+): Promise<T> {
+  core.startGroup(groupName);
+  try {
+    return await f();
+  } finally {
+    core.endGroup();
+  }
+}
+
 /** Format a duration for use in logs. */
 export function formatDuration(durationMs: number) {
   if (durationMs < 1000) {