ensureSharedHistory fix

Author: ecraig12345

packages/beachball/src/git/ensureSharedHistory.ts

@@ -6,8 +6,14 @@ import { BeachballError } from '../types/BeachballError';
 
 /**
  * Ensure that adequate history is available to check for changes between HEAD and `options.branch`.
- * Otherwise attempting to get changes will fail with an error "no merge base".
- * (This is mostly an issue with CI builds that use shallow clones.)
+ * Otherwise attempting to get changes will fail with an error "no merge base". (This is mostly an
+ * issue with CI builds that use shallow clones and may not have a tracking branch configured for
+ * the remote target branch.)
+ *
+ * Since repo may be very large, it's important to avoid fetching the entire history or extra refs.
+ * If the repo is shallow and no common commit is found after a normal fetch, this function will
+ * iteratively deepen the history (100 commits at a time by default) and check for a common commit
+ * each round. If still not found, it will unshallow the clone.
  *
  * Throws an error if history is inadequate and cannot be fixed.
  */
@@ -33,26 +39,13 @@ export function ensureSharedHistory(
     }
 
     if (!remote) {
-      // If the remote isn't specified, even if fetching is allowed it will be unclear what to
-      // compare against
+      // If the remote isn't specified, even if fetching is allowed, it will be unclear what to
+      // compare against, so throw an error.
       throw new BeachballError(
         `Target branch "${branch}" doesn't exist locally, and a remote name wasn't specified and couldn't be inferred. ` +
           'Please set "repository" in your root package.json or include a remote in the beachball "branch" setting.'
       );
     }
-
-    // If fetching the requested branch isn't already (probably) configured, add it to the list so
-    // it can be fetched in the next step. Otherwise the ref <remote>/<remoteBranch> won't exist locally.
-    const fetchConfig = git(['config', '--get-all', `remote.${remote}.fetch`], { cwd }).stdout.trim();
-    if (!fetchConfig.includes(`${remote}/*`) && !fetchConfig.includes(branch)) {
-      console.log(`Adding branch "${remoteBranch}" to fetch config for remote "${remote}"`);
-      const result = git(['remote', 'set-branches', '--add', remote, remoteBranch], { cwd });
-      if (!result.success) {
-        throw new BeachballError(
-          `Failed to add branch "${remoteBranch}" to fetch config for remote "${remote}":\n${result.stderr}`
-        );
-      }
-    }
   }
 
   if (fetch) {
@@ -89,6 +82,8 @@ export function ensureSharedHistory(
 
       // Try fetching more history
       isConnected = deepenHistory({ remote, remoteBranch, branch, depth, cwd, verbose });
+    } else {
+      // Repo isn't shallow, so potentially we just need to add a ref to be connected
     }
 
     if (!isConnected) {

packages/beachball/src/git/fetch.ts

@@ -3,10 +3,16 @@ import { getGitEnv } from './gitAsync';
 
 type GitFetchParams = {
   cwd: string;
-  /** Remote to fetch from. If not specified, fetches all remotes. */
-  remote?: string;
-  /** Branch to fetch. This will be ignored if `remote` is not also specified. */
-  branch?: string;
+  /**
+   * Remote to fetch from. This should almost always be set but might be an empty string
+   * if the repo somehow has no remotes configured.
+   */
+  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: 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`) */
@@ -20,6 +26,9 @@ type GitFetchParams = {
  * Wrapper for `git fetch`. If `verbose` is true, log the command before starting, and display output
  * on stdout (except in tests). In tests with `verbose`, the output will be logged all together to
  * `console.log` when the command finishes (for easier mocking/capturing).
+ *
+ * This converts `remote` and `branch` into a fully qualified refspec, so it doesn't matter if
+ * 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;
@@ -31,22 +40,29 @@ export function gitFetch(params: GitFetchParams): GitProcessOutput & { errorMess
 
   const extraArgs = depth ? [`--depth=${depth}`] : deepen ? [`--deepen=${deepen}`] : unshallow ? ['--unshallow'] : [];
 
-  let description = remote
-    ? `Fetching ${branch ? `branch "${branch}" from ` : ''}remote "${remote}"`
-    : 'Fetching all remotes';
+  // Be specific with the 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;
 
-  if (extraArgs.length) {
-    description += ` (with ${extraArgs.join(' ')})`;
-  }
+  const shortDescription = `Fetching ${resolvedBranch ? `branch "${branch}" from remote "${remote}"` : 'all remotes'}`;
 
+  let description = shortDescription;
+  resolvedBranch && (description += ` (${resolvedBranch})`);
+  extraArgs.length && (description += ` (with ${extraArgs.join(' ')})`);
   shouldLog && console.log(description + '...');
 
   const result: ReturnType<typeof gitFetch> = git(
     [
       'fetch',
       ...extraArgs,
       // If the remote is unknown, don't specify the branch (fetching a branch without a remote is invalid)
-      ...(remote && branch ? [remote, branch] : remote ? [remote] : []),
+      ...(resolvedBranch ? [remote, resolvedBranch] : []),
     ],
     { cwd, stdio: shouldLog === 'live' ? 'inherit' : 'pipe' }
   );
@@ -59,7 +75,7 @@ export function gitFetch(params: GitFetchParams): GitProcessOutput & { errorMess
     result.stderr && log(result.stderr);
   }
 
-  let message = `${description} ${result.success ? 'completed successfully' : `failed (code ${result.status})`}`;
+  let message = `${shortDescription} ${result.success ? 'completed successfully' : `failed (code ${result.status})`}`;
   if (shouldLog) {
     log(message);
     message += ' - see above for details';