Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions change/change-e0ebd254-f3c4-4b0b-aa4f-533fe9300419.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"type": "patch",
"comment": "Fully fix iterative deepening for latest git",
"packageName": "beachball",
"email": "elcraig@microsoft.com",
"dependentChangeType": "patch"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});

Expand Down Expand Up @@ -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]);
});

Expand All @@ -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}`,
]);
});

Expand Down
29 changes: 29 additions & 0 deletions packages/beachball/src/__functional__/git/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
19 changes: 17 additions & 2 deletions packages/beachball/src/git/ensureSharedHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)`);
}
Expand All @@ -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)`);
}
Expand Down Expand Up @@ -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';
}
Expand Down
26 changes: 17 additions & 9 deletions packages/beachball/src/git/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`) */
Expand All @@ -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) {
Expand All @@ -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 + '...');

Expand All @@ -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' }
);
Expand Down