src/goEnvironmentStatus.ts: clear pre-installed terminal PATH mutation
Changes to vscode.EnvironmentVariableCollection persist across
vscode sessions, so when we decide not to mutate PATH, we need to clear
the preexisting changes. The new function 'clearGoRuntimeBaseFromPATH'
reverses almost all of what addGoRuntimeBaseToPATH did on the persisted
state.
Manually tested by setting/unsetting "go.alternateTools".
Also, fixes the case where 'go.alternateTools' is set as an alternate
tool name rather than an absolute path - in that case, we search the
alternate tool from PATH. In this case, the reason shouldn't be 'path'.
Manually tested by setting
"go.alternateTools": { "go": "mygo.sh" }
where "mygo.sh" is another binary that exists in PATH and shells out go.
In addition to this, this change fixes an exception thrown while
calling updateIntegratedTerminal when no 'go' executable is found from
the default PATH. In that case, getBinPathFromEnvVar returns null,
and path.dirname throws an error for non-string params.
This results in the failure of updating terminal's PATH change and
the users will not see the picked go from the terminal.
This is a bug preexisted before 0.17.1
Manually tested by launching code without go in PATH.
Updates golang/vscode-go#679
Fixes golang/vscode-go#713
Change-Id: I240694cb4425e81998299ab38097393a0f3faf46
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/258557
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/goEnvironmentStatus.ts b/src/goEnvironmentStatus.ts
index 8a3d38d..07d596e 100644
--- a/src/goEnvironmentStatus.ts
+++ b/src/goEnvironmentStatus.ts
@@ -303,6 +303,16 @@
// PATH value cached before addGoRuntimeBaseToPath modified.
let defaultPathEnv = '';
+function pathEnvVarName(): string|undefined {
+ if (process.env.hasOwnProperty('PATH')) {
+ return 'PATH';
+ } else if (process.platform === 'win32' && process.env.hasOwnProperty('Path')) {
+ return 'Path';
+ } else {
+ return;
+ }
+}
+
// 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?
@@ -310,12 +320,8 @@
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 {
+ const pathEnvVar = pathEnvVarName();
+ if (!pathEnvVar) {
logVerbose(`couldn't find PATH property in process.env`);
return;
}
@@ -359,14 +365,33 @@
process.env[pathEnvVar] = pathVars.join(path.delimiter);
}
+// Clear terminal PATH environment modification previously installed
+// using addGoRuntimeBaseToPATH.
+// In particular, changes to vscode.EnvironmentVariableCollection persist across
+// vscode sessions, so when we decide not to mutate PATH, we need to clear
+// the preexisting changes.
+export function clearGoRuntimeBaseFromPATH() {
+ if (terminalCreationListener) {
+ const l = terminalCreationListener;
+ terminalCreationListener = undefined;
+ l.dispose();
+ }
+ const pathEnvVar = pathEnvVarName();
+ if (!pathEnvVar) {
+ logVerbose(`couldn't find PATH property in process.env`);
+ return;
+ }
+ environmentVariableCollection?.delete(pathEnvVar);
+}
+
/**
* update the PATH variable in the given terminal to default to the currently selected Go
*/
export async function updateIntegratedTerminal(terminal: vscode.Terminal): Promise<void> {
if (!terminal) { return; }
const gorootBin = path.join(getCurrentGoRoot(), 'bin');
- const defaultGoRuntimeBin = path.dirname(getBinPathFromEnvVar('go', defaultPathEnv, false));
- if (gorootBin === defaultGoRuntimeBin) {
+ const defaultGoRuntime = getBinPathFromEnvVar('go', defaultPathEnv, false);
+ if (defaultGoRuntime && gorootBin === path.dirname(defaultGoRuntime)) {
return;
}
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index bc4f5a4..d80fe21 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -12,7 +12,7 @@
import util = require('util');
import vscode = require('vscode');
import { toolExecutionEnvironment, toolInstallationEnvironment } from './goEnv';
-import { addGoRuntimeBaseToPATH, initGoStatusBar } from './goEnvironmentStatus';
+import { addGoRuntimeBaseToPATH, clearGoRuntimeBaseFromPATH, initGoStatusBar } from './goEnvironmentStatus';
import { getLanguageServerToolPath } from './goLanguageServer';
import { logVerbose } from './goLogging';
import { restartLanguageServer } from './goMain';
@@ -392,6 +392,9 @@
// version of go than the system default found from PATH (or Path).
if (why !== 'path') {
addGoRuntimeBaseToPATH(path.join(getCurrentGoRoot(), 'bin'));
+ } else {
+ // clear pre-existing terminal PATH mutation logic set up by this extension.
+ clearGoRuntimeBaseFromPATH();
}
initGoStatusBar();
// TODO: restart language server or synchronize with language server update.
diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts
index e0c2118..f133df8 100644
--- a/src/utils/pathUtils.ts
+++ b/src/utils/pathUtils.ts
@@ -19,7 +19,8 @@
export const envPath = process.env['PATH'] || (process.platform === 'win32' ? process.env['Path'] : null);
-export function getBinPathFromEnvVar(toolName: string, envVarValue: string, appendBinToPath: boolean): string {
+// find the tool's path from the given PATH env var, or null if the tool is not found.
+export function getBinPathFromEnvVar(toolName: string, envVarValue: string, appendBinToPath: boolean): string|null {
toolName = correctBinname(toolName);
if (envVarValue) {
const paths = envVarValue.split(path.delimiter);
@@ -45,7 +46,7 @@
return r.binPath;
}
-// Is same as getBinPAthWithPreferredGopathGoroot, but returns why the
+// Is same as getBinPathWithPreferredGopathGoroot, but returns why the
// returned path was chosen.
export function getBinPathWithPreferredGopathGorootWithExplanation(
toolName: string,
@@ -65,10 +66,11 @@
}
const binname = alternateTool && !path.isAbsolute(alternateTool) ? alternateTool : toolName;
+ const found = (why: string) => binname === toolName ? why : 'alternateTool';
const pathFromGoBin = getBinPathFromEnvVar(binname, process.env['GOBIN'], false);
if (pathFromGoBin) {
binPathCache[toolName] = pathFromGoBin;
- return {binPath: pathFromGoBin, why: 'gobin'};
+ return {binPath: pathFromGoBin, why: binname === toolName ? 'gobin' : 'alternateTool'};
}
for (const preferred of preferredGopaths) {
@@ -77,7 +79,7 @@
const pathFrompreferredGoPath = getBinPathFromEnvVar(binname, preferred, true);
if (pathFrompreferredGoPath) {
binPathCache[toolName] = pathFrompreferredGoPath;
- return {binPath: pathFrompreferredGoPath, why: 'gopath'};
+ return {binPath: pathFrompreferredGoPath, why: found('gopath')};
}
}
}
@@ -86,14 +88,14 @@
const pathFromGoRoot = getBinPathFromEnvVar(binname, preferredGoroot || getCurrentGoRoot(), true);
if (pathFromGoRoot) {
binPathCache[toolName] = pathFromGoRoot;
- return {binPath: pathFromGoRoot, why: 'goroot'};
+ return {binPath: pathFromGoRoot, why: found('goroot')};
}
// Finally search PATH parts
const pathFromPath = getBinPathFromEnvVar(binname, envPath, false);
if (pathFromPath) {
binPathCache[toolName] = pathFromPath;
- return {binPath: pathFromPath, why: 'path'};
+ return {binPath: pathFromPath, why: found('path')};
}
// Check default path for go
