Skip to content

Commit 772b35f

Browse files
authored
fix(amazonq): improve lsp cleanup logic (#7075)
## Problem When switching between different manifests, a newly downloaded version might chronologically be older than all previously downloaded versions, even though it's marked as the latest version in its own manifest. This is why we are getting the EPIPE error when trying out the alpha manifest. The cleanup deletes the language server right after it was downloaded ## Solution In such cases, we skip the cleanup process to preserve this version. Otherwise we will get an EPIPE error. At this point the version that was downloaded shouldn't be delisted, so we don't want to make sure its not removed --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 05cf555 commit 772b35f

File tree

3 files changed

+25
-4
lines changed

3 files changed

+25
-4
lines changed

packages/core/src/shared/lsp/baseLspInstaller.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@ export abstract class BaseLspInstaller<T extends ResourcePaths = ResourcePaths,
5151

5252
await this.postInstall(assetDirectory)
5353

54-
const deletedVersions = await cleanLspDownloads(manifest.versions, nodePath.dirname(assetDirectory))
54+
const deletedVersions = await cleanLspDownloads(
55+
installationResult.version,
56+
manifest.versions,
57+
nodePath.dirname(assetDirectory)
58+
)
5559
this.logger.debug(`cleaning old LSP versions deleted ${deletedVersions.length} versions`)
5660

5761
return {

packages/core/src/shared/lsp/utils/cleanup.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ function isDelisted(manifestVersions: LspVersion[], targetVersion: string): bool
2323
* @param downloadDirectory
2424
* @returns array of deleted versions.
2525
*/
26-
export async function cleanLspDownloads(manifestVersions: LspVersion[], downloadDirectory: string): Promise<string[]> {
26+
export async function cleanLspDownloads(
27+
latestInstalledVersion: string,
28+
manifestVersions: LspVersion[],
29+
downloadDirectory: string
30+
): Promise<string[]> {
2731
const downloadedVersions = await getDownloadedVersions(downloadDirectory)
2832
const [delistedVersions, remainingVersions] = partition(downloadedVersions, (v: string) =>
2933
isDelisted(manifestVersions, v)
@@ -40,6 +44,15 @@ export async function cleanLspDownloads(manifestVersions: LspVersion[], download
4044
}
4145

4246
for (const v of sort(remainingVersions).slice(0, -2)) {
47+
/**
48+
* When switching between different manifests, the following edge case can occur:
49+
* A newly downloaded version might chronologically be older than all previously downloaded versions,
50+
* even though it's marked as the latest version in its own manifest.
51+
* In such cases, we skip the cleanup process to preserve this version. Otherwise we will get an EPIPE error
52+
*/
53+
if (v === latestInstalledVersion) {
54+
continue
55+
}
4356
await fs.delete(path.join(downloadDirectory, v), { force: true, recursive: true })
4457
deletedVersions.push(v)
4558
}

packages/core/src/test/shared/lsp/utils/cleanup.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ describe('cleanLSPDownloads', function () {
4141

4242
it('keeps two newest versions', async function () {
4343
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
44-
const deleted = await cleanLspDownloads([], installationDir.fsPath)
44+
const deleted = await cleanLspDownloads('2.1.1', [], installationDir.fsPath)
4545

4646
const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
4747
assert.strictEqual(result.length, 2)
@@ -53,6 +53,7 @@ describe('cleanLSPDownloads', function () {
5353
it('deletes delisted versions', async function () {
5454
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
5555
const deleted = await cleanLspDownloads(
56+
'2.1.1',
5657
[{ serverVersion: '1.1.1', isDelisted: true, targets: [] }],
5758
installationDir.fsPath
5859
)
@@ -67,6 +68,7 @@ describe('cleanLSPDownloads', function () {
6768
it('handles case where less than 2 versions are not delisted', async function () {
6869
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
6970
const deleted = await cleanLspDownloads(
71+
'1.0.1',
7072
[
7173
{ serverVersion: '1.1.1', isDelisted: true, targets: [] },
7274
{ serverVersion: '2.1.1', isDelisted: true, targets: [] },
@@ -83,7 +85,7 @@ describe('cleanLSPDownloads', function () {
8385

8486
it('handles case where less than 2 versions exist', async function () {
8587
await fakeInstallVersions(['1.0.0'], installationDir.fsPath)
86-
const deleted = await cleanLspDownloads([], installationDir.fsPath)
88+
const deleted = await cleanLspDownloads('1.0.0', [], installationDir.fsPath)
8789

8890
const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
8991
assert.strictEqual(result.length, 1)
@@ -93,6 +95,7 @@ describe('cleanLSPDownloads', function () {
9395
it('does not install delisted version when no other option exists', async function () {
9496
await fakeInstallVersions(['1.0.0'], installationDir.fsPath)
9597
const deleted = await cleanLspDownloads(
98+
'1.0.0',
9699
[{ serverVersion: '1.0.0', isDelisted: true, targets: [] }],
97100
installationDir.fsPath
98101
)
@@ -105,6 +108,7 @@ describe('cleanLSPDownloads', function () {
105108
it('ignores invalid versions', async function () {
106109
await fakeInstallVersions(['1.0.0', '.DS_STORE'], installationDir.fsPath)
107110
const deleted = await cleanLspDownloads(
111+
'1.0.0',
108112
[{ serverVersion: '1.0.0', isDelisted: true, targets: [] }],
109113
installationDir.fsPath
110114
)

0 commit comments

Comments
 (0)