[release] 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 #146
Updates #26

Change-Id: I9b0e42787308e17547067460960b5fdd8b678991
GitHub-Last-Rev: 995c1a3
GitHub-Pull-Request: #252
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/239697
Reviewed-by: Brayden Cloud <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>
(cherry picked from commit 057186f)
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/239981

Author: hyangah

src/goEnvironmentStatus.ts

@@ -18,20 +18,15 @@ let goEnvStatusbarItem: vscode.StatusBarItem;
  * 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();
 }
 

src/goInstallTools.ts

@@ -12,6 +12,7 @@ import { SemVer } from 'semver';
 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 async function promptForUpdatingTool(toolName: string, newVersion?: SemVe
 }
 
 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 @@ export function updateGoVarsFromConfig(): Promise<void> {
 			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 @@ export function updateGoVarsFromConfig(): Promise<void> {
 			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() {

src/goMain.ts

@@ -18,7 +18,7 @@ import {
 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 @@ export function activate(ctx: vscode.ExtensionContext): void {
 			);
 		})
 	);
-	showHideStatus(vscode.window.activeTextEditor);
-
-	// show the go environment status bar item
-	initGoStatusBar();
-
 	const testCodeLensProvider = new GoRunTestCodeLensProvider();
 	const referencesCodeLensProvider = new GoReferencesCodeLensProvider();
 
@@ -407,8 +402,14 @@ export function activate(ctx: vscode.ExtensionContext): void {
 				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();

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 getBinPathFromEnvVar(toolName: string, envVarValue: string, appe
 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 @@ export function getBinPathWithPreferredGopath(
 	}
 
 	// 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;

src/util.ts

@@ -136,11 +136,13 @@ export class GoVersion {
 	}
 }
 
+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 @@ export function getGoConfig(uri?: vscode.Uri): vscode.WorkspaceConfiguration {
 		}
 	}
 	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 @@ export async function getGoVersion(): Promise<GoVersion | undefined> {
 		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 @@ export async function getGoVersion(): Promise<GoVersion | undefined> {
 			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,14 +435,15 @@ function resolveToolsGopath(): string {
 	}
 }
 
-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];
 
 	return getBinPathWithPreferredGopath(
 		tool,
 		tool === 'go' ? [] : [getToolsGopath(), getCurrentGoPath()],
 		resolvePath(alternateToolPath),
+		useCache
 	);
 }
 

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")
+}