Skip to content

Commit 3d8749a

Browse files
committed
Fixes issues with worktree delete
1 parent ddd7ae9 commit 3d8749a

File tree

5 files changed

+126
-43
lines changed

5 files changed

+126
-43
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
1313
- Fixes [#3657](https://github.com/gitkraken/vscode-gitlens/issues/3657) - Creating a worktree from within a worktree chooses the wrong path
1414
- Fixes [#3090](https://github.com/gitkraken/vscode-gitlens/issues/3090) - Manually created empty bare clone repositories in a trusted directory crash worktree view since LocalGitProvider.findRepositoryUri returns just ".git" — thanks to [PR #3092](https://github.com/gitkraken/vscode-gitlens/pull/3092) by Dawn Hwang ([@hwangh95](https://github.com/hwangh95))
1515
- Fixes [#3667](https://github.com/gitkraken/vscode-gitlens/issues/3667) - Makes Launchpad search by repo name
16+
- Fixes failure to prompt to delete the branch after deleting a worktree when a modal was shown (e.g. when prompting for force)
17+
- Fixes issues when git fails to delete the worktree folder on Windows
1618

1719
## [15.6.0] - 2024-10-07
1820

src/commands/git/worktree.ts

Lines changed: 60 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { proBadge, proBadgeSuperscript } from '../../constants';
55
import type { Container } from '../../container';
66
import { CancellationError } from '../../errors';
77
import { PlusFeatures } from '../../features';
8+
import { executeGitCommand } from '../../git/actions';
89
import { convertLocationToOpenFlags, convertOpenFlagsToLocation, reveal } from '../../git/actions/worktree';
910
import {
1011
ApplyPatchCommitError,
@@ -15,7 +16,7 @@ import {
1516
WorktreeDeleteErrorReason,
1617
} from '../../git/errors';
1718
import { uncommitted, uncommittedStaged } from '../../git/models/constants';
18-
import type { GitReference } from '../../git/models/reference';
19+
import type { GitBranchReference, GitReference } from '../../git/models/reference';
1920
import {
2021
getNameWithoutRemote,
2122
getReferenceFromBranch,
@@ -69,7 +70,6 @@ import {
6970
pickWorktreesStep,
7071
pickWorktreeStep,
7172
} from '../quickCommand.steps';
72-
import { getSteps } from '../quickWizard.utils';
7373

7474
interface Context {
7575
repos: Repository[];
@@ -864,20 +864,20 @@ export class WorktreeGitCommand extends QuickCommand<State> {
864864

865865
state.flags = result;
866866

867-
endSteps(state);
868-
869-
const branchesToDelete = [];
867+
const branchesToDelete: GitBranchReference[] = [];
870868

871869
for (const uri of state.uris) {
872-
let retry = false;
873870
let skipHasChangesPrompt = false;
874-
do {
875-
retry = false;
876-
const force = state.flags.includes('--force');
877-
const deleteBranches = state.flags.includes('--delete-branches');
871+
let succeeded = false;
872+
873+
const deleteBranches = state.flags.includes('--delete-branches');
874+
let force = state.flags.includes('--force');
875+
const worktree = context.worktrees.find(wt => wt.uri.toString() === uri.toString());
876+
877+
while (true) {
878+
succeeded = false;
878879

879880
try {
880-
const worktree = context.worktrees.find(wt => wt.uri.toString() === uri.toString());
881881
if (force) {
882882
let status;
883883
try {
@@ -899,16 +899,35 @@ export class WorktreeGitCommand extends QuickCommand<State> {
899899
}
900900

901901
await state.repo.git.deleteWorktree(uri, { force: force });
902-
if (deleteBranches && worktree?.branch) {
903-
branchesToDelete.push(getReferenceFromBranch(worktree?.branch));
904-
}
902+
succeeded = true;
905903
} catch (ex) {
906904
skipHasChangesPrompt = false;
907905

908906
if (WorktreeDeleteError.is(ex)) {
909-
if (ex.reason === WorktreeDeleteErrorReason.MainWorkingTree) {
910-
void window.showErrorMessage('Unable to delete the main worktree');
911-
} else if (!force) {
907+
if (ex.reason === WorktreeDeleteErrorReason.DefaultWorkingTree) {
908+
void window.showErrorMessage('Cannot delete the default worktree.');
909+
break;
910+
}
911+
912+
if (ex.reason === WorktreeDeleteErrorReason.DirectoryNotEmpty) {
913+
const openFolder: MessageItem = { title: 'Open Folder' };
914+
const confirm: MessageItem = { title: 'OK', isCloseAffordance: true };
915+
const result = await window.showErrorMessage(
916+
`Unable to fully clean up the delete worktree in '${uri.fsPath}' because the folder is not empty.`,
917+
{ modal: true },
918+
openFolder,
919+
confirm,
920+
);
921+
922+
if (result === openFolder) {
923+
void revealInFileExplorer(uri);
924+
}
925+
926+
succeeded = true;
927+
break;
928+
}
929+
930+
if (!force) {
912931
const confirm: MessageItem = { title: 'Force Delete' };
913932
const cancel: MessageItem = { title: 'Cancel', isCloseAffordance: true };
914933
const result = await window.showErrorMessage(
@@ -921,31 +940,39 @@ export class WorktreeGitCommand extends QuickCommand<State> {
921940
);
922941

923942
if (result === confirm) {
924-
state.flags.push('--force');
925-
retry = true;
943+
force = true;
926944
skipHasChangesPrompt = ex.reason === WorktreeDeleteErrorReason.HasChanges;
945+
continue;
927946
}
947+
948+
break;
928949
}
929-
} else {
930-
void showGenericErrorMessage(`Unable to delete worktree in '${uri.fsPath}.`);
931950
}
951+
952+
void showGenericErrorMessage(`Unable to delete worktree in '${uri.fsPath}. ex=${String(ex)}`);
932953
}
933-
} while (retry);
954+
955+
break;
956+
}
957+
958+
if (succeeded && deleteBranches && worktree?.branch) {
959+
branchesToDelete.push(getReferenceFromBranch(worktree?.branch));
960+
}
934961
}
935962

963+
endSteps(state);
964+
936965
if (branchesToDelete.length) {
937-
yield* getSteps(
938-
this.container,
939-
{
940-
command: 'branch',
941-
state: {
942-
subcommand: 'delete',
943-
repo: state.repo,
944-
references: branchesToDelete,
945-
},
966+
// Don't use `getSteps` here because this is a whole new flow, a
967+
// and because of the modals above it won't even work (since the modals will trigger the quick pick to hide)
968+
void executeGitCommand({
969+
command: 'branch',
970+
state: {
971+
subcommand: 'delete',
972+
repo: state.repo,
973+
references: branchesToDelete,
946974
},
947-
this.pickedVia,
948-
);
975+
});
949976
}
950977
}
951978
}

src/env/node/git/git.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export const GitErrors = {
7777
changesWouldBeOverwritten: /Your local changes to the following files would be overwritten/i,
7878
commitChangesFirst: /Please, commit your changes before you can/i,
7979
conflict: /^CONFLICT \([^)]+\): \b/m,
80+
failedToDeleteDirectoryNotEmpty: /failed to delete '(.*?)': Directory not empty/i,
8081
invalidObjectName: /invalid object name: (.*)\s/i,
8182
invalidObjectNameList: /could not open object name list: (.*)\s/i,
8283
noFastForward: /\(non-fast-forward\)/i,

src/env/node/git/localGitProvider.ts

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { exec } from 'child_process';
12
import { promises as fs, readdir, realpath } from 'fs';
23
import { homedir, hostname, tmpdir, userInfo } from 'os';
34
import path, { resolve as resolvePath } from 'path';
@@ -6081,27 +6082,78 @@ export class LocalGitProvider implements GitProvider, Disposable {
60816082
' Please install a more recent version of Git and try again.',
60826083
);
60836084

6085+
path = normalizePath(typeof path === 'string' ? path : path.fsPath);
60846086
try {
6085-
await this.git.worktree__remove(
6086-
repoPath,
6087-
normalizePath(typeof path === 'string' ? path : path.fsPath),
6088-
options,
6089-
);
6090-
this.container.events.fire('git:cache:reset', { repoPath: repoPath, caches: ['worktrees'] });
6087+
await this.git.worktree__remove(repoPath, path, options);
60916088
} catch (ex) {
60926089
Logger.error(ex, scope);
60936090

60946091
const msg = String(ex);
60956092

60966093
if (GitErrors.mainWorkingTree.test(msg)) {
6097-
throw new WorktreeDeleteError(WorktreeDeleteErrorReason.MainWorkingTree, ex);
6094+
throw new WorktreeDeleteError(WorktreeDeleteErrorReason.DefaultWorkingTree, ex);
60986095
}
60996096

61006097
if (GitErrors.uncommittedChanges.test(msg)) {
61016098
throw new WorktreeDeleteError(WorktreeDeleteErrorReason.HasChanges, ex);
61026099
}
61036100

6101+
if (GitErrors.failedToDeleteDirectoryNotEmpty.test(msg)) {
6102+
Logger.warn(
6103+
scope,
6104+
`Failed to fully delete worktree '${path}' because it is not empty. Attempting to delete it manually.`,
6105+
scope,
6106+
);
6107+
try {
6108+
await fs.rm(path, { force: true, recursive: true });
6109+
return;
6110+
} catch (ex) {
6111+
if (isWindows) {
6112+
const match = /EPERM: operation not permitted, unlink '(.*?)'/i.exec(ex.message);
6113+
if (match != null) {
6114+
Logger.warn(
6115+
scope,
6116+
`Failed to manually delete '${path}' because it is in use. Attempting to forcefully delete it.`,
6117+
scope,
6118+
);
6119+
6120+
function deleteInUseSymlink(symlink: string) {
6121+
return new Promise((resolve, reject) => {
6122+
exec(`del "${symlink}"`, (ex, stdout, stderr) => {
6123+
if (ex) {
6124+
reject(ex instanceof Error ? ex : new Error(ex));
6125+
} else if (stderr) {
6126+
reject(new Error(stderr));
6127+
} else {
6128+
resolve(stdout);
6129+
}
6130+
});
6131+
});
6132+
}
6133+
6134+
const [, file] = match;
6135+
try {
6136+
await deleteInUseSymlink(file);
6137+
await fs.rm(path, { force: true, recursive: true, maxRetries: 1, retryDelay: 1 });
6138+
return;
6139+
} catch (ex) {
6140+
Logger.error(
6141+
ex,
6142+
scope,
6143+
`Failed to forcefully delete '${path}' because it is in use.`,
6144+
scope,
6145+
);
6146+
}
6147+
}
6148+
}
6149+
6150+
throw new WorktreeDeleteError(WorktreeDeleteErrorReason.DirectoryNotEmpty, ex);
6151+
}
6152+
}
6153+
61046154
throw new WorktreeDeleteError(undefined, ex);
6155+
} finally {
6156+
this.container.events.fire('git:cache:reset', { repoPath: repoPath, caches: ['worktrees'] });
61056157
}
61066158
}
61076159

src/git/errors.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,8 @@ export class WorktreeCreateError extends Error {
449449

450450
export const enum WorktreeDeleteErrorReason {
451451
HasChanges,
452-
MainWorkingTree,
452+
DefaultWorkingTree,
453+
DirectoryNotEmpty,
453454
}
454455

455456
export class WorktreeDeleteError extends Error {
@@ -476,8 +477,8 @@ export class WorktreeDeleteError extends Error {
476477
case WorktreeDeleteErrorReason.HasChanges:
477478
message = 'Unable to delete worktree because there are uncommitted changes';
478479
break;
479-
case WorktreeDeleteErrorReason.MainWorkingTree:
480-
message = 'Unable to delete worktree because it is a main working tree';
480+
case WorktreeDeleteErrorReason.DefaultWorkingTree:
481+
message = 'Cannot delete worktree because it is the default working tree';
481482
break;
482483
}
483484
}

0 commit comments

Comments
 (0)