Fully fix iterative deepening for latest git

Author: ecraig12345

change/change-e0ebd254-f3c4-4b0b-aa4f-533fe9300419.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "type": "patch",
+      "comment": "Fully fix iterative deepening for latest git",
+      "packageName": "beachball",
+      "email": "elcraig@microsoft.com",
+      "dependentChangeType": "patch"
+    }
+  ]
+}

packages/beachball/src/__functional__/git/ensureSharedHistory.test.ts

@@ -251,7 +251,8 @@ describe('ensureSharedHistory', () => {
     expect(logs.mocks.warn).not.toHaveBeenCalled();
     expect(logs.mocks.error).not.toHaveBeenCalled();
 
-    const deepen = `fetch --deepen=2 origin ${defaultRefSpec}`;
+    const testRefSpec = `+refs/heads/${testBranch}:refs/remotes/origin/${testBranch}`;
+    const deepen = `fetch --deepen=2 origin ${defaultRefSpec} ${testRefSpec}`;
     expect(filteredGitCalls()).toEqual([`fetch --depth=2 origin ${defaultRefSpec}`, deepen, deepen, deepen]);
   });
 
@@ -283,7 +284,8 @@ describe('ensureSharedHistory', () => {
     expect(logs.mocks.warn).not.toHaveBeenCalled();
     expect(logs.mocks.error).not.toHaveBeenCalled();
 
-    const deepen = `fetch --deepen=2 origin ${defaultRefSpec}`;
+    const testRefSpec = `+refs/heads/${testBranch}:refs/remotes/origin/${testBranch}`;
+    const deepen = `fetch --deepen=2 origin ${defaultRefSpec} ${testRefSpec}`;
     expect(filteredGitCalls()).toEqual([`fetch --depth=2 origin ${defaultRefSpec}`, deepen, deepen, deepen]);
   });
 
@@ -301,13 +303,14 @@ describe('ensureSharedHistory', () => {
     });
 
     expect(logs.getMockLines('all')).toMatch("Still didn't find a common commit after deepening by 3. Unshallowing...");
-    const deepen = `fetch --deepen=1 origin ${defaultRefSpec}`;
+    const testRefSpec = `+refs/heads/${testBranch}:refs/remotes/origin/${testBranch}`;
+    const deepen = `fetch --deepen=1 origin ${defaultRefSpec} ${testRefSpec}`;
     expect(filteredGitCalls()).toEqual([
       `fetch --depth=1 origin ${defaultRefSpec}`,
       deepen,
       deepen,
       deepen,
-      `fetch --unshallow origin ${defaultRefSpec}`,
+      `fetch --unshallow origin ${defaultRefSpec} ${testRefSpec}`,
     ]);
   });
 

packages/beachball/src/__functional__/git/fetch.test.ts

@@ -154,6 +154,35 @@ describe('gitFetch', () => {
     );
   });
 
+  it('fetches multiple branches in a single invocation', () => {
+    // Multiple refspecs let one --deepen / --unshallow cover both refs in a single network
+    // round-trip — used by ensureSharedHistory when it has to deepen both HEAD and the target.
+    gitOverride = noOpSuccess;
+    const otherBranch = 'feature';
+    const res = gitFetch({
+      cwd: repo.rootPath,
+      remote: defaultRemoteName,
+      branch: [defaultBranchName, otherBranch],
+      deepen: 5,
+      verbose: true,
+    });
+
+    const refspec1 = `+refs/heads/${defaultBranchName}:refs/remotes/${defaultRemoteName}/${defaultBranchName}`;
+    const refspec2 = `+refs/heads/${otherBranch}:refs/remotes/${defaultRemoteName}/${otherBranch}`;
+    expect(gitSpy).toHaveBeenCalledWith(
+      ['fetch', '--deepen=5', defaultRemoteName, refspec1, refspec2],
+      expect.anything()
+    );
+    expect(res).toMatchObject({ success: true });
+    expect(logs.mocks.log).toHaveBeenCalledWith(
+      `Fetching branches "${defaultBranchName}", "${otherBranch}" from remote "${defaultRemoteName}" ` +
+        `(${refspec1} ${refspec2}) (with --deepen=5)...`
+    );
+    expect(logs.mocks.log).toHaveBeenCalledWith(
+      `Fetching branches "${defaultBranchName}", "${otherBranch}" from remote "${defaultRemoteName}" completed successfully`
+    );
+  });
+
   it('preserves the tracking ref after a real fetch', () => {
     // With a bare branch name like 'master' as the refspec source, git can fail to resolve it
     // on the remote and treat it as absent, pruning refs/remotes/origin/master (exit code 0).

packages/beachball/src/git/ensureSharedHistory.ts

@@ -110,11 +110,19 @@ function deepenHistory(params: {
 
   console.log(`This is a shallow clone. Deepening to check for changes...`);
 
+  // git fetch --deepen only deepens the histories of the refs explicitly listed in the
+  // refspec args, not other local refs. To find the common ancestor, both the target branch
+  // and HEAD (when on a different branch) need enough history to reach it. We pass both
+  // refspecs to a single git invocation so one --deepen / --unshallow covers both refs in one
+  // network round-trip.
+  const headBranch = getHeadBranch(cwd);
+  const branchesToFetch = headBranch && headBranch !== remoteBranch ? [remoteBranch, headBranch] : [remoteBranch];
+
   // Iteratively deepen the history
   const maxAttempts = 3;
   for (let attempt = 1; attempt <= maxAttempts; attempt++) {
     console.log(`Deepening by ${depth} more commits (attempt ${attempt}/${maxAttempts})...`);
-    const result = gitFetch({ remote, branch: remoteBranch, deepen: depth, cwd, verbose });
+    const result = gitFetch({ remote, branch: branchesToFetch, deepen: depth, cwd, verbose });
     if (!result.success) {
       throw new BeachballError(`Failed to fetch more history (see above for details)`);
     }
@@ -130,7 +138,7 @@ function deepenHistory(params: {
 
   // No common commit was found and the repo is still shallow, so fully unshallow it
   console.log(`Still didn't find a common commit after deepening by ${depth * maxAttempts}. Unshallowing...`);
-  const result = gitFetch({ remote, branch: remoteBranch, unshallow: true, cwd, verbose });
+  const result = gitFetch({ remote, branch: branchesToFetch, unshallow: true, cwd, verbose });
   if (!result.success) {
     throw new BeachballError(`Failed to unshallow repo (see above for details)`);
   }
@@ -187,6 +195,13 @@ function hasBranchRef(branch: string, cwd: string): boolean {
   return git(['rev-parse', '--verify', branch], { cwd }).success;
 }
 
+/** Returns the current branch name, or undefined if in detached HEAD state */
+function getHeadBranch(cwd: string): string | undefined {
+  const result = git(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd });
+  const branch = result.stdout.trim();
+  return result.success && branch !== 'HEAD' ? branch : undefined;
+}
+
 function isShallowRepository(cwd: string): boolean {
   return git(['rev-parse', '--is-shallow-repository'], { cwd }).stdout.trim() === 'true';
 }

packages/beachball/src/git/fetch.ts

@@ -9,10 +9,13 @@ type GitFetchParams = {
    */
   remote: string;
   /**
-   * Branch to fetch. It will be converted to a full refspec for fetching:
-   * e.g. `branch: 'main', remote: 'origin'` will be converted to `+main:refs/remotes/origin/main`.
+   * Branch(es) to fetch. Each will be converted to a full refspec for fetching:
+   * e.g. `branch: 'main', remote: 'origin'` will be converted to `+refs/heads/main:refs/remotes/origin/main`.
+   * Pass an array to fetch multiple branches in a single git invocation, which lets a single
+   * `--deepen` or `--unshallow` apply to all of them (saves network round-trips when both HEAD
+   * and the target branch need deepening).
    */
-  branch: string;
+  branch: string | string[];
   /** Set depth to this number of commits (mutually exclusive with `deepen` and `unshallow`) */
   depth?: number;
   /** Deepen a shallow clone by this number of commits (mutually exclusive with `depth` and `unshallow`) */
@@ -31,7 +34,8 @@ type GitFetchParams = {
  * the remote branch is tracked or not in the local repository.
  */
 export function gitFetch(params: GitFetchParams): GitProcessOutput & { errorMessage?: string } {
-  const { remote, branch, depth, deepen, unshallow, cwd, verbose } = params;
+  const { remote, depth, deepen, unshallow, cwd, verbose } = params;
+  const branches = Array.isArray(params.branch) ? params.branch : [params.branch];
   const { shouldLog } = getGitEnv(verbose);
 
   if ([depth, deepen, unshallow].filter(v => v !== undefined).length > 1) {
@@ -40,20 +44,24 @@ export function gitFetch(params: GitFetchParams): GitProcessOutput & { errorMess
 
   const extraArgs = depth ? [`--depth=${depth}`] : deepen ? [`--deepen=${deepen}`] : unshallow ? ['--unshallow'] : [];
 
-  // Be specific with the ref being fetched, so we don't have to worry about tracking configs.
+  // Be specific with each ref being fetched, so we don't have to worry about tracking configs.
   // In git fetch <remote> +<src>:<dst>...
   // - The + means allow non-fast-forward updates (in case the remote was force pushed).
   // - <src> refs/heads/${branch} is resolved against the remote's advertised refs. The fully
   //   qualified ref is unambiguous, whereas bare branch names can be silently misresolved,
   //   causing git to treat the ref as absent and delete the local tracking ref.
   // - <dst> refs/remotes/${remote}/${branch} is resolved locally and only moves the tracking ref
   //   for the remote branch, not the local refs/heads/${branch} or its tracking config.
-  const resolvedBranch = remote ? `+refs/heads/${branch}:refs/remotes/${remote}/${branch}` : undefined;
+  const resolvedRefspecs = remote ? branches.map(b => `+refs/heads/${b}:refs/remotes/${remote}/${b}`) : [];
 
-  const shortDescription = `Fetching ${resolvedBranch ? `branch "${branch}" from remote "${remote}"` : 'all remotes'}`;
+  const branchLabel =
+    branches.length > 1 ? `branches ${branches.map(b => `"${b}"`).join(', ')}` : `branch "${branches[0]}"`;
+  const shortDescription = `Fetching ${
+    resolvedRefspecs.length ? `${branchLabel} from remote "${remote}"` : 'all remotes'
+  }`;
 
   let description = shortDescription;
-  resolvedBranch && (description += ` (${resolvedBranch})`);
+  resolvedRefspecs.length && (description += ` (${resolvedRefspecs.join(' ')})`);
   extraArgs.length && (description += ` (with ${extraArgs.join(' ')})`);
   shouldLog && console.log(description + '...');
 
@@ -62,7 +70,7 @@ export function gitFetch(params: GitFetchParams): GitProcessOutput & { errorMess
       'fetch',
       ...extraArgs,
       // If the remote is unknown, don't specify the branch (fetching a branch without a remote is invalid)
-      ...(resolvedBranch ? [remote, resolvedBranch] : []),
+      ...(resolvedRefspecs.length ? [remote, ...resolvedRefspecs] : []),
     ],
     { cwd, stdio: shouldLog === 'live' ? 'inherit' : 'pipe' }
   );