src/goDebugConfiguration: combine envFile and env

With the resolveDebugConfigurationWithSubstitutedVariables,
we can read envFile and combine it into the env property
from the extension host.

Delve DAP will handle only the env arg and not have any
special handling for envFile. So, processing it from the
extension side makes more sense for long term.

This move allows us to centralize the .env file read support.
For backwards compatibility, I left the logic in the old DA
but removed it from the new delve DAP DA.

Fixes #452
Updates #23

Change-Id: I641eb2e62051985ba3486901483ad796256aba2c
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/248659
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Polina Sokolova <polina@google.com>

src/debugAdapter/goDebug.ts

@@ -259,8 +259,6 @@ interface LaunchRequestArguments extends DebugProtocol.LaunchRequestArguments {
 	buildFlags?: string;
 	init?: string;
 	trace?: 'verbose' | 'log' | 'error';
-	/** Optional path to .env file. */
-	envFile?: string | string[];
 	backend?: string;
 	output?: string;
 	/** Delve LoadConfig parameters */
@@ -273,6 +271,12 @@ interface LaunchRequestArguments extends DebugProtocol.LaunchRequestArguments {
 
 	showGlobalVariables?: boolean;
 	packagePathToGoModPathMap: { [key: string]: string };
+
+	/** Optional path to .env file. */
+	// TODO: deprecate .env file processing from DA.
+	// We expect the extension processes .env files
+	// and send the information to DA using the 'env' property.
+	envFile?: string | string[];
 }
 
 interface AttachRequestArguments extends DebugProtocol.AttachRequestArguments {

src/debugAdapter2/goDlvDebug.ts

@@ -20,7 +20,6 @@ import {
 	TerminatedEvent
 } from 'vscode-debugadapter';
 import { DebugProtocol } from 'vscode-debugprotocol';
-import { parseEnvFiles } from '../utils/envUtils';
 import { envPath, getBinPathWithPreferredGopathGoroot } from '../utils/goPath';
 import { killProcessTree } from '../utils/processUtils';
 
@@ -57,8 +56,6 @@ interface LaunchRequestArguments extends DebugProtocol.LaunchRequestArguments {
 	buildFlags?: string;
 	init?: string;
 	trace?: 'verbose' | 'log' | 'error';
-	/** Optional path to .env file. */
-	envFile?: string | string[];
 	backend?: string;
 	output?: string;
 	/** Delve LoadConfig parameters */
@@ -610,10 +607,11 @@ export class GoDlvDapDebugSession extends LoggingDebugSession {
 			goRunArgs.push(...launchArgs.args);
 		}
 
-		// Read env from disk and merge into env variables.
-		const fileEnvs = parseEnvFiles(launchArgs.envFile);
+		// launchArgs.env includes all the environment variables
+		// including vscode-go's toolsExecutionEnvironment (PATH, GOPATH, ...),
+		// and those read from .env files.
 		const launchArgsEnv = launchArgs.env || {};
-		const programEnv = Object.assign({}, process.env, fileEnvs, launchArgsEnv);
+		const programEnv = Object.assign({}, process.env, launchArgsEnv);
 
 		log(`Current working directory: ${dirname}`);
 		const goExe = getBinPathWithPreferredGopathGoroot('go', []);

src/goDebugConfiguration.ts

@@ -12,6 +12,7 @@ import { promptForMissingTool } from './goInstallTools';
 import { packagePathToGoModPathMap } from './goModules';
 import { getFromGlobalState, updateGlobalState } from './stateUtils';
 import { getBinPath, getCurrentGoPath, getGoConfig } from './util';
+import { parseEnvFiles } from './utils/envUtils';
 
 export class GoDebugConfigurationProvider implements vscode.DebugConfigurationProvider {
 	constructor(private defaultDebugAdapterType: string = 'go') { }
@@ -60,20 +61,9 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 
 		debugConfiguration['packagePathToGoModPathMap'] = packagePathToGoModPathMap;
 
-		const gopath = getCurrentGoPath(folder ? folder.uri : undefined);
-		if (!debugConfiguration['env']) {
-			debugConfiguration['env'] = { GOPATH: gopath };
-		} else if (!debugConfiguration['env']['GOPATH']) {
-			debugConfiguration['env']['GOPATH'] = gopath;
-		}
-
 		const goConfig = getGoConfig(folder && folder.uri);
-		const goToolsEnvVars = toolExecutionEnvironment();
-		Object.keys(goToolsEnvVars).forEach((key) => {
-			if (!debugConfiguration['env'].hasOwnProperty(key)) {
-				debugConfiguration['env'][key] = goToolsEnvVars[key];
-			}
-		});
+
+		combineEnvFilesAndEnv(folder, debugConfiguration);
 
 		const dlvConfig = goConfig.get<any>('delveConfig');
 		let useApiV1 = false;
@@ -146,3 +136,18 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 		});
 	}
 }
+
+// combineEnvFilesAndEnv 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
+// because it is already applied.
+function combineEnvFilesAndEnv(
+	folder: vscode.WorkspaceFolder, debugConfiguration: vscode.DebugConfiguration) {
+	const goToolsEnvVars = toolExecutionEnvironment(folder?.uri); // also includes GOPATH: getCurrentGoPath().
+	const fileEnvs = parseEnvFiles(debugConfiguration['envFile']);
+	const env = debugConfiguration['env'] || {};
+
+	debugConfiguration['env'] = Object.assign(goToolsEnvVars, fileEnvs, env);
+	debugConfiguration['envFile'] = undefined;  // unset, since we already processed.
+}

src/goEnv.ts

@@ -44,9 +44,9 @@ export function toolInstallationEnvironment(): NodeJS.Dict<string> {
 
 // toolExecutionEnvironment returns the environment in which tools should
 // be executed. It always returns a new object.
-export function toolExecutionEnvironment(): NodeJS.Dict<string> {
+export function toolExecutionEnvironment(uri?: vscode.Uri): NodeJS.Dict<string> {
 	const env = newEnvironment();
-	const gopath = getCurrentGoPath();
+	const gopath = getCurrentGoPath(uri);
 	if (gopath) {
 		env['GOPATH'] = gopath;
 	}

test/integration/goDebugConfiguration.test.ts

@@ -0,0 +1,100 @@
+import assert = require('assert');
+import fs = require('fs');
+import os = require('os');
+import path = require('path');
+import sinon = require('sinon');
+import vscode = require('vscode');
+import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
+import goEnv = require('../../src/goEnv');
+import { updateGoVarsFromConfig } from '../../src/goInstallTools';
+import { getCurrentGoPath, rmdirRecursive } from '../../src/util';
+
+suite('Debug Environment Variable Merge Test', () => {
+	const debugConfigProvider = new GoDebugConfigurationProvider();
+
+	suiteSetup(async () => {
+		await updateGoVarsFromConfig();
+
+		// Set up the test fixtures.
+		const fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'fixtures');
+		const filePath = path.join(fixtureSourcePath, 'baseTest', 'test.go');
+		await vscode.workspace.openTextDocument(vscode.Uri.file(filePath));
+	});
+
+	suiteTeardown(() => {
+		vscode.commands.executeCommand('workbench.action.closeActiveEditor');
+	});
+
+	let sandbox: sinon.SinonSandbox;
+	let tmpDir = '';
+	const toolExecutionEnv: NodeJS.Dict<string> = {};
+	setup(() => {
+		tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'godebugconfig_test'));
+		sandbox = sinon.createSandbox();
+
+	});
+
+	teardown(() => {
+		sandbox.restore();
+		rmdirRecursive(tmpDir);
+	});
+
+	interface Input {
+		env?: { [key: string]: any };
+		envFile?: string|string[];
+		toolsEnv?: { [key: string]: any};
+	}
+
+	function runTest(input: Input,	expected: { [key: string]: any}) {
+		sandbox.stub(goEnv, 'toolExecutionEnvironment').returns(input.toolsEnv || {});
+		const config = debugConfigProvider.resolveDebugConfigurationWithSubstitutedVariables(undefined, {
+			type: 'go',
+			name: 'Launch',
+			request: 'launch',
+			env: input.env,
+			envFile: input.envFile,
+		});
+
+		const actual = config.env;
+		assert.deepStrictEqual(actual, expected);
+	}
+
+	test('works with empty launchArgs', () => {
+		runTest({}, {});
+	});
+
+	test('toolsEnvVars is propagated', () => {
+		const toolsEnv = {
+			GOPATH: '/gopath',
+			GOOS: 'valueFromToolsEnv'};
+
+		runTest({toolsEnv}, {
+			GOPATH: '/gopath',
+			GOOS: 'valueFromToolsEnv'});
+	});
+
+	test('preserves settings from launchArgs.env', () => {
+		const env = {GOPATH: 'valueFromEnv', GOOS: 'valueFromEnv2'};
+		runTest({env}, {
+			GOPATH: 'valueFromEnv',
+			GOOS: 'valueFromEnv2'});
+	});
+
+	test('preserves settings from launchArgs.envFile', () => {
+		const envFile = path.join(tmpDir, 'env');
+		fs.writeFileSync(envFile, 'GOPATH=valueFromEnvFile');
+		runTest({envFile}, {GOPATH: 'valueFromEnvFile'});
+	});
+
+	test('launchArgs.env overwrites launchArgs.envFile', () => {
+		const env = {SOMEVAR1: 'valueFromEnv'};
+		const envFile = path.join(tmpDir, 'env');
+		fs.writeFileSync(envFile, [
+			'SOMEVAR1=valueFromEnvFile1',
+			'SOMEVAR2=valueFromEnvFile2'].join('\n'));
+
+		runTest({ env, envFile }, {
+			SOMEVAR1: 'valueFromEnv',
+			SOMEVAR2: 'valueFromEnvFile2'});
+	});
+});