src/goInstallTools.ts: fix PATH adjustment when a different go is chosen
With commits d93a0ae and a5e40ca (microsoft/vscode-go#3152), we tried to
include the go binary's path to the PATH (Path on windows) in order to ensure
all underlying go tools (gofmt, cgo, gopls, ...) that simply invoke 'go'
can find the matching go binary by searching the PATH.
We found that trick does not work when users specifies a different go version
using go.alternateTools and the specified binary is not named 'go'.
For example, golang.org provides an easy way to install extra versions of Go
https://golang.org/doc/install#extra_versions through a wrapper, whose
name includes the version. Users who take this approach should be able to
configure to pick up the chosen version with
```
"go.alternateTools": {
"go": "/Users/username/go/bin/go1.13.11"
}
```
Previously, we just added /Users/username/go/bin (the go binary directory name)
to PATH. Because there is no 'go' binary in this directory, the underlying
tools failed to pick the right go tool.
In this CL, we instead use the GOROOT (found from go env call) and add
GOROOT/bin to the PATH.
In this CL
- We also arrange to call updateGoVarsFromConfig only when the relevant
configs are changed (onDidChangeConfiguration setup in goMain.ts).
Previously, this was called almost on every file save events if the repository
has a workspace setting file (.vscode/setting.json).
- We also changed initGoStatusBar to be called after the goroot is updated.
That eliminates an extra call path (initGoStatusBar -> ... -> updateGoVarsFromConfig,
and also, reflects goroot changes correctly when the configuration is updated.
Updates golang/vscode-go#146
Updates golang/vscode-go#26
Change-Id: I9b0e42787308e17547067460960b5fdd8b678991
GitHub-Last-Rev: 995c1a39ccbc1b9294d2be4f7fac6131e5edeb1a
GitHub-Pull-Request: golang/vscode-go#252
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/239697
Reviewed-by: Brayden Cloud <bcloud@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/src/goEnvironmentStatus.ts b/src/goEnvironmentStatus.ts
index ee6f9d7..dbb3d8a 100644
--- a/src/goEnvironmentStatus.ts
+++ b/src/goEnvironmentStatus.ts
@@ -18,20 +18,15 @@
* Initialize the status bar item with current Go binary
*/
export async function initGoStatusBar() {
- goEnvStatusbarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 50);
-
- // make goroot default to go.goroot and fallback on $PATH
- const goroot = await getActiveGoRoot();
- if (!goroot) {
- // TODO: prompt user to install Go
- vscode.window.showErrorMessage('No Go command could be found.');
+ if (!goEnvStatusbarItem) {
+ goEnvStatusbarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left, 50);
}
-
// set Go version and command
const version = await getGoVersion();
+
+ hideGoStatusBar();
goEnvStatusbarItem.text = formatGoVersion(version.format());
goEnvStatusbarItem.command = 'go.environment.choose';
-
showGoStatusBar();
}
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index a72ad34..1781391 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -12,6 +12,7 @@
import util = require('util');
import vscode = require('vscode');
import { toolInstallationEnvironment } from './goEnv';
+import { initGoStatusBar } from './goEnvironmentStatus';
import { getLanguageServerToolPath } from './goLanguageServer';
import { restartLanguageServer } from './goMain';
import { envPath, getCurrentGoRoot, getToolFromToolPath, setCurrentGoRoot } from './goPath';
@@ -335,36 +336,22 @@
}
export function updateGoVarsFromConfig(): Promise<void> {
- if (getCurrentGoRoot() && process.env['GOPATH'] && process.env['GOPROXY'] && process.env['GOBIN']) {
+ // FIXIT: when user changes the environment variable settings or go.gopath, the following
+ // condition prevents from updating the process.env accordingly, so the extension will lie.
+ // Needs to clean up.
+ if (process.env['GOPATH'] && process.env['GOPROXY'] && process.env['GOBIN']) {
return Promise.resolve();
}
- // If GOPATH is still not set, then use the one from `go env`
- const goRuntimePath = getBinPath('go');
+ // FIXIT: if updateGoVarsFromConfig is called again after addGoRuntimeBaseToPATH sets PATH,
+ // the go chosen by getBinPath based on PATH will not change.
+ const goRuntimePath = getBinPath('go', false);
if (!goRuntimePath) {
vscode.window.showErrorMessage(
`Failed to run "go env" to find GOPATH as the "go" binary cannot be found in either GOROOT(${getCurrentGoRoot()}) or PATH(${envPath})`
);
return;
}
- const goRuntimeBasePath = path.dirname(goRuntimePath);
-
- // cgo and a few other Go tools expect Go binary to be in the path
- let pathEnvVar: string;
- if (process.env.hasOwnProperty('PATH')) {
- pathEnvVar = 'PATH';
- } else if (process.platform === 'win32' && process.env.hasOwnProperty('Path')) {
- pathEnvVar = 'Path';
- }
- if (
- goRuntimeBasePath &&
- pathEnvVar &&
- process.env[pathEnvVar] &&
- (<string>process.env[pathEnvVar]).split(path.delimiter).indexOf(goRuntimeBasePath) === -1
- ) {
- // Place the goRuntimeBasePath to the front so tools use the same version of go.
- process.env[pathEnvVar] = goRuntimeBasePath + path.delimiter + process.env[pathEnvVar];
- }
return new Promise<void>((resolve, reject) => {
cp.execFile(goRuntimePath, ['env', 'GOPATH', 'GOROOT', 'GOPROXY', 'GOBIN'], (err, stdout, stderr) => {
@@ -375,7 +362,7 @@
if (!process.env['GOPATH'] && envOutput[0].trim()) {
process.env['GOPATH'] = envOutput[0].trim();
}
- if (!getCurrentGoRoot() && envOutput[1] && envOutput[1].trim()) {
+ if (envOutput[1] && envOutput[1].trim()) {
setCurrentGoRoot(envOutput[1].trim());
}
if (!process.env['GOPROXY'] && envOutput[2] && envOutput[2].trim()) {
@@ -384,11 +371,48 @@
if (!process.env['GOBIN'] && envOutput[3] && envOutput[3].trim()) {
process.env['GOBIN'] = envOutput[3].trim();
}
+
+ // cgo, gopls, and other underlying tools will inherit the environment and attempt
+ // to locate 'go' from the PATH env var.
+ addGoRuntimeBaseToPATH(path.join(getCurrentGoRoot(), 'bin'));
+ initGoStatusBar();
+ // TODO: restart language server or synchronize with language server update.
+
return resolve();
});
});
}
+// PATH value cached before addGoRuntimeBaseToPath modified.
+let defaultPathEnv = '';
+
+// addGoRuntimeBaseToPATH adds the given path to the front of the PATH environment variable.
+// It removes duplicates.
+// TODO: can we avoid changing PATH but utilize toolExecutionEnv?
+function addGoRuntimeBaseToPATH(newGoRuntimeBase: string) {
+ if (!newGoRuntimeBase) {
+ return;
+ }
+
+ let pathEnvVar: string;
+ if (process.env.hasOwnProperty('PATH')) {
+ pathEnvVar = 'PATH';
+ } else if (process.platform === 'win32' && process.env.hasOwnProperty('Path')) {
+ pathEnvVar = 'Path';
+ } else {
+ return;
+ }
+
+ if (!defaultPathEnv) { // cache the default value
+ defaultPathEnv = <string>process.env[pathEnvVar];
+ }
+
+ let pathVars = defaultPathEnv.split(path.delimiter);
+ pathVars = pathVars.filter((p) => p !== newGoRuntimeBase);
+ pathVars.unshift(newGoRuntimeBase);
+ process.env[pathEnvVar] = pathVars.join(path.delimiter);
+}
+
let alreadyOfferedToInstallTools = false;
export async function offerToInstallTools() {
diff --git a/src/goMain.ts b/src/goMain.ts
index 31088b7..e10fa8d 100644
--- a/src/goMain.ts
+++ b/src/goMain.ts
@@ -18,7 +18,7 @@
import { GoDebugConfigurationProvider } from './goDebugConfiguration';
import { extractFunction, extractVariable } from './goDoctor';
import { toolExecutionEnvironment } from './goEnv';
-import { chooseGoEnvironment, initGoStatusBar } from './goEnvironmentStatus';
+import { chooseGoEnvironment } from './goEnvironmentStatus';
import { runFillStruct } from './goFillStruct';
import * as goGenerateTests from './goGenerateTests';
import { goGetPackage } from './goGetPackage';
@@ -167,11 +167,6 @@
);
})
);
- showHideStatus(vscode.window.activeTextEditor);
-
- // show the go environment status bar item
- initGoStatusBar();
-
const testCodeLensProvider = new GoRunTestCodeLensProvider();
const referencesCodeLensProvider = new GoReferencesCodeLensProvider();
@@ -407,8 +402,14 @@
return;
}
const updatedGoConfig = getGoConfig();
- updateGoVarsFromConfig();
+ if (e.affectsConfiguration('go.goroot') ||
+ e.affectsConfiguration('go.alternateTools') ||
+ e.affectsConfiguration('go.gopath') ||
+ e.affectsConfiguration('go.toolsEnvVars') ||
+ e.affectsConfiguration('go.testEnvFile')) {
+ updateGoVarsFromConfig();
+ }
// If there was a change in "toolsGopath" setting, then clear cache for go tools
if (getToolsGopath() !== getToolsGopath(false)) {
clearCacheForTools();
diff --git a/src/goPath.ts b/src/goPath.ts
index 6f4d742..1e07c2b 100644
--- a/src/goPath.ts
+++ b/src/goPath.ts
@@ -12,6 +12,7 @@
import fs = require('fs');
import os = require('os');
import path = require('path');
+import { getGoConfig } from './util';
let binPathCache: { [bin: string]: string } = {};
@@ -34,14 +35,16 @@
export function getBinPathWithPreferredGopath(
toolName: string,
preferredGopaths: string[],
- alternateTool?: string
+ alternateTool?: string,
+ useCache = true,
) {
if (alternateTool && path.isAbsolute(alternateTool) && executableFileExists(alternateTool)) {
binPathCache[toolName] = alternateTool;
return alternateTool;
}
- if (binPathCache[toolName]) {
+ // FIXIT: this cache needs to be invalidated when go.goroot or go.alternateTool is changed.
+ if (useCache && binPathCache[toolName]) {
return binPathCache[toolName];
}
@@ -64,7 +67,7 @@
}
// Check GOROOT (go, gofmt, godoc would be found here)
- const pathFromGoRoot = getBinPathFromEnvVar(binname, getCurrentGoRoot(), true);
+ const pathFromGoRoot = getBinPathFromEnvVar(binname, getGoConfig().get('goroot') || getCurrentGoRoot(), true);
if (pathFromGoRoot) {
binPathCache[toolName] = pathFromGoRoot;
return pathFromGoRoot;
diff --git a/src/util.ts b/src/util.ts
index 556e52f..ba5b760 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -136,11 +136,13 @@
}
}
+let cachedGoBinPath: string | undefined;
let cachedGoVersion: GoVersion | undefined;
let vendorSupport: boolean | undefined;
let toolsGopath: string;
-export function getGoConfig(uri?: vscode.Uri): vscode.WorkspaceConfiguration {
+// getGoConfig is declared as an exported const rather than a function, so it can be stubbbed in testing.
+export const getGoConfig = (uri?: vscode.Uri) => {
if (!uri) {
if (vscode.window.activeTextEditor) {
uri = vscode.window.activeTextEditor.document.uri;
@@ -149,7 +151,7 @@
}
}
return vscode.workspace.getConfiguration('go', uri);
-}
+};
export function byteOffsetAt(document: vscode.TextDocument, position: vscode.Position): number {
const offset = document.offsetAt(position);
@@ -311,7 +313,7 @@
warn(`unable to locate "go" binary in GOROOT (${getCurrentGoRoot()}) or PATH (${envPath})`);
return;
}
- if (cachedGoVersion) {
+ if (cachedGoBinPath === goRuntimePath && cachedGoVersion) {
if (cachedGoVersion.isValid()) {
return Promise.resolve(cachedGoVersion);
}
@@ -324,6 +326,7 @@
warn(`failed to run "${goRuntimePath} version": stdout: ${stdout}, stderr: ${stderr}`);
return;
}
+ cachedGoBinPath = goRuntimePath;
cachedGoVersion = new GoVersion(goRuntimePath, stdout);
} catch (err) {
warn(`failed to run "${goRuntimePath} version": ${err}`);
@@ -432,7 +435,7 @@
}
}
-export function getBinPath(tool: string): string {
+export function getBinPath(tool: string, useCache = true): string {
const alternateTools: { [key: string]: string } = getGoConfig().get('alternateTools');
const alternateToolPath: string = alternateTools[tool];
@@ -440,6 +443,7 @@
tool,
tool === 'go' ? [] : [getToolsGopath(), getCurrentGoPath()],
resolvePath(alternateToolPath),
+ useCache
);
}
diff --git a/test/fixtures/testhelpers/fakego.go b/test/fixtures/testhelpers/fakego.go
new file mode 100644
index 0000000..8116833
--- /dev/null
+++ b/test/fixtures/testhelpers/fakego.go
@@ -0,0 +1,47 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Licensed under the MIT License.
+// See LICENSE in the project root for license information.
+
+// This is a helper used to fake a go binary.
+// It currently fakes `go env` and `go version` commands.
+// For `go env`, it returns FAKEGOROOT for 'GOROOT', and an empty string for others.
+// For `go version`, it returns FAKEGOVERSION or a default dev version string.
+package main
+
+import (
+ "fmt"
+ "os"
+)
+
+func main() {
+ args := os.Args
+
+ if len(args) <= 1 {
+ return
+ }
+ switch args[1] {
+ case "env":
+ fakeEnv(args[2:])
+ case "version":
+ fakeVersion()
+ default:
+ fmt.Fprintf(os.Stderr, "not implemented")
+ os.Exit(1)
+ }
+ os.Exit(0)
+}
+
+func fakeEnv(args []string) {
+ for _, a := range args {
+ fmt.Println(os.Getenv("FAKE" + a))
+ }
+}
+
+func fakeVersion() {
+ ver := os.Getenv("FAKEGOVERSION")
+ if ver != "" {
+ fmt.Println(ver)
+ return
+ }
+ fmt.Println("go version devel +a07e2819 Thu Jun 18 20:58:26 2020 +0000 darwin/amd64")
+}
diff --git a/test/integration/statusbar.test.ts b/test/integration/statusbar.test.ts
index f55d34e..42d166e 100644
--- a/test/integration/statusbar.test.ts
+++ b/test/integration/statusbar.test.ts
@@ -5,14 +5,22 @@
*--------------------------------------------------------*/
import * as assert from 'assert';
+import cp = require('child_process');
+import fs = require('fs');
import { describe, it } from 'mocha';
-
-import { disposeGoStatusBar, formatGoVersion, getGoEnvironmentStatusbarItem, initGoStatusBar } from '../../src/goEnvironmentStatus';
-import { getGoVersion } from '../../src/util';
+import os = require('os');
+import path = require('path');
+import sinon = require('sinon');
+import util = require('util');
+import { WorkspaceConfiguration } from 'vscode';
+import { disposeGoStatusBar, formatGoVersion, getGoEnvironmentStatusbarItem } from '../../src/goEnvironmentStatus';
+import { updateGoVarsFromConfig } from '../../src/goInstallTools';
+import { getCurrentGoRoot } from '../../src/goPath';
+import ourutil = require('../../src/util');
describe('#initGoStatusBar()', function () {
- this.beforeAll(() => {
- initGoStatusBar();
+ this.beforeAll(async () => {
+ await updateGoVarsFromConfig(); // should initialize the status bar.
});
this.afterAll(() => {
@@ -23,8 +31,8 @@
assert.notEqual(getGoEnvironmentStatusbarItem(), undefined);
});
- it('should create a status bar item with a label matching go.goroot version', async () => {
- const version = await getGoVersion();
+ it('should create a status bar item with a label matching go.goroot version', async () => {
+ const version = await ourutil.getGoVersion();
const versionLabel = formatGoVersion(version.format());
assert.equal(
getGoEnvironmentStatusbarItem().text,
@@ -33,3 +41,109 @@
);
});
});
+
+describe('#updateGoVarsFromConfig()', function () {
+ this.timeout(10000);
+
+ let defaultGoConfig: WorkspaceConfiguration | undefined;
+ let tmpRoot: string | undefined;
+ let tmpRootBin: string | undefined;
+ let sandbox: sinon.SinonSandbox | undefined;
+
+ this.beforeAll(async () => {
+ defaultGoConfig = ourutil.getGoConfig();
+
+ tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rootchangetest'));
+ tmpRootBin = path.join(tmpRoot, 'bin');
+
+ // build a fake go binary and place it in tmpRootBin.
+ fs.mkdirSync(tmpRootBin);
+
+ const fixtureSourcePath = path.join(__dirname, '..', '..', '..', 'test', 'fixtures', 'testhelpers');
+ const execFile = util.promisify(cp.execFile);
+ const goRuntimePath = ourutil.getBinPath('go');
+ const { stdout, stderr } = await execFile(
+ goRuntimePath, ['build', '-o', path.join(tmpRootBin, 'go'), path.join(fixtureSourcePath, 'fakego.go')]);
+ if (stderr) {
+ assert.fail(`failed to build the fake go binary required for testing: ${stderr}`);
+ }
+ });
+
+ this.afterAll(() => {
+ ourutil.rmdirRecursive(tmpRoot);
+ });
+
+ this.beforeEach(() => {
+ sandbox = sinon.createSandbox();
+ });
+
+ this.afterEach(() => {
+ sandbox.restore();
+ });
+
+ function pathEnvVar(): string[] {
+ let paths = [] as string[];
+ if (process.env.hasOwnProperty('PATH')) {
+ paths = process.env['PATH'].split(path.delimiter);
+ } else if (process.platform === 'win32' && process.env.hasOwnProperty('Path')) {
+ paths = process.env['Path'].split(path.delimiter);
+ }
+ return paths;
+ }
+
+ it('should have a sensible goroot with the default setting', async () => {
+ await updateGoVarsFromConfig();
+
+ const b = getGoEnvironmentStatusbarItem();
+ assert.ok(b.text.startsWith('Go'), `go env statusbar item = ${b.text}, want "Go..."`);
+ assert.equal(pathEnvVar()[0], [path.join(getCurrentGoRoot(), 'bin')],
+ `the first element in PATH must match the current GOROOT/bin`);
+ });
+
+ it('should recognize the adjusted goroot using go.goroot', async () => {
+ // stub geteGoConfig to return "go.goroot": tmpRoot.
+ const getGoConfigStub = sandbox.stub(ourutil, 'getGoConfig').returns({
+ get: (s: string) => {
+ if (s === 'goroot') { return tmpRoot; }
+ return defaultGoConfig.get(s);
+ },
+ } as WorkspaceConfiguration);
+
+ // adjust the fake go binary's behavior.
+ process.env['FAKEGOROOT'] = tmpRoot;
+ process.env['FAKEGOVERSION'] = 'go version go2.0.0 darwin/amd64';
+
+ await updateGoVarsFromConfig();
+
+ sandbox.assert.calledWith(getGoConfigStub);
+ assert.equal((await ourutil.getGoVersion()).format(), '2.0.0');
+ assert.equal(getGoEnvironmentStatusbarItem().text, 'Go 2.0.0');
+ assert.equal(pathEnvVar()[0], [path.join(getCurrentGoRoot(), 'bin')],
+ `the first element in PATH must match the current GOROOT/bin`);
+ });
+
+ it('should recognize the adjusted goroot using go.alternateTools', async () => {
+ // "go.alternateTools" : {"go": "go3"}
+ fs.copyFileSync(path.join(tmpRootBin, 'go'), path.join(tmpRootBin, 'go3'));
+
+ const getGoConfigStub = sandbox.stub(ourutil, 'getGoConfig').returns({
+ get: (s: string) => {
+ if (s === 'alternateTools') {
+ return { go: path.join(tmpRootBin, 'go3') };
+ }
+ return defaultGoConfig.get(s);
+ },
+ } as WorkspaceConfiguration);
+
+ process.env['FAKEGOROOT'] = tmpRoot;
+ process.env['FAKEGOVERSION'] = 'go version go3.0.0 darwin/amd64';
+
+ await updateGoVarsFromConfig();
+
+ sandbox.assert.calledWith(getGoConfigStub);
+ assert.equal((await ourutil.getGoVersion()).format(), '3.0.0');
+ assert.equal(getGoEnvironmentStatusbarItem().text, 'Go 3.0.0');
+ assert.equal(pathEnvVar()[0], [path.join(getCurrentGoRoot(), 'bin')],
+ `the first element in PATH must match the current GOROOT/bin`);
+ });
+});
