Skip to content

Commit

Permalink
src: refactor tools env variables to handle installation and execution
Browse files Browse the repository at this point in the history
I modified this change to focus only on addressing the differences between environment variables when installing tools vs. when running them. https://golang.org/cl/233325 now handles the issue reports on restart.

Change-Id: I9d03e2c8f9244bade19606e9d9c6892da9fa0c66
GitHub-Last-Rev: e6f4db3
GitHub-Pull-Request: golang#28
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/232863
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
  • Loading branch information
stamblerre committed May 27, 2020
1 parent bc0a099 commit ab4483e
Show file tree
Hide file tree
Showing 29 changed files with 177 additions and 145 deletions.
8 changes: 4 additions & 4 deletions src/goBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import path = require('path');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { buildDiagnosticCollection } from './goMain';
import { isModSupported } from './goModules';
import { getNonVendorPackages } from './goPackages';
Expand All @@ -16,7 +17,6 @@ import {
getGoConfig,
getModuleCache,
getTempFilePath,
getToolsEnvVars,
getWorkspaceFolderPath,
handleDiagnosticErrors,
ICheckResult,
Expand Down Expand Up @@ -100,14 +100,14 @@ export async function goBuild(
return [];
}

const buildEnv = Object.assign({}, getToolsEnvVars());
const buildEnv = toolExecutionEnvironment();
const tmpPath = getTempFilePath('go-code-check');
const isTestFile = fileUri && fileUri.fsPath.endsWith('_test.go');
const buildFlags: string[] = isTestFile
? getTestFlags(goConfig)
: Array.isArray(goConfig['buildFlags'])
? [...goConfig['buildFlags']]
: [];
? [...goConfig['buildFlags']]
: [];
const buildArgs: string[] = isTestFile ? ['test', '-c'] : ['build'];

if (goConfig['installDependenciesWhenBuilding'] === true && !isMod) {
Expand Down
5 changes: 3 additions & 2 deletions src/goDebugConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@

import path = require('path');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { packagePathToGoModPathMap } from './goModules';
import { getFromGlobalState, updateGlobalState } from './stateUtils';
import { getBinPath, getCurrentGoPath, getGoConfig, getToolsEnvVars } from './util';
import { getBinPath, getCurrentGoPath, getGoConfig } from './util';

export class GoDebugConfigurationProvider implements vscode.DebugConfigurationProvider {
public provideDebugConfigurations(
Expand Down Expand Up @@ -61,7 +62,7 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
}

const goConfig = getGoConfig(folder && folder.uri);
const goToolsEnvVars = getToolsEnvVars();
const goToolsEnvVars = toolExecutionEnvironment();
Object.keys(goToolsEnvVars).forEach((key) => {
if (!debugConfiguration['env'].hasOwnProperty(key)) {
debugConfiguration['env'][key] = goToolsEnvVars[key];
Expand Down
8 changes: 4 additions & 4 deletions src/goDeclaration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import cp = require('child_process');
import path = require('path');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
import { getModFolderPath, promptToUpdateToolForModules } from './goModules';
import {
Expand All @@ -16,7 +17,6 @@ import {
getFileArchive,
getGoConfig,
getModuleCache,
getToolsEnvVars,
getWorkspaceFolderPath,
goKeywords,
isPositionInString,
Expand Down Expand Up @@ -132,7 +132,7 @@ function definitionLocation_godef(
return Promise.reject(missingToolMsg + godefTool);
}
const offset = byteOffsetAt(input.document, input.position);
const env = getToolsEnvVars();
const env = toolExecutionEnvironment();
let p: cp.ChildProcess;
if (token) {
token.onCancellationRequested(() => killTree(p.pid));
Expand Down Expand Up @@ -223,7 +223,7 @@ function definitionLocation_gogetdoc(
return Promise.reject(missingToolMsg + 'gogetdoc');
}
const offset = byteOffsetAt(input.document, input.position);
const env = getToolsEnvVars();
const env = toolExecutionEnvironment();
let p: cp.ChildProcess;
if (token) {
token.onCancellationRequested(() => killTree(p.pid));
Expand Down Expand Up @@ -297,7 +297,7 @@ function definitionLocation_guru(
return Promise.reject(missingToolMsg + 'guru');
}
const offset = byteOffsetAt(input.document, input.position);
const env = getToolsEnvVars();
const env = toolExecutionEnvironment();
let p: cp.ChildProcess;
if (token) {
token.onCancellationRequested(() => killTree(p.pid));
Expand Down
7 changes: 4 additions & 3 deletions src/goDoctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
import cp = require('child_process');
import { dirname, isAbsolute } from 'path';
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { getBinPath, getToolsEnvVars } from './util';
import { getBinPath } from './util';

/**
* Extracts function out of current selection and replaces the current selection with a call to the extracted function.
Expand Down Expand Up @@ -78,15 +79,15 @@ function runGoDoctor(
'-w',
'-pos',
`${selection.start.line + 1},${selection.start.character + 1}:${selection.end.line + 1},${
selection.end.character
selection.end.character
}`,
'-file',
fileName,
type,
newName
],
{
env: getToolsEnvVars(),
env: toolExecutionEnvironment(),
cwd: dirname(fileName)
},
(err, stdout, stderr) => {
Expand Down
69 changes: 69 additions & 0 deletions src/goEnv.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*---------------------------------------------------------
* Copyright (C) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE in the project root for license information.
*--------------------------------------------------------*/

'use strict';

import path = require('path');
import vscode = require('vscode');
import { getCurrentGoPath, getGoConfig, getToolsGopath, resolvePath } from './util';

// toolInstallationEnvironment returns the environment in which tools should
// be installed. It always returns a new object.
export function toolInstallationEnvironment(): NodeJS.Dict<string> {
const env = newEnvironment();

// If the go.toolsGopath is set, use its value as the GOPATH for `go` processes.
// Else use the Current Gopath
let toolsGopath = getToolsGopath();
if (toolsGopath) {
// User has explicitly chosen to use toolsGopath, so ignore GOBIN.
env['GOBIN'] = '';
} else {
toolsGopath = getCurrentGoPath();
}
if (!toolsGopath) {
const msg = 'Cannot install Go tools. Set either go.gopath or go.toolsGopath in settings.';
vscode.window.showInformationMessage(msg, 'Open User Settings', 'Open Workspace Settings').then((selected) => {
switch (selected) {
case 'Open User Settings':
vscode.commands.executeCommand('workbench.action.openGlobalSettings');
break;
case 'Open Workspace Settings':
vscode.commands.executeCommand('workbench.action.openWorkspaceSettings');
break;
}
});
return;
}
env['GOPATH'] = toolsGopath;

return env;
}

// toolExecutionEnvironment returns the environment in which tools should
// be executed. It always returns a new object.
export function toolExecutionEnvironment(): NodeJS.Dict<string> {
const env = newEnvironment();
const gopath = getCurrentGoPath();
if (gopath) {
env['GOPATH'] = gopath;
}
return env;
}

function newEnvironment(): NodeJS.Dict<string> {
const toolsEnvVars = getGoConfig()['toolsEnvVars'];
const env = Object.assign({}, process.env, toolsEnvVars);

// The http.proxy setting takes precedence over environment variables.
const httpProxy = vscode.workspace.getConfiguration('http', null).get('proxy');
if (httpProxy && typeof httpProxy === 'string') {
env['http_proxy'] = httpProxy;
env['HTTP_PROXY'] = httpProxy;
env['https_proxy'] = httpProxy;
env['HTTPS_PROXY'] = httpProxy;
}
return env;
}
5 changes: 3 additions & 2 deletions src/goFillStruct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

import cp = require('child_process');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { byteOffsetAt, getBinPath, getFileArchive, getToolsEnvVars, makeMemoizedByteOffsetConverter } from './util';
import { byteOffsetAt, getBinPath, getFileArchive, makeMemoizedByteOffsetConverter } from './util';

// Interface for the output from fillstruct
interface GoFillStructOutput {
Expand Down Expand Up @@ -59,7 +60,7 @@ function execFillStruct(editor: vscode.TextEditor, args: string[]): Promise<void
const tabsCount = getTabsCount(editor);

return new Promise<void>((resolve, reject) => {
const p = cp.execFile(fillstruct, args, { env: getToolsEnvVars() }, (err, stdout, stderr) => {
const p = cp.execFile(fillstruct, args, { env: toolExecutionEnvironment() }, (err, stdout, stderr) => {
try {
if (err && (<any>err).code === 'ENOENT') {
promptForMissingTool('fillstruct');
Expand Down
5 changes: 3 additions & 2 deletions src/goFormat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
import cp = require('child_process');
import path = require('path');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
import { getBinPath, getGoConfig, getToolsEnvVars, killTree } from './util';
import { getBinPath, getGoConfig, killTree } from './util';

export class GoDocumentFormattingEditProvider implements vscode.DocumentFormattingEditProvider {
public provideDocumentFormattingEdits(
Expand Down Expand Up @@ -70,7 +71,7 @@ export class GoDocumentFormattingEditProvider implements vscode.DocumentFormatti
return reject();
}

const env = getToolsEnvVars();
const env = toolExecutionEnvironment();
const cwd = path.dirname(document.fileName);
let stdout = '';
let stderr = '';
Expand Down
5 changes: 3 additions & 2 deletions src/goGenerateTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import cp = require('child_process');
import path = require('path');
import vscode = require('vscode');

import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { GoDocumentSymbolProvider } from './goOutline';
import { outputChannel } from './goStatus';
import { getBinPath, getGoConfig, getToolsEnvVars } from './util';
import { getBinPath, getGoConfig } from './util';

const generatedWord = 'Generated ';

Expand Down Expand Up @@ -172,7 +173,7 @@ function generateTests(conf: Config, goConfig: vscode.WorkspaceConfiguration): P
args = args.concat(['-all', conf.dir]);
}

cp.execFile(cmd, args, { env: getToolsEnvVars() }, (err, stdout, stderr) => {
cp.execFile(cmd, args, { env: toolExecutionEnvironment() }, (err, stdout, stderr) => {
outputChannel.appendLine('Generating Tests: ' + cmd + ' ' + args.join(' '));

try {
Expand Down
5 changes: 3 additions & 2 deletions src/goImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
import cp = require('child_process');
import { dirname } from 'path';
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { getBinPath, getToolsEnvVars } from './util';
import { getBinPath } from './util';

// Supports only passing interface, see TODO in implCursor to finish
const inputRegex = /^(\w+\ \*?\w+\ )?([\w./]+)$/;
Expand Down Expand Up @@ -49,7 +50,7 @@ function runGoImpl(args: string[], insertPos: vscode.Position, editor: vscode.Te
const p = cp.execFile(
goimpl,
args,
{ env: getToolsEnvVars(), cwd: dirname(editor.document.fileName) },
{ env: toolExecutionEnvironment(), cwd: dirname(editor.document.fileName) },
(err, stdout, stderr) => {
if (err && (<any>err).code === 'ENOENT') {
promptForMissingTool('impl');
Expand Down
4 changes: 2 additions & 2 deletions src/goImplementations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
import cp = require('child_process');
import path = require('path');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { envPath } from './goPath';
import {
byteOffsetAt,
canonicalizeGOPATHPrefix,
getBinPath,
getGoConfig,
getToolsEnvVars,
getWorkspaceFolderPath,
killTree
} from './util';
Expand Down Expand Up @@ -66,7 +66,7 @@ export class GoImplementationProvider implements vscode.ImplementationProvider {
if (token.isCancellationRequested) {
return resolve(null);
}
const env = getToolsEnvVars();
const env = toolExecutionEnvironment();
const listProcess = cp.execFile(
goRuntimePath,
['list', '-e', '-json'],
Expand Down
5 changes: 3 additions & 2 deletions src/goImport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

import cp = require('child_process');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { promptForMissingTool } from './goInstallTools';
import { documentSymbols, GoOutlineImportsOptions } from './goOutline';
import { getImportablePackages } from './goPackages';
import { envPath } from './goPath';
import { getBinPath, getImportPath, getToolsEnvVars, parseFilePrelude } from './util';
import { getBinPath, getImportPath, parseFilePrelude } from './util';

const missingToolMsg = 'Missing tool: ';

Expand Down Expand Up @@ -183,7 +184,7 @@ export function addImportToWorkspace() {
);
return;
}
const env = getToolsEnvVars();
const env = toolExecutionEnvironment();

cp.execFile(goRuntimePath, ['list', '-f', '{{.Dir}}', importPath], { env }, (err, stdout, stderr) => {
const dirs = (stdout || '').split('\n');
Expand Down
5 changes: 3 additions & 2 deletions src/goInstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
import cp = require('child_process');
import path = require('path');
import vscode = require('vscode');
import { toolExecutionEnvironment } from './goEnv';
import { isModSupported } from './goModules';
import { envPath, getCurrentGoWorkspaceFromGOPATH } from './goPath';
import { outputChannel } from './goStatus';
import { getBinPath, getCurrentGoPath, getGoConfig, getModuleCache, getToolsEnvVars } from './util';
import { getBinPath, getCurrentGoPath, getGoConfig, getModuleCache } from './util';

export async function installCurrentPackage(): Promise<void> {
const editor = vscode.window.activeTextEditor;
Expand All @@ -32,7 +33,7 @@ export async function installCurrentPackage(): Promise<void> {
return;
}

const env = Object.assign({}, getToolsEnvVars());
const env = toolExecutionEnvironment();
const cwd = path.dirname(editor.document.uri.fsPath);
const isMod = await isModSupported(editor.document.uri);

Expand Down
Loading

0 comments on commit ab4483e

Please sign in to comment.