Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Fixes #2122 tools are also found at GOBIN path #3001

Merged
merged 4 commits into from
Jan 28, 2020
Merged

Fixes #2122 tools are also found at GOBIN path #3001

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e19f3f
go tools are also resolved on GOBIN path. Logging correct path on GOB…
marcel-basel Jan 25, 2020
d917583
Merge branch 'master' of https://github.com/microsoft/vscode-go into …
marcel-basel Jan 27, 2020
8660153
no copy of process.env, rem. condition, ren. const
marcel-basel Jan 27, 2020
83e3360
When using GOBIN, the message should reflect the same
ramya-rao-a Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/goInstallTools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,12 @@ export function installTools(missing: Tool[], goVersion: GoVersion): Promise<voi
return;
}

let installingMsg = `Installing ${missing.length} ${missing.length > 1 ? 'tools' : 'tool'} at ${toolsGopath}${
path.sep
}bin`;
let binpath = toolsGopath + path.sep + "bin";
if(envForTools['GOBIN'] != undefined && envForTools['GOBIN'] != ''){
binpath = "GOBIN path " + envForTools['GOBIN']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If GOBIN is set, then we can essentially skip the code path

vscode-go/src/goInstallTools.ts

Lines 124 to 145 in 8660153

} else {
toolsGopath = getCurrentGoPath();
outputChannel.appendLine(`go.toolsGopath setting is not set. Using GOPATH ${toolsGopath}`);
}
if (toolsGopath) {
const paths = toolsGopath.split(path.delimiter);
toolsGopath = paths[0];
envForTools['GOPATH'] = toolsGopath;
} else {
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;
}
right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have to set the current go path envForTools['GOPATH'] = toolsGopath; since envForTools is passed as options when go get child process is created. GOBIN is only setting the path where the executable should be installed. Right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm fair point

}

let installingMsg = `Installing ${missing.length} ${missing.length > 1 ? 'tools' : 'tool'} at ${binpath}`;

// If the user is on Go >= 1.11, tools should be installed with modules enabled.
// This ensures that users get the latest tagged version, rather than master,
Expand Down
6 changes: 6 additions & 0 deletions src/goPath.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ export function getBinPathWithPreferredGopath(toolName: string, preferredGopaths
}

const binname = alternateTool && !path.isAbsolute(alternateTool) ? alternateTool : toolName;
const pathFromGoBin = getBinPathFromEnvVar(binname, process.env['GOBIN'], false);
if (pathFromGoBin) {
binPathCache[toolName] = pathFromGoBin;
return pathFromGoBin;
}

for (const preferred of preferredGopaths) {
if (typeof preferred === 'string') {
// Search in the preferred GOPATH workspace's bin folder
Expand Down