src/goDebugConfiguration: do not merge toolExecutionEnvironment to 'env'
toolExecutionEnvironment is populated based on settings.json. The extension
merged these env vars into 'env' automatically in order to make the separate
legacy debug adapter process use the same env vars when invoking `dlv`.
In dlv-dap mode, the extension host spawns dlv processes and can control
the environment variables easily. Instead of mutating the launch parameter's
env property by merging the toolExecutionEnvironment, this CL uses the
env vars only when spawning the dlv process.
This makes the debug adapter trace's launch message logging more compact
and easier to share/inspect the request.
Change-Id: I352528e4907e9b62e79c0f2b4d79f97fcd32c5dc
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/349749
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
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 af5919e..273c92f 100644
--- a/src/goDebugConfiguration.ts
+++ b/src/goDebugConfiguration.ts
@@ -366,12 +366,24 @@
debugConfiguration: vscode.DebugConfiguration,
token?: vscode.CancellationToken
): vscode.DebugConfiguration {
- // Reads debugConfiguration.envFile and
- // combines the environment variables from all the env files and
- // debugConfiguration.env, on top of the tools execution environment variables.
- // It also unsets 'envFile' from the user-suppled debugConfiguration
+ const debugAdapter = debugConfiguration['debugAdapter'];
+ if (debugAdapter === '') {
+ return null;
+ }
+
+ // Read debugConfiguration.envFile and
+ // combine the environment variables from all the env files and
+ // debugConfiguration.env.
+ // We also unset 'envFile' from the user-suppled debugConfiguration
// because it is already applied.
- const goToolsEnvVars = toolExecutionEnvironment(folder?.uri); // also includes GOPATH: getCurrentGoPath().
+ //
+ // For legacy mode, we merge the environment variables on top of
+ // the tools execution environment variables and update the debugConfiguration
+ // because VS Code directly handles launch of the legacy debug adapter.
+ // For dlv-dap mode, we do not merge the tools execution environment
+ // variables here to reduce the number of environment variables passed
+ // as launch/attach parameters.
+ const goToolsEnvVars = debugAdapter === 'legacy' ? toolExecutionEnvironment(folder?.uri) : {};
const fileEnvs = parseEnvFiles(debugConfiguration['envFile']);
const env = debugConfiguration['env'] || {};
@@ -381,7 +393,7 @@
const entriesWithRelativePaths = ['cwd', 'output', 'program'].filter(
(attr) => debugConfiguration[attr] && !path.isAbsolute(debugConfiguration[attr])
);
- if (debugConfiguration['debugAdapter'] === 'dlv-dap') {
+ if (debugAdapter === 'dlv-dap') {
// relative paths -> absolute paths
if (entriesWithRelativePaths.length > 0) {
const workspaceRoot = folder?.uri.fsPath;
diff --git a/src/goDebugFactory.ts b/src/goDebugFactory.ts
index c1ecc45..cf9cea7 100644
--- a/src/goDebugFactory.ts
+++ b/src/goDebugFactory.ts
@@ -16,6 +16,7 @@
import { Logger, TimestampedLogger } from './goLogging';
import { DebugProtocol } from 'vscode-debugprotocol';
import { getWorkspaceFolderPath } from './util';
+import { toolExecutionEnvironment } from './goEnv';
export class GoDebugAdapterDescriptorFactory implements vscode.DebugAdapterDescriptorFactory {
constructor(private outputChannel?: vscode.OutputChannel) {}
@@ -345,7 +346,9 @@
logConsole: (msg: string) => void
): Promise<ChildProcess> {
const launchArgsEnv = launchAttachArgs.env || {};
- const env = Object.assign({}, process.env, launchArgsEnv);
+ const goToolsEnvVars = toolExecutionEnvironment();
+ // launchArgsEnv is user-requested env vars (envFiles + env).
+ const env = Object.assign(goToolsEnvVars, launchArgsEnv);
const dlvPath = launchAttachArgs.dlvToolPath ?? getTool('dlv-dap');
diff --git a/test/integration/goDebug.test.ts b/test/integration/goDebug.test.ts
index 467588a..8ebb288 100644
--- a/test/integration/goDebug.test.ts
+++ b/test/integration/goDebug.test.ts
@@ -13,6 +13,7 @@
import * as path from 'path';
import * as sinon from 'sinon';
import * as proxy from '../../src/goDebugFactory';
+import * as vscode from 'vscode';
import { DebugConfiguration, DebugProtocolMessage } from 'vscode';
import { DebugClient } from 'vscode-debugadapter-testsupport';
import { ILocation } from 'vscode-debugadapter-testsupport/lib/debugClient';
@@ -24,6 +25,7 @@
PackageBuildInfo,
RemoteSourcesAndPackages
} from '../../src/debugAdapter/goDebug';
+import * as extConfig from '../../src/config';
import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
import { getBinPath, rmdirRecursive } from '../../src/util';
import { killProcessTree } from '../../src/utils/processUtils';
@@ -552,6 +554,70 @@
});
});
+ suite('env', () => {
+ let sandbox: sinon.SinonSandbox;
+
+ setup(() => {
+ sandbox = sinon.createSandbox();
+ });
+ teardown(async () => sandbox.restore());
+
+ test('env var from go.toolsEnvVars is respected', async () => {
+ const PROGRAM = path.join(DATA_ROOT, 'envTest');
+ const FILE = path.join(PROGRAM, 'main.go');
+ const BREAKPOINT_LINE = 10;
+
+ const goConfig = Object.create(vscode.workspace.getConfiguration('go'), {
+ toolsEnvVars: {
+ value: { FOO: 'BAR' }
+ }
+ });
+ const configStub = sandbox.stub(extConfig, 'getGoConfig').returns(goConfig);
+
+ const config = {
+ name: 'Launch',
+ type: 'go',
+ request: 'launch',
+ mode: 'debug',
+ program: PROGRAM,
+ args: ['FOO']
+ };
+ const debugConfig = await initializeDebugConfig(config);
+ await dc.hitBreakpoint(debugConfig, getBreakpointLocation(FILE, BREAKPOINT_LINE));
+ await assertLocalVariableValue('v', '"BAR"');
+
+ await dc.continueRequest({ threadId: 1 }); // continue until completion for cleanup.
+ });
+
+ test('env var from launch config is respected', async () => {
+ const PROGRAM = path.join(DATA_ROOT, 'envTest');
+ const FILE = path.join(PROGRAM, 'main.go');
+ const BREAKPOINT_LINE = 10;
+
+ const goConfig = Object.create(vscode.workspace.getConfiguration('go'), {
+ toolsEnvVars: {
+ value: { FOO: 'BAR' }
+ }
+ });
+ const configStub = sandbox.stub(extConfig, 'getGoConfig').returns(goConfig);
+
+ const config = {
+ name: 'Launch',
+ type: 'go',
+ request: 'launch',
+ mode: 'debug',
+ program: PROGRAM,
+ args: ['FOO'],
+ env: { FOO: 'BAZ' }
+ };
+ const debugConfig = await initializeDebugConfig(config);
+ await dc.hitBreakpoint(debugConfig, getBreakpointLocation(FILE, BREAKPOINT_LINE));
+ await assertLocalVariableValue('v', '"BAZ"');
+
+ await dc.continueRequest({ threadId: 1 }); // continue until completion for cleanup.
+ });
+ });
+
suite('launch', () => {
test('should run program to the end', async () => {
const PROGRAM = path.join(DATA_ROOT, 'baseTest');
diff --git a/test/integration/goDebugConfiguration.test.ts b/test/integration/goDebugConfiguration.test.ts
index 8ef111c..14f8930 100644
--- a/test/integration/goDebugConfiguration.test.ts
+++ b/test/integration/goDebugConfiguration.test.ts
@@ -45,6 +45,7 @@
});
interface Input {
+ debugAdapter?: 'dlv-dap' | 'legacy';
env?: { [key: string]: any };
envFile?: string | string[];
toolsEnv?: { [key: string]: any };
@@ -57,7 +58,8 @@
name: 'Launch',
request: 'launch',
env: input.env,
- envFile: input.envFile
+ envFile: input.envFile,
+ debugAdapter: input.debugAdapter
});
const actual = config.env;
@@ -68,14 +70,18 @@
runTest({}, {});
});
- test('toolsEnvVars is propagated', () => {
+ test('toolsEnvVars is propagated (legacy)', () => {
+ const debugAdapter = 'legacy';
const toolsEnv = {
GOPATH: '/gopath',
GOOS: 'valueFromToolsEnv'
};
runTest(
- { toolsEnv },
+ {
+ debugAdapter,
+ toolsEnv
+ },
{
GOPATH: '/gopath',
GOOS: 'valueFromToolsEnv'
@@ -83,6 +89,20 @@
);
});
+ test('toolsEnvVars is not propagated', () => {
+ const toolsEnv = {
+ GOPATH: '/gopath',
+ GOOS: 'valueFromToolsEnv'
+ };
+
+ runTest(
+ {
+ toolsEnv
+ },
+ {}
+ );
+ });
+
test('preserves settings from launchArgs.env', () => {
const env = { GOPATH: 'valueFromEnv', GOOS: 'valueFromEnv2' };
runTest(
@@ -114,7 +134,26 @@
);
});
- test('launchArgs.env overwrites toolsEnvVar', () => {
+ test('launchArgs.env overwrites toolsEnvVar (legacy)', () => {
+ const toolsEnv = {
+ GOPATH: '/gopath',
+ SOMEVAR1: 'valueFromToolsEnvVar1',
+ SOMEVAR2: 'valueFromToolsEnvVar2'
+ };
+
+ const debugAdapter = 'legacy';
+ const env = { SOMEVAR1: 'valueFromEnv' };
+ runTest(
+ { debugAdapter, env, toolsEnv },
+ {
+ GOPATH: '/gopath',
+ SOMEVAR1: 'valueFromEnv',
+ SOMEVAR2: 'valueFromToolsEnvVar2'
+ }
+ );
+ });
+
+ test('launchArgs.env is respected, toolsEnvVar is ignored (dlv-dap)', () => {
const toolsEnv = {
GOPATH: '/gopath',
SOMEVAR1: 'valueFromToolsEnvVar1',
@@ -125,14 +164,12 @@
runTest(
{ env, toolsEnv },
{
- GOPATH: '/gopath',
- SOMEVAR1: 'valueFromEnv',
- SOMEVAR2: 'valueFromToolsEnvVar2'
+ SOMEVAR1: 'valueFromEnv'
}
);
});
- test('launchArgs.envFile overwrites toolsEnvVar', () => {
+ test('launchArgs.envFile overwrites toolsEnvVar (legacy)', () => {
const toolsEnv = {
GOPATH: '/gopath',
SOMEVAR1: 'valueFromToolsEnvVar1',
@@ -141,8 +178,9 @@
const envFile = path.join(tmpDir, 'env');
fs.writeFileSync(envFile, ['SOMEVAR2=valueFromEnvFile2'].join('\n'));
+ const debugAdapter = 'legacy';
runTest(
- { toolsEnv, envFile },
+ { debugAdapter, toolsEnv, envFile },
{
GOPATH: '/gopath',
SOMEVAR1: 'valueFromToolsEnvVar1',
@@ -150,6 +188,24 @@
}
);
});
+
+ test('launchArgs.envFile is repected, and toolsEnvVar is ignored (dlv-dap)', () => {
+ const toolsEnv = {
+ GOPATH: '/gopath',
+ SOMEVAR1: 'valueFromToolsEnvVar1',
+ SOMEVAR2: 'valueFromToolsEnvVar2'
+ };
+ const envFile = path.join(tmpDir, 'env');
+ fs.writeFileSync(envFile, ['SOMEVAR2=valueFromEnvFile2'].join('\n'));
+
+ const debugAdapter = 'dlv-dap';
+ runTest(
+ { debugAdapter, toolsEnv, envFile },
+ {
+ SOMEVAR2: 'valueFromEnvFile2'
+ }
+ );
+ });
});
suite('Debug Configuration Merge User Settings', () => {
diff --git a/test/testdata/envTest/main.go b/test/testdata/envTest/main.go
new file mode 100644
index 0000000..bf5cbec
--- /dev/null
+++ b/test/testdata/envTest/main.go
@@ -0,0 +1,11 @@
+package main
+
+import "os"
+
+func main() {
+ if len(os.Args) != 2 {
+ os.Exit(1)
+ }
+ v := os.Getenv(os.Args[1])
+ println(v)
+}
