src/goDebugConfiguration: handle directory with '.' in its name
parseProgramSync assumed 'program' is a directory if it does not
contain '.'. That was a hack to avoid lstatSync and implement/test
parseProgramSync without requiring the program to be a valid,
existing file or directory. However, this does not work for users
whose folder name contains '.'. (e.g., golang/vscode-go#1826)
Readd the lstat check to examine whether the program is a directory.
This required to change tests to create dummy files and folders.
While we are here, rework parseProgramSync.
- move parseProgramSync to goDebugConfiguration, the only call
site of this function.
- rename it to parseDebugProgramArgSync and call it only for
debug/test/auto launch requests.
- call parseDebugProgramArgSync for externally launched debug
adapter settings too because it does `program` validation.
But in that case, we won't mess with `program` or `__buildDir`.
- previously we accepted cases for which parseDebugProgramArgSync
throws an error (including files other than .go) because the
hack is not perfect and delve dap and go build may know better
on how to handle that. But now build errors can be hidden in DEBUG
CONSOLE so they fail to catch users' attention
(e.g., golang/vscode-go#1826)
So, let's stop and don't proceed if parseDebugProgramArgSync fails.
Updates golang/vscode-go#1769
Updates golang/vscode-go#1826
Change-Id: If413a0ca0c5814bcaa45ea69ceff8bcfde558370
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/353990
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
diff --git a/src/goDebugConfiguration.ts b/src/goDebugConfiguration.ts
index d4352c7..c21ec0a 100644
--- a/src/goDebugConfiguration.ts
+++ b/src/goDebugConfiguration.ts
@@ -7,10 +7,10 @@
'use strict';
+import { lstatSync } from 'fs';
import path = require('path');
import vscode = require('vscode');
import { getGoConfig } from './config';
-import { parseProgramArgSync } from './goDebugFactory';
import { toolExecutionEnvironment } from './goEnv';
import {
declinedToolInstall,
@@ -20,8 +20,8 @@
shouldUpdateTool
} from './goInstallTools';
import { packagePathToGoModPathMap } from './goModules';
-import { getTool, getToolAtVersion } from './goTools';
-import { pickGoProcess, pickProcess, pickProcessByName } from './pickProcess';
+import { getToolAtVersion } from './goTools';
+import { pickProcess, pickProcessByName } from './pickProcess';
import { getFromGlobalState, updateGlobalState } from './stateUtils';
import { getBinPath, getGoVersion } from './util';
import { parseEnvFiles } from './utils/envUtils';
@@ -413,28 +413,26 @@
// Compute the launch dir heuristically, and translate the dirname in program to a path relative to buildDir.
// We skip this step when working with externally launched debug adapter
// because we do not control the adapter's launch process.
- if (
- debugConfiguration.request === 'launch' &&
- // Presence of the following attributes indicates externally launched debug adapter.
- !debugConfiguration.port &&
- !debugConfiguration.host &&
- !debugConfiguration.debugServer
- ) {
+ 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.
+ // parseDebugProgramArgSync will throw an error if `program` is invalid.
+ const { program, dirname, programIsDirectory } = parseDebugProgramArgSync(
+ debugConfiguration['program']
+ );
+ if (
+ dirname &&
+ // Presence of the following attributes indicates externally launched debug adapter.
+ // Don't mess with 'program' if the debug adapter was launched externally.
+ !debugConfiguration.port &&
+ !debugConfiguration.debugServer
+ ) {
+ debugConfiguration['__buildDir'] = dirname;
+ debugConfiguration['program'] = programIsDirectory
+ ? '.'
+ : '.' + path.sep + path.relative(dirname, program);
}
}
}
@@ -463,3 +461,29 @@
});
}
}
+
+// parseDebugProgramArgSync parses program arg of debug/auto/test launch requests.
+export function parseDebugProgramArgSync(
+ program: string
+): { program: string; dirname: string; programIsDirectory: boolean } {
+ if (!program) {
+ throw new Error('The program attribute is missing in the debug configuration in launch.json');
+ }
+ try {
+ const pstats = lstatSync(program);
+ if (pstats.isDirectory()) {
+ return { program, dirname: program, programIsDirectory: true };
+ }
+ const ext = path.extname(program);
+ if (ext === '.go') {
+ // TODO(hyangah): .s?
+ return { program, dirname: path.dirname(program), programIsDirectory: false };
+ }
+ } catch (e) {
+ console.log(`parseDebugProgramArgSync failed: ${e}`);
+ }
+ // shouldn't reach here if program was a valid directory or .go file.
+ throw new Error(
+ `The program attribute '${program}' must be a valid directory or .go file in debug/test/auto modes.`
+ );
+}
diff --git a/src/goDebugFactory.ts b/src/goDebugFactory.ts
index 27964cf..b58f665 100644
--- a/src/goDebugFactory.ts
+++ b/src/goDebugFactory.ts
@@ -494,43 +494,3 @@
});
});
}
-
-export function parseProgramArgSync(
- launchAttachArgs: vscode.DebugConfiguration
-): { program: string; dirname: string; programIsDirectory: boolean } {
- // attach request:
- // irrelevant
- if (launchAttachArgs.request !== 'launch') return;
-
- 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');
- }
-
- // 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 '${program}' must be a directory or .go file in debug and test modes.`
- );
- }
- }
- // Otherwise, let delve handle.
- return { program, dirname: '', programIsDirectory: false };
-}
diff --git a/test/integration/goDebug.test.ts b/test/integration/goDebug.test.ts
index 8ebb288..24a53cc 100644
--- a/test/integration/goDebug.test.ts
+++ b/test/integration/goDebug.test.ts
@@ -26,12 +26,11 @@
RemoteSourcesAndPackages
} from '../../src/debugAdapter/goDebug';
import * as extConfig from '../../src/config';
-import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
+import { GoDebugConfigurationProvider, parseDebugProgramArgSync } from '../../src/goDebugConfiguration';
import { getBinPath, rmdirRecursive } from '../../src/util';
import { killProcessTree } from '../../src/utils/processUtils';
import getPort = require('get-port');
import util = require('util');
-import { parseProgramArgSync } from '../../src/goDebugFactory';
import { TimestampedLogger } from '../../src/goLogging';
// For debugging test and streaming the trace instead of buffering, set this.
@@ -2207,8 +2206,8 @@
// that the second test could build the binary, and then the
// first test could delete that binary during cleanup before the
// second test has a chance to run it.
- if (!config['output'] && config['mode'] !== 'remote') {
- const dir = parseProgramArgSync(config).dirname;
+ if (!config['output'] && ['debug', 'auto', 'test'].includes(config['mode'])) {
+ const dir = parseDebugProgramArgSync(config['program']).dirname;
config['output'] = path.join(dir, `__debug_bin_${testNumber}`);
}
testNumber++;
diff --git a/test/integration/goDebugConfiguration.test.ts b/test/integration/goDebugConfiguration.test.ts
index 4162436..d874546 100644
--- a/test/integration/goDebugConfiguration.test.ts
+++ b/test/integration/goDebugConfiguration.test.ts
@@ -14,16 +14,17 @@
import goEnv = require('../../src/goEnv');
import { isInPreviewMode } from '../../src/goLanguageServer';
import { MockCfg } from '../mocks/MockCfg';
+import { fileURLToPath } from 'url';
suite('Debug Environment Variable Merge Test', () => {
const debugConfigProvider = new GoDebugConfigurationProvider();
+ // Set up the test fixtures.
+ const fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'testdata');
+ const filePath = path.join(fixtureSourcePath, 'baseTest', 'test.go');
+
suiteSetup(async () => {
await updateGoVarsFromConfig();
-
- // Set up the test fixtures.
- const fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'testdata');
- const filePath = path.join(fixtureSourcePath, 'baseTest', 'test.go');
await vscode.workspace.openTextDocument(vscode.Uri.file(filePath));
});
@@ -59,7 +60,8 @@
request: 'launch',
env: input.env,
envFile: input.envFile,
- debugAdapter: input.debugAdapter
+ debugAdapter: input.debugAdapter,
+ program: filePath
});
const actual = config.env;
@@ -497,9 +499,106 @@
});
});
+function writeEmptyFile(filename: string) {
+ const dir = path.dirname(filename);
+ if (!fs.existsSync(dir)) {
+ createDirRecursively(dir);
+ }
+ try {
+ fs.writeFileSync(filename, '');
+ } catch (e) {
+ console.log(`failed to write a file: ${e}`);
+ }
+}
+
+function createDirRecursively(dir: string) {
+ try {
+ fs.mkdirSync(dir, { recursive: true });
+ } catch (e) {
+ console.log(`failed to create directory: ${e}`);
+ }
+}
+
+suite('Debug Configuration With Invalid Program', () => {
+ const debugConfigProvider = new GoDebugConfigurationProvider();
+
+ let workspaceDir = '';
+ setup(() => {
+ workspaceDir = fs.mkdtempSync(path.join(os.tmpdir(), 'godebugrelpath_test'));
+ });
+
+ teardown(() => {
+ rmdirRecursive(workspaceDir);
+ });
+
+ function debugConfig(adapter: string) {
+ return {
+ name: 'Launch',
+ type: 'go',
+ request: 'launch',
+ mode: 'auto',
+ debugAdapter: adapter,
+ program: path.join('foo', 'bar.go'),
+ cwd: '.',
+ output: 'debug'
+ };
+ }
+
+ test('empty, undefined program is an error', () => {
+ const config = debugConfig('dlv-dap');
+ config.program = '';
+
+ const workspaceFolder = {
+ uri: vscode.Uri.file(workspaceDir),
+ name: 'test',
+ index: 0
+ };
+ assert.throws(() => {
+ debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(workspaceFolder, config);
+ }, /The program attribute is missing/);
+ });
+
+ test('non-existing file/directory is an error', () => {
+ const config = debugConfig('dlv-dap');
+ config.program = '/notexists';
+
+ const workspaceFolder = {
+ uri: vscode.Uri.file(workspaceDir),
+ name: 'test',
+ index: 0
+ };
+ assert.throws(() => {
+ debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(workspaceFolder, config);
+ }, /The program attribute.* must be a valid directory or .go file/);
+ });
+
+ test('files other than .go file with debug/test/auto mode is an error', () => {
+ writeEmptyFile(path.join(workspaceDir, 'foo', 'bar.test'));
+ const config = debugConfig('dlv-dap');
+ config.program = path.join(workspaceDir, 'foo', 'bar.test');
+ const workspaceFolder = {
+ uri: vscode.Uri.file(workspaceDir),
+ name: 'test',
+ index: 0
+ };
+ assert.throws(() => {
+ debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(workspaceFolder, config);
+ }, /The program attribute.* must be a valid directory or .go file/);
+ });
+});
+
suite('Debug Configuration Converts Relative Paths', () => {
const debugConfigProvider = new GoDebugConfigurationProvider();
+ let workspaceDir = '';
+ setup(() => {
+ workspaceDir = fs.mkdtempSync(path.join(os.tmpdir(), 'godebugrelpath_test'));
+ });
+
+ teardown(() => {
+ rmdirRecursive(workspaceDir);
+ });
+
function debugConfig(adapter: string) {
return {
name: 'Launch',
@@ -514,11 +613,14 @@
}
test('resolve relative paths with workspace root in dlv-dap mode, exec mode does not set __buildDir', () => {
+ writeEmptyFile(path.join(workspaceDir, 'foo', 'bar.exe'));
+
const config = debugConfig('dlv-dap');
config.mode = 'exec';
config.program = path.join('foo', 'bar.exe');
+
const workspaceFolder = {
- uri: vscode.Uri.file(os.tmpdir()),
+ uri: vscode.Uri.file(workspaceDir),
name: 'test',
index: 0
};
@@ -529,18 +631,20 @@
assert.deepStrictEqual(
{ program, cwd, __buildDir },
{
- program: path.join(os.tmpdir(), 'foo', 'bar.exe'),
- cwd: os.tmpdir(),
+ program: path.join(workspaceDir, 'foo', 'bar.exe'),
+ cwd: workspaceDir,
__buildDir: undefined
}
);
});
test('program and __buildDir are updated while resolving debug configuration in dlv-dap mode', () => {
+ createDirRecursively(path.join(workspaceDir, 'foo', 'bar', 'pkg'));
+
const config = debugConfig('dlv-dap');
config.program = path.join('foo', 'bar', 'pkg');
const workspaceFolder = {
- uri: vscode.Uri.file(os.tmpdir()),
+ uri: vscode.Uri.file(workspaceDir),
name: 'test',
index: 0
};
@@ -554,19 +658,21 @@
{ program, cwd, output, __buildDir },
{
program: '.',
- cwd: os.tmpdir(),
- output: path.join(os.tmpdir(), 'debug'),
- __buildDir: path.join(os.tmpdir(), 'foo', 'bar', 'pkg')
+ cwd: workspaceDir,
+ output: path.join(workspaceDir, 'debug'),
+ __buildDir: path.join(workspaceDir, 'foo', 'bar', 'pkg')
}
);
});
test('program and __buildDir are not updated when working with externally launched adapters', () => {
+ createDirRecursively(path.join(workspaceDir, 'foo', 'bar', 'pkg'));
+
const config: vscode.DebugConfiguration = debugConfig('dlv-dap');
config.program = path.join('foo', 'bar', 'pkg');
config.port = 12345;
const workspaceFolder = {
- uri: vscode.Uri.file(os.tmpdir()),
+ uri: vscode.Uri.file(workspaceDir),
name: 'test',
index: 0
};
@@ -577,19 +683,21 @@
assert.deepStrictEqual(
{ program, cwd, __buildDir },
{
- program: path.join(os.tmpdir(), 'foo', 'bar', 'pkg'),
- cwd: os.tmpdir(),
+ program: path.join(workspaceDir, 'foo', 'bar', 'pkg'),
+ cwd: workspaceDir,
__buildDir: undefined
}
);
});
test('program and __buildDir are not updated when working with externally launched adapters (debugServer)', () => {
+ createDirRecursively(path.join(workspaceDir, 'foo', 'bar', 'pkg'));
+
const config: vscode.DebugConfiguration = debugConfig('dlv-dap');
config.program = path.join('foo', 'bar', 'pkg');
config.debugServer = 4777;
const workspaceFolder = {
- uri: vscode.Uri.file(os.tmpdir()),
+ uri: vscode.Uri.file(workspaceDir),
name: 'test',
index: 0
};
@@ -600,40 +708,70 @@
assert.deepStrictEqual(
{ program, cwd, __buildDir },
{
- program: path.join(os.tmpdir(), 'foo', 'bar', 'pkg'),
- cwd: os.tmpdir(),
+ program: path.join(workspaceDir, 'foo', 'bar', 'pkg'),
+ cwd: workspaceDir,
__buildDir: undefined
}
);
});
- test('empty, undefined paths are not affected', () => {
- const config = debugConfig('dlv-dap');
- config.program = undefined;
- config.cwd = '';
- delete config.output;
+ test('directory as program still works when directory name contains .', () => {
+ createDirRecursively(path.join(workspaceDir, 'foo.test'));
+ const config: vscode.DebugConfiguration = debugConfig('dlv-dap');
+ config.program = 'foo.test';
const workspaceFolder = {
- uri: vscode.Uri.file(os.tmpdir()),
+ uri: vscode.Uri.file(workspaceDir),
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: undefined,
+ program: '.',
+ cwd: workspaceDir,
+ __buildDir: path.join(workspaceDir, 'foo.test')
+ }
+ );
+ });
+
+ test('empty, undefined paths are not affected', () => {
+ writeEmptyFile(path.join(workspaceDir, 'bar_test.go'));
+
+ const config = debugConfig('dlv-dap');
+ config.program = 'bar_test.go';
+ config.cwd = '';
+ delete config.output;
+
+ const workspaceFolder = {
+ uri: vscode.Uri.file(workspaceDir),
+ name: 'test',
+ index: 0
+ };
+ const {
+ program,
+ cwd,
+ output,
+ __buildDir
+ } = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(workspaceFolder, config);
+ assert.deepStrictEqual(
+ { program, cwd, output, __buildDir },
+ {
+ program: '.' + path.sep + 'bar_test.go',
cwd: '',
- output: undefined
+ output: undefined,
+ __buildDir: workspaceDir
}
);
});
test('relative paths with no workspace root are not expanded', () => {
const config = debugConfig('dlv-dap');
+ config.program = '.'; // the program must be a valid directory or .go file.
const {
program,
cwd,
@@ -643,18 +781,20 @@
assert.deepStrictEqual(
{ program, cwd, output, __buildDir },
{
- program: '.' + path.sep + 'bar.go',
+ program: '.',
cwd: '.',
output: 'debug',
- __buildDir: 'foo'
+ __buildDir: '.'
}
);
});
test('do not affect relative paths (workspace) in legacy mode', () => {
+ writeEmptyFile(path.join(workspaceDir, 'foo', 'bar.go'));
+
const config = debugConfig('legacy');
const workspaceFolder = {
- uri: vscode.Uri.file(os.tmpdir()),
+ uri: vscode.Uri.file(workspaceDir),
name: 'test',
index: 0
};
@@ -674,6 +814,7 @@
test('do not affect relative paths (no workspace) in legacy mode', () => {
const config = debugConfig('legacy');
+ config.program = '.'; // program must be a valid directory or .go file.
const { program, cwd, output } = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(
undefined,
config
@@ -681,7 +822,7 @@
assert.deepStrictEqual(
{ program, cwd, output },
{
- program: path.join('foo', 'bar.go'),
+ program: '.',
cwd: '.',
output: 'debug'
}
