diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 67f61509e8ba0..e251884f02983 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -1,5 +1,6 @@ import { QuickInputButtons } from 'vscode'; import type { Container } from '../../container'; +import { BranchError, BranchErrorReason } from '../../git/errors'; import type { GitBranchReference, GitReference } from '../../git/models/reference'; import { getNameWithoutRemote, @@ -10,7 +11,7 @@ import { import { Repository } from '../../git/models/repository'; import type { GitWorktree } from '../../git/models/worktree'; import { getWorktreesByBranch } from '../../git/models/worktree'; -import { showGenericErrorMessage } from '../../messages'; +import { showGenericErrorMessage, showGitBranchNotFullyMergedPrompt } from '../../messages'; import type { QuickPickItemOfT } from '../../quickpicks/items/common'; import { createQuickPickSeparator } from '../../quickpicks/items/common'; import type { FlagsQuickPickItem } from '../../quickpicks/items/flags'; @@ -427,7 +428,7 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage('Unable to create branch'); + return showGenericErrorMessage(ex); } } } @@ -558,10 +559,35 @@ export class BranchGitCommand extends QuickCommand { state.flags = result; endSteps(state); - state.repo.branchDelete(state.references, { - force: state.flags.includes('--force'), - remote: state.flags.includes('--remotes'), - }); + + for (const ref of state.references) { + try { + await state.repo.git.deleteBranch(ref, { + force: state.flags.includes('--force'), + remote: state.flags.includes('--remotes'), + }); + } catch (ex) { + // TODO likely need some better error handling here + Logger.error(ex); + if (ex instanceof BranchError && ex.reason === BranchErrorReason.BranchNotFullyMerged) { + const shouldRetryWithForce = await showGitBranchNotFullyMergedPrompt(ref.name); + if (shouldRetryWithForce) { + try { + await state.repo.git.deleteBranch(ref, { + force: true, + remote: state.flags.includes('--remotes'), + }); + } catch (ex) { + Logger.error(ex); + await showGenericErrorMessage(ex); + } + } + continue; + } + + await showGenericErrorMessage(ex); + } + } } } @@ -669,7 +695,7 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage('Unable to rename branch'); + return showGenericErrorMessage(ex); } } } diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 6f9b571ea868c..cc9939057dbfd 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -12,6 +12,8 @@ import { GitErrorHandling } from '../../../git/commandOptions'; import { BlameIgnoreRevsFileBadRevisionError, BlameIgnoreRevsFileError, + BranchError, + BranchErrorReason, CherryPickError, CherryPickErrorReason, FetchError, @@ -73,6 +75,10 @@ const textDecoder = new TextDecoder('utf8'); const rootSha = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; export const GitErrors = { + noRemoteReference: /unable to delete '.+?': remote ref does not exist/i, + invalidBranchName: /fatal: '.+?' is not a valid branch name/i, + branchAlreadyExists: /fatal: A branch named '.+?' already exists/i, + branchNotFullyMerged: /error: The branch '.+?' is not fully merged/i, badRevision: /bad revision '(.*?)'/i, cantLockRef: /cannot lock ref|unable to update local ref/i, changesWouldBeOverwritten: /Your local changes to the following files would be overwritten/i, @@ -165,6 +171,13 @@ function getStdinUniqueKey(): number { type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true }; export type PushForceOptions = { withLease: true; ifIncludes?: boolean } | { withLease: false; ifIncludes?: never }; +const branchErrorAndReason: [RegExp, BranchErrorReason][] = [ + [GitErrors.noRemoteReference, BranchErrorReason.NoRemoteReference], + [GitErrors.invalidBranchName, BranchErrorReason.InvalidBranchName], + [GitErrors.branchAlreadyExists, BranchErrorReason.BranchAlreadyExists], + [GitErrors.branchNotFullyMerged, BranchErrorReason.BranchNotFullyMerged], +]; + const tagErrorAndReason: [RegExp, TagErrorReason][] = [ [GitErrors.tagAlreadyExists, TagErrorReason.TagAlreadyExists], [GitErrors.tagNotFound, TagErrorReason.TagNotFound], @@ -520,8 +533,18 @@ export class Git { } } - branch(repoPath: string, ...args: string[]) { - return this.git({ cwd: repoPath }, 'branch', ...args); + async branch(repoPath: string, ...args: string[]): Promise { + try { + await this.git({ cwd: repoPath }, 'branch', ...args); + } catch (ex) { + const msg: string = ex?.toString() ?? ''; + for (const [error, reason] of branchErrorAndReason) { + if (error.test(msg) || error.test(ex.stderr ?? '')) { + throw new BranchError(reason, ex); + } + } + throw new BranchError(BranchErrorReason.Other, ex); + } } branch__set_upstream(repoPath: string, branch: string, remote: string, remoteBranch: string) { @@ -976,6 +999,10 @@ export class Git { publish?: boolean; remote?: string; upstream?: string; + delete?: { + remote: string; + branch: string; + }; }, ): Promise { const params = ['push']; @@ -1003,6 +1030,8 @@ export class Git { } } else if (options.remote) { params.push(options.remote); + } else if (options.delete) { + params.push('-d', options.delete.remote, options.delete.branch); } try { @@ -1023,9 +1052,13 @@ export class Git { /! \[rejected\].*\(remote ref updated since checkout\)/m.test(ex.stderr || '') ) { reason = PushErrorReason.PushRejectedWithLeaseIfIncludes; + } else if (/error: unable to delete '(.*?)': remote ref does not exist/m.test(ex.stderr || '')) { + reason = PushErrorReason.PushRejectedRefNotExists; } else { reason = PushErrorReason.PushRejected; } + } else if (/error: unable to delete '(.*?)': remote ref does not exist/m.test(ex.stderr || '')) { + reason = PushErrorReason.PushRejectedRefNotExists; } else { reason = PushErrorReason.PushRejected; } @@ -1037,7 +1070,12 @@ export class Git { reason = PushErrorReason.NoUpstream; } - throw new PushError(reason, ex, options?.branch, options?.remote); + throw new PushError( + reason, + ex, + options?.branch || options?.delete?.branch, + options?.remote || options?.delete?.remote, + ); } } @@ -1591,7 +1629,12 @@ export class Git { async rev_list( repoPath: string, ref: string, - options?: { all?: boolean; maxParents?: number; since?: string }, + options?: { + all?: boolean; + maxParents?: number; + maxResults?: number; + since?: string; + }, ): Promise { const params = ['rev-list']; if (options?.all) { @@ -1602,6 +1645,10 @@ export class Git { params.push(`--max-parents=${options.maxParents}`); } + if (options?.maxResults != null) { + params.push(`-n ${options.maxResults}`); + } + if (options?.since) { params.push(`--since="${options.since}"`, '--date-order'); } @@ -1871,6 +1918,10 @@ export class Git { return data.length === 0 ? undefined : data.trim(); } + async update_ref(repoPath: string, ...args: string[]): Promise { + await this.git({ cwd: repoPath }, 'update-ref', ...args); + } + show( repoPath: string, options?: { cancellation?: CancellationToken; configs?: readonly string[] }, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 75f5e9b1cbfd3..a329335ddefef 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -24,6 +24,7 @@ import { ApplyPatchCommitErrorReason, BlameIgnoreRevsFileBadRevisionError, BlameIgnoreRevsFileError, + BranchError, CherryPickError, CherryPickErrorReason, FetchError, @@ -1277,12 +1278,86 @@ export class LocalGitProvider implements GitProvider, Disposable { @log() async createBranch(repoPath: string, name: string, ref: string): Promise { - await this.git.branch(repoPath, name, ref); + try { + await this.git.branch(repoPath, name, ref); + } catch (ex) { + if (ex instanceof BranchError) { + throw ex.WithBranch(name); + } + + throw ex; + } } @log() async renameBranch(repoPath: string, oldName: string, newName: string): Promise { - await this.git.branch(repoPath, '-m', oldName, newName); + try { + await this.git.branch(repoPath, '-m', oldName, newName); + } catch (ex) { + if (ex instanceof BranchError) { + throw ex.WithBranch(oldName); + } + + throw ex; + } + } + + @log() + async deleteBranch( + repoPath: string, + branch: GitBranchReference, + options: { force?: boolean; remote?: boolean }, + ): Promise { + try { + if (branch.remote) { + await this.git.push(repoPath, { + delete: { + remote: getRemoteNameFromBranchName(branch.name), + branch: branch.remote ? getBranchNameWithoutRemote(branch.name) : branch.name, + }, + }); + return; + } + + const args = ['--delete']; + if (options.force) { + args.push('--force'); + } + + if (!options.remote || !branch.upstream) { + await this.git.branch(repoPath, ...args, branch.ref); + return; + } + + const remote = getRemoteNameFromBranchName(branch.upstream.name); + const remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, { + maxResults: 1, + }); + + await this.git.branch(repoPath, '--delete', '--remotes', `${remote}/${branch.ref}`); + + try { + await this.git.branch(repoPath, ...args, branch.ref); + } catch (ex) { + // If it fails, restore the remote branch + await this.git.update_ref(repoPath, `refs/remotes/${remote}/${branch.ref}`, remoteCommit?.[0] ?? ''); + await this.git.branch__set_upstream(repoPath, branch.name, remote, branch.ref); + throw ex; + } + + await this.git.push(repoPath, { + delete: { + remote: remote, + branch: getBranchNameWithoutRemote(branch.upstream.name), + }, + }); + } catch (ex) { + if (ex instanceof BranchError) { + throw ex.WithBranch(branch.name); + } + + throw ex; + } } @log() diff --git a/src/git/errors.ts b/src/git/errors.ts index e1ef081fdfb25..16a4e5de4e496 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -148,6 +148,7 @@ export const enum PushErrorReason { PushRejected, PushRejectedWithLease, PushRejectedWithLeaseIfIncludes, + PushRejectedRefNotExists, PermissionDenied, RemoteConnection, NoUpstream, @@ -197,6 +198,11 @@ export class PushError extends Error { remote ? ` to ${remote}` : '' } because some refs failed to push or the push was rejected. The tip of the remote-tracking branch has been updated since the last checkout. Try pulling first.`; break; + case PushErrorReason.PushRejectedRefNotExists: + message = `Unable to delete branch${branch ? ` '${branch}'` : ''}${ + remote ? ` on ${remote}` : '' + }, the remote reference does not exist`; + break; case PushErrorReason.PermissionDenied: message = `${baseMessage} because you don't have permission to push to this remote repository.`; break; @@ -218,6 +224,61 @@ export class PushError extends Error { } } +export const enum BranchErrorReason { + BranchAlreadyExists, + BranchNotFullyMerged, + NoRemoteReference, + InvalidBranchName, + Other, +} + +export class BranchError extends Error { + static is(ex: unknown, reason?: BranchErrorReason): ex is BranchError { + return ex instanceof BranchError && (reason == null || ex.reason === reason); + } + + readonly original?: Error; + readonly reason: BranchErrorReason | undefined; + + constructor(reason?: BranchErrorReason, original?: Error, branch?: string); + constructor(message?: string, original?: Error); + constructor(messageOrReason: string | BranchErrorReason | undefined, original?: Error, branch?: string) { + let reason: BranchErrorReason | undefined; + if (typeof messageOrReason !== 'string') { + reason = messageOrReason as BranchErrorReason; + } + + const message = + typeof messageOrReason === 'string' ? messageOrReason : BranchError.buildBranchErrorMessage(reason, branch); + super(message); + + this.original = original; + this.reason = reason; + Error.captureStackTrace?.(this, BranchError); + } + + private static buildBranchErrorMessage(reason?: BranchErrorReason, branch?: string): string { + const baseMessage = `Unable to perform action ${branch ? `with branch '${branch}'` : 'on branch'}`; + switch (reason) { + case BranchErrorReason.BranchAlreadyExists: + return `${baseMessage} because it already exists`; + case BranchErrorReason.BranchNotFullyMerged: + return `${baseMessage} because it is not fully merged`; + case BranchErrorReason.NoRemoteReference: + return `${baseMessage} because the remote reference does not exist`; + case BranchErrorReason.InvalidBranchName: + return `${baseMessage} because the branch name is invalid`; + default: + return baseMessage; + } + } + + WithBranch(branchName: string): this { + this.message = BranchError.buildBranchErrorMessage(this.reason, branchName); + return this; + } +} + export const enum PullErrorReason { Conflict, GitIdentity, diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 162279a814baf..543347130c621 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -120,6 +120,11 @@ export interface BranchContributorOverview { export interface GitProviderRepository { createBranch?(repoPath: string, name: string, ref: string): Promise; renameBranch?(repoPath: string, oldName: string, newName: string): Promise; + deleteBranch?( + repoPath: string, + branches: GitBranchReference, + options?: { force?: boolean; remote?: boolean }, + ): Promise; createTag?(repoPath: string, name: string, ref: string, message?: string): Promise; deleteTag?(repoPath: string, name: string): Promise; addRemote?(repoPath: string, name: string, url: string, options?: { fetch?: boolean }): Promise; diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 479657d170dd1..e6d018e302460 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1377,6 +1377,21 @@ export class GitProviderService implements Disposable { return provider.renameBranch(path, oldName, newName); } + @log() + deleteBranch( + repoPath: string, + branch: GitBranchReference, + options?: { force?: boolean; remote?: boolean }, + ): Promise { + const { provider, path } = this.getProvider(repoPath); + if (provider.deleteBranch == null) throw new ProviderNotSupportedError(provider.descriptor.name); + + return provider.deleteBranch(path, branch, { + force: options?.force, + remote: options?.remote, + }); + } + @log() createTag(repoPath: string | Uri, name: string, ref: string, message?: string): Promise { const { provider, path } = this.getProvider(repoPath); diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index 49c0a3c4a972c..ecb236631f355 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -15,7 +15,7 @@ import { debug, log, logName } from '../../system/decorators/log'; import { memoize } from '../../system/decorators/memoize'; import type { Deferrable } from '../../system/function'; import { debounce } from '../../system/function'; -import { filter, groupByMap, join, map, min, some } from '../../system/iterable'; +import { filter, join, map, min, some } from '../../system/iterable'; import { getLoggableName, Logger } from '../../system/logger'; import { getLogScope } from '../../system/logger.scope'; import { updateRecordValue } from '../../system/object'; @@ -26,9 +26,8 @@ import { configuration } from '../../system/vscode/configuration'; import type { GitProviderDescriptor, GitProviderRepository } from '../gitProvider'; import type { GitProviderService } from '../gitProviderService'; import type { GitBranch } from './branch'; -import { getBranchNameWithoutRemote, getRemoteNameFromBranchName } from './branch'; import type { GitBranchReference, GitReference } from './reference'; -import { getNameWithoutRemote, isBranchReference } from './reference'; +import { isBranchReference } from './reference'; import type { GitRemote } from './remote'; import type { GitWorktree } from './worktree'; @@ -591,49 +590,6 @@ export class Repository implements Disposable { return remote; } - @log() - branchDelete(branches: GitBranchReference | GitBranchReference[], options?: { force?: boolean; remote?: boolean }) { - if (!Array.isArray(branches)) { - branches = [branches]; - } - - const localBranches = branches.filter(b => !b.remote); - if (localBranches.length !== 0) { - const args = ['--delete']; - if (options?.force) { - args.push('--force'); - } - void this.runTerminalCommand('branch', ...args, ...branches.map(b => b.ref)); - - if (options?.remote) { - const trackingBranches = localBranches.filter(b => b.upstream != null); - if (trackingBranches.length !== 0) { - const branchesByOrigin = groupByMap(trackingBranches, b => - getRemoteNameFromBranchName(b.upstream!.name), - ); - - for (const [remote, branches] of branchesByOrigin.entries()) { - void this.runTerminalCommand( - 'push', - '-d', - remote, - ...branches.map(b => getBranchNameWithoutRemote(b.upstream!.name)), - ); - } - } - } - } - - const remoteBranches = branches.filter(b => b.remote); - if (remoteBranches.length !== 0) { - const branchesByOrigin = groupByMap(remoteBranches, b => getRemoteNameFromBranchName(b.name)); - - for (const [remote, branches] of branchesByOrigin.entries()) { - void this.runTerminalCommand('push', '-d', remote, ...branches.map(b => getNameWithoutRemote(b))); - } - } - } - @log() cherryPick(...args: string[]) { void this.runTerminalCommand('cherry-pick', ...args); diff --git a/src/messages.ts b/src/messages.ts index 5027cd29eac8b..6702cd2f39dde 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -138,6 +138,18 @@ export function showGitVersionUnsupportedErrorMessage( ); } +export async function showGitBranchNotFullyMergedPrompt(branchName: string): Promise { + const confirm = { title: 'Retry with --force flag' }; + const result = await showMessage( + 'warn', + `Unable to delete branch '${branchName}'. It is not fully merged.`, + undefined, + null, + confirm, + ); + return result === confirm; +} + export async function showPreReleaseExpiredErrorMessage(version: string) { const upgrade = { title: 'Upgrade' }; const switchToRelease = { title: 'Switch to Release Version' };