[release] src/goDebugConfiguration: massage launch config for debug/test

When the current build directory is resolved to a path
different from the program's directory due to soft/symbolic links,
the go command can be confused and complain that the absolute
path in program is outside the module of the current directory.

This CL avoids such problem by making dlv always use a relative
path as the build target for build/test mode (and auto for testing).
Before starting the debug session, we massage the launch config:

  program: /foo/bar.go -> program: ./bar.go, __buildDir: /foo
  program: /foo/bar -> program: ., __buildDir: /foo/bar

Previously we find the package directory just before spawning
dlv dap and spawn the dlv dap process from the package directory.
With this CL, we find the package directory when resolving
the debug configuration before debug session.

This introduces __buildDir attribute which is internal.
(There is an ongoing work to introduce 'buildDir' to dlv DAP
so we use internal attribute to make it clear.)

Also, this made the resolveDebugConfigurationWithSubstitutedVariables
accept relative paths without workspace root. Just the behavior
is undefined. (The motivation of change in this part is
the testing. We have 'output' property or some others, that
are like relative path. I guess most users wouldn't care much.
Delve is cool with relative paths so we do our best to
resolve that wrt the workspace root.
Otherwise, just let delve decide.

Fixes #1713
Fixes #1680
Fixes #1677
Updates #1348

Change-Id: I434c43131b27d9c58058450c502e1b30c58ea690
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/344790
Trust: Hyang-Ah Hana Kim <[email protected]>
Trust: Suzy Mueller <[email protected]>
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/346095

Author: hyangah

src/goDebugConfiguration.ts

@@ -10,6 +10,7 @@
 import path = require('path');
 import vscode = require('vscode');
 import { getGoConfig } from './config';
+import { parseProgramArgSync } from './goDebugFactory';
 import { toolExecutionEnvironment } from './goEnv';
 import {
 	declinedToolInstall,
@@ -243,8 +244,6 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 
 		const dlvToolPath = getBinPath(debugAdapter);
 		if (!path.isAbsolute(dlvToolPath)) {
-			const tool = getTool(debugAdapter);
-
 			// If user has not already declined to install this tool,
 			// prompt for it. Otherwise continue and have the lack of
 			// dlv binary be caught later.
@@ -382,28 +381,49 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 		const entriesWithRelativePaths = ['cwd', 'output', 'program'].filter(
 			(attr) => debugConfiguration[attr] && !path.isAbsolute(debugConfiguration[attr])
 		);
-		if (debugConfiguration['debugAdapter'] === 'dlv-dap' && entriesWithRelativePaths.length > 0) {
-			const workspaceRoot = folder?.uri.fsPath;
-			if (!workspaceRoot) {
-				this.showWarning(
-					'relativePathsWithoutWorkspaceFolder',
-					'Relative paths without a workspace folder for `cwd`, `program`, or `output` are not allowed.'
-				);
-				return null;
+		if (debugConfiguration['debugAdapter'] === 'dlv-dap') {
+			// relative paths -> absolute paths
+			if (entriesWithRelativePaths.length > 0) {
+				const workspaceRoot = folder?.uri.fsPath;
+				if (workspaceRoot) {
+					entriesWithRelativePaths.forEach((attr) => {
+						debugConfiguration[attr] = path.join(workspaceRoot, debugConfiguration[attr]);
+					});
+				} else {
+					this.showWarning(
+						'relativePathsWithoutWorkspaceFolder',
+						'Behavior when using relative paths without a workspace folder for `cwd`, `program`, or `output` is undefined.'
+					);
+				}
+			}
+			// compute build dir, and translate the dirname in program to a path relative to buildDir.
+			if (debugConfiguration.request === 'launch') {
+				const mode = debugConfiguration['mode'] || 'debug';
+				if (['debug', 'test', 'auto'].includes(mode)) {
+					// Massage config to build the target from the package directory
+					// with a relative path. (https://github.com/golang/vscode-go/issues/1713)
+					try {
+						const { program, dirname, programIsDirectory } = parseProgramArgSync(debugConfiguration);
+						if (dirname) {
+							debugConfiguration['__buildDir'] = dirname;
+							debugConfiguration['program'] = programIsDirectory
+								? '.'
+								: '.' + path.sep + path.relative(dirname, program);
+						}
+					} catch (e) {
+						this.showWarning('invalidProgramArg', `Invalid 'program': ${e}`);
+						// keep going - just in case dlv knows how to handle this better.
+					}
+				}
 			}
-			entriesWithRelativePaths.forEach((attr) => {
-				debugConfiguration[attr] = path.join(workspaceRoot, debugConfiguration[attr]);
-			});
 		}
-
 		if (debugConfiguration.request === 'attach' && debugConfiguration['mode'] === 'local') {
 			// processId needs to be an int, but the substituted variables from pickGoProcess and pickProcess
 			// become a string. Convert any strings to integers.
 			if (typeof debugConfiguration['processId'] === 'string') {
 				debugConfiguration['processId'] = parseInt(debugConfiguration['processId'], 10);
 			}
 		}
-
 		return debugConfiguration;
 	}
 

src/goDebugFactory.ts

@@ -38,6 +38,7 @@ export class GoDebugAdapterDescriptorFactory implements vscode.DebugAdapterDescr
 		configuration: vscode.DebugConfiguration
 	): Promise<vscode.ProviderResult<vscode.DebugAdapterDescriptor>> {
 		const logger = new TimestampedLogger(configuration.trace, this.outputChannel);
+		logger.debug(`Config: ${JSON.stringify(configuration)}`);
 		const d = new DelveDAPOutputAdapter(configuration, logger);
 		return new vscode.DebugAdapterInlineImplementation(d);
 	}
@@ -356,13 +357,9 @@ function spawnDlvDapServerProcess(
 		throw new Error('Cannot find Delve debugger (dlv dap)');
 	}
 	let dir = getWorkspaceFolderPath();
-	if (launchAttachArgs.request === 'launch') {
-		try {
-			dir = parseProgramArgSync(launchAttachArgs).dirname;
-		} catch (err) {
-			logErr(`Program arg: ${launchAttachArgs.program}\n${err}\n`);
-			throw err; // rethrow so the caller knows it failed.
-		}
+	if (launchAttachArgs.request === 'launch' && launchAttachArgs['__buildDir']) {
+		// __buildDir is the directory determined during resolving debug config
+		dir = launchAttachArgs['__buildDir'];
 	}
 
 	const dlvArgs = new Array<string>();
@@ -408,7 +405,7 @@ function spawnDlvDapServerProcess(
 
 	const logDestStream = logDest ? fs.createWriteStream(logDest) : undefined;
 
-	logConsole(`Starting: ${dlvPath} ${dlvArgs.join(' ')}\n`);
+	logConsole(`Starting: ${dlvPath} ${dlvArgs.join(' ')} from ${dir}\n`);
 
 	// TODO(hyangah): In module-module workspace mode, the program should be build in the super module directory
 	// where go.work (gopls.mod) file is present. Where dlv runs determines the build directory currently. Two options:
@@ -491,27 +488,37 @@ function spawnDlvDapServerProcess(
 export function parseProgramArgSync(
 	launchAttachArgs: vscode.DebugConfiguration
 ): { program: string; dirname: string; programIsDirectory: boolean } {
-	const program = launchAttachArgs.program;
-	let programIsDirectory = false;
+	// attach request:
+	//   irrelevant
+	if (launchAttachArgs.request !== 'launch') return;
 
-	if (launchAttachArgs.mode === 'replay') {
-		// Skip program parsing on modes that do not require a program
-		return { program: '', dirname: '', programIsDirectory: programIsDirectory };
-	}
+	const mode = launchAttachArgs.mode || 'debug';
+	const program = launchAttachArgs.program;
 
 	if (!program) {
 		throw new Error('The program attribute is missing in the debug configuration in launch.json');
 	}
 
-	try {
-		programIsDirectory = fs.lstatSync(program).isDirectory();
-	} catch (e) {
-		// TODO(hyangah): why can't the program be a package name?
-		throw new Error('The program attribute must point to valid directory, .go file or executable.');
-	}
-	if (!programIsDirectory && launchAttachArgs.mode !== 'exec' && path.extname(program) !== '.go') {
-		throw new Error('The program attribute must be a directory or .go file in debug and test mode');
+	// debug, test, auto mode in launch request:
+	//   program ends with .go file -> file, otherwise -> programIsDirectory.
+	// exec mode
+	//   program should be executable.
+	// other modes:
+	//   not relevant
+	if (['debug', 'test', 'auto'].includes(mode)) {
+		// `auto` shouldn't happen other than in testing.
+		const ext = path.extname(program);
+		if (ext === '') {
+			// the last path element doesn't have . or the first char is .
+			// Treat this like a directory.
+			return { program, dirname: program, programIsDirectory: true };
+		}
+		if (ext === '.go') {
+			return { program, dirname: path.dirname(program), programIsDirectory: false };
+		} else {
+			throw new Error('The program attribute must be a directory or .go file in debug and test mode');
+		}
 	}
-	const dirname = programIsDirectory ? program : path.dirname(program);
-	return { program, dirname, programIsDirectory };
+	// Otherwise, let delve handle.
+	return { program, dirname: '', programIsDirectory: false };
 }

test/integration/goDebug.test.ts

@@ -2081,6 +2081,41 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 				await new Promise((resolve) => setTimeout(resolve, 2_000));
 			});
 		});
+
+		suite('substitutePath with symlink', () => {
+			let realPath: string;
+			let symlinkPath: string;
+
+			suiteSetup(() => {
+				realPath = copyDirectory('baseTest');
+				symlinkPath = path.join(tmpDir, 'symlinked');
+				fs.symlinkSync(realPath, symlinkPath, 'dir');
+			});
+			suiteTeardown(() => {
+				fs.unlinkSync(symlinkPath);
+				rmdirRecursive(realPath);
+			});
+			test('should stop on a breakpoint', async function () {
+				if (!isDlvDap) this.skip(); // BUG: the legacy adapter fails with 'breakpoint verification mismatch' error.
+				const FILE = path.join(symlinkPath, 'test.go');
+				const BREAKPOINT_LINE = 11;
+				const config = {
+					name: 'Launch',
+					type: 'go',
+					request: 'launch',
+					mode: 'debug',
+					program: FILE,
+					substitutePath: [
+						{
+							from: symlinkPath,
+							to: realPath
+						}
+					]
+				};
+				const debugConfig = await initializeDebugConfig(config);
+				await dc.hitBreakpoint(debugConfig, getBreakpointLocation(FILE, BREAKPOINT_LINE));
+			});
+		});
 	});
 
 	let testNumber = 0;
@@ -2110,7 +2145,12 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 		}
 		testNumber++;
 
-		const debugConfig = await debugConfigProvider.resolveDebugConfiguration(undefined, config);
+		let debugConfig = await debugConfigProvider.resolveDebugConfiguration(undefined, config);
+		debugConfig = await debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(
+			undefined,
+			debugConfig
+		);
+
 		if (isDlvDap) {
 			dlvDapAdapter = await DelveDAPDebugAdapterOnSocket.create(debugConfig);
 			const port = await dlvDapAdapter.serve();

test/integration/goDebugConfiguration.test.ts

@@ -452,23 +452,50 @@ suite('Debug Configuration Converts Relative Paths', () => {
 		};
 	}
 
-	test('resolve relative paths with workspace root in dlv-dap mode', () => {
+	test('resolve relative paths with workspace root in dlv-dap mode, exec mode does not set __buildDir', () => {
 		const config = debugConfig('dlv-dap');
+		config.mode = 'exec';
+		config.program = path.join('foo', 'bar.exe');
 		const workspaceFolder = {
 			uri: vscode.Uri.file(os.tmpdir()),
 			name: 'test',
 			index: 0
 		};
-		const { program, cwd, output } = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(
+		const { program, cwd, __buildDir } = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(
 			workspaceFolder,
 			config
 		);
 		assert.deepStrictEqual(
-			{ program, cwd, output },
+			{ program, cwd, __buildDir },
+			{
+				program: path.join(os.tmpdir(), 'foo', 'bar.exe'),
+				cwd: os.tmpdir(),
+				__buildDir: undefined
+			}
+		);
+	});
+
+	test('program and __buildDir are updated while resolving debug configuration in dlv-dap mode', () => {
+		const config = debugConfig('dlv-dap');
+		config.program = path.join('foo', 'bar', 'pkg');
+		const workspaceFolder = {
+			uri: vscode.Uri.file(os.tmpdir()),
+			name: 'test',
+			index: 0
+		};
+		const {
+			program,
+			cwd,
+			output,
+			__buildDir
+		} = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(workspaceFolder, config);
+		assert.deepStrictEqual(
+			{ program, cwd, output, __buildDir },
 			{
-				program: path.join(os.tmpdir(), 'foo/bar.go'),
+				program: '.',
 				cwd: os.tmpdir(),
-				output: path.join(os.tmpdir(), 'debug')
+				output: path.join(os.tmpdir(), 'debug'),
+				__buildDir: path.join(os.tmpdir(), 'foo', 'bar', 'pkg')
 			}
 		);
 	});
@@ -498,10 +525,23 @@ suite('Debug Configuration Converts Relative Paths', () => {
 		);
 	});
 
-	test('disallow relative paths with no workspace root', () => {
+	test('relative paths with no workspace root are not expanded', () => {
 		const config = debugConfig('dlv-dap');
-		const got = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(undefined, config);
-		assert.strictEqual(got, null);
+		const {
+			program,
+			cwd,
+			output,
+			__buildDir
+		} = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(undefined, config);
+		assert.deepStrictEqual(
+			{ program, cwd, output, __buildDir },
+			{
+				program: '.' + path.sep + 'bar.go',
+				cwd: '.',
+				output: 'debug',
+				__buildDir: 'foo'
+			}
+		);
 	});
 
 	test('do not affect relative paths (workspace) in legacy mode', () => {