Skip to content

Commit f122b06

Browse files
firelizzard18h9jiang
authored andcommitted
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 #1627. Change-Id: I9ad15752271c84fa069106c3a96206b6edcc3797 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/661195 kokoro-CI: kokoro <[email protected]> Reviewed-by: Hongxiang Jiang <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 6e371c5 commit f122b06

File tree

3 files changed

+109
-15
lines changed

3 files changed

+109
-15
lines changed

extension/src/goDebugConfiguration.ts

+85-15
Original file line numberDiff line numberDiff line change
@@ -488,24 +488,45 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
488488
debugConfiguration['env'] = Object.assign(goToolsEnvVars, fileEnvs, env);
489489
debugConfiguration['envFile'] = undefined; // unset, since we already processed.
490490

491-
const entriesWithRelativePaths = ['cwd', 'output', 'program'].filter(
492-
(attr) => debugConfiguration[attr] && !path.isAbsolute(debugConfiguration[attr])
493-
);
494491
if (debugAdapter === 'dlv-dap') {
495-
// 1. Relative paths -> absolute paths
496-
if (entriesWithRelativePaths.length > 0) {
497-
const workspaceRoot = folder?.uri.fsPath;
498-
if (workspaceRoot) {
499-
entriesWithRelativePaths.forEach((attr) => {
500-
debugConfiguration[attr] = path.join(workspaceRoot, debugConfiguration[attr]);
501-
});
502-
} else {
492+
// If the user provides a relative path outside of a workspace
493+
// folder, warn them, but only once.
494+
let didWarn = false;
495+
const makeRelative = (s: string) => {
496+
if (folder) {
497+
return path.join(folder.uri.fsPath, s);
498+
}
499+
500+
if (!didWarn) {
501+
didWarn = true;
503502
this.showWarning(
504503
'relativePathsWithoutWorkspaceFolder',
505504
'Behavior when using relative paths without a workspace folder for `cwd`, `program`, or `output` is undefined.'
506505
);
507506
}
508-
}
507+
508+
return s;
509+
};
510+
511+
// 1. Relative paths -> absolute paths
512+
['cwd', 'output', 'program'].forEach((attr) => {
513+
const value = debugConfiguration[attr];
514+
if (!value || path.isAbsolute(value)) return;
515+
516+
// Make the path relative (the program attribute needs
517+
// additional checks).
518+
if (attr !== 'program') {
519+
debugConfiguration[attr] = makeRelative(value);
520+
return;
521+
}
522+
523+
// If the program could be a package URL, don't alter it unless
524+
// we can confirm that it is also a file path.
525+
if (!isPackageUrl(value) || isFsPath(value, folder?.uri.fsPath)) {
526+
debugConfiguration[attr] = makeRelative(value);
527+
}
528+
});
529+
509530
// 2. For launch debug/test modes that builds the debug target,
510531
// delve needs to be launched from the right directory (inside the main module of the target).
511532
// Compute the launch dir heuristically, and translate the dirname in program to a path relative to buildDir.
@@ -518,7 +539,8 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
518539
// with a relative path. (https://github.com/golang/vscode-go/issues/1713)
519540
// parseDebugProgramArgSync will throw an error if `program` is invalid.
520541
const { program, dirname, programIsDirectory } = parseDebugProgramArgSync(
521-
debugConfiguration['program']
542+
debugConfiguration['program'],
543+
folder?.uri.fsPath
522544
);
523545
if (
524546
dirname &&
@@ -583,11 +605,15 @@ export async function maybeJoinFlags(dlvToolPath: string, flags: string | string
583605

584606
// parseDebugProgramArgSync parses program arg of debug/auto/test launch requests.
585607
export function parseDebugProgramArgSync(
586-
program: string
587-
): { program: string; dirname: string; programIsDirectory: boolean } {
608+
program: string,
609+
cwd?: string
610+
): { program: string; dirname?: string; programIsDirectory: boolean } {
588611
if (!program) {
589612
throw new Error('The program attribute is missing in the debug configuration in launch.json');
590613
}
614+
if (isPackageUrl(program) && !isFsPath(program, cwd)) {
615+
return { program, programIsDirectory: true };
616+
}
591617
try {
592618
const pstats = lstatSync(program);
593619
if (pstats.isDirectory()) {
@@ -606,3 +632,47 @@ export function parseDebugProgramArgSync(
606632
`The program attribute '${program}' must be a valid directory or .go file in debug/test/auto modes.`
607633
);
608634
}
635+
636+
/**
637+
* Returns true if the given string is an absolute path or refers to a file or
638+
* directory in the current working directory, or `wd` if specified.
639+
* @param s The prospective file or directory path.
640+
* @param wd The working directory to use instead of `process.cwd()`.
641+
*/
642+
function isFsPath(s: string, wd?: string) {
643+
// If it's absolute, it's a path.
644+
if (path.isAbsolute(s)) return;
645+
646+
// If the caller specifies a working directory, make the prospective path
647+
// absolute.
648+
if (wd) s = path.join(wd, s);
649+
650+
try {
651+
// If lstat doesn't throw, the path refers to a file or directory.
652+
lstatSync(s);
653+
return true;
654+
} catch (error) {
655+
// ENOENT means nothing exists at the specified path.
656+
if (error instanceof Error && 'code' in error && (error as NodeJS.ErrnoException).code === 'ENOENT') {
657+
return false;
658+
}
659+
660+
// If the error is something unexpected, rethrow it.
661+
throw error;
662+
}
663+
}
664+
665+
function isPackageUrl(s: string) {
666+
// If the string does not contain `/` and ends with .go it is most likely
667+
// intended to be a file path. If the file exists it would be caught by
668+
// isFsPath, but otherwise "the file doesn't exist" is much less confusing
669+
// than "the package doesn't exist" if the user is trying to execute a test
670+
// file and got the path wrong.
671+
if (s.match(/^[^/]*\.go$/)) {
672+
return s;
673+
}
674+
675+
// If the string starts with domain.tld/ and it doesn't reference a file,
676+
// assume it's a package URL
677+
return s.match(/^[a-zA-Z0-9][a-zA-Z0-9-]{0,61}[a-zA-Z0-9](\.[a-zA-Z]{2,})+\//);
678+
}

extension/test/integration/goDebug.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -2051,6 +2051,7 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean, withConsole?: string) =>
20512051
// second test has a chance to run it.
20522052
if (!config['output'] && ['debug', 'auto', 'test'].includes(config['mode'])) {
20532053
const dir = parseDebugProgramArgSync(config['program']).dirname;
2054+
if (!dir) throw new Error('Debug configuration does not define an output directory');
20542055
config['output'] = path.join(dir, `__debug_bin_${testNumber}`);
20552056
}
20562057
testNumber++;

extension/test/integration/goDebugConfiguration.test.ts

+23
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,29 @@ suite('Debug Configuration Converts Relative Paths', () => {
717717
);
718718
});
719719

720+
test('allow package path in dlv-dap mode', () => {
721+
const config = debugConfig('dlv-dap');
722+
config.program = 'example.com/foo/bar';
723+
724+
const workspaceFolder = {
725+
uri: vscode.Uri.file(workspaceDir),
726+
name: 'test',
727+
index: 0
728+
};
729+
const { program, cwd, __buildDir } = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(
730+
workspaceFolder,
731+
config
732+
)!;
733+
assert.deepStrictEqual(
734+
{ program, cwd, __buildDir },
735+
{
736+
program: 'example.com/foo/bar',
737+
cwd: workspaceDir,
738+
__buildDir: undefined
739+
}
740+
);
741+
});
742+
720743
test('program and __buildDir are updated while resolving debug configuration in dlv-dap mode', () => {
721744
createDirRecursively(path.join(workspaceDir, 'foo', 'bar', 'pkg'));
722745

0 commit comments

Comments
 (0)