[vscode-go] src/goMain: suggest tool updates if tools were compiled with old go
39 views
Skip to first unread message
Hyang-Ah Hana Kim (Gerrit)
Feb 14, 2022, 6:07:44 PM2/14/22
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Suzy Mueller, kokoro, golang-co...@googlegroups.com
Hyang-Ah Hana Kim submitted this change.
Approvals:
Suzy Mueller: Looks good to me, approved; Trusted
Hyang-Ah Hana Kim: Trusted; Run TryBots
kokoro: TryBots succeeded
src/goMain: suggest tool updates if tools were compiled with old go
Suggest update only if the go major version is newer than the
go version used to compile the tools.
And change GoVersion constructor to reject the old dev version format
used before go1.17.
Fixes golang/vscode-go#1998
Change-Id: I9aeea0438d5530b1b0f6190784a9eb489efbb877
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/385181
Trust: Hyang-Ah Hana Kim <hya...@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hya...@gmail.com>
TryBot-Result: kokoro <noreply...@google.com>
Reviewed-by: Suzy Mueller <suz...@golang.org>
Trust: Suzy Mueller <suz...@golang.org>
---
M src/goInstallTools.ts
M src/goMain.ts
M src/util.ts
M test/integration/install.test.ts
M test/integration/utils.test.ts
5 files changed, 209 insertions(+), 19 deletions(-)
diff --git a/src/goInstallTools.ts b/src/goInstallTools.ts
index d8f7b13..10f6b46 100644
--- a/src/goInstallTools.ts
+++ b/src/goInstallTools.ts
@@ -703,9 +703,15 @@
path golang.org/x/tools/gopls
mod golang.org/x/tools/gopls (devel)
dep github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
+
+ if the binary was built with a dev version of go, in module mode.
+ /Users/hakim/go/bin/gopls: devel go1.18-41f485b9a7 Mon Jan 31 13:43:52 2022 +0000
+ path golang.org/x/tools/gopls
+ mod golang.org/x/tools/gopls v0.8.0-pre.1 h1:6iHi9bCJ8XndQtBEFFG/DX+eTJrf2lKFv4GI3zLeDOo=
+ ...
*/
const lines = stdout.split('\n', 3);
- const goVersion = lines[0].split(/\s+/)[1];
+ const goVersion = lines[0] && lines[0].match(/\s+(go\d+.\d+\S*)/)[1];
const moduleVersion = lines[2].split(/\s+/)[3];
return { goVersion, moduleVersion };
} catch (e) {
diff --git a/src/goMain.ts b/src/goMain.ts
index f9bdccc..585d5f2 100644
--- a/src/goMain.ts
+++ b/src/goMain.ts
@@ -72,7 +72,7 @@
testPrevious,
testWorkspace
} from './goTest';
-import { getConfiguredTools } from './goTools';
+import { getConfiguredTools, Tool } from './goTools';
import { vetCode } from './goVet';
import { pickGoProcess, pickProcess } from './pickProcess';
import {
@@ -95,6 +95,7 @@
getGoVersion,
getToolsGopath,
getWorkspaceFolderPath,
+ GoVersion,
handleDiagnosticErrors,
isGoPathSet,
resolvePath
@@ -828,10 +829,88 @@
}
}
+// exported for testing
+export async function listOutdatedTools(configuredGoVersion: GoVersion, allTools: Tool[]): Promise<Tool[]> {
+ if (!configuredGoVersion || !configuredGoVersion.sv) {
+ return [];
+ }
+
+ const { major, minor } = configuredGoVersion.sv;
+
+ const oldTools = await Promise.all(
+ allTools.map(async (tool) => {
+ const toolPath = getBinPath(tool.name);
+ if (!path.isAbsolute(toolPath)) {
+ return;
+ }
+ const m = await inspectGoToolVersion(toolPath);
+ if (!m) {
+ console.log(`failed to get go tool version: ${toolPath}`);
+ return;
+ }
+ const { goVersion } = m;
+ if (!goVersion) {
+ // TODO: we cannot tell whether the tool was compiled with a newer version of go
+ // or compiled in an unconventional way.
+ return;
+ }
+ const toolGoVersion = new GoVersion('', `go version ${goVersion} os/arch`);
+ if (!toolGoVersion || !toolGoVersion.sv) {
+ return tool;
+ }
+ if (
+ major > toolGoVersion.sv.major ||
+ (major === toolGoVersion.sv.major && minor > toolGoVersion.sv.minor)
+ ) {
+ return tool;
+ }
+ // special case: if the tool was compiled with beta or rc, and the current
+ // go version is a stable version, let's ask to recompile.
+ if (
+ major === toolGoVersion.sv.major &&
+ minor === toolGoVersion.sv.minor &&
+ (goVersion.includes('beta') || goVersion.includes('rc')) &&
+ // We assume tools compiled with different rc/beta need to be recompiled.
+ // We test the inequality by checking whether the exact beta or rc version
+ // appears in the `go version` output. e.g.,
+ // configuredGoVersion.version goVersion(tool) update
+ // 'go version go1.18 ...' 'go1.18beta1' Yes
+ // 'go version go1.18beta1 ...' 'go1.18beta1' No
+ // 'go version go1.18beta2 ...' 'go1.18beta1' Yes
+ // 'go version go1.18rc1 ...' 'go1.18beta1' Yes
+ // 'go version go1.18rc1 ...' 'go1.18' No
+ // 'go version devel go1.18-deadbeaf ...' 'go1.18beta1' No (* rare)
+ !configuredGoVersion.version.includes(goVersion)
+ ) {
+ return tool;
+ }
+ return;
+ })
+ );
+ return oldTools.filter((tool) => !!tool);
+}
+
async function suggestUpdates(ctx: vscode.ExtensionContext) {
- // TODO(hyangah): this is to clean up the unused key. Delete this code in 2021 Dec.
- if (ctx.globalState.get('toolsGoInfo')) {
- ctx.globalState.update('toolsGoInfo', null);
+ const configuredGoVersion = await getGoVersion();
+ if (!configuredGoVersion || configuredGoVersion.lt('1.12')) {
+ // User is using an ancient or a dev version of go. Don't suggest updates -
+ // user should know what they are doing.
+ return;
+ }
+
+ const allTools = getConfiguredTools(configuredGoVersion, getGoConfig(), getGoplsConfig());
+ const toolsToUpdate = await listOutdatedTools(configuredGoVersion, allTools);
+ if (toolsToUpdate.length > 0) {
+ const updateToolsCmdText = 'Update tools';
+ const selected = await vscode.window.showWarningMessage(
+ `Tools (${toolsToUpdate.map((tool) => tool.name).join(', ')}) need recompiling to work with ${
+ configuredGoVersion.version
+ }`,
+ updateToolsCmdText
+ );
+ if (selected === updateToolsCmdText) {
+ installTools(toolsToUpdate, configuredGoVersion);
+ }
}
}
diff --git a/src/util.ts b/src/util.ts
index 132db11..ff69431 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -96,7 +96,7 @@
constructor(public binaryPath: string, public version: string) {
const matchesRelease = /^go version go(\d\.\d+\S*)\s+/.exec(version);
- const matchesDevel = /^go version devel (\S+)\s+/.exec(version);
+ const matchesDevel = /^go version devel go(\d\.\d+\S*)\s+/.exec(version);
if (matchesRelease) {
// note: semver.parse does not work with Go version string like go1.14.
const sv = semver.coerce(matchesRelease[1]);
diff --git a/test/integration/install.test.ts b/test/integration/install.test.ts
index 6272c8a..3de0755 100644
--- a/test/integration/install.test.ts
+++ b/test/integration/install.test.ts
@@ -21,6 +21,9 @@
import vscode = require('vscode');
import { isInPreviewMode } from '../../src/goLanguageServer';
import { allToolsInformation } from '../../src/goToolsInformation';
+import { listOutdatedTools } from '../../src/goMain';
+import * as goInstallTools from '../../src/goInstallTools';
+import * as utilModule from '../../src/util';
interface installationTestCase {
name: string;
@@ -238,27 +241,112 @@
suite('getConfiguredTools', () => {
test('do not require legacy tools when using language server', async () => {
- const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: true }, {});
+ const configured = getConfiguredTools(
+ fakeGoVersion('go version go1.15.6 linux/amd64'),
+ { useLanguageServer: true },
+ {}
+ );
const got = configured.map((tool) => tool.name) ?? [];
assert(got.includes('gopls'), `omitted 'gopls': ${JSON.stringify(got)}`);
assert(!got.includes('guru') && !got.includes('gocode'), `suggested legacy tools: ${JSON.stringify(got)}`);
});
test('do not require gopls when not using language server', async () => {
- const configured = getConfiguredTools(fakeGoVersion('1.15.6'), { useLanguageServer: false }, {});
+ const configured = getConfiguredTools(
+ fakeGoVersion('go version go1.15.6 linux/amd64'),
+ { useLanguageServer: false },
+ {}
+ );
const got = configured.map((tool) => tool.name) ?? [];
assert(!got.includes('gopls'), `suggested 'gopls': ${JSON.stringify(got)}`);
assert(got.includes('guru') && got.includes('gocode'), `omitted legacy tools: ${JSON.stringify(got)}`);
});
test('do not require gopls when the go version is old', async () => {
- const configured = getConfiguredTools(fakeGoVersion('1.9'), { useLanguageServer: true }, {});
+ const configured = getConfiguredTools(
+ fakeGoVersion('go version go1.9 linux/amd64'),
+ { useLanguageServer: true },
+ {}
+ );
const got = configured.map((tool) => tool.name) ?? [];
assert(!got.includes('gopls'), `suggested 'gopls' for old go: ${JSON.stringify(got)}`);
assert(got.includes('guru') && got.includes('gocode'), `omitted legacy tools: ${JSON.stringify(got)}`);
});
});
-function fakeGoVersion(version: string) {
- return new GoVersion('/path/to/go', `go version go${version} windows/amd64`);
+function fakeGoVersion(versionStr: string) {
+ return new GoVersion('/path/to/go', versionStr);
}
+
+suite('listOutdatedTools', () => {
+ let sandbox: sinon.SinonSandbox;
+ setup(() => {
+ sandbox = sinon.createSandbox();
+ });
+ teardown(() => sandbox.restore());
+
+ async function runTest(goVersion: string | undefined, tools: { [key: string]: string | undefined }) {
+ const binPathStub = sandbox.stub(utilModule, 'getBinPath');
+ const versionStub = sandbox.stub(goInstallTools, 'inspectGoToolVersion');
+ for (const tool in tools) {
+ binPathStub.withArgs(tool).returns(`/bin/${tool}`);
+ versionStub.withArgs(`/bin/${tool}`).returns(Promise.resolve({ goVersion: tools[tool] }));
+ }
+
+ const toolsToCheck = Object.keys(tools).map((tool) => getToolAtVersion(tool));
+
+ const configuredGoVersion = goVersion ? fakeGoVersion(goVersion) : undefined;
+ const toolsToUpdate = await listOutdatedTools(configuredGoVersion, toolsToCheck);
+ return toolsToUpdate.map((tool) => tool.name).filter((tool) => !!tool);
+ }
+
+ test('minor version difference requires updates', async () => {
+ const x = await runTest('go version go1.18 linux/amd64', {
+ gopls: 'go1.16', // 1.16 < 1.18
+ dlv: 'go1.17', // 1.17 < 1.18
+ staticcheck: 'go1.18', // 1.18 == 1.18
+ gotests: 'go1.19' // 1.19 > 1.18
+ });
+ assert.deepStrictEqual(x, ['gopls', 'dlv']);
+ });
+ test('patch version difference does not require updates', async () => {
+ const x = await runTest('go version go1.16.1 linux/amd64', {
+ gopls: 'go1.16', // 1.16 < 1.16.1
+ dlv: 'go1.16.1', // 1.16.1 == 1.16.1
+ staticcheck: 'go1.16.2', // 1.16.2 > 1.16.1
+ gotests: 'go1.16rc1' // 1.16rc1 != 1.16.1
+ });
+ assert.deepStrictEqual(x, ['gotests']);
+ });
+ test('go is beta version', async () => {
+ const x = await runTest('go version go1.18beta2 linux/amd64', {
+ gopls: 'go1.17.1', // 1.17.1 < 1.18beta2
+ dlv: 'go1.18beta1', // 1.18beta1 != 1.18beta2
+ staticcheck: 'go1.18beta2', // 1.18beta2 == 1.18beta2
+ gotests: 'go1.18' // 1.18 > 1.18beta2
+ });
+ assert.deepStrictEqual(x, ['gopls', 'dlv']);
+ });
+ test('go is dev version', async () => {
+ const x = await runTest('go version devel go1.18-41f485b9a7 linux/amd64', {
+ gopls: 'go1.17.1',
+ dlv: 'go1.18beta1',
+ staticcheck: 'go1.18',
+ gotests: 'go1.19'
+ });
+ assert.deepStrictEqual(x, []);
+ });
+ test('go is unknown version', async () => {
+ const x = await runTest('', {
+ gopls: 'go1.17.1'
+ });
+ assert.deepStrictEqual(x, []);
+ });
+ test('tools are unknown versions', async () => {
+ const x = await runTest('go version go1.17 linux/amd64', {
+ gopls: undefined, // this can be because gopls was compiled with go1.18 or it's too old.
+ dlv: 'go1.16.1'
+ });
+ assert.deepStrictEqual(x, ['dlv']);
+ });
+});
diff --git a/test/integration/utils.test.ts b/test/integration/utils.test.ts
index c13a939..690a27e 100644
--- a/test/integration/utils.test.ts
+++ b/test/integration/utils.test.ts
@@ -30,15 +30,9 @@
// [input, wantFormat, wantFormatIncludePrerelease, wantIsValid]
const testCases: [string | undefined, string, string, boolean][] = [
[
- 'go version devel +a295d59d Fri Jun 26 19:00:25 2020 +0000 darwin/amd64',
- 'devel +a295d59d',
- 'devel +a295d59d',
- true
- ],
- [
'go version devel go1.17-756fd56bbf Thu Apr 29 01:15:34 2021 +0000 darwin/amd64',
- 'devel go1.17-756fd56bbf',
- 'devel go1.17-756fd56bbf',
+ 'devel 1.17-756fd56bbf',
+ 'devel 1.17-756fd56bbf',
true
],
['go version go1.14 darwin/amd64', '1.14.0', '1.14', true],
To view, visit change 385181. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages