src/goInstallTools.ts: add GOROOT/bin to PATH when it wasn't found from PATH
The fix for golang/vscode-go#679 changed to prepend GOROOT/bin to PATH or Path
only if users explicitly configured to use go different from what's found from PATH.
I forgot the case where go was not found from PATH and the extension picked up
a common default go installation directory. (C:\Go\bin or /usr/local/go/bin).
This change rewrote the fix - rewrote getBinPath to return why it chose the
path as well. Then, we mutate the PATH env var if getBinPath picked
go with a reason other than it's in PATH (why === 'path').
Since getBinPath and getBinPathWithPreferredGopathGoroot are
used in many places, this CL introduces getBinPathWithExplanation and
getBinPath wraps it.
Updates golang/vscode-go#679
Fixes golang/vscode-go#713
Change-Id: Ie00612fcef2cf4c2a187a263da04b342182c030b
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/258316
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/goInstallTools.ts b/src/goInstallTools.ts
index 4065f89..bc4f5a4 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -31,6 +31,7 @@
import { getFromWorkspaceState } from './stateUtils';
import {
getBinPath,
+ getBinPathWithExplanation,
getGoConfig,
getGoVersion,
getTempFilePath,
@@ -38,7 +39,7 @@
GoVersion,
rmdirRecursive,
} from './util';
-import { correctBinname, envPath, getCurrentGoRoot, getToolFromToolPath, setCurrentGoRoot } from './utils/pathUtils';
+import { envPath, getCurrentGoRoot, getToolFromToolPath, setCurrentGoRoot } from './utils/pathUtils';
// declinedUpdates tracks the tools that the user has declined to update.
const declinedUpdates: Tool[] = [];
@@ -344,7 +345,9 @@
}
export function updateGoVarsFromConfig(): Promise<void> {
- const goRuntimePath = getBinPath('go', false);
+ const {binPath, why} = getBinPathWithExplanation('go', false);
+ const goRuntimePath = binPath;
+
logVerbose(`updateGoVarsFromConfig: found 'go' in ${goRuntimePath}`);
if (!goRuntimePath || !path.isAbsolute(goRuntimePath)) {
// getBinPath returns the absolute path to the tool if it exists.
@@ -386,8 +389,8 @@
// cgo, gopls, and other underlying tools will inherit the environment and attempt
// to locate 'go' from the PATH env var.
// Update the PATH only if users configured to use a different
- // version of go than the system default.
- if (!!goPickedByExtension()) {
+ // version of go than the system default found from PATH (or Path).
+ if (why !== 'path') {
addGoRuntimeBaseToPATH(path.join(getCurrentGoRoot(), 'bin'));
}
initGoStatusBar();
@@ -398,41 +401,6 @@
});
}
-// The go command is picked up by searching directories in PATH by default.
-// But users can override it and force the extension to pick a different
-// one by configuring
-//
-// 1) with the go.environment.choose command, which stores the selection
-// in the workspace memento with the key 'selectedGo',
-// 2) with 'go.alternateTools': { 'go': ... } setting, or
-// 3) with 'go.goroot' setting
-//
-// goPickedByExtension returns the chosen path if the default path should
-// be overridden by above methods.
-// TODO: This logic is duplicated in getBinPath. Centralize this logic.
-function goPickedByExtension(): string | undefined {
- // getFromWorkspaceState('selectedGo')
- const selectedGoPath: string = getFromWorkspaceState('selectedGo')?.binpath;
- if (selectedGoPath) {
- return selectedGoPath;
- }
-
- const cfg = getGoConfig();
-
- // 'go.alternateTools.go'
- const alternateTools: { [key: string]: string } = cfg.get('alternateTools');
- const alternateToolPath: string = alternateTools['go'];
- if (alternateToolPath) {
- return alternateToolPath;
- }
- // 'go.goroot'
- const goRoot: string = cfg.get('goroot');
- if (goRoot) {
- return path.join(goRoot, 'bin', correctBinname('go'));
- }
- return undefined;
-}
-
let alreadyOfferedToInstallTools = false;
export async function offerToInstallTools() {
diff --git a/src/util.ts b/src/util.ts
index fa94f7b..ee361b4 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -20,7 +20,7 @@
import {
envPath,
fixDriveCasingInWindows,
- getBinPathWithPreferredGopathGoroot,
+ getBinPathWithPreferredGopathGorootWithExplanation,
getCurrentGoRoot,
getInferredGopath,
resolveHomeDir,
@@ -477,7 +477,15 @@
}
}
+// getBinPath returns the path to the tool.
export function getBinPath(tool: string, useCache = true): string {
+ const r = getBinPathWithExplanation(tool, useCache);
+ return r.binPath;
+}
+
+// getBinPathWithExplanation returns the path to the tool, and the explanation on why
+// the path was chosen. See getBinPathWithPreferredGopathGorootWithExplanation for details.
+export function getBinPathWithExplanation(tool: string, useCache = true): {binPath: string, why?: string} {
const cfg = getGoConfig();
const alternateTools: { [key: string]: string } = cfg.get('alternateTools');
const alternateToolPath: string = alternateTools[tool];
@@ -487,7 +495,7 @@
selectedGoPath = getFromWorkspaceState('selectedGo')?.binpath;
}
- return getBinPathWithPreferredGopathGoroot(
+ return getBinPathWithPreferredGopathGorootWithExplanation(
tool,
tool === 'go' ? [] : [getToolsGopath(), getCurrentGoPath()],
tool === 'go' && cfg.get('goroot') ? resolvePath(cfg.get('goroot')) : undefined,
diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts
index 74fe93c..e0c2118 100644
--- a/src/utils/pathUtils.ts
+++ b/src/utils/pathUtils.ts
@@ -38,23 +38,37 @@
preferredGopaths: string[],
preferredGoroot?: string,
alternateTool?: string,
- useCache = true,
+ useCache = true
): string {
+ const r = getBinPathWithPreferredGopathGorootWithExplanation(
+ toolName, preferredGopaths, preferredGoroot, alternateTool, useCache);
+ return r.binPath;
+}
+
+// Is same as getBinPAthWithPreferredGopathGoroot, but returns why the
+// returned path was chosen.
+export function getBinPathWithPreferredGopathGorootWithExplanation(
+ toolName: string,
+ preferredGopaths: string[],
+ preferredGoroot?: string,
+ alternateTool?: string,
+ useCache = true,
+): {binPath: string, why?: string} {
if (alternateTool && path.isAbsolute(alternateTool) && executableFileExists(alternateTool)) {
binPathCache[toolName] = alternateTool;
- return alternateTool;
+ return {binPath: alternateTool, why: 'alternateTool'};
}
// FIXIT: this cache needs to be invalidated when go.goroot or go.alternateTool is changed.
if (useCache && binPathCache[toolName]) {
- return binPathCache[toolName];
+ return {binPath: binPathCache[toolName], why: 'cached'};
}
const binname = alternateTool && !path.isAbsolute(alternateTool) ? alternateTool : toolName;
const pathFromGoBin = getBinPathFromEnvVar(binname, process.env['GOBIN'], false);
if (pathFromGoBin) {
binPathCache[toolName] = pathFromGoBin;
- return pathFromGoBin;
+ return {binPath: pathFromGoBin, why: 'gobin'};
}
for (const preferred of preferredGopaths) {
@@ -63,7 +77,7 @@
const pathFrompreferredGoPath = getBinPathFromEnvVar(binname, preferred, true);
if (pathFrompreferredGoPath) {
binPathCache[toolName] = pathFrompreferredGoPath;
- return pathFrompreferredGoPath;
+ return {binPath: pathFrompreferredGoPath, why: 'gopath'};
}
}
}
@@ -72,14 +86,14 @@
const pathFromGoRoot = getBinPathFromEnvVar(binname, preferredGoroot || getCurrentGoRoot(), true);
if (pathFromGoRoot) {
binPathCache[toolName] = pathFromGoRoot;
- return pathFromGoRoot;
+ return {binPath: pathFromGoRoot, why: 'goroot'};
}
// Finally search PATH parts
const pathFromPath = getBinPathFromEnvVar(binname, envPath, false);
if (pathFromPath) {
binPathCache[toolName] = pathFromPath;
- return pathFromPath;
+ return {binPath: pathFromPath, why: 'path'};
}
// Check default path for go
@@ -87,13 +101,13 @@
const defaultPathForGo = process.platform === 'win32' ? 'C:\\Go\\bin\\go.exe' : '/usr/local/go/bin/go';
if (executableFileExists(defaultPathForGo)) {
binPathCache[toolName] = defaultPathForGo;
- return defaultPathForGo;
+ return {binPath: defaultPathForGo, why: 'default'};
}
- return;
+ return {binPath: ''};
}
// Else return the binary name directly (this will likely always fail downstream)
- return toolName;
+ return {binPath: toolName};
}
/**
