extension/src/goDebugConfiguration: fix debugging package urls A package URL such as example.com/foo/bar is a valid program for dlv debug. This updates goDebugConfiguration's handling of the program attribute to prevent it from A) treating it as a path (prepending the workspace folder path) and B) throwing an error due to it not being a valid filesystem path. Updates golang/vscode-go#1627. Change-Id: I9ad15752271c84fa069106c3a96206b6edcc3797 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/661195 kokoro-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Hongxiang Jiang <hxjiang@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/extension/src/goDebugConfiguration.ts b/extension/src/goDebugConfiguration.ts index 723a9ad..94c7e12 100644 --- a/extension/src/goDebugConfiguration.ts +++ b/extension/src/goDebugConfiguration.ts
@@ -488,24 +488,45 @@ debugConfiguration['env'] = Object.assign(goToolsEnvVars, fileEnvs, env); debugConfiguration['envFile'] = undefined; // unset, since we already processed. - const entriesWithRelativePaths = ['cwd', 'output', 'program'].filter( - (attr) => debugConfiguration[attr] && !path.isAbsolute(debugConfiguration[attr]) - ); if (debugAdapter === 'dlv-dap') { - // 1. 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 { + // If the user provides a relative path outside of a workspace + // folder, warn them, but only once. + let didWarn = false; + const makeRelative = (s: string) => { + if (folder) { + return path.join(folder.uri.fsPath, s); + } + + if (!didWarn) { + didWarn = true; this.showWarning( 'relativePathsWithoutWorkspaceFolder', 'Behavior when using relative paths without a workspace folder for `cwd`, `program`, or `output` is undefined.' ); } - } + + return s; + }; + + // 1. Relative paths -> absolute paths + ['cwd', 'output', 'program'].forEach((attr) => { + const value = debugConfiguration[attr]; + if (!value || path.isAbsolute(value)) return; + + // Make the path relative (the program attribute needs + // additional checks). + if (attr !== 'program') { + debugConfiguration[attr] = makeRelative(value); + return; + } + + // If the program could be a package URL, don't alter it unless + // we can confirm that it is also a file path. + if (!isPackageUrl(value) || isFsPath(value, folder?.uri.fsPath)) { + debugConfiguration[attr] = makeRelative(value); + } + }); + // 2. For launch debug/test modes that builds the debug target, // delve needs to be launched from the right directory (inside the main module of the target). // Compute the launch dir heuristically, and translate the dirname in program to a path relative to buildDir. @@ -518,7 +539,8 @@ // with a relative path. (https://github.com/golang/vscode-go/issues/1713) // parseDebugProgramArgSync will throw an error if `program` is invalid. const { program, dirname, programIsDirectory } = parseDebugProgramArgSync( - debugConfiguration['program'] + debugConfiguration['program'], + folder?.uri.fsPath ); if ( dirname && @@ -583,11 +605,15 @@ // parseDebugProgramArgSync parses program arg of debug/auto/test launch requests. export function parseDebugProgramArgSync( - program: string -): { program: string; dirname: string; programIsDirectory: boolean } { + program: string, + cwd?: string +): { program: string; dirname?: string; programIsDirectory: boolean } { if (!program) { throw new Error('The program attribute is missing in the debug configuration in launch.json'); } + if (isPackageUrl(program) && !isFsPath(program, cwd)) { + return { program, programIsDirectory: true }; + } try { const pstats = lstatSync(program); if (pstats.isDirectory()) { @@ -606,3 +632,47 @@ `The program attribute '${program}' must be a valid directory or .go file in debug/test/auto modes.` ); } + +/** + * Returns true if the given string is an absolute path or refers to a file or + * directory in the current working directory, or `wd` if specified. + * @param s The prospective file or directory path. + * @param wd The working directory to use instead of `process.cwd()`. + */ +function isFsPath(s: string, wd?: string) { + // If it's absolute, it's a path. + if (path.isAbsolute(s)) return; + + // If the caller specifies a working directory, make the prospective path + // absolute. + if (wd) s = path.join(wd, s); + + try { + // If lstat doesn't throw, the path refers to a file or directory. + lstatSync(s); + return true; + } catch (error) { + // ENOENT means nothing exists at the specified path. + if (error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code === 'ENOENT') { + return false; + } + + // If the error is something unexpected, rethrow it. + throw error; + } +} + +function isPackageUrl(s: string) { + // If the string does not contain `/` and ends with .go it is most likely + // intended to be a file path. If the file exists it would be caught by + // isFsPath, but otherwise "the file doesn't exist" is much less confusing + // than "the package doesn't exist" if the user is trying to execute a test + // file and got the path wrong. + if (s.match(/^[^/]*\.go$/)) { + return s; + } + + // If the string starts with domain.tld/ and it doesn't reference a file, + // assume it's a package URL + return s.match(/^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9](\.[a-zA-Z]{2,})+\//); +}
diff --git a/extension/test/integration/goDebug.test.ts b/extension/test/integration/goDebug.test.ts index 8db072b..cd0ef62 100644 --- a/extension/test/integration/goDebug.test.ts +++ b/extension/test/integration/goDebug.test.ts
@@ -2051,6 +2051,7 @@ // second test has a chance to run it. if (!config['output'] && ['debug', 'auto', 'test'].includes(config['mode'])) { const dir = parseDebugProgramArgSync(config['program']).dirname; + if (!dir) throw new Error('Debug configuration does not define an output directory'); config['output'] = path.join(dir, `__debug_bin_${testNumber}`); } testNumber++;
diff --git a/extension/test/integration/goDebugConfiguration.test.ts b/extension/test/integration/goDebugConfiguration.test.ts index e2e21a1..877e72c 100644 --- a/extension/test/integration/goDebugConfiguration.test.ts +++ b/extension/test/integration/goDebugConfiguration.test.ts
@@ -717,6 +717,29 @@ ); }); + test('allow package path in dlv-dap mode', () => { + const config = debugConfig('dlv-dap'); + config.program = 'example.com/foo/bar'; + + const workspaceFolder = { + uri: vscode.Uri.file(workspaceDir), + name: 'test', + index: 0 + }; + const { program, cwd, __buildDir } = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables( + workspaceFolder, + config + )!; + assert.deepStrictEqual( + { program, cwd, __buildDir }, + { + program: 'example.com/foo/bar', + 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'));