mhutchie#372 Improved handling of repositories on Mapped Network Driv…

…es on Windows with Git >= 2.25.0.

src/dataSource.ts

@@ -7,10 +7,11 @@ import { AskpassEnvironment, AskpassManager } from './askpass/askpassManager';
 import { getConfig } from './config';
 import { Logger } from './logger';
 import { CommitOrdering, DateType, DeepWriteable, ErrorInfo, GitCommit, GitCommitDetails, GitCommitStash, GitConfigLocation, GitFileChange, GitFileStatus, GitPushBranchMode, GitRepoSettings, GitResetMode, GitSignatureStatus, GitStash, MergeActionOn, RebaseActionOn, SquashMessageFormat, Writeable } from './types';
-import { GitExecutable, UNABLE_TO_FIND_GIT_MSG, UNCOMMITTED, abbrevCommit, constructIncompatibleGitVersionMessage, getPathFromStr, getPathFromUri, isGitAtLeastVersion, openGitTerminal, realpath, resolveSpawnOutput } from './utils';
+import { GitExecutable, UNABLE_TO_FIND_GIT_MSG, UNCOMMITTED, abbrevCommit, constructIncompatibleGitVersionMessage, getPathFromStr, getPathFromUri, isGitAtLeastVersion, openGitTerminal, pathWithTrailingSlash, realpath, resolveSpawnOutput } from './utils';
 import { Disposable } from './utils/disposable';
 import { Event } from './utils/event';
 
+const DRIVE_LETTER_PATH_REGEX = /^[a-z]:\//;
 const EOL_REGEX = /\r\n|\r|\n/g;
 const INVALID_BRANCH_REGEX = /^\(.* .*\)$/;
 const GIT_LOG_SEPARATOR = 'XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb';
@@ -505,20 +506,31 @@ export class DataSource extends Disposable {
 
 	/**
 	 * Get the root of the repository containing the specified path.
-	 * @param repoPath The path contained in the repository.
-	 * @returns The root of the repository.
-	 */
-	public repoRoot(repoPath: string) {
-		return this.spawnGit(['rev-parse', '--show-toplevel'], repoPath, (stdout) => getPathFromUri(vscode.Uri.file(path.normalize(stdout.trim())))).then(async (canonicalRoot) => {
-			let path = repoPath;
+	 * @param pathOfPotentialRepo The path that is potentially a repository (or is contained within a repository).
+	 * @returns STRING => The root of the repository, NULL => `pathOfPotentialRepo` is not in a repository.
+	 */
+	public repoRoot(pathOfPotentialRepo: string) {
+		return this.spawnGit(['rev-parse', '--show-toplevel'], pathOfPotentialRepo, (stdout) => getPathFromUri(vscode.Uri.file(path.normalize(stdout.trim())))).then(async (pathReturnedByGit) => {
+			if (process.platform === 'win32') {
+				// On Windows Mapped Network Drives with Git >= 2.25.0, `git rev-parse --show-toplevel` returns the UNC Path for the Mapped Network Drive, instead of the Drive Letter.
+				// Attempt to replace the UNC Path with the Drive Letter.
+				let driveLetterPathMatch: RegExpMatchArray | null;
+				if ((driveLetterPathMatch = pathOfPotentialRepo.match(DRIVE_LETTER_PATH_REGEX)) && !pathReturnedByGit.match(DRIVE_LETTER_PATH_REGEX)) {
+					const realPathForDriveLetter = pathWithTrailingSlash(await realpath(driveLetterPathMatch[0], true));
+					if (realPathForDriveLetter !== driveLetterPathMatch[0] && pathReturnedByGit.startsWith(realPathForDriveLetter)) {
+						pathReturnedByGit = driveLetterPathMatch[0] + pathReturnedByGit.substring(realPathForDriveLetter.length);
+					}
+				}
+			}
+			let path = pathOfPotentialRepo;
 			let first = path.indexOf('/');
 			while (true) {
-				if (canonicalRoot === path || canonicalRoot === await realpath(path)) return path;
+				if (pathReturnedByGit === path || pathReturnedByGit === await realpath(path)) return path;
 				let next = path.lastIndexOf('/');
 				if (first !== next && next > -1) {
 					path = path.substring(0, next);
 				} else {
-					return canonicalRoot;
+					return pathReturnedByGit;
 				}
 			}
 		}).catch(() => null); // null => path is not in a repo

src/tsconfig.json

@@ -14,6 +14,11 @@
 		"removeComments": true,
 		"sourceMap": true,
 		"strict": true,
-		"target": "es6"
+		"target": "es6",
+		"types": [
+			"node",
+			"vscode",
+			"./utils/@types/node"
+		]
 	}
 }

src/utils.ts

@@ -63,11 +63,12 @@ export function isPathInWorkspace(path: string) {
 /**
  * Get the normalised canonical absolute path (i.e. resolves symlinks in `path`).
  * @param path The path.
+ * @param native Use the native realpath.
  * @returns The normalised canonical absolute path.
  */
-export function realpath(path: string) {
-	return new Promise<string>(resolve => {
-		fs.realpath(path, (err, resolvedPath) => resolve(err !== null ? path : getPathFromUri(vscode.Uri.file(resolvedPath))));
+export function realpath(path: string, native: boolean = false) {
+	return new Promise<string>((resolve) => {
+		(native ? fs.realpath.native : fs.realpath)(path, (err, resolvedPath) => resolve(err !== null ? path : getPathFromUri(vscode.Uri.file(resolvedPath))));
 	});
 }
 

src/utils/@types/node/index.d.ts

@@ -0,0 +1,13 @@
+/**
+ * This file contains types for additional methods used by Git Graph, that were added between the
+ * version of @types/node (8.10.62) and the version of Node.js (10.11.0) used by the minimum
+ * version of Visual Studio Code that Git Graph supports. Unfortunately @types/node 10.11.0 can't
+ * be used, as it is not compatible with Typescript >= 3.7.2. Once the minimum version of Visual
+ * Studio Code that Git Graph supports is increased, such that it's version of @types/node is
+ * compatible with Typescript >= 3.7.2, @types/node will be updated, and this file will be removed.
+ */
+declare module 'fs' {
+	namespace realpath {
+		function native(path: PathLike, callback: (err: NodeJS.ErrnoException | null, resolvedPath: string) => void): void;
+	}
+}

tests/dataSource.test.ts

@@ -8,6 +8,7 @@ jest.mock('../src/logger');
 import * as cp from 'child_process';
 import * as fs from 'fs';
 import * as iconv from 'iconv-lite';
+import * as path from 'path';
 import { ConfigurationChangeEvent } from 'vscode';
 import { DataSource } from '../src/dataSource';
 import { Logger } from '../src/logger';
@@ -24,6 +25,7 @@ beforeAll(() => {
 	onDidChangeConfiguration = new EventEmitter<ConfigurationChangeEvent>();
 	onDidChangeGitExecutable = new EventEmitter<utils.GitExecutable>();
 	logger = new Logger();
+	jest.spyOn(path, 'normalize').mockImplementation((p) => p);
 });
 
 afterAll(() => {
@@ -3466,6 +3468,15 @@ describe('DataSource', () => {
 	});
 
 	describe('getSubmodules', () => {
+		let platform: NodeJS.Platform;
+		beforeEach(() => {
+			platform = process.platform;
+			Object.defineProperty(process, 'platform', { value: 'not-windows' });
+		});
+		afterEach(() => {
+			Object.defineProperty(process, 'platform', { value: platform });
+		});
+
 		it('Should return no submodules if no .gitmodules file exists', async () => {
 			// Setup
 			const spyOnReadFile = jest.spyOn(fs, 'readFile');
@@ -3511,6 +3522,15 @@ describe('DataSource', () => {
 	});
 
 	describe('repoRoot', () => {
+		let platform: NodeJS.Platform;
+		beforeEach(() => {
+			platform = process.platform;
+			Object.defineProperty(process, 'platform', { value: 'not-windows' });
+		});
+		afterEach(() => {
+			Object.defineProperty(process, 'platform', { value: platform });
+		});
+
 		it('Should return the same directory when called from the root of the repository', async () => {
 			// Setup
 			mockGitSuccessOnce('/path/to/repo/root');
@@ -3574,6 +3594,87 @@ describe('DataSource', () => {
 			expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['rev-parse', '--show-toplevel'], expect.objectContaining({ cwd: '/path' }));
 		});
 
+		describe('Windows Mapped Network Drive Resolution', () => {
+			it('Should not alter non-network share drives', async () => {
+				// Setup
+				mockGitSuccessOnce('c:/path/to/repo/root');
+				Object.defineProperty(process, 'platform', { value: 'win32' });
+				const spyOnRealpath = jest.spyOn(utils, 'realpath');
+
+				// Run
+				const result = await dataSource.repoRoot('c:/path/to/repo/root');
+
+				// Assert
+				expect(result).toBe('c:/path/to/repo/root');
+				expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['rev-parse', '--show-toplevel'], expect.objectContaining({ cwd: 'c:/path/to/repo/root' }));
+				expect(spyOnRealpath).toHaveBeenCalledTimes(0);
+			});
+
+			it('Should resolve the UNC Path Prefix of a path on a network share', async () => {
+				// Setup
+				mockGitSuccessOnce('//network/drive/path/to/repo/root');
+				Object.defineProperty(process, 'platform', { value: 'win32' });
+				const spyOnRealpath = jest.spyOn(utils, 'realpath');
+				spyOnRealpath.mockResolvedValueOnce('//network/drive/');
+
+				// Run
+				const result = await dataSource.repoRoot('a:/path/to/repo/root');
+
+				// Assert
+				expect(result).toBe('a:/path/to/repo/root');
+				expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['rev-parse', '--show-toplevel'], expect.objectContaining({ cwd: 'a:/path/to/repo/root' }));
+				expect(spyOnRealpath).toBeCalledWith('a:/', true);
+			});
+
+			it('Should resolve the UNC Path Prefix of a path on a network share (when native realpath doesn\'t return a trailing slash)', async () => {
+				// Setup
+				mockGitSuccessOnce('//network/drive/path/to/repo/root');
+				Object.defineProperty(process, 'platform', { value: 'win32' });
+				const spyOnRealpath = jest.spyOn(utils, 'realpath');
+				spyOnRealpath.mockResolvedValueOnce('//network/drive');
+
+				// Run
+				const result = await dataSource.repoRoot('a:/path/to/repo/root');
+
+				// Assert
+				expect(result).toBe('a:/path/to/repo/root');
+				expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['rev-parse', '--show-toplevel'], expect.objectContaining({ cwd: 'a:/path/to/repo/root' }));
+				expect(spyOnRealpath).toBeCalledWith('a:/', true);
+			});
+
+			it('Should not adjust the path if the native realpath can\'t resolve the Mapped Network Drive Letter', async () => {
+				// Setup
+				mockGitSuccessOnce('//network/drive/path/to/repo/root');
+				Object.defineProperty(process, 'platform', { value: 'win32' });
+				const spyOnRealpath = jest.spyOn(utils, 'realpath');
+				spyOnRealpath.mockResolvedValueOnce('a:/');
+
+				// Run
+				const result = await dataSource.repoRoot('a:/path/to/repo/root');
+
+				// Assert
+				expect(result).toBe('//network/drive/path/to/repo/root');
+				expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['rev-parse', '--show-toplevel'], expect.objectContaining({ cwd: 'a:/path/to/repo/root' }));
+				expect(spyOnRealpath).toBeCalledWith('a:/', true);
+			});
+
+			it('Should not adjust the path if the native realpath resolves the Mapped Network Drive Letter to a different UNC Path Prefix', async () => {
+				// Setup
+				mockGitSuccessOnce('//network/drive/path/to/repo/root');
+				Object.defineProperty(process, 'platform', { value: 'win32' });
+				const spyOnRealpath = jest.spyOn(utils, 'realpath');
+				spyOnRealpath.mockResolvedValueOnce('//other/network/drive/');
+
+				// Run
+				const result = await dataSource.repoRoot('a:/path/to/repo/root');
+
+				// Assert
+				expect(result).toBe('//network/drive/path/to/repo/root');
+				expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['rev-parse', '--show-toplevel'], expect.objectContaining({ cwd: 'a:/path/to/repo/root' }));
+				expect(spyOnRealpath).toBeCalledWith('a:/', true);
+			});
+		});
+
 		it('Should return NULL when git threw an error', async () => {
 			// Setup
 			mockGitThrowingErrorOnce();

tests/tsconfig.json

@@ -12,6 +12,12 @@
 		"removeComments": true,
 		"sourceMap": true,
 		"strict": true,
-		"target": "es6"
+		"target": "es6",
+		"types": [
+			"jest",
+			"node",
+			"vscode",
+			"../src/utils/@types/node"
+		]
 	}
 }