src/goInstallTools: use GOBIN correctly installing dlv-dap/gocode-gomod
We incorrectly added `bin` to the final binary path
when the destination was determined by GOBIN env var.
Changed install.test.ts to set the temporary GOPATH instead of
stubbing getToolsGopath. When populating tools installation
environment, the extension ignores GOBIN if users configured
go.toolsGopath currently, so depending on toolsGopath to point
to the temporary GOPATH prevents from testing GOBIN-related
behavior.
Fixes golang/vscode-go#1430
Change-Id: Id6450e9422a791f6d5e7810f6bbbec1bc695e924
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/310769
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: Rebecca Stambler <rstambler@golang.org>
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index 48cbf1b..b234b51 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -33,6 +33,7 @@
getBinPath,
getBinPathWithExplanation,
getCheckForToolsUpdatesConfig,
+ getCurrentGoPath,
getGoVersion,
getTempFilePath,
getWorkspaceFolderPath,
@@ -280,12 +281,16 @@
logVerbose(`install: ${goBinary} ${args.join(' ')}\n${stdout}${stderr}`);
if (hasModSuffix(tool) || tool.name === 'dlv-dap') {
// Actual installation of the -gomod tool and dlv-dap is done by running go build.
- const gopath = env['GOBIN'] || env['GOPATH'];
- if (!gopath) {
+ let destDir = env['GOBIN'];
+ if (!destDir) {
+ const gopath0 = env['GOPATH']?.split(path.delimiter)[0];
+ destDir = gopath0 ? path.join(gopath0, 'bin') : undefined;
+ }
+ if (!destDir) {
throw new Error('GOBIN/GOPATH not configured in environment');
}
- const destDir = gopath.split(path.delimiter)[0];
- const outputFile = path.join(destDir, 'bin', process.platform === 'win32' ? `${tool.name}.exe` : tool.name);
+ const outputFile = path.join(destDir, correctBinname(tool.name));
+
// go build does not take @version suffix yet.
const importPath = getImportPath(tool, goVersion);
await execFile(goBinary, ['build', '-o', outputFile, importPath], opts);
diff --git a/test/integration/install.test.ts b/test/integration/install.test.ts
index a4375d6..85dd96c 100644
--- a/test/integration/install.test.ts
+++ b/test/integration/install.test.ts
@@ -7,7 +7,6 @@
import AdmZip = require('adm-zip');
import * as assert from 'assert';
import * as config from '../../src/config';
-import { toolInstallationEnvironment } from '../../src/goEnv';
import { inspectGoToolVersion, installTools } from '../../src/goInstallTools';
import { allToolsInformation, getConfiguredTools, getTool, getToolAtVersion } from '../../src/goTools';
import { getBinPath, getGoVersion, GoVersion, rmdirRecursive } from '../../src/util';
@@ -39,7 +38,7 @@
let tmpToolsGopath: string;
let tmpToolsGopath2: string;
let sandbox: sinon.SinonSandbox;
- let toolsGopathStub: sinon.SinonStub;
+ let toolsGopath: string;
setup(() => {
// Create a temporary directory in which to install tools.
@@ -48,11 +47,9 @@
// a temporary directory to be used as the second GOPATH element.
tmpToolsGopath2 = fs.mkdtempSync(path.join(os.tmpdir(), 'install-test2'));
- const toolsGopath = tmpToolsGopath + path.delimiter + tmpToolsGopath2;
+ toolsGopath = tmpToolsGopath + path.delimiter + tmpToolsGopath2;
sandbox = sinon.createSandbox();
- const utils = require('../../src/util');
- toolsGopathStub = sandbox.stub(utils, 'getToolsGopath').returns(toolsGopath);
});
teardown(async () => {
@@ -74,7 +71,10 @@
// runTest actually executes the logic of the test.
// If withLocalProxy is true, the test does not require internet.
- async function runTest(testCases: installationTestCase[], withLocalProxy?: boolean) {
+ // If withGOBIN is true, the test will set GOBIN env var.
+ async function runTest(testCases: installationTestCase[], withLocalProxy?: boolean, withGOBIN?: boolean) {
+ const gobin = withLocalProxy && withGOBIN ? path.join(tmpToolsGopath, 'gobin') : undefined;
+
let proxyDir: string;
let configStub: sinon.SinonStub;
if (withLocalProxy) {
@@ -83,14 +83,21 @@
toolsEnvVars: {
value: {
GOPROXY: url.pathToFileURL(proxyDir),
- GOSUMDB: 'off'
+ GOSUMDB: 'off',
+ GOBIN: gobin
}
- }
+ },
+ gopath: { value: toolsGopath }
});
configStub = sandbox.stub(config, 'getGoConfig').returns(goConfig);
} else {
- const env = toolInstallationEnvironment();
- console.log(`Installing tools using GOPROXY=${env['GOPROXY']}`);
+ const goConfig = Object.create(vscode.workspace.getConfiguration('go'), {
+ toolsEnvVars: {
+ value: { GOBIN: gobin }
+ },
+ gopath: { value: toolsGopath }
+ });
+ configStub = sandbox.stub(config, 'getGoConfig').returns(goConfig);
}
const missingTools = testCases.map((tc) => getToolAtVersion(tc.name));
@@ -104,7 +111,9 @@
checks.push(
new Promise<void>(async (resolve) => {
// Check that the expect tool has been installed to $GOPATH/bin.
- const bin = path.join(tmpToolsGopath, 'bin', correctBinname(tc.name));
+ const bin = gobin
+ ? path.join(gobin, correctBinname(tc.name))
+ : path.join(tmpToolsGopath, 'bin', correctBinname(tc.name));
const ok = await exists(bin);
if (!ok) {
assert.fail(`expected ${bin}, not found`);
@@ -124,8 +133,6 @@
}
await Promise.all(checks);
- sandbox.assert.calledWith(toolsGopathStub);
-
if (withLocalProxy) {
sandbox.assert.calledWith(configStub);
rmdirRecursive(proxyDir);
@@ -160,6 +167,22 @@
);
});
+ test('Install multiple tools with a local proxy & GOBIN', async () => {
+ await runTest(
+ [
+ { name: 'gopls', versions: ['v0.1.0', 'v1.0.0-pre.1', 'v1.0.0'], wantVersion: 'v1.0.0' },
+ { name: 'guru', versions: ['v1.0.0'], wantVersion: 'v1.0.0' },
+ {
+ name: 'dlv-dap',
+ versions: ['v1.0.0', 'master'],
+ wantVersion: 'v' + getTool('dlv-dap').latestVersion!.toString()
+ }
+ ],
+ true, // LOCAL PROXY
+ true // GOBIN
+ );
+ });
+
test('Install all tools via GOPROXY', async () => {
// Only run this test if we are in CI before a Nightly release.
if (!shouldRunSlowTests()) {
