Fixes change detection if worktree is outside .git

src/env/node/git/git.ts

@@ -1474,11 +1474,35 @@ export class Git {
 		}
 	}
 
-	async rev_parse__git_dir(cwd: string): Promise<string | undefined> {
-		const data = await this.git<string>({ cwd: cwd, errors: GitErrorHandling.Ignore }, 'rev-parse', '--git-dir');
-		// Make sure to normalize: https://github.com/git-for-windows/git/issues/2478
+	async rev_parse__git_dir(cwd: string): Promise<{ path: string; commonPath?: string } | undefined> {
+		const data = await this.git<string>(
+			{ cwd: cwd, errors: GitErrorHandling.Ignore },
+			'rev-parse',
+			'--git-dir',
+			'--git-common-dir',
+		);
+		if (data.length === 0) return undefined;
+
 		// Keep trailing spaces which are part of the directory name
-		return data.length === 0 ? undefined : normalizePath(data.trimLeft().replace(/[\r|\n]+$/, ''));
+		let [dotGitPath, commonDotGitPath] = data.split('\n').map(r => r.trimStart());
+
+		// Make sure to normalize: https://github.com/git-for-windows/git/issues/2478
+
+		if (!isAbsolute(dotGitPath)) {
+			dotGitPath = joinPaths(cwd, dotGitPath);
+		}
+		dotGitPath = normalizePath(dotGitPath);
+
+		if (commonDotGitPath) {
+			if (!isAbsolute(commonDotGitPath)) {
+				commonDotGitPath = joinPaths(cwd, commonDotGitPath);
+			}
+			commonDotGitPath = normalizePath(commonDotGitPath);
+
+			return { path: dotGitPath, commonPath: commonDotGitPath !== dotGitPath ? commonDotGitPath : undefined };
+		}
+
+		return { path: dotGitPath };
 	}
 
 	async rev_parse__show_toplevel(cwd: string): Promise<string | undefined> {

src/env/node/git/localGitProvider.ts

@@ -29,6 +29,7 @@ import {
 	WorktreeDeleteErrorReason,
 } from '../../../git/errors';
 import type {
+	GitDir,
 	GitProvider,
 	GitProviderDescriptor,
 	NextComparisonUrisResult,
@@ -184,7 +185,7 @@ const stashSummaryRegex =
 const reflogCommands = ['merge', 'pull'];
 
 interface RepositoryInfo {
-	gitDir?: string;
+	gitDir?: GitDir;
 	user?: GitUser | null;
 }
 
@@ -2494,10 +2495,35 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	}
 
 	@debug()
+	async getGitDir(repoPath: string): Promise<GitDir> {
+		const repo = this._repoInfoCache.get(repoPath);
+		if (repo?.gitDir != null) return repo.gitDir;
+
+		const gitDirPaths = await this.git.rev_parse__git_dir(repoPath);
+
+		let gitDir: GitDir;
+		if (gitDirPaths != null) {
+			gitDir = {
+				uri: Uri.file(gitDirPaths.path),
+				commonUri: gitDirPaths.commonPath != null ? Uri.file(gitDirPaths.commonPath) : undefined,
+			};
+		} else {
+			gitDir = {
+				uri: this.getAbsoluteUri('.git', repoPath),
+			};
+		}
+		this._repoInfoCache.set(repoPath, { ...repo, gitDir: gitDir });
+
+		return gitDir;
+	}
+
+	@debug()
 	async getLastFetchedTimestamp(repoPath: string): Promise<number | undefined> {
 		try {
 			const gitDir = await this.getGitDir(repoPath);
-			const stats = await workspace.fs.stat(this.container.git.getAbsoluteUri(`${gitDir}/FETCH_HEAD`, repoPath));
+			const stats = await workspace.fs.stat(
+				this.container.git.getAbsoluteUri(Uri.joinPath(gitDir.commonUri ?? gitDir.uri, 'FETCH_HEAD'), repoPath),
+			);
 			// If the file is empty, assume the fetch failed, and don't update the timestamp
 			if (stats.size > 0) return stats.mtime;
 		} catch {}
@@ -2505,16 +2531,6 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		return undefined;
 	}
 
-	private async getGitDir(repoPath: string): Promise<string> {
-		const repo = this._repoInfoCache.get(repoPath);
-		if (repo?.gitDir != null) return repo.gitDir;
-
-		const gitDir = normalizePath((await this.git.rev_parse__git_dir(repoPath)) || '.git');
-		this._repoInfoCache.set(repoPath, { ...repo, gitDir: gitDir });
-
-		return gitDir;
-	}
-
 	@log()
 	async getLog(
 		repoPath: string,

src/git/gitProvider.ts

@@ -28,6 +28,11 @@ import type { RemoteProviders } from './remotes/remoteProviders';
 import type { RichRemoteProvider } from './remotes/richRemoteProvider';
 import type { GitSearch, SearchQuery } from './search';
 
+export interface GitDir {
+	readonly uri: Uri;
+	readonly commonUri?: Uri;
+}
+
 export const enum GitProviderId {
 	Git = 'git',
 	GitHub = 'github',
@@ -270,6 +275,7 @@ export interface GitProvider extends Disposable {
 		options?: { filters?: GitDiffFilter[] | undefined; similarityThreshold?: number | undefined },
 	): Promise<GitFile[] | undefined>;
 	getFileStatusForCommit(repoPath: string, uri: Uri, ref: string): Promise<GitFile | undefined>;
+	getGitDir?(repoPath: string): Promise<GitDir | undefined>;
 	getLastFetchedTimestamp(repoPath: string): Promise<number | undefined>;
 	getLog(
 		repoPath: string,

src/git/gitProviderService.ts

@@ -32,6 +32,7 @@ import { getBestPath, getScheme, isAbsolute, maybeUri, normalizePath } from '../
 import { cancellable, fastestSettled, getSettledValue, isPromise, PromiseCancelledError } from '../system/promise';
 import { VisitedPathsTrie } from '../system/trie';
 import type {
+	GitDir,
 	GitProvider,
 	GitProviderDescriptor,
 	GitProviderId,
@@ -278,7 +279,7 @@ export class GitProviderService implements Disposable {
 			for (const folder of e.removed) {
 				const repository = this._repositories.getClosest(folder.uri);
 				if (repository != null) {
-					this._repositories.remove(repository.uri);
+					this._repositories.remove(repository.uri, false);
 					removed.push(repository);
 				}
 			}
@@ -427,7 +428,7 @@ export class GitProviderService implements Disposable {
 
 				for (const repository of [...this._repositories.values()]) {
 					if (repository?.provider.id === id) {
-						this._repositories.remove(repository.uri);
+						this._repositories.remove(repository.uri, false);
 						removed.push(repository);
 					}
 				}
@@ -1513,6 +1514,12 @@ export class GitProviderService implements Disposable {
 	}
 
 	@debug()
+	getGitDir(repoPath: string): Promise<GitDir | undefined> {
+		const { provider, path } = this.getProvider(repoPath);
+		return Promise.resolve(provider.getGitDir?.(path));
+	}
+
+	@debug()
 	getLastFetchedTimestamp(repoPath: string | Uri): Promise<number | undefined> {
 		const { provider, path } = this.getProvider(repoPath);
 		return provider.getLastFetchedTimestamp(path);

src/git/models/repository.ts

@@ -200,7 +200,6 @@ export class Repository implements Disposable {
 	private _providers: RemoteProviders | undefined;
 	private _remotes: Promise<GitRemote[]> | undefined;
 	private _remotesDisposable: Disposable | undefined;
-	private _repoWatcherDisposable: Disposable | undefined;
 	private _suspended: boolean;
 
 	constructor(
@@ -239,38 +238,68 @@ export class Repository implements Disposable {
 		this._suspended = suspended;
 		this._closed = closed;
 
-		const watcher = workspace.createFileSystemWatcher(
-			new RelativePattern(
-				this.uri,
-				'{\
-**/.git/config,\
-**/.git/index,\
-**/.git/HEAD,\
-**/.git/*_HEAD,\
-**/.git/MERGE_*,\
-**/.git/refs/**,\
-**/.git/rebase-merge/**,\
-**/.git/sequencer/**,\
-**/.git/worktrees/**,\
-**/.gitignore\
-}',
-			),
-		);
 		this._disposable = Disposable.from(
-			watcher,
-			watcher.onDidChange(this.onRepositoryChanged, this),
-			watcher.onDidCreate(this.onRepositoryChanged, this),
-			watcher.onDidDelete(this.onRepositoryChanged, this),
+			this.setupRepoWatchers(),
 			configuration.onDidChange(this.onConfigurationChanged, this),
 		);
+
 		this.onConfigurationChanged();
 	}
 
+	private setupRepoWatchers() {
+		let disposable: Disposable | undefined;
+
+		void this.setupRepoWatchersCore().then(d => (disposable = d));
+
+		return {
+			dispose: () => void disposable?.dispose(),
+		};
+	}
+
+	private async setupRepoWatchersCore() {
+		const disposables: Disposable[] = [];
+
+		const watcher = workspace.createFileSystemWatcher(new RelativePattern(this.uri, '**/.gitignore'));
+		disposables.push(
+			watcher,
+			watcher.onDidChange(this.onGitIgnoreChanged, this),
+			watcher.onDidCreate(this.onGitIgnoreChanged, this),
+			watcher.onDidDelete(this.onGitIgnoreChanged, this),
+		);
+
+		function watch(this: Repository, uri: Uri, pattern: string) {
+			const watcher = workspace.createFileSystemWatcher(new RelativePattern(uri, pattern));
+
+			disposables.push(
+				watcher,
+				watcher.onDidChange(e => this.onRepositoryChanged(e, uri)),
+				watcher.onDidCreate(e => this.onRepositoryChanged(e, uri)),
+				watcher.onDidDelete(e => this.onRepositoryChanged(e, uri)),
+			);
+			return watcher;
+		}
+
+		const gitDir = await this.container.git.getGitDir(this.path);
+		if (gitDir != null) {
+			if (gitDir?.commonUri == null) {
+				watch.call(
+					this,
+					gitDir.uri,
+					'{index,HEAD,*_HEAD,MERGE_*,config,refs/**,rebase-merge/**,sequencer/**,worktrees/**}',
+				);
+			} else {
+				watch.call(this, gitDir.uri, '{index,HEAD,*_HEAD,MERGE_*,rebase-merge/**,sequencer/**}');
+				watch.call(this, gitDir.commonUri, '{config,refs/**,worktrees/**}');
+			}
+		}
+
+		return Disposable.from(...disposables);
+	}
+
 	dispose() {
 		this.stopWatchingFileSystem();
 
 		this._remotesDisposable?.dispose();
-		this._repoWatcherDisposable?.dispose();
 		this._disposable.dispose();
 	}
 
@@ -311,24 +340,29 @@ export class Repository implements Disposable {
 	}
 
 	@debug()
-	private onRepositoryChanged(uri: Uri | undefined) {
+	private onGitIgnoreChanged(_uri: Uri) {
+		this.fireChange(RepositoryChange.Ignores);
+	}
+
+	@debug()
+	private onRepositoryChanged(uri: Uri | undefined, base: Uri) {
+		// TODO@eamodio Revisit -- as I can't seem to get this to work as a negative glob pattern match when creating the watcher
+		if (uri?.path.includes('/fsmonitor--daemon/')) {
+			return;
+		}
+
 		this._lastFetched = undefined;
 
 		const match =
 			uri != null
-				? /(?<ignore>\/\.gitignore)|\.git\/(?<type>config|index|HEAD|FETCH_HEAD|ORIG_HEAD|CHERRY_PICK_HEAD|MERGE_HEAD|REBASE_HEAD|rebase-merge|refs\/(?:heads|remotes|stash|tags)|worktrees)/.exec(
-						uri.path,
+				? // Move worktrees first, since if it is in a worktree it isn't affecting this repo directly
+				  /(worktrees|index|HEAD|FETCH_HEAD|ORIG_HEAD|CHERRY_PICK_HEAD|MERGE_HEAD|REBASE_HEAD|rebase-merge|config|refs\/(?:heads|remotes|stash|tags))/.exec(
+						this.container.git.getRelativePath(uri, base),
 				  )
 				: undefined;
-		if (match?.groups != null) {
-			const { ignore, type } = match.groups;
-
-			if (ignore) {
-				this.fireChange(RepositoryChange.Ignores);
-				return;
-			}
 
-			switch (type) {
+		if (match != null) {
+			switch (match[1]) {
 				case 'config':
 					this.resetCaches();
 					this.fireChange(RepositoryChange.Config, RepositoryChange.Remotes);