From 26ec6f7a27a25e4f01713d0c279fa7b6be1ac577 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 30 Jul 2024 12:27:55 -0400 Subject: [PATCH 01/25] Adds ability to install a missing bsc version --- .vscode/launch.json | 4 +-- package-lock.json | 16 ++++++--- package.json | 2 +- src/LanguageServerManager.spec.ts | 57 ++++++++++++++++++++++++++++- src/LanguageServerManager.ts | 59 +++++++++++++++++++++++++++++-- 5 files changed, 127 insertions(+), 11 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 6a33081f..356d879b 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -75,7 +75,7 @@ "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", "args": [ "--timeout", - "0" + "987654" ], "internalConsoleOptions": "openOnSessionStart" } @@ -104,4 +104,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/package-lock.json b/package-lock.json index bfac465b..8c4a4f2c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56,7 +56,7 @@ "@types/fs-extra": "^5.0.4", "@types/glob": "^7.1.1", "@types/mocha": "^7.0.2", - "@types/node": "^12.12.0", + "@types/node": "^20.14.10", "@types/node-ssdp": "^3.3.0", "@types/prompt": "^1.1.2", "@types/resolve": "^1.20.6", @@ -1467,9 +1467,12 @@ "dev": true }, "node_modules/@types/node": { - "version": "12.20.55", - "resolved": "https://registry.npmjs.org/@types/node/-/node-12.20.55.tgz", - "integrity": "sha512-J8xLz7q2OFulZ2cyGTLE1TbbZcjpno7FaN6zdJNrgAdrJ+DZzh/uFR6YrTb4C+nXakvud8Q4+rbhoIWlYQbUFQ==" + "version": "20.14.10", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.14.10.tgz", + "integrity": "sha512-MdiXf+nDuMvY0gJKxyfZ7/6UFsETO7mGKF54MVD/ekJS6HdFtpZFBgrh6Pseu64XTb2MLyFPlbW6hj8HYRQNOQ==", + "dependencies": { + "undici-types": "~5.26.4" + } }, "node_modules/@types/node-ssdp": { "version": "3.3.1", @@ -10542,6 +10545,11 @@ "integrity": "sha512-+A5Sja4HP1M08MaXya7p5LvjuM7K6q/2EaC0+iovj/wOcMsTzMvDFbasi/oSapiwOlt252IqsKqPjCl7huKS0A==", "dev": true }, + "node_modules/undici-types": { + "version": "5.26.5", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-5.26.5.tgz", + "integrity": "sha512-JlCMO+ehdEIKqlFxk6IfVoAUVmgz7cU7zD/h9XZ0qzeosSHmUJVOzSQvvYSYWXkFXC+IfLKSIffhv0sVZup6pA==" + }, "node_modules/universalify": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", diff --git a/package.json b/package.json index 3bfbf516..c08e8ff6 100644 --- a/package.json +++ b/package.json @@ -98,7 +98,7 @@ "@types/fs-extra": "^5.0.4", "@types/glob": "^7.1.1", "@types/mocha": "^7.0.2", - "@types/node": "^12.12.0", + "@types/node": "^20.14.10", "@types/node-ssdp": "^3.3.0", "@types/prompt": "^1.1.2", "@types/resolve": "^1.20.6", diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 581cfbd6..d17e6b82 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -15,6 +15,7 @@ import { LanguageClient, State } from 'vscode-languageclient/node'; +import * as childProcess from 'child_process'; const Module = require('module'); const sinon = createSandbox(); @@ -75,7 +76,7 @@ describe('LanguageServerManager', () => { //disable starting so we can manually test sinon.stub(languageServerManager, 'syncVersionAndTryRun').callsFake(() => Promise.resolve()); - await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']); + await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository'], languageServerManager['logger']); languageServerManager['lspRunTracker'].debounceDelay = 100; @@ -323,4 +324,58 @@ describe('LanguageServerManager', () => { expect(bsdkPath).to.eql(null); }); }); + + describe.only('ensureBscVersionInstalled', function() { + //these tests take a long time (due to running `npm install`) + this.timeout(20_000); + + const storageDir = s`${tempDir}/brighterscript-storage`; + beforeEach(() => { + fsExtra.removeSync(storageDir); + (languageServerManager['context'] as any).globalStorageUri = URI.file(storageDir); + }); + + it('installs a bsc version when not present', async () => { + expect( + await languageServerManager['ensureBscVersionInstalled']('0.65.0') + ).to.eql(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); + expect( + fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`) + ).to.be.true; + }); + + it('reuses the same bsc version when already exists', async () => { + let stub = sinon.stub(childProcess, 'exec'); + expect( + await languageServerManager['ensureBscVersionInstalled']('0.65.0') + ).to.eql(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); + expect( + fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`) + ).to.be.true; + expect(stub.called).to.be.false; + }); + + it('repairs a broken bsc version', async () => { + let stub = sinon.stub(fsExtra, 'remove'); + fsExtra.ensureDirSync( + s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript` + ); + fsExtra.writeFileSync( + s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript/package.json`, + 'bad json' + ); + + expect( + await languageServerManager['ensureBscVersionInstalled']('0.65.1') + ).to.eql(s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`); + expect( + fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`) + ).to.be.true; + + //make sure we deleted the bad folder + expect( + s`${stub.getCalls()[0].args[0]}` + ).to.eql(s`${storageDir}/packages/brighterscript-0.65.1`); + }); + }); }); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 9e264bff..f46dae0b 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -16,7 +16,8 @@ import { window, workspace } from 'vscode'; -import { BusyStatus, NotificationName, Logger } from 'brighterscript'; +import { BusyStatus, NotificationName, standardizePath as s } from 'brighterscript'; +import { Logger } from '@rokucommunity/logger'; import { CustomCommands, Deferred } from 'brighterscript'; import type { CodeWithSourceMap } from 'source-map'; import BrightScriptDefinitionProvider from './BrightScriptDefinitionProvider'; @@ -29,6 +30,7 @@ import { util } from './util'; import { LanguageServerInfoCommand, languageServerInfoCommand } from './commands/LanguageServerInfoCommand'; import * as fsExtra from 'fs-extra'; import { EventEmitter } from 'eventemitter3'; +import * as childProcess from 'child_process'; /** * Tracks the running/stopped state of the language server. When the lsp crashes, vscode will restart it. After the 5th crash, they'll leave it permanently crashed. @@ -88,10 +90,13 @@ export class LanguageServerManager { return this.definitionRepository.provider; } + private logger: Logger; public async init( context: vscode.ExtensionContext, - definitionRepository: DefinitionRepository + definitionRepository: DefinitionRepository, + logger: Logger ) { + this.logger = logger; this.context = context; this.definitionRepository = definitionRepository; @@ -266,7 +271,7 @@ export class LanguageServerManager { if (event.status === BusyStatus.busy) { timeoutHandle = setTimeout(() => { const delay = Date.now() - event.timestamp; - this.client.outputChannel.appendLine(`${logger.getTimestamp()} language server has been 'busy' for ${delay}ms. most recent busyStatus event: ${JSON.stringify(event, undefined, 4)}`); + this.client.outputChannel.appendLine(`${logger.formatTimestamp(new Date())} language server has been 'busy' for ${delay}ms. most recent busyStatus event: ${JSON.stringify(event, undefined, 4)}`); }, 60_000); //clear any existing timeout @@ -386,6 +391,7 @@ export class LanguageServerManager { * and if different, re-launch the specific version of the language server' */ public async syncVersionAndTryRun() { + await this.ensureBscVersionInstalled('0.67.3'); const bsdkPath = await this.getBsdkPath(); //if the path to bsc is different, spin down the old server and start a new one @@ -460,6 +466,53 @@ export class LanguageServerManager { ).toString() ); } + + /** + * Ensure that the specified bsc version is installed in the global storage directory. + * @param version + * @param retryCount the number of times we should retry before giving up + * @returns full path to the root of where the brighterscript module is installed + */ + private async ensureBscVersionInstalled(version: string, retryCount = 1) { + console.log('Ensuring bsc version is installed', version); + const bscNpmDir = s`${this.context.globalStorageUri.fsPath}/packages/brighterscript-${version}`; + if (await fsExtra.pathExists(bscNpmDir) === false) { + //write a simple package.json file referencing the version of brighterscript we want + await fsExtra.outputJson(`${bscNpmDir}/package.json`, { + name: 'vscode-brighterscript-host', + private: true, + version: '1.0.0', + dependencies: { + 'brighterscript': version + } + }); + await new Promise((resolve, reject) => { + const process = childProcess.exec(`npm install`, { + cwd: bscNpmDir + }); + process.on('error', (err) => { + console.error(err); + reject(err); + }); + process.on('close', (code) => { + if (code === 0) { + resolve(); + } + }); + }); + } + const bscPath = s`${bscNpmDir}/node_modules/brighterscript`; + + //if the module is invalid, try again + if (await fsExtra.pathExists(`${bscPath}/dist/index.js`) === false && retryCount > 0) { + console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); + //remove the dir and try again + await fsExtra.remove(bscNpmDir); + return this.ensureBscVersionInstalled(version, retryCount - 1); + } + + return bscPath; + } } export const languageServerManager = new LanguageServerManager(); From 4bd3d8dbc1feb3a6f44b24e42ade9f65650d64c6 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 30 Jul 2024 13:09:32 -0400 Subject: [PATCH 02/25] Fix broken test --- src/LanguageServerManager.spec.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index d17e6b82..5128420d 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -345,14 +345,17 @@ describe('LanguageServerManager', () => { }); it('reuses the same bsc version when already exists', async () => { - let stub = sinon.stub(childProcess, 'exec'); + let spy = sinon.spy(childProcess, 'exec'); + fsExtra.ensureDirSync( + s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript/dist/index.js` + ); expect( await languageServerManager['ensureBscVersionInstalled']('0.65.0') ).to.eql(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); expect( fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`) ).to.be.true; - expect(stub.called).to.be.false; + expect(spy.called).to.be.false; }); it('repairs a broken bsc version', async () => { From 83eeef08e64d164cef2a3f57ab2431d1129c943a Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 30 Jul 2024 16:02:17 -0400 Subject: [PATCH 03/25] Add support for loading lsp from npm or url --- package-lock.json | 8 +- package.json | 2 +- src/ActiveDeviceManager.ts | 4 +- src/LanguageServerManager.spec.ts | 4 +- src/LanguageServerManager.ts | 133 +++++++++++------- .../LanguageServerInfoCommand.spec.ts | 15 ++ src/commands/LanguageServerInfoCommand.ts | 66 ++++++++- src/mockVscode.spec.ts | 6 + 8 files changed, 180 insertions(+), 58 deletions(-) diff --git a/package-lock.json b/package-lock.json index 652b5652..8e0a9881 100644 --- a/package-lock.json +++ b/package-lock.json @@ -72,7 +72,7 @@ "chalk": "^4.1.2", "changelog-parser": "^2.8.0", "coveralls-next": "^4.2.0", - "dayjs": "^1.11.7", + "dayjs": "^1.11.12", "deferred": "^0.7.11", "eslint": "^8.10.0", "eslint-plugin-github": "^4.3.5", @@ -3559,9 +3559,9 @@ } }, "node_modules/dayjs": { - "version": "1.11.7", - "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.11.7.tgz", - "integrity": "sha512-+Yw9U6YO5TQohxLcIkrXBeY73WP3ejHWVvx8XCk3gxvQDCTEmS48ZrSZCKciI7Bhl/uCMyxYtE9UqRILmFphkQ==" + "version": "1.11.12", + "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.11.12.tgz", + "integrity": "sha512-Rt2g+nTbLlDWZTwwrIXjy9MeiZmSDI375FvZs72ngxx8PDC6YXOeR3q5LAuPzjZQxhiWdRKac7RKV+YyQYfYIg==" }, "node_modules/debounce": { "version": "1.2.1", diff --git a/package.json b/package.json index d585fdf7..ca14ff5d 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "chalk": "^4.1.2", "changelog-parser": "^2.8.0", "coveralls-next": "^4.2.0", - "dayjs": "^1.11.7", + "dayjs": "^1.11.12", "deferred": "^0.7.11", "eslint": "^8.10.0", "eslint-plugin-github": "^4.3.5", diff --git a/src/ActiveDeviceManager.ts b/src/ActiveDeviceManager.ts index cc50e90b..cfb46576 100644 --- a/src/ActiveDeviceManager.ts +++ b/src/ActiveDeviceManager.ts @@ -261,8 +261,8 @@ class RokuFinder extends EventEmitter { } private readonly client: Client; - private intervalId: NodeJS.Timer | null = null; - private timeoutId: NodeJS.Timer | null = null; + private intervalId: NodeJS.Timeout | null = null; + private timeoutId: NodeJS.Timeout | null = null; private running = false; public start(timeout: number) { diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 5128420d..679eee1c 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -76,7 +76,7 @@ describe('LanguageServerManager', () => { //disable starting so we can manually test sinon.stub(languageServerManager, 'syncVersionAndTryRun').callsFake(() => Promise.resolve()); - await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository'], languageServerManager['logger']); + await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']); languageServerManager['lspRunTracker'].debounceDelay = 100; @@ -325,7 +325,7 @@ describe('LanguageServerManager', () => { }); }); - describe.only('ensureBscVersionInstalled', function() { + describe('ensureBscVersionInstalled', function() { //these tests take a long time (due to running `npm install`) this.timeout(20_000); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index f46dae0b..291a44be 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -31,6 +31,7 @@ import { LanguageServerInfoCommand, languageServerInfoCommand } from './commands import * as fsExtra from 'fs-extra'; import { EventEmitter } from 'eventemitter3'; import * as childProcess from 'child_process'; +import * as semver from 'semver'; /** * Tracks the running/stopped state of the language server. When the lsp crashes, vscode will restart it. After the 5th crash, they'll leave it permanently crashed. @@ -90,13 +91,10 @@ export class LanguageServerManager { return this.definitionRepository.provider; } - private logger: Logger; public async init( context: vscode.ExtensionContext, - definitionRepository: DefinitionRepository, - logger: Logger + definitionRepository: DefinitionRepository ) { - this.logger = logger; this.context = context; this.definitionRepository = definitionRepository; @@ -391,7 +389,6 @@ export class LanguageServerManager { * and if different, re-launch the specific version of the language server' */ public async syncVersionAndTryRun() { - await this.ensureBscVersionInstalled('0.67.3'); const bsdkPath = await this.getBsdkPath(); //if the path to bsc is different, spin down the old server and start a new one @@ -421,15 +418,19 @@ export class LanguageServerManager { } /** - * Get the full path to the brighterscript module where the LanguageServer should be run + * Get the full path to the brighterscript module where the LanguageServer should be run. + * If `brightscript.bsdk` is a version number or a URL, install that version in global storage and return that path. + * if it's a relative path, resolve it to the workspace folder and return that path. */ private async getBsdkPath() { //if there's a bsdk entry in the workspace settings, assume the path is relative to the workspace if (this.workspaceConfigIncludesBsdkKey()) { let bsdk = vscode.workspace.getConfiguration('brightscript', vscode.workspace.workspaceFile).get('bsdk'); - return bsdk === 'embedded' - ? this.embeddedBscInfo.path - : path.resolve(path.dirname(vscode.workspace.workspaceFile.fsPath), bsdk); + if (bsdk === 'embedded') { + return this.embeddedBscInfo.path; + } + let bscPath = await this.ensureBscVersionInstalled(bsdk); + return path.resolve(path.dirname(vscode.workspace.workspaceFile.fsPath), bscPath); } const folderResults = new Set(); @@ -437,11 +438,14 @@ export class LanguageServerManager { for (const folder of vscode.workspace.workspaceFolders) { const bsdk = vscode.workspace.getConfiguration('brightscript', folder).get('bsdk'); if (bsdk) { - folderResults.add( - bsdk === 'embedded' - ? this.embeddedBscInfo.path - : path.resolve(folder.uri.fsPath, bsdk) - ); + if (bsdk === 'embedded') { + folderResults.add(this.embeddedBscInfo.path); + } else { + let bscPath = await this.ensureBscVersionInstalled(bsdk); + folderResults.add( + path.resolve(folder.uri.fsPath, bscPath) + ); + } } } const values = [...folderResults.values()]; @@ -473,42 +477,75 @@ export class LanguageServerManager { * @param retryCount the number of times we should retry before giving up * @returns full path to the root of where the brighterscript module is installed */ - private async ensureBscVersionInstalled(version: string, retryCount = 1) { - console.log('Ensuring bsc version is installed', version); - const bscNpmDir = s`${this.context.globalStorageUri.fsPath}/packages/brighterscript-${version}`; - if (await fsExtra.pathExists(bscNpmDir) === false) { - //write a simple package.json file referencing the version of brighterscript we want - await fsExtra.outputJson(`${bscNpmDir}/package.json`, { - name: 'vscode-brighterscript-host', - private: true, - version: '1.0.0', - dependencies: { - 'brighterscript': version - } - }); - await new Promise((resolve, reject) => { - const process = childProcess.exec(`npm install`, { - cwd: bscNpmDir - }); - process.on('error', (err) => { - console.error(err); - reject(err); - }); - process.on('close', (code) => { - if (code === 0) { - resolve(); + private async ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { + let folderName: string; + let packageJsonEntry: string; + //if this is a URL + if (/^(http|https):\/\//.test(bsdkEntry)) { + folderName = `brighterscript-${btoa(bsdkEntry.trim())}`.substring(0, 30); + packageJsonEntry = bsdkEntry.trim(); + + //this is a valid semantic version + } else if (semver.valid(bsdkEntry)) { + folderName = `brighterscript-${bsdkEntry}`; + packageJsonEntry = bsdkEntry; + + //assume this is a folder path, return as-is + } else { + return bsdkEntry; + } + + let bscPath: string; + + const action = async () => { + + console.log('Ensuring bsc version is installed', packageJsonEntry); + const bscNpmDir = s`${this.context.globalStorageUri.fsPath}/packages/${folderName}`; + if (await fsExtra.pathExists(bscNpmDir) === false) { + //write a simple package.json file referencing the version of brighterscript we want + await fsExtra.outputJson(`${bscNpmDir}/package.json`, { + name: 'vscode-brighterscript-host', + private: true, + version: '1.0.0', + dependencies: { + 'brighterscript': packageJsonEntry } }); - }); - } - const bscPath = s`${bscNpmDir}/node_modules/brighterscript`; - - //if the module is invalid, try again - if (await fsExtra.pathExists(`${bscPath}/dist/index.js`) === false && retryCount > 0) { - console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); - //remove the dir and try again - await fsExtra.remove(bscNpmDir); - return this.ensureBscVersionInstalled(version, retryCount - 1); + await new Promise((resolve, reject) => { + const process = childProcess.exec(`npm install`, { + cwd: bscNpmDir + }); + process.on('error', (err) => { + console.error(err); + reject(err); + }); + process.on('close', (code) => { + if (code === 0) { + resolve(); + } + }); + }); + } + bscPath = s`${bscNpmDir}/node_modules/brighterscript`; + + //if the module is invalid, try again + if (await fsExtra.pathExists(`${bscPath}/dist/index.js`) === false && retryCount > 0) { + console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); + //remove the dir and try again + await fsExtra.remove(bscNpmDir); + return this.ensureBscVersionInstalled(bsdkEntry, retryCount - 1, false); + } + }; + + //show a progress spinner if configured to do so + if (showProgress) { + await vscode.window.withProgress({ + title: 'Installing brighterscript language server ' + packageJsonEntry, + location: vscode.ProgressLocation.Notification, + cancellable: false + }, action); + } else { + await action(); } return bscPath; diff --git a/src/commands/LanguageServerInfoCommand.spec.ts b/src/commands/LanguageServerInfoCommand.spec.ts index 4500a7c3..ddd0a36e 100644 --- a/src/commands/LanguageServerInfoCommand.spec.ts +++ b/src/commands/LanguageServerInfoCommand.spec.ts @@ -35,6 +35,21 @@ describe('LanguageServerInfoCommand', () => { fsExtra.removeSync(tempDir); }); + describe('getBscVersionsFromNpm', () => { + it('returns a list of versions', async () => { + const results = await command['getBscVersionsFromNpm'](); + // `results` is entire list of all bsc versions, live from npm. so we obviously can't make a test that ensure they're all correct. + // so just check that certain values are sorted correctly + expect(results.map(x => x.version).filter(x => x.startsWith('0.64'))).to.eql([ + '0.64.4', + '0.64.3', + '0.64.2', + '0.64.1', + '0.64.0' + ]); + }); + }); + describe('discoverBrighterScriptVersions', () => { function writePackage(version: string) { fsExtra.outputJsonSync(s`${tempDir}/package.json`, { diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index cdd8e5e3..b11feb15 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -3,6 +3,11 @@ import { LANGUAGE_SERVER_NAME, languageServerManager } from '../LanguageServerMa import * as path from 'path'; import * as resolve from 'resolve'; import * as fsExtra from 'fs-extra'; +import * as childProcess from 'child_process'; +import { firstBy } from 'thenby'; +import * as dayjs from 'dayjs'; +import * as relativeTime from 'dayjs/plugin/relativeTime'; +dayjs.extend(relativeTime); export class LanguageServerInfoCommand { public static commandName = 'extension.brightscript.languageServer.info'; @@ -73,9 +78,45 @@ export class LanguageServerInfoCommand { }); } } + return versions; } + private async getBscVersionsFromNpm() { + const versions = await new Promise((resolve, reject) => { + const process = childProcess.exec(`npm view brighterscript time --json`); + + process.stdout.on('data', (data) => { + try { + const versions = JSON.parse(data); + delete versions.created; + delete versions.modified; + resolve(versions); + } catch (error) { + reject(error); + } + }); + + process.stderr.on('data', (error) => { + reject(error); + }); + + process.on('exit', (code) => { + if (code !== 0) { + reject(new Error(`Process exited with code ${code}`)); + } + }); + }); + return Object.entries(versions) + .map(x => { + return { + version: x[0], + date: x[1] + }; + }) + .sort(firstBy(x => x.date, -1)); + } + /** * If this changes the user/folder/workspace settings, that will trigger a reload of the language server so there's no need to * call the reload manually @@ -84,7 +125,30 @@ export class LanguageServerInfoCommand { const versions = this.discoverBrighterScriptVersions( vscode.workspace.workspaceFolders.map(x => this.getWorkspaceOrFolderPath(x.uri.fsPath)) ); - let selection = await vscode.window.showQuickPick(versions, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }); + + //start the request right now, we will leverage it later + const versionsFromNpmPromise = this.getBscVersionsFromNpm(); + + //get the full list of versions from npm + versions.push({ + label: '$(package) Install from npm', + description: '', + detail: '', + command: async () => { + let versionsFromNpm = (await versionsFromNpmPromise).map(x => ({ + label: x.version, + detail: x.version, + description: dayjs(x.date).fromNow(true) + ' ago' + })); + return await vscode.window.showQuickPick(versionsFromNpm, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; + } + } as any); + + let selection = await vscode.window.showQuickPick(versions, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; + + //if the selection has a command, run it before continuing; + selection = await selection?.command() ?? selection; + if (selection) { const config = vscode.workspace.getConfiguration('brightscript'); //quickly clear the setting, then set it again so we are guaranteed to trigger a change event diff --git a/src/mockVscode.spec.ts b/src/mockVscode.spec.ts index 79f3f279..3751ceec 100644 --- a/src/mockVscode.spec.ts +++ b/src/mockVscode.spec.ts @@ -34,6 +34,9 @@ export let vscode = { CodeAction: class { }, Diagnostic: class { }, CallHierarchyItem: class { }, + ProgressLocation: { + Notification: 1 + }, QuickPickItemKind: QuickPickItemKind, StatusBarAlignment: { Left: 1, @@ -161,6 +164,9 @@ export let vscode = { onDidCloseTextDocument: () => { } }, window: { + withProgress: (options, action) => { + return action(); + }, showInputBox: () => { }, createStatusBarItem: () => { return { From 24ef473fb75e670a037f3ce2e3c781053b6a3a46 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 6 Aug 2024 13:41:51 -0400 Subject: [PATCH 04/25] Proper url hashing --- package-lock.json | 32 +++++++++++++++++++ package.json | 1 + src/LanguageServerManager.spec.ts | 19 ++++++++++- src/LanguageServerManager.ts | 4 ++- .../LanguageServerInfoCommand.spec.ts | 3 +- 5 files changed, 56 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e0a9881..c57adbea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,6 +28,7 @@ "iconv-lite": "0.4.24", "jszip": "^3.10.1", "just-throttle": "^4.0.1", + "md5": "^2.3.0", "net": "^1.0.2", "node-cache": "^4.2.0", "node-ssdp": "^4.0.0", @@ -3018,6 +3019,14 @@ "changelog-parser": "bin/cli.js" } }, + "node_modules/charenc": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/charenc/-/charenc-0.0.2.tgz", + "integrity": "sha512-yrLQ/yVUFXkzg7EDQsPieE/53+0RlaWTs+wBrvW36cyilJ2SaDWfl4Yj7MtLTXleV9uEKefbAGUPv2/iWSooRA==", + "engines": { + "node": "*" + } + }, "node_modules/check-error": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/check-error/-/check-error-1.0.2.tgz", @@ -3471,6 +3480,14 @@ "node": ">= 8" } }, + "node_modules/crypt": { + "version": "0.0.2", + "resolved": "https://registry.npmjs.org/crypt/-/crypt-0.0.2.tgz", + "integrity": "sha512-mCxBlsHFYh9C+HVpiEacem8FEBnMXgU9gy4zmNC+SXAZNB/1idgp/aulFJ4FgCi7GPEVbfyng092GqL2k2rmow==", + "engines": { + "node": "*" + } + }, "node_modules/css-select": { "version": "5.1.0", "resolved": "https://registry.npmjs.org/css-select/-/css-select-5.1.0.tgz", @@ -6090,6 +6107,11 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/is-buffer": { + "version": "1.1.6", + "resolved": "https://registry.npmjs.org/is-buffer/-/is-buffer-1.1.6.tgz", + "integrity": "sha512-NcdALwpXkTm5Zvvbk7owOUSvVvBKDgKP5/ewfXEznmQFfs4ZRmanOeKBTjRVjka3QFoN6XJ+9F3USqfHqTaU5w==" + }, "node_modules/is-callable": { "version": "1.2.7", "resolved": "https://registry.npmjs.org/is-callable/-/is-callable-1.2.7.tgz", @@ -7006,6 +7028,16 @@ "node": ">= 12" } }, + "node_modules/md5": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/md5/-/md5-2.3.0.tgz", + "integrity": "sha512-T1GITYmFaKuO91vxyoQMFETst+O71VUPEU3ze5GNzDm0OWdP8v1ziTaAEPUr/3kLsY3Sftgz242A1SetQiDL7g==", + "dependencies": { + "charenc": "0.0.2", + "crypt": "0.0.2", + "is-buffer": "~1.1.6" + } + }, "node_modules/mdurl": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/mdurl/-/mdurl-1.0.1.tgz", diff --git a/package.json b/package.json index ca14ff5d..6c10184d 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,7 @@ "iconv-lite": "0.4.24", "jszip": "^3.10.1", "just-throttle": "^4.0.1", + "md5": "^2.3.0", "net": "^1.0.2", "node-cache": "^4.2.0", "node-ssdp": "^4.0.0", diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 679eee1c..aed4316f 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -65,7 +65,10 @@ describe('LanguageServerManager', () => { }); } - afterEach(() => { + afterEach(function() { + //deleting certain directories take a while + this.timeout(30_000); + sinon.restore(); fsExtra.removeSync(tempDir); }); @@ -358,6 +361,20 @@ describe('LanguageServerManager', () => { expect(spy.called).to.be.false; }); + it('installs from url', async () => { + fsExtra.ensureDirSync( + s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript/dist/index.js` + ); + expect( + await languageServerManager['ensureBscVersionInstalled']( + 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz' + ) + ).to.eql(s`${storageDir}/packages/brighterscript-028738851c072bf844c10c260d6d2c65/node_modules/brighterscript`); + expect( + fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-028738851c072bf844c10c260d6d2c65/node_modules/brighterscript`) + ).to.be.true; + }); + it('repairs a broken bsc version', async () => { let stub = sinon.stub(fsExtra, 'remove'); fsExtra.ensureDirSync( diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 291a44be..973a0b64 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -32,6 +32,7 @@ import * as fsExtra from 'fs-extra'; import { EventEmitter } from 'eventemitter3'; import * as childProcess from 'child_process'; import * as semver from 'semver'; +import * as md5 from 'md5'; /** * Tracks the running/stopped state of the language server. When the lsp crashes, vscode will restart it. After the 5th crash, they'll leave it permanently crashed. @@ -482,7 +483,8 @@ export class LanguageServerManager { let packageJsonEntry: string; //if this is a URL if (/^(http|https):\/\//.test(bsdkEntry)) { - folderName = `brighterscript-${btoa(bsdkEntry.trim())}`.substring(0, 30); + //hash the URL to create a unique folder name. There is next to zero possibility these will clash, so the hash should be fine. + folderName = `brighterscript-${md5(bsdkEntry.trim())}`; packageJsonEntry = bsdkEntry.trim(); //this is a valid semantic version diff --git a/src/commands/LanguageServerInfoCommand.spec.ts b/src/commands/LanguageServerInfoCommand.spec.ts index ddd0a36e..5e389d8f 100644 --- a/src/commands/LanguageServerInfoCommand.spec.ts +++ b/src/commands/LanguageServerInfoCommand.spec.ts @@ -35,7 +35,8 @@ describe('LanguageServerInfoCommand', () => { fsExtra.removeSync(tempDir); }); - describe('getBscVersionsFromNpm', () => { + describe('getBscVersionsFromNpm', function() { + this.timeout(20_000); it('returns a list of versions', async () => { const results = await command['getBscVersionsFromNpm'](); // `results` is entire list of all bsc versions, live from npm. so we obviously can't make a test that ensure they're all correct. From 7bb3902d0aff2442bd3c2bfd06ad7cee40cb5971 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 20 Aug 2024 13:15:24 -0400 Subject: [PATCH 05/25] Add command to clear local npm cache --- package.json | 6 +++++ src/BrightScriptCommands.spec.ts | 2 +- src/BrightScriptCommands.ts | 6 ++++- src/LanguageServerManager.spec.ts | 23 ++++++++++++++----- src/LanguageServerManager.ts | 7 ++++++ src/commands/ClearNpmPackageCacheCommand.ts | 14 +++++++++++ src/extension.ts | 3 ++- .../WebviewViewProviderManager.spec.ts | 2 +- 8 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 src/commands/ClearNpmPackageCacheCommand.ts diff --git a/package.json b/package.json index 6c10184d..e2693ac3 100644 --- a/package.json +++ b/package.json @@ -3085,6 +3085,12 @@ "title": "Open SceneGraph Inspector In New Window", "category": "BrighterScript", "icon": "$(link-external)" + }, + { + "command": "extension.brightscript.clearNpmPackageCache", + "title": "Clear the extension's local node_modules cache", + "category": "BrighterScript", + "icon": "$(link-external)" } ], "keybindings": [ diff --git a/src/BrightScriptCommands.spec.ts b/src/BrightScriptCommands.spec.ts index 5e135554..dfa80c38 100644 --- a/src/BrightScriptCommands.spec.ts +++ b/src/BrightScriptCommands.spec.ts @@ -22,7 +22,7 @@ describe('BrightScriptFileUtils ', () => { let languagesMock; beforeEach(() => { - commands = new BrightScriptCommands({} as any, {} as any, {} as any, {} as any, {} as any); + commands = new BrightScriptCommands({} as any, {} as any, {} as any, {} as any, {} as any, {} as any); commandsMock = sinon.mock(commands); languagesMock = sinon.mock(vscode.languages); }); diff --git a/src/BrightScriptCommands.ts b/src/BrightScriptCommands.ts index 2a8a67ad..dc6b210f 100644 --- a/src/BrightScriptCommands.ts +++ b/src/BrightScriptCommands.ts @@ -14,6 +14,8 @@ import type { ActiveDeviceManager } from './ActiveDeviceManager'; import * as xml2js from 'xml2js'; import { firstBy } from 'thenby'; import type { UserInputManager } from './managers/UserInputManager'; +import { clearNpmPackageCacheCommand } from './commands/ClearNpmPackageCacheCommand'; +import type { LanguageServerManager } from './LanguageServerManager'; export class BrightScriptCommands { @@ -22,7 +24,8 @@ export class BrightScriptCommands { private whatsNewManager: WhatsNewManager, private context: vscode.ExtensionContext, private activeDeviceManager: ActiveDeviceManager, - private userInputManager: UserInputManager + private userInputManager: UserInputManager, + private languageServerManager: LanguageServerManager ) { this.fileUtils = new BrightScriptFileUtils(); } @@ -39,6 +42,7 @@ export class BrightScriptCommands { languageServerInfoCommand.register(this.context); captureScreenshotCommand.register(this.context, this); rekeyAndPackageCommand.register(this.context, this, this.userInputManager); + clearNpmPackageCacheCommand.register(this.context, this.languageServerManager); this.registerGeneralCommands(); diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index aed4316f..913811b7 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -34,6 +34,8 @@ Module.prototype.require = function hijacked(file) { const tempDir = s`${process.cwd()}/.tmp`; describe('LanguageServerManager', () => { + const storageDir = s`${tempDir}/brighterscript-storage`; + let languageServerManager: LanguageServerManager; beforeEach(() => { @@ -49,6 +51,10 @@ describe('LanguageServerManager', () => { update: () => { } } } as unknown as ExtensionContext; + + fsExtra.removeSync(storageDir); + (languageServerManager['context'] as any).globalStorageUri = URI.file(storageDir); + }); function stubConstructClient(processor?: (LanguageClient) => void) { @@ -332,12 +338,6 @@ describe('LanguageServerManager', () => { //these tests take a long time (due to running `npm install`) this.timeout(20_000); - const storageDir = s`${tempDir}/brighterscript-storage`; - beforeEach(() => { - fsExtra.removeSync(storageDir); - (languageServerManager['context'] as any).globalStorageUri = URI.file(storageDir); - }); - it('installs a bsc version when not present', async () => { expect( await languageServerManager['ensureBscVersionInstalled']('0.65.0') @@ -398,4 +398,15 @@ describe('LanguageServerManager', () => { ).to.eql(s`${storageDir}/packages/brighterscript-0.65.1`); }); }); + + describe('clearNpmPackageCache', () => { + it('clears the cache', async () => { + fsExtra.ensureFileSync(`${storageDir}/test.txt`); + expect(fsExtra.pathExistsSync(`${storageDir}/packages/test.txt`)).to.be.true; + + await languageServerManager.clearNpmPackageCache(); + + expect(fsExtra.pathExistsSync(`${storageDir}/packages/test.txt`)).to.be.false; + }); + }); }); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 973a0b64..564c005d 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -552,6 +552,13 @@ export class LanguageServerManager { return bscPath; } + + /** + * Clear all packages stored in the npm cache for this extension + */ + public async clearNpmPackageCache() { + await fsExtra.emptyDir(s`${this.context.globalStorageUri.fsPath}/packages`); + } } export const languageServerManager = new LanguageServerManager(); diff --git a/src/commands/ClearNpmPackageCacheCommand.ts b/src/commands/ClearNpmPackageCacheCommand.ts new file mode 100644 index 00000000..4a60ab69 --- /dev/null +++ b/src/commands/ClearNpmPackageCacheCommand.ts @@ -0,0 +1,14 @@ +import * as vscode from 'vscode'; +import type { LanguageServerManager } from '../LanguageServerManager'; + +export class ClearNpmPackageCacheCommand { + public static commandName = 'extension.brightscript.clearNpmPackageCache'; + + public register(context: vscode.ExtensionContext, languageServerManager: LanguageServerManager) { + context.subscriptions.push(vscode.commands.registerCommand(ClearNpmPackageCacheCommand.commandName, async () => { + await languageServerManager.clearNpmPackageCache(); + })); + } +} + +export const clearNpmPackageCacheCommand = new ClearNpmPackageCacheCommand(); diff --git a/src/extension.ts b/src/extension.ts index e18cf065..fe153bf2 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -75,7 +75,8 @@ export class Extension { this.whatsNewManager, context, activeDeviceManager, - userInputManager + userInputManager, + languageServerManager ); this.rtaManager = new RtaManager(context); diff --git a/src/managers/WebviewViewProviderManager.spec.ts b/src/managers/WebviewViewProviderManager.spec.ts index fd073fad..cc111979 100644 --- a/src/managers/WebviewViewProviderManager.spec.ts +++ b/src/managers/WebviewViewProviderManager.spec.ts @@ -15,7 +15,7 @@ describe('WebviewViewProviderManager', () => { const config = {} as BrightScriptLaunchConfiguration; let webviewViewProviderManager: WebviewViewProviderManager; let rtaManager: RtaManager; - const brightScriptCommands = new BrightScriptCommands({} as any, {} as any, {} as any, {} as any, {} as any); + const brightScriptCommands = new BrightScriptCommands({} as any, {} as any, {} as any, {} as any, {} as any, {} as any); before(() => { context = { From 8c6600fbd7ef1a23700673dca5e945ed2e1c7c27 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Tue, 20 Aug 2024 13:25:14 -0400 Subject: [PATCH 06/25] Add language server menu option for clearing cached packages --- src/LanguageServerManager.spec.ts | 2 +- src/commands/ClearNpmPackageCacheCommand.ts | 4 ++-- src/commands/LanguageServerInfoCommand.ts | 9 +++++++++ src/commands/VscodeCommand.ts | 3 ++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 913811b7..af419aa5 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -401,7 +401,7 @@ describe('LanguageServerManager', () => { describe('clearNpmPackageCache', () => { it('clears the cache', async () => { - fsExtra.ensureFileSync(`${storageDir}/test.txt`); + fsExtra.ensureFileSync(`${storageDir}/packages/test.txt`); expect(fsExtra.pathExistsSync(`${storageDir}/packages/test.txt`)).to.be.true; await languageServerManager.clearNpmPackageCache(); diff --git a/src/commands/ClearNpmPackageCacheCommand.ts b/src/commands/ClearNpmPackageCacheCommand.ts index 4a60ab69..2f30ee9b 100644 --- a/src/commands/ClearNpmPackageCacheCommand.ts +++ b/src/commands/ClearNpmPackageCacheCommand.ts @@ -1,11 +1,11 @@ import * as vscode from 'vscode'; import type { LanguageServerManager } from '../LanguageServerManager'; +import { VscodeCommand } from './VscodeCommand'; export class ClearNpmPackageCacheCommand { - public static commandName = 'extension.brightscript.clearNpmPackageCache'; public register(context: vscode.ExtensionContext, languageServerManager: LanguageServerManager) { - context.subscriptions.push(vscode.commands.registerCommand(ClearNpmPackageCacheCommand.commandName, async () => { + context.subscriptions.push(vscode.commands.registerCommand(VscodeCommand.clearNpmPackageCache, async () => { await languageServerManager.clearNpmPackageCache(); })); } diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index b11feb15..f9418834 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -7,6 +7,8 @@ import * as childProcess from 'child_process'; import { firstBy } from 'thenby'; import * as dayjs from 'dayjs'; import * as relativeTime from 'dayjs/plugin/relativeTime'; +import { util } from '../util'; +import { VscodeCommand } from './VscodeCommand'; dayjs.extend(relativeTime); export class LanguageServerInfoCommand { @@ -26,6 +28,13 @@ export class LanguageServerInfoCommand { label: `View language server logs`, description: ``, command: this.focusLanguageServerOutputChannel.bind(this) + }, { + label: `Delete cached brighterscript versions`, + description: ``, + command: async () => { + await vscode.commands.executeCommand(VscodeCommand.clearNpmPackageCache); + await vscode.window.showInformationMessage('All cached brighterscript versions have been removed'); + } }]; let selection = await vscode.window.showQuickPick(commands, { placeHolder: `BrighterScript Project Info` }); diff --git a/src/commands/VscodeCommand.ts b/src/commands/VscodeCommand.ts index 2b1c9aa4..15bb8d8f 100644 --- a/src/commands/VscodeCommand.ts +++ b/src/commands/VscodeCommand.ts @@ -19,5 +19,6 @@ export enum VscodeCommand { rokuAppOverlaysViewRemoveAllOverlays = 'extension.brightscript.rokuAppOverlaysView.removeAllOverlays', rokuFileSystemViewRefresh = 'extension.brightscript.rokuFileSystemView.refresh', disconnectFromDevice = 'extension.brightscript.disconnectFromDevice', - openSceneGraphInspectorInPanel = 'extension.brightscript.openSceneGraphInspectorInPanel' + openSceneGraphInspectorInPanel = 'extension.brightscript.openSceneGraphInspectorInPanel', + clearNpmPackageCache = 'extension.brightscript.clearNpmPackageCache' } From 43fa9da8fb3bcb0f12adf7bf50ecd5826e0f2c3d Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 11 Sep 2024 11:11:23 -0400 Subject: [PATCH 07/25] Delete bsc versions after 45 days of inactivity --- package.json | 6 ++ src/GlobalStateManager.ts | 2 +- src/LanguageServerManager.spec.ts | 58 ++++++++++- src/LanguageServerManager.ts | 115 +++++++++++++++++++--- src/commands/LanguageServerInfoCommand.ts | 3 +- src/mockVscode.spec.ts | 5 +- 6 files changed, 164 insertions(+), 25 deletions(-) diff --git a/package.json b/package.json index e2693ac3..8c719187 100644 --- a/package.json +++ b/package.json @@ -1811,6 +1811,12 @@ "description": "Path to the BrighterScript module to use for the BrightScript and BrighterScript language features", "scope": "resource" }, + "brightscript.npmCacheRetentionDays": { + "type": "integer", + "description": "How long should the extension keep around unused extension-managed npm packages such as brighterscript", + "default": 45, + "scope": "resource" + }, "brightscript.enableLanguageServer": { "type": "boolean", "description": "Enable the Language Server, which includes things like syntax checking, intellisense, completions, etc.", diff --git a/src/GlobalStateManager.ts b/src/GlobalStateManager.ts index 29c2f12d..0145b627 100644 --- a/src/GlobalStateManager.ts +++ b/src/GlobalStateManager.ts @@ -14,6 +14,7 @@ export class GlobalStateManager { sendRemoteTextHistory: 'sendRemoteTextHistory', debugProtocolPopupSnoozeUntilDate: 'debugProtocolPopupSnoozeUntilDate', debugProtocolPopupSnoozeValue: 'debugProtocolPopupSnoozeValue' + }; private remoteTextHistoryLimit: number; private remoteTextHistoryEnabled: boolean; @@ -38,7 +39,6 @@ export class GlobalStateManager { void this.context.globalState.update(this.keys.lastSeenReleaseNotesVersion, value); } - public get debugProtocolPopupSnoozeUntilDate(): Date { const epoch = this.context.globalState.get(this.keys.debugProtocolPopupSnoozeUntilDate); if (epoch) { diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index af419aa5..0e6ec25d 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -16,6 +16,9 @@ import { State } from 'vscode-languageclient/node'; import * as childProcess from 'child_process'; +import { util } from './util'; +import * as dayjs from 'dayjs'; +import { GlobalStateManager } from './GlobalStateManager'; const Module = require('module'); const sinon = createSandbox(); @@ -44,13 +47,11 @@ describe('LanguageServerManager', () => { new DeclarationProvider() ); languageServerManager['context'] = { + ...vscode.context, asAbsolutePath: vscode.context.asAbsolutePath, - subscriptions: [], - globalState: { - get: () => { }, - update: () => { } - } + subscriptions: [] } as unknown as ExtensionContext; + languageServerManager['globalStateManager'] = new GlobalStateManager(languageServerManager['context']); fsExtra.removeSync(storageDir); (languageServerManager['context'] as any).globalStorageUri = URI.file(storageDir); @@ -409,4 +410,51 @@ describe('LanguageServerManager', () => { expect(fsExtra.pathExistsSync(`${storageDir}/packages/test.txt`)).to.be.false; }); }); + + describe('deleteOutdatedBscVersions', () => { + beforeEach(() => { + //prevent lsp from actually running + sinon.stub(languageServerManager as any, 'syncVersionAndTryRun').returns(Promise.resolve()); + }); + + it('runs after a short delay after init', async () => { + const stub = sinon.stub(languageServerManager as any, 'deleteOutdatedBscVersions').callsFake(() => { }); + + languageServerManager['outdatedBscVersionDeleteDelay'] = 50; + + await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']); + + expect(stub.called).to.be.false; + + await util.sleep(100); + expect(stub.called).to.be.true; + }); + + it('deletes bsc versions that are older than the specified number of days', async () => { + //create a vew bsc versions + fsExtra.ensureDirSync(`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); + fsExtra.ensureDirSync(`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`); + fsExtra.ensureDirSync(`${storageDir}/packages/brighterscript-0.65.2/node_modules/brighterscript`); + + //mark the first and third as outdated + await languageServerManager['updateBscVersionUsageDate']( + s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`, + dayjs().subtract(46, 'day').toDate() + ); + await languageServerManager['updateBscVersionUsageDate']( + s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`, + dayjs().subtract(20, 'day').toDate() + ); + await languageServerManager['updateBscVersionUsageDate']( + s`${storageDir}/packages/brighterscript-0.65.2/node_modules/brighterscript`, + dayjs().subtract(60, 'day').toDate() + ); + + await languageServerManager.deleteOutdatedBscVersions(); + + expect(fsExtra.pathExistsSync(`${storageDir}/packages/brighterscript-0.65.0`)).to.be.false; + expect(fsExtra.pathExistsSync(`${storageDir}/packages/brighterscript-0.65.1`)).to.be.true; + expect(fsExtra.pathExistsSync(`${storageDir}/packages/brighterscript-0.65.2`)).to.be.false; + }); + }); }); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 564c005d..911fe3f9 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -33,6 +33,8 @@ import { EventEmitter } from 'eventemitter3'; import * as childProcess from 'child_process'; import * as semver from 'semver'; import * as md5 from 'md5'; +import type { GlobalStateManager } from './GlobalStateManager'; +import * as dayjs from 'dayjs'; /** * Tracks the running/stopped state of the language server. When the lsp crashes, vscode will restart it. After the 5th crash, they'll leave it permanently crashed. @@ -92,6 +94,11 @@ export class LanguageServerManager { return this.definitionRepository.provider; } + /** + * The delay after init before we delete any outdated bsc versions + */ + private outdatedBscVersionDeleteDelay = 5 * 60 * 1000; + public async init( context: vscode.ExtensionContext, definitionRepository: DefinitionRepository @@ -99,6 +106,16 @@ export class LanguageServerManager { this.context = context; this.definitionRepository = definitionRepository; + //anytime the window changes focus, save the current brighterscript version + vscode.window.onDidChangeWindowState(async (e) => { + await this.updateBscVersionUsageDate(this.selectedBscInfo.path); + }); + + //in about 5 minutes, clean up any outdated bsc versions (delayed to prevent slower startup times) + setTimeout(() => { + void this.deleteOutdatedBscVersions(); + }, this.outdatedBscVersionDeleteDelay); + //if the lsp is permanently stopped by vscode, ask the user if they want to restart it again. this.lspRunTracker.on('stopped', async () => { //stop the statusbar spinner @@ -472,6 +489,36 @@ export class LanguageServerManager { ); } + private get packagesDir() { + return s`${this.context.globalStorageUri.fsPath}/packages`; + } + + /** + * Extract some info about the bsc version string + * @param version can be a semantic version, a URL, or a folder path + * @returns + */ + private parseBscVersion(version: string) { + //if this is a URL + if (/^(http|https):\/\//.test(version)) { + //hash the URL to create a unique folder name. There is next to zero possibility these will clash, so the hash should be fine. + return { + folderName: `brighterscript-${md5(version.trim())}`, + packageJsonEntry: version.trim() + }; + + //this is a valid semantic version + } else if (semver.valid(version)) { + return { + folderName: `brighterscript-${version}`, + packageJsonEntry: version + }; + //assume this is a folder path, return undefined + } else { + return undefined; + } + } + /** * Ensure that the specified bsc version is installed in the global storage directory. * @param version @@ -479,21 +526,11 @@ export class LanguageServerManager { * @returns full path to the root of where the brighterscript module is installed */ private async ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { - let folderName: string; - let packageJsonEntry: string; - //if this is a URL - if (/^(http|https):\/\//.test(bsdkEntry)) { - //hash the URL to create a unique folder name. There is next to zero possibility these will clash, so the hash should be fine. - folderName = `brighterscript-${md5(bsdkEntry.trim())}`; - packageJsonEntry = bsdkEntry.trim(); - //this is a valid semantic version - } else if (semver.valid(bsdkEntry)) { - folderName = `brighterscript-${bsdkEntry}`; - packageJsonEntry = bsdkEntry; + const { folderName, packageJsonEntry } = this.parseBscVersion(bsdkEntry) ?? {}; - //assume this is a folder path, return as-is - } else { + //assume this is a folder path, return as-is + if (!folderName || !packageJsonEntry) { return bsdkEntry; } @@ -502,7 +539,7 @@ export class LanguageServerManager { const action = async () => { console.log('Ensuring bsc version is installed', packageJsonEntry); - const bscNpmDir = s`${this.context.globalStorageUri.fsPath}/packages/${folderName}`; + const bscNpmDir = s`${this.packagesDir}/${folderName}`; if (await fsExtra.pathExists(bscNpmDir) === false) { //write a simple package.json file referencing the version of brighterscript we want await fsExtra.outputJson(`${bscNpmDir}/package.json`, { @@ -557,7 +594,55 @@ export class LanguageServerManager { * Clear all packages stored in the npm cache for this extension */ public async clearNpmPackageCache() { - await fsExtra.emptyDir(s`${this.context.globalStorageUri.fsPath}/packages`); + await fsExtra.emptyDir(this.packagesDir); + } + + /** + * Update the last-used date of a specific instance of brighterscript so we can prevent it from being cleaned up too soon + * @param bscPath + */ + private async updateBscVersionUsageDate(bscPath: string, date = new Date()) { + const usage = this.context.globalState.get('bsc-usage', {}); + usage[s`${bscPath}`] = date.getTime(); + await this.context.globalState.update('bsc-usage', usage); + } + + /** + * The number of days to retain a cached npm package before deleting it. + */ + private get npmCacheRetentionDays() { + return vscode.workspace.getConfiguration('brightscript')?.get?.('npmCacheRetentionDays', 45) ?? 45; + } + + /** + * Delete any brighterscript versions that haven't been used in a while + */ + public async deleteOutdatedBscVersions() { + //build a date that represents the cutoff date (i.e. 45 days ago) + const cutoffDate = dayjs().subtract(this.npmCacheRetentionDays, 'days'); + + //get a list of bsc locations that have been recently used + const bscLocationsToKeep = Object.entries( + this.context.globalState.get>('bsc-usage', {}) + + //keep versions that have been used recently + ).filter(([, lastUsedTime]) => { + //if this version was used within the past 45 days, keep it + return dayjs(lastUsedTime).diff(cutoffDate) > 0; + }).map(x => x[0]); + + //get a list of folders in our internal packages directory + const dirs = (await fsExtra.readdir(this.packagesDir)); + + //delete any dir not in the keep list + for (const dir of dirs) { + const fullDir = s`${this.packagesDir}/${dir}/node_modules/brighterscript`; + if (bscLocationsToKeep.includes(fullDir) === false) { + try { + await fsExtra.remove(s`${this.packagesDir}/${dir}`); + } catch { } + } + } } } diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index f9418834..ac36d120 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -7,7 +7,6 @@ import * as childProcess from 'child_process'; import { firstBy } from 'thenby'; import * as dayjs from 'dayjs'; import * as relativeTime from 'dayjs/plugin/relativeTime'; -import { util } from '../util'; import { VscodeCommand } from './VscodeCommand'; dayjs.extend(relativeTime); @@ -156,7 +155,7 @@ export class LanguageServerInfoCommand { let selection = await vscode.window.showQuickPick(versions, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; //if the selection has a command, run it before continuing; - selection = await selection?.command() ?? selection; + selection = await selection?.command?.() ?? selection; if (selection) { const config = vscode.workspace.getConfiguration('brightscript'); diff --git a/src/mockVscode.spec.ts b/src/mockVscode.spec.ts index 3751ceec..c99eb680 100644 --- a/src/mockVscode.spec.ts +++ b/src/mockVscode.spec.ts @@ -99,8 +99,8 @@ export let vscode = { update: function(key: string, value: any) { this._data[key] = value; }, - get: function(key: string) { - return this._data[key]; + get: function(key: string, defaultData) { + return this._data[key] ?? defaultData; } } as any, workspaceState: { @@ -176,6 +176,7 @@ export let vscode = { dispose: () => { } }; }, + onDidChangeWindowState: () => { }, createQuickPick: () => { class QuickPick { private emitter = new EventEmitter(); From 13b6abfd3d5bfe4f09b35cea88504f570cd65e5b Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 11 Sep 2024 11:23:54 -0400 Subject: [PATCH 08/25] Add command to view packages dir in explorer --- package.json | 2 +- src/LanguageServerManager.ts | 3 +-- src/commands/LanguageServerInfoCommand.ts | 9 ++++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 8c719187..81f5bd67 100644 --- a/package.json +++ b/package.json @@ -1812,7 +1812,7 @@ "scope": "resource" }, "brightscript.npmCacheRetentionDays": { - "type": "integer", + "type": "number", "description": "How long should the extension keep around unused extension-managed npm packages such as brighterscript", "default": 45, "scope": "resource" diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 911fe3f9..a1ec7d0b 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -33,7 +33,6 @@ import { EventEmitter } from 'eventemitter3'; import * as childProcess from 'child_process'; import * as semver from 'semver'; import * as md5 from 'md5'; -import type { GlobalStateManager } from './GlobalStateManager'; import * as dayjs from 'dayjs'; /** @@ -489,7 +488,7 @@ export class LanguageServerManager { ); } - private get packagesDir() { + public get packagesDir() { return s`${this.context.globalStorageUri.fsPath}/packages`; } diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index ac36d120..067c6176 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -5,9 +5,10 @@ import * as resolve from 'resolve'; import * as fsExtra from 'fs-extra'; import * as childProcess from 'child_process'; import { firstBy } from 'thenby'; +import { VscodeCommand } from './VscodeCommand'; +import URI from 'vscode-uri'; import * as dayjs from 'dayjs'; import * as relativeTime from 'dayjs/plugin/relativeTime'; -import { VscodeCommand } from './VscodeCommand'; dayjs.extend(relativeTime); export class LanguageServerInfoCommand { @@ -27,6 +28,12 @@ export class LanguageServerInfoCommand { label: `View language server logs`, description: ``, command: this.focusLanguageServerOutputChannel.bind(this) + }, { + label: `View BrighterScript version cache folder`, + description: ``, + command: async () => { + await vscode.commands.executeCommand('revealFileInOS', URI.file(languageServerManager.packagesDir)); + } }, { label: `Delete cached brighterscript versions`, description: ``, From 45cf3f269c31c03a32282f7575a5deb2820cc5a8 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 16 Sep 2024 10:51:21 -0400 Subject: [PATCH 09/25] Prevent installing same bsc dependency multiple times --- src/LanguageServerManager.spec.ts | 32 +++++++++++++++++++++++++-- src/LanguageServerManager.ts | 36 +++++++++++++++++-------------- src/util.ts | 27 ++++++++++++++++++++--- 3 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 0e6ec25d..09c312e1 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -15,7 +15,6 @@ import { LanguageClient, State } from 'vscode-languageclient/node'; -import * as childProcess from 'child_process'; import { util } from './util'; import * as dayjs from 'dayjs'; import { GlobalStateManager } from './GlobalStateManager'; @@ -348,8 +347,37 @@ describe('LanguageServerManager', () => { ).to.be.true; }); + it('does not run multiple installs for the same version at the same time', async () => { + let spy = sinon.stub(util, 'exec').callsFake(async (command, options) => { + //simulate that the bsc code was installed + fsExtra.outputFileSync(`${options.cwd}/node_modules/brighterscript/dist/index.js`, ''); + //ensure both requests have the opportunity to run at same time + await util.sleep(200); + }); + //request the install multiple times without waiting for them + const promises = [ + languageServerManager['ensureBscVersionInstalled']('0.65.0'), + languageServerManager['ensureBscVersionInstalled']('0.65.0'), + languageServerManager['ensureBscVersionInstalled']('0.65.1') + ]; + //now wait for them to finish + expect( + await Promise.all(promises) + ).to.eql([ + s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`, + s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`, + s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript` + ]); + + //the spy should have only been called once for each unique version + expect(spy.getCalls().map(x => x.args[1].cwd)).to.eql([ + s`${storageDir}/packages/brighterscript-0.65.0`, + s`${storageDir}/packages/brighterscript-0.65.1` + ]); + }); + it('reuses the same bsc version when already exists', async () => { - let spy = sinon.spy(childProcess, 'exec'); + let spy = sinon.spy(util, 'exec'); fsExtra.ensureDirSync( s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript/dist/index.js` ); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index a1ec7d0b..905718a5 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -30,7 +30,6 @@ import { util } from './util'; import { LanguageServerInfoCommand, languageServerInfoCommand } from './commands/LanguageServerInfoCommand'; import * as fsExtra from 'fs-extra'; import { EventEmitter } from 'eventemitter3'; -import * as childProcess from 'child_process'; import * as semver from 'semver'; import * as md5 from 'md5'; import * as dayjs from 'dayjs'; @@ -524,8 +523,24 @@ export class LanguageServerManager { * @param retryCount the number of times we should retry before giving up * @returns full path to the root of where the brighterscript module is installed */ - private async ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { + private ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { + let timeout = util.sleep(3 * 60 * 1000); + this.ensureBscVersionInstalledPromise = Promise.race([ + //only block for a few minutes, then let the next one try + timeout, + this.ensureBscVersionInstalledPromise + ]).then(() => { + return this._ensureBscVersionInstalled(bsdkEntry, retryCount, showProgress); + }).catch((error) => { + console.error(error); + }).finally(() => { + timeout.cancel(); + }); + return this.ensureBscVersionInstalledPromise; + } + private ensureBscVersionInstalledPromise = Promise.resolve(undefined); + private async _ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { const { folderName, packageJsonEntry } = this.parseBscVersion(bsdkEntry) ?? {}; //assume this is a folder path, return as-is @@ -549,19 +564,8 @@ export class LanguageServerManager { 'brighterscript': packageJsonEntry } }); - await new Promise((resolve, reject) => { - const process = childProcess.exec(`npm install`, { - cwd: bscNpmDir - }); - process.on('error', (err) => { - console.error(err); - reject(err); - }); - process.on('close', (code) => { - if (code === 0) { - resolve(); - } - }); + await util.exec('npm install', { + cwd: bscNpmDir }); } bscPath = s`${bscNpmDir}/node_modules/brighterscript`; @@ -571,7 +575,7 @@ export class LanguageServerManager { console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); //remove the dir and try again await fsExtra.remove(bscNpmDir); - return this.ensureBscVersionInstalled(bsdkEntry, retryCount - 1, false); + return this._ensureBscVersionInstalled(bsdkEntry, retryCount - 1, false); } }; diff --git a/src/util.ts b/src/util.ts index be5799f7..c0510a7e 100644 --- a/src/util.ts +++ b/src/util.ts @@ -10,6 +10,7 @@ import { EXTENSION_ID, ROKU_DEBUG_VERSION } from './constants'; import type { DeviceInfo } from 'roku-deploy'; import * as request from 'postman-request'; import type { Response, CoreOptions } from 'request'; +import * as childProcess from 'child_process'; class Util { public async readDir(dirPath: string) { @@ -268,9 +269,14 @@ class Util { * Get a promise that resolves after the given number of milliseconds. */ public sleep(milliseconds: number) { - return new Promise((resolve) => { - setTimeout(resolve, milliseconds); - }); + let handle: NodeJS.Timeout; + const promise = new Promise((resolve) => { + handle = setTimeout(resolve, milliseconds); + }) as Promise & { cancel: () => void }; + promise.cancel = () => { + clearTimeout(handle); + }; + return promise; } /** @@ -424,6 +430,21 @@ class Util { spinner.dispose(); } } + + public async exec(command: string, options?: childProcess.ExecOptions) { + await new Promise((resolve, reject) => { + const process = childProcess.exec(command, options); + process.on('error', (err) => { + console.error(err); + reject(err); + }); + process.on('close', (code) => { + if (code === 0) { + resolve(); + } + }); + }); + } } const util = new Util(); From 25f240d608b76e37a1287dca7f5406dbb4a6da5f Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 16 Sep 2024 15:09:26 -0400 Subject: [PATCH 10/25] Better messaging around removing cached brighterscript versions --- src/LanguageServerManager.ts | 78 ++++++++++++----------- src/commands/LanguageServerInfoCommand.ts | 23 +++++-- src/util.ts | 16 +++++ 3 files changed, 73 insertions(+), 44 deletions(-) diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 905718a5..901a9b93 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -524,10 +524,11 @@ export class LanguageServerManager { * @returns full path to the root of where the brighterscript module is installed */ private ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { - let timeout = util.sleep(3 * 60 * 1000); + let timeout: ReturnType; + this.ensureBscVersionInstalledPromise = Promise.race([ //only block for a few minutes, then let the next one try - timeout, + timeout = util.sleep(3 * 60 * 1000), this.ensureBscVersionInstalledPromise ]).then(() => { return this._ensureBscVersionInstalled(bsdkEntry, retryCount, showProgress); @@ -548,46 +549,49 @@ export class LanguageServerManager { return bsdkEntry; } + function runWithProgress(action) { + //show a progress spinner if configured to do so + if (showProgress) { + return vscode.window.withProgress({ + title: 'Installing brighterscript language server ' + packageJsonEntry, + location: vscode.ProgressLocation.Notification, + cancellable: false + }, action); + } else { + return action(); + } + } + let bscPath: string; - const action = async () => { - - console.log('Ensuring bsc version is installed', packageJsonEntry); - const bscNpmDir = s`${this.packagesDir}/${folderName}`; - if (await fsExtra.pathExists(bscNpmDir) === false) { - //write a simple package.json file referencing the version of brighterscript we want - await fsExtra.outputJson(`${bscNpmDir}/package.json`, { - name: 'vscode-brighterscript-host', - private: true, - version: '1.0.0', - dependencies: { - 'brighterscript': packageJsonEntry - } - }); + + console.log('Ensuring bsc version is installed', packageJsonEntry); + const bscNpmDir = s`${this.packagesDir}/${folderName}`; + //if the npm dir isn't there, install it + if (await fsExtra.pathExists(bscNpmDir) === false) { + //write a simple package.json file referencing the version of brighterscript we want + await fsExtra.outputJson(`${bscNpmDir}/package.json`, { + name: 'vscode-brighterscript-host', + private: true, + version: '1.0.0', + dependencies: { + 'brighterscript': packageJsonEntry + } + }); + await runWithProgress(async () => { await util.exec('npm install', { cwd: bscNpmDir }); - } - bscPath = s`${bscNpmDir}/node_modules/brighterscript`; - - //if the module is invalid, try again - if (await fsExtra.pathExists(`${bscPath}/dist/index.js`) === false && retryCount > 0) { - console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); - //remove the dir and try again - await fsExtra.remove(bscNpmDir); - return this._ensureBscVersionInstalled(bsdkEntry, retryCount - 1, false); - } - }; - - //show a progress spinner if configured to do so - if (showProgress) { - await vscode.window.withProgress({ - title: 'Installing brighterscript language server ' + packageJsonEntry, - location: vscode.ProgressLocation.Notification, - cancellable: false - }, action); - } else { - await action(); + }); + } + bscPath = s`${bscNpmDir}/node_modules/brighterscript`; + + //if the module is invalid, try again + if (await fsExtra.pathExists(`${bscPath}/dist/index.js`) === false && retryCount > 0) { + console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); + //remove the dir and try again + await fsExtra.remove(bscNpmDir); + return this._ensureBscVersionInstalled(bsdkEntry, retryCount - 1, false); } return bscPath; diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index 067c6176..d5679804 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -9,6 +9,7 @@ import { VscodeCommand } from './VscodeCommand'; import URI from 'vscode-uri'; import * as dayjs from 'dayjs'; import * as relativeTime from 'dayjs/plugin/relativeTime'; +import { util } from '../util'; dayjs.extend(relativeTime); export class LanguageServerInfoCommand { @@ -35,11 +36,19 @@ export class LanguageServerInfoCommand { await vscode.commands.executeCommand('revealFileInOS', URI.file(languageServerManager.packagesDir)); } }, { - label: `Delete cached brighterscript versions`, + label: `Remove cached brighterscript versions`, description: ``, command: async () => { - await vscode.commands.executeCommand(VscodeCommand.clearNpmPackageCache); - await vscode.window.showInformationMessage('All cached brighterscript versions have been removed'); + await util.runWithProgress(async () => { + await vscode.commands.executeCommand(VscodeCommand.clearNpmPackageCache); + }, { + title: 'Removing cached brighterscript versions' + }); + + void vscode.window.showInformationMessage('All cached brighterscript versions have been removed'); + + //restart the language server since we might have just removed the one we're using + await this.restartLanguageServer(); } }]; @@ -137,7 +146,7 @@ export class LanguageServerInfoCommand { * call the reload manually */ public async selectBrighterScriptVersion() { - const versions = this.discoverBrighterScriptVersions( + const quickPickItems = this.discoverBrighterScriptVersions( vscode.workspace.workspaceFolders.map(x => this.getWorkspaceOrFolderPath(x.uri.fsPath)) ); @@ -145,7 +154,7 @@ export class LanguageServerInfoCommand { const versionsFromNpmPromise = this.getBscVersionsFromNpm(); //get the full list of versions from npm - versions.push({ + quickPickItems.push({ label: '$(package) Install from npm', description: '', detail: '', @@ -159,7 +168,7 @@ export class LanguageServerInfoCommand { } } as any); - let selection = await vscode.window.showQuickPick(versions, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; + let selection = await vscode.window.showQuickPick(quickPickItems, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; //if the selection has a command, run it before continuing; selection = await selection?.command?.() ?? selection; @@ -170,7 +179,7 @@ export class LanguageServerInfoCommand { await config.update('bsdk', undefined); //if the user picked "use embedded version", then remove the setting - if (versions.indexOf(selection) === 0) { + if (quickPickItems.indexOf(selection) === 0) { //setting to undefined means "remove" await config.update('bsdk', 'embedded'); return 'embedded'; diff --git a/src/util.ts b/src/util.ts index c0510a7e..e738713d 100644 --- a/src/util.ts +++ b/src/util.ts @@ -445,6 +445,22 @@ class Util { }); }); } + + /** + * Run an action with option for a progress spinner. If `showProgress` is `false` then no progress is shown and instead the action is run directly + */ + public async runWithProgress(action: () => PromiseLike, options: Partial & { showProgress?: boolean }) { + //show a progress spinner if configured to do so + if (options?.showProgress !== false) { + return vscode.window.withProgress({ + location: vscode.ProgressLocation.Notification, + cancellable: false, + ...options + }, action); + } else { + return action(); + } + } } const util = new Util(); From 87a922887996288a694aefa992b9357020e24e30 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Mon, 16 Sep 2024 15:51:12 -0400 Subject: [PATCH 11/25] Fix bug keeping test process alive for too long --- src/LanguageServerManager.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 09c312e1..95f581c4 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -55,6 +55,9 @@ describe('LanguageServerManager', () => { fsExtra.removeSync(storageDir); (languageServerManager['context'] as any).globalStorageUri = URI.file(storageDir); + //this delay is used to clean up old versions. for testing, have it trigger instantly so it doesn't keep the testing process alive + languageServerManager['outdatedBscVersionDeleteDelay'] = 0; + }); function stubConstructClient(processor?: (LanguageClient) => void) { From 97c7dd3cd7b2aade67f8efaf399ca872b4ec8ee9 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 18 Sep 2024 13:40:04 -0400 Subject: [PATCH 12/25] Add `LocalPackageManager` class, not finished yet --- src/managers/LocalPackageManager.spec.ts | 62 +++++++++++++ src/managers/LocalPackageManager.ts | 113 +++++++++++++++++++++++ src/util.ts | 37 ++++++++ 3 files changed, 212 insertions(+) create mode 100644 src/managers/LocalPackageManager.spec.ts create mode 100644 src/managers/LocalPackageManager.ts diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts new file mode 100644 index 00000000..494f2b43 --- /dev/null +++ b/src/managers/LocalPackageManager.spec.ts @@ -0,0 +1,62 @@ +import { LocalPackageManager, PackageCatalog } from './LocalPackageManager'; +import { standardizePath as s } from 'brighterscript'; +import * as fsExtra from 'fs-extra'; +import { expect } from 'chai'; +import { util } from '../util'; +import { createSandbox } from 'sinon'; +const sinon = createSandbox(); + +const cwd = s`${__dirname}/../../`; +const tempDir = s`${cwd}/.tmp`; + +describe.only('LocalPackageManager', function() { + this.timeout(10_000); + const storageDir = s`${tempDir}/storage`; + let manager: LocalPackageManager; + + beforeEach(() => { + manager = new LocalPackageManager(storageDir); + + fsExtra.emptyDirSync(storageDir); + sinon.restore(); + }); + + afterEach(() => { + fsExtra.removeSync(storageDir); + sinon.restore(); + }); + + function expectCatalogEquals(expectedCatalog: PackageCatalog) { + const catalog = fsExtra.readJsonSync(manager['catalogPath']); + //remove the `lastUpdated` property because it's not deterministic + for (let packageName in catalog.packages) { + for (let version in catalog.packages[packageName]) { + delete catalog.packages[packageName][version].lastUpdated; + } + } + expect(catalog).to.eql(expectedCatalog); + } + + it('installs a package when missing', async () => { + await manager.install('is-odd', '1.0.0'); + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; + }); + + it('skips install when package is already there', async () => { + const stub = sinon.stub(util, 'spawnAsync').callsFake(() => Promise.resolve()); + + fsExtra.ensureDirSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd`); + fsExtra.outputJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`, { + name: 'is-odd', + customKey: 'test' + }); + + await manager.install('is-odd', '1.0.0'); + + expect( + fsExtra.readJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`).customKey + ).to.eql('test'); + + expect(stub.called).to.be.false; + }); +}); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts new file mode 100644 index 00000000..7a6d78d6 --- /dev/null +++ b/src/managers/LocalPackageManager.ts @@ -0,0 +1,113 @@ +/* eslint-disable @typescript-eslint/consistent-indexed-object-style */ +import * as fsExtra from 'fs-extra'; +import { standardizePath as s } from 'brighterscript'; +import { util } from '../util'; + +/** + * Manages all node_module packages that are installed by this extension + */ +export class LocalPackageManager { + constructor( + public readonly storageLocation: string + ) { + this.catalogPath = s`${this.storageLocation}/catalog.json`; + } + + private catalogPath: string; + + /** + * Load the catalog object from disk + */ + private getCatalog(): PackageCatalog { + //load from disk + return fsExtra.readJsonSync(this.catalogPath, { throws: false }) || {}; + } + + /** + * Write the catalog object to disk + */ + private setCatalog(catalog: PackageCatalog) { + fsExtra.outputJsonSync(this.catalogPath, catalog); + } + + private setCatalogPackageInfo(packageName: string, version: string, info: PackageInfo) { + const catalog = this.getCatalog(); + + catalog.packages ??= {}; + catalog.packages[packageName] ??= {}; + catalog.packages[packageName][version] = info; + this.setCatalog(catalog); + } + + private getPackageDir(packageName: string, version: string): string { + return s`${this.storageLocation}/${packageName}/${version}`; + } + + public isInstalled(packageName: string, version: string) { + return fsExtra.pathExistsSync( + this.getPackageDir(packageName, version) + ); + } + + /** + * Install a package with the given name and version information + * @param packageName the name of the package + * @param version the versionInfo of the package. See {versionInfo} for more details + */ + public async install(packageName: string, version: string) { + const packageDir = this.getPackageDir(packageName, version); + + //if this package is already installed, skip the install + if (this.isInstalled(packageName, version)) { + return; + } + + fsExtra.ensureDirSync(packageDir); + + //write a simple package.json file referencing the version of brighterscript we want + await fsExtra.outputJson(`${packageDir}/package.json`, { + name: 'vscode-brighterscript-host', + private: true, + version: '1.0.0', + dependencies: { + [packageName]: version + } + }); + + //install the package + await util.spawnNpmAsync(['install'], { + cwd: packageDir + }); + + //update the catalog + this.setCatalogPackageInfo(packageName, version, { + dir: packageDir, + installDate: Date.now() + }); + } + + public dispose() { + + } +} + +/** + * The versionInfo of a package. This can be a specific version number, a semver range, a url to a package, or a release channel + */ +export type VersionInfo = string; + +export interface PackageCatalog { + packages: { + [packageName: string]: { + [version: string]: PackageInfo; + }; + }; +} + +export interface PackageInfo { + /** + * The path to the package on disk + */ + dir: string; + installDate: number; +} diff --git a/src/util.ts b/src/util.ts index e738713d..2605091e 100644 --- a/src/util.ts +++ b/src/util.ts @@ -446,6 +446,43 @@ class Util { }); } + /** + * Determine if the current OS is running a version of windows + */ + private isWindowsPlatform() { + return process.platform.startsWith('win'); + } + + /** + * Spawn an npm command and return a promise. + * This is necessary because spawn requires the file extension (.cmd) on windows. + * @param args - the list of args to pass to npm. Any undefined args will be removed from the list, so feel free to use ternary outside to simplify things + */ + spawnNpmAsync(args: Array, options?: childProcess.SpawnOptions) { + //filter out undefined args + args = args.filter(arg => arg !== undefined); + return this.spawnAsync( + this.isWindowsPlatform() ? 'npm.cmd' : 'npm', + args, + options + ); + } + + /** + * Executes an exec command and returns a promise that completes when it's finished + */ + spawnAsync(command: string, args?: string[], options?: childProcess.SpawnOptions) { + return new Promise((resolve, reject) => { + const child = childProcess.spawn(command, args ?? [], { + ...(options ?? {}), + stdio: 'inherit' + }); + child.addListener('error', reject); + child.addListener('exit', resolve); + }); + } + + /** * Run an action with option for a progress spinner. If `showProgress` is `false` then no progress is shown and instead the action is run directly */ From a26f523ae12d5fcde7ff61491f49ea299aa373bc Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 18 Sep 2024 13:46:27 -0400 Subject: [PATCH 13/25] Add ability to delete all of a given package --- src/managers/LocalPackageManager.spec.ts | 54 ++++++++++++++++-------- src/managers/LocalPackageManager.ts | 13 ++++++ 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 494f2b43..23bd8b71 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -1,4 +1,5 @@ -import { LocalPackageManager, PackageCatalog } from './LocalPackageManager'; +import type { PackageCatalog } from './LocalPackageManager'; +import { LocalPackageManager } from './LocalPackageManager'; import { standardizePath as s } from 'brighterscript'; import * as fsExtra from 'fs-extra'; import { expect } from 'chai'; @@ -9,8 +10,8 @@ const sinon = createSandbox(); const cwd = s`${__dirname}/../../`; const tempDir = s`${cwd}/.tmp`; -describe.only('LocalPackageManager', function() { - this.timeout(10_000); +describe.only('LocalPackageManager', () => { + const storageDir = s`${tempDir}/storage`; let manager: LocalPackageManager; @@ -37,26 +38,43 @@ describe.only('LocalPackageManager', function() { expect(catalog).to.eql(expectedCatalog); } - it('installs a package when missing', async () => { - await manager.install('is-odd', '1.0.0'); - expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; - }); + describe('install', function() { + this.timeout(10_000); + + it('installs a package when missing', async () => { + await manager.install('is-odd', '1.0.0'); + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; + }); + + it('skips install when package is already there', async () => { + const stub = sinon.stub(util, 'spawnAsync').callsFake(() => Promise.resolve()); - it('skips install when package is already there', async () => { - const stub = sinon.stub(util, 'spawnAsync').callsFake(() => Promise.resolve()); + fsExtra.ensureDirSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd`); + fsExtra.outputJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`, { + name: 'is-odd', + customKey: 'test' + }); - fsExtra.ensureDirSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd`); - fsExtra.outputJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`, { - name: 'is-odd', - customKey: 'test' + await manager.install('is-odd', '1.0.0'); + + expect( + fsExtra.readJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`).customKey + ).to.eql('test'); + + expect(stub.called).to.be.false; }); + }); - await manager.install('is-odd', '1.0.0'); + describe('removeAll', () => { + it('removes all packages', async () => { + fsExtra.ensureDirSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd`); + fsExtra.ensureDirSync(`${storageDir}/is-odd/1.1.0/node_modules/is-odd`); + fsExtra.ensureDirSync(`${storageDir}/is-even/1.1.0/node_modules/is-even`); - expect( - fsExtra.readJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`).customKey - ).to.eql('test'); + await manager.removeAll('is-odd'); - expect(stub.called).to.be.false; + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd`)).to.be.false; + expect(fsExtra.pathExistsSync(`${storageDir}/is-even`)).to.be.true; + }); }); }); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 7a6d78d6..de77e220 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -86,6 +86,19 @@ export class LocalPackageManager { }); } + /** + * Remove all packages with the given name + * @param packageName the name of the package that will have all versions removed + */ + public async removeAll(packageName: string) { + //delete the package folder + await fsExtra.remove(s`${this.storageLocation}/${packageName}`); + + const catalog = this.getCatalog(); + delete catalog?.packages?.[packageName]; + this.setCatalog(catalog); + } + public dispose() { } From 94090540a8d09b6b6105d20aa6509390dcfa3112 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 18 Sep 2024 14:03:27 -0400 Subject: [PATCH 14/25] Add ability to remove a specific package. --- src/managers/LocalPackageManager.spec.ts | 76 ++++++++++++++++++++++-- src/managers/LocalPackageManager.ts | 16 +++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 23bd8b71..40fcf6f8 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -27,12 +27,18 @@ describe.only('LocalPackageManager', () => { sinon.restore(); }); - function expectCatalogEquals(expectedCatalog: PackageCatalog) { + function expectCatalogEquals(expectedCatalog: { packages: Record> }) { const catalog = fsExtra.readJsonSync(manager['catalogPath']); //remove the `lastUpdated` property because it's not deterministic for (let packageName in catalog.packages) { for (let version in catalog.packages[packageName]) { - delete catalog.packages[packageName][version].lastUpdated; + delete catalog.packages[packageName][version].installDate; + + //coerce all paths to the same dir separator + catalog.packages[packageName][version].dir = s`${catalog.packages[packageName][version].dir}`; + if (expectedCatalog?.packages?.[packageName]?.[version]?.dir) { + expectedCatalog.packages[packageName][version].dir = s`${expectedCatalog.packages[packageName][version].dir}`; + } } } expect(catalog).to.eql(expectedCatalog); @@ -63,13 +69,75 @@ describe.only('LocalPackageManager', () => { expect(stub.called).to.be.false; }); + + it('installs multiple versions at the same time', async () => { + await Promise.all([ + manager.install('is-odd', '1.0.0'), + manager.install('is-odd', '2.0.0'), + manager.install('is-even', '1.0.0') + ]); + expectCatalogEquals({ + packages: { + 'is-odd': { + '1.0.0': { + dir: `${storageDir}/is-odd/1.0.0` + }, + '2.0.0': { + dir: `${storageDir}/is-odd/2.0.0` + } + }, + 'is-even': { + '1.0.0': { + dir: `${storageDir}/is-even/1.0.0` + } + } + } + }); + + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd/package.json`)).to.be.true; + expect(fsExtra.pathExistsSync(`${storageDir}/is-even/1.0.0/node_modules/is-even/package.json`)).to.be.true; + }); + + }); + + describe('remove', () => { + it('removes a specific package version', async () => { + await Promise.all([ + manager.install('is-odd', '1.0.0'), + manager.install('is-odd', '2.0.0'), + manager.install('is-even', '1.0.0') + ]); + + await manager.remove('is-odd', '2.0.0'); + + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd/package.json`)).to.be.false; + expect(fsExtra.pathExistsSync(`${storageDir}/is-even/1.0.0/node_modules/is-even/package.json`)).to.be.true; + + expectCatalogEquals({ + packages: { + 'is-odd': { + '1.0.0': { + dir: `${storageDir}/is-odd/1.0.0` + } + }, + 'is-even': { + '1.0.0': { + dir: `${storageDir}/is-even/1.0.0` + } + } + } + }); + + }); }); describe('removeAll', () => { it('removes all packages', async () => { fsExtra.ensureDirSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd`); - fsExtra.ensureDirSync(`${storageDir}/is-odd/1.1.0/node_modules/is-odd`); - fsExtra.ensureDirSync(`${storageDir}/is-even/1.1.0/node_modules/is-even`); + fsExtra.ensureDirSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd`); + fsExtra.ensureDirSync(`${storageDir}/is-even/1.0.0/node_modules/is-even`); await manager.removeAll('is-odd'); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index de77e220..c7749988 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -86,6 +86,22 @@ export class LocalPackageManager { }); } + /** + * Remove a specific version of a package + * @param packageName name of the package + * @param version version of the package to remove + */ + public async remove(packageName: string, version: VersionInfo) { + const packageDir = this.getPackageDir(packageName, version); + if (packageDir) { + await fsExtra.remove(packageDir); + + const catalog = this.getCatalog(); + delete catalog.packages?.[packageName]?.[version]; + this.setCatalog(catalog); + } + } + /** * Remove all packages with the given name * @param packageName the name of the package that will have all versions removed From 0aeed72e4363f76eaee5b581ec6a25d27e633178 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 18 Sep 2024 14:12:45 -0400 Subject: [PATCH 15/25] Fix coverage issues --- src/managers/LocalPackageManager.spec.ts | 24 ++++++++++++++++++++++++ src/managers/LocalPackageManager.ts | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 40fcf6f8..02dd539e 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -131,9 +131,27 @@ describe.only('LocalPackageManager', () => { }); }); + + it('does not crash when removing missing package', async () => { + await manager.remove('is-odd', '1.0.0'); + }); + + it('does not crash when packageDir is undefined', async () => { + sinon.stub(manager as any, 'getPackageDir').returns(undefined); + await manager.remove('is-odd', '1.0.0'); + }); }); describe('removeAll', () => { + it('removes entries from the catalog', async () => { + await manager.install('is-odd', '1.0.0'); + await manager.removeAll('is-odd'); + }); + + it('handles undefined package name', async () => { + await manager.removeAll(undefined as string); + }); + it('removes all packages', async () => { fsExtra.ensureDirSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd`); fsExtra.ensureDirSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd`); @@ -145,4 +163,10 @@ describe.only('LocalPackageManager', () => { expect(fsExtra.pathExistsSync(`${storageDir}/is-even`)).to.be.true; }); }); + + describe('dispose', () => { + it('works', () => { + manager.dispose(); + }); + }); }); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index c7749988..f87eee57 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -111,7 +111,7 @@ export class LocalPackageManager { await fsExtra.remove(s`${this.storageLocation}/${packageName}`); const catalog = this.getCatalog(); - delete catalog?.packages?.[packageName]; + delete catalog.packages?.[packageName]; this.setCatalog(catalog); } From 998f953e6c69ad4745980a493bd7ed4c7fb43a1e Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 18 Sep 2024 14:50:27 -0400 Subject: [PATCH 16/25] Update LanguageServermanager to use localPackageManager --- src/BrightScriptCommands.ts | 8 +-- src/LanguageServerManager.spec.ts | 15 +---- src/LanguageServerManager.ts | 73 +++++---------------- src/commands/ClearNpmPackageCacheCommand.ts | 6 +- src/commands/LanguageServerInfoCommand.ts | 14 ++-- src/extension.ts | 10 ++- src/managers/LocalPackageManager.spec.ts | 42 ++++++++++-- src/managers/LocalPackageManager.ts | 16 ++++- src/util.ts | 2 +- 9 files changed, 91 insertions(+), 95 deletions(-) diff --git a/src/BrightScriptCommands.ts b/src/BrightScriptCommands.ts index dc6b210f..c81e722a 100644 --- a/src/BrightScriptCommands.ts +++ b/src/BrightScriptCommands.ts @@ -15,7 +15,7 @@ import * as xml2js from 'xml2js'; import { firstBy } from 'thenby'; import type { UserInputManager } from './managers/UserInputManager'; import { clearNpmPackageCacheCommand } from './commands/ClearNpmPackageCacheCommand'; -import type { LanguageServerManager } from './LanguageServerManager'; +import type { LocalPackageManager } from './managers/LocalPackageManager'; export class BrightScriptCommands { @@ -25,7 +25,7 @@ export class BrightScriptCommands { private context: vscode.ExtensionContext, private activeDeviceManager: ActiveDeviceManager, private userInputManager: UserInputManager, - private languageServerManager: LanguageServerManager + private localPackageManager: LocalPackageManager ) { this.fileUtils = new BrightScriptFileUtils(); } @@ -39,10 +39,10 @@ export class BrightScriptCommands { public registerCommands() { brighterScriptPreviewCommand.register(this.context); - languageServerInfoCommand.register(this.context); + languageServerInfoCommand.register(this.context, this.localPackageManager); captureScreenshotCommand.register(this.context, this); rekeyAndPackageCommand.register(this.context, this, this.userInputManager); - clearNpmPackageCacheCommand.register(this.context, this.languageServerManager); + clearNpmPackageCacheCommand.register(this.context, this.localPackageManager); this.registerGeneralCommands(); diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 95f581c4..383bc320 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -88,7 +88,7 @@ describe('LanguageServerManager', () => { //disable starting so we can manually test sinon.stub(languageServerManager, 'syncVersionAndTryRun').callsFake(() => Promise.resolve()); - await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']); + await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository'], languageServerManager['localPackageManager']); languageServerManager['lspRunTracker'].debounceDelay = 100; @@ -431,17 +431,6 @@ describe('LanguageServerManager', () => { }); }); - describe('clearNpmPackageCache', () => { - it('clears the cache', async () => { - fsExtra.ensureFileSync(`${storageDir}/packages/test.txt`); - expect(fsExtra.pathExistsSync(`${storageDir}/packages/test.txt`)).to.be.true; - - await languageServerManager.clearNpmPackageCache(); - - expect(fsExtra.pathExistsSync(`${storageDir}/packages/test.txt`)).to.be.false; - }); - }); - describe('deleteOutdatedBscVersions', () => { beforeEach(() => { //prevent lsp from actually running @@ -453,7 +442,7 @@ describe('LanguageServerManager', () => { languageServerManager['outdatedBscVersionDeleteDelay'] = 50; - await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository']); + await languageServerManager.init(languageServerManager['context'], languageServerManager['definitionRepository'], languageServerManager['localPackageManager']); expect(stub.called).to.be.false; diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 901a9b93..a12cb3d0 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -33,6 +33,7 @@ import { EventEmitter } from 'eventemitter3'; import * as semver from 'semver'; import * as md5 from 'md5'; import * as dayjs from 'dayjs'; +import type { LocalPackageManager } from './managers/LocalPackageManager'; /** * Tracks the running/stopped state of the language server. When the lsp crashes, vscode will restart it. After the 5th crash, they'll leave it permanently crashed. @@ -92,6 +93,8 @@ export class LanguageServerManager { return this.definitionRepository.provider; } + private localPackageManager: LocalPackageManager; + /** * The delay after init before we delete any outdated bsc versions */ @@ -99,11 +102,15 @@ export class LanguageServerManager { public async init( context: vscode.ExtensionContext, - definitionRepository: DefinitionRepository + definitionRepository: DefinitionRepository, + localPackageManager: LocalPackageManager ) { this.context = context; + this.definitionRepository = definitionRepository; + this.localPackageManager = localPackageManager; + //anytime the window changes focus, save the current brighterscript version vscode.window.onDidChangeWindowState(async (e) => { await this.updateBscVersionUsageDate(this.selectedBscInfo.path); @@ -487,10 +494,6 @@ export class LanguageServerManager { ); } - public get packagesDir() { - return s`${this.context.globalStorageUri.fsPath}/packages`; - } - /** * Extract some info about the bsc version string * @param version can be a semantic version, a URL, or a folder path @@ -549,59 +552,17 @@ export class LanguageServerManager { return bsdkEntry; } - function runWithProgress(action) { + //install this version of brighterscript + return util.runWithProgress({ + title: 'Installing brighterscript language server ' + packageJsonEntry, + location: vscode.ProgressLocation.Notification, + cancellable: false, //show a progress spinner if configured to do so - if (showProgress) { - return vscode.window.withProgress({ - title: 'Installing brighterscript language server ' + packageJsonEntry, - location: vscode.ProgressLocation.Notification, - cancellable: false - }, action); - } else { - return action(); - } - } - - let bscPath: string; + showProgress: showProgress && !this.localPackageManager.isInstalled('brighterscript', packageJsonEntry) + }, async () => { - - console.log('Ensuring bsc version is installed', packageJsonEntry); - const bscNpmDir = s`${this.packagesDir}/${folderName}`; - //if the npm dir isn't there, install it - if (await fsExtra.pathExists(bscNpmDir) === false) { - //write a simple package.json file referencing the version of brighterscript we want - await fsExtra.outputJson(`${bscNpmDir}/package.json`, { - name: 'vscode-brighterscript-host', - private: true, - version: '1.0.0', - dependencies: { - 'brighterscript': packageJsonEntry - } - }); - await runWithProgress(async () => { - await util.exec('npm install', { - cwd: bscNpmDir - }); - }); - } - bscPath = s`${bscNpmDir}/node_modules/brighterscript`; - - //if the module is invalid, try again - if (await fsExtra.pathExists(`${bscPath}/dist/index.js`) === false && retryCount > 0) { - console.log(`Failed to load brighterscript module at ${bscNpmDir}. Deleting directory and trying again`); - //remove the dir and try again - await fsExtra.remove(bscNpmDir); - return this._ensureBscVersionInstalled(bsdkEntry, retryCount - 1, false); - } - - return bscPath; - } - - /** - * Clear all packages stored in the npm cache for this extension - */ - public async clearNpmPackageCache() { - await fsExtra.emptyDir(this.packagesDir); + return this.localPackageManager.install('brighterscript', packageJsonEntry); + }); } /** diff --git a/src/commands/ClearNpmPackageCacheCommand.ts b/src/commands/ClearNpmPackageCacheCommand.ts index 2f30ee9b..673e4663 100644 --- a/src/commands/ClearNpmPackageCacheCommand.ts +++ b/src/commands/ClearNpmPackageCacheCommand.ts @@ -1,12 +1,12 @@ import * as vscode from 'vscode'; -import type { LanguageServerManager } from '../LanguageServerManager'; import { VscodeCommand } from './VscodeCommand'; +import type { LocalPackageManager } from '../managers/LocalPackageManager'; export class ClearNpmPackageCacheCommand { - public register(context: vscode.ExtensionContext, languageServerManager: LanguageServerManager) { + public register(context: vscode.ExtensionContext, localPackageManager: LocalPackageManager) { context.subscriptions.push(vscode.commands.registerCommand(VscodeCommand.clearNpmPackageCache, async () => { - await languageServerManager.clearNpmPackageCache(); + await localPackageManager.removeAll(); })); } } diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index d5679804..6b3ba428 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -7,15 +7,17 @@ import * as childProcess from 'child_process'; import { firstBy } from 'thenby'; import { VscodeCommand } from './VscodeCommand'; import URI from 'vscode-uri'; -import * as dayjs from 'dayjs'; import * as relativeTime from 'dayjs/plugin/relativeTime'; import { util } from '../util'; +import type { LocalPackageManager } from '../managers/LocalPackageManager'; +import { standardizePath as s } from 'brighterscript'; +import * as dayjs from 'dayjs'; dayjs.extend(relativeTime); export class LanguageServerInfoCommand { public static commandName = 'extension.brightscript.languageServer.info'; - public register(context: vscode.ExtensionContext) { + public register(context: vscode.ExtensionContext, localPackageManager: LocalPackageManager) { context.subscriptions.push(vscode.commands.registerCommand(LanguageServerInfoCommand.commandName, async () => { const commands = [{ label: `Change Selected BrighterScript Version`, @@ -33,16 +35,16 @@ export class LanguageServerInfoCommand { label: `View BrighterScript version cache folder`, description: ``, command: async () => { - await vscode.commands.executeCommand('revealFileInOS', URI.file(languageServerManager.packagesDir)); + await vscode.commands.executeCommand('revealFileInOS', URI.file(s`${localPackageManager.storageLocation}/brighterscript`)); } }, { label: `Remove cached brighterscript versions`, description: ``, command: async () => { - await util.runWithProgress(async () => { - await vscode.commands.executeCommand(VscodeCommand.clearNpmPackageCache); - }, { + await util.runWithProgress({ title: 'Removing cached brighterscript versions' + }, async () => { + await vscode.commands.executeCommand(VscodeCommand.clearNpmPackageCache); }); void vscode.window.showInformationMessage('All cached brighterscript versions have been removed'); diff --git a/src/extension.ts b/src/extension.ts index fe153bf2..c6558b00 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -30,6 +30,8 @@ import { ViewProviderId } from './viewProviders/ViewProviderId'; import { DiagnosticManager } from './managers/DiagnosticManager'; import { EXTENSION_ID } from './constants'; import { UserInputManager } from './managers/UserInputManager'; +import { LocalPackageManager } from './managers/LocalPackageManager'; +import { standardizePath as s } from 'brighterscript'; export class Extension { public outputChannel: vscode.OutputChannel; @@ -63,6 +65,10 @@ export class Extension { }) ); + let localPackageManager = new LocalPackageManager( + s`${context.globalStorageUri.fsPath}/packages` + ); + this.telemetryManager.sendStartupEvent(); let activeDeviceManager = new ActiveDeviceManager(); let userInputManager = new UserInputManager( @@ -76,7 +82,7 @@ export class Extension { context, activeDeviceManager, userInputManager, - languageServerManager + localPackageManager ); this.rtaManager = new RtaManager(context); @@ -103,7 +109,7 @@ export class Extension { const definitionRepo = new DefinitionRepository(declarationProvider); //initialize the LanguageServerManager - void languageServerManager.init(context, definitionRepo); + void languageServerManager.init(context, definitionRepo, localPackageManager); //register a tree data provider for this extension's "RENDEZVOUS" view in the debug area let rendezvousViewProvider = new RendezvousViewProvider(context); diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 02dd539e..11e8eec2 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -20,6 +20,20 @@ describe.only('LocalPackageManager', () => { fsExtra.emptyDirSync(storageDir); sinon.restore(); + + //mock the npm install command to speed up the tests + sinon.stub(util, 'spawnNpmAsync').callsFake(async (args: string[], options) => { + let spawnCwd = s`${options.cwd?.toString()}`; + if (args[0] !== 'install' || !spawnCwd.startsWith(storageDir)) { + throw new Error(`Invalid cwd: ${spawnCwd}`); + } + + //get the dependency name + const packageName = Object.keys( + fsExtra.readJsonSync(`${spawnCwd}/package.json`).dependencies + )[0]; + await fsExtra.outputJson(s`${spawnCwd}/node_modules/${packageName}/package.json`, {}); + }); }); afterEach(() => { @@ -109,7 +123,7 @@ describe.only('LocalPackageManager', () => { manager.install('is-even', '1.0.0') ]); - await manager.remove('is-odd', '2.0.0'); + await manager.removePackageVersion('is-odd', '2.0.0'); expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd/package.json`)).to.be.false; @@ -133,23 +147,23 @@ describe.only('LocalPackageManager', () => { }); it('does not crash when removing missing package', async () => { - await manager.remove('is-odd', '1.0.0'); + await manager.removePackageVersion('is-odd', '1.0.0'); }); it('does not crash when packageDir is undefined', async () => { sinon.stub(manager as any, 'getPackageDir').returns(undefined); - await manager.remove('is-odd', '1.0.0'); + await manager.removePackageVersion('is-odd', '1.0.0'); }); }); - describe('removeAll', () => { + describe('removePackage', () => { it('removes entries from the catalog', async () => { await manager.install('is-odd', '1.0.0'); - await manager.removeAll('is-odd'); + await manager.removePackage('is-odd'); }); it('handles undefined package name', async () => { - await manager.removeAll(undefined as string); + await manager.removePackage(undefined as string); }); it('removes all packages', async () => { @@ -157,13 +171,27 @@ describe.only('LocalPackageManager', () => { fsExtra.ensureDirSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd`); fsExtra.ensureDirSync(`${storageDir}/is-even/1.0.0/node_modules/is-even`); - await manager.removeAll('is-odd'); + await manager.removePackage('is-odd'); expect(fsExtra.pathExistsSync(`${storageDir}/is-odd`)).to.be.false; expect(fsExtra.pathExistsSync(`${storageDir}/is-even`)).to.be.true; }); }); + describe('removeAll', () => { + it('removes everything from the storage dir', async () => { + await manager.install('is-odd', '1.0.0'); + + expect(fsExtra.pathExistsSync(`${storageDir}/catalog.json`)).to.be.true; + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd`)).to.be.true; + + await manager.removeAll(); + + expect(fsExtra.pathExistsSync(`${storageDir}/catalog.json`)).to.be.false; + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd`)).to.be.false; + }); + }); + describe('dispose', () => { it('works', () => { manager.dispose(); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index f87eee57..1c65c1cb 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -53,8 +53,9 @@ export class LocalPackageManager { * Install a package with the given name and version information * @param packageName the name of the package * @param version the versionInfo of the package. See {versionInfo} for more details + * @returns the absolute path to the installed package */ - public async install(packageName: string, version: string) { + public async install(packageName: string, version: string): Promise { const packageDir = this.getPackageDir(packageName, version); //if this package is already installed, skip the install @@ -84,6 +85,8 @@ export class LocalPackageManager { dir: packageDir, installDate: Date.now() }); + + return s`${packageDir}/node_modules/${packageName}`; } /** @@ -91,7 +94,7 @@ export class LocalPackageManager { * @param packageName name of the package * @param version version of the package to remove */ - public async remove(packageName: string, version: VersionInfo) { + public async removePackageVersion(packageName: string, version: VersionInfo) { const packageDir = this.getPackageDir(packageName, version); if (packageDir) { await fsExtra.remove(packageDir); @@ -106,7 +109,7 @@ export class LocalPackageManager { * Remove all packages with the given name * @param packageName the name of the package that will have all versions removed */ - public async removeAll(packageName: string) { + public async removePackage(packageName: string) { //delete the package folder await fsExtra.remove(s`${this.storageLocation}/${packageName}`); @@ -115,6 +118,13 @@ export class LocalPackageManager { this.setCatalog(catalog); } + /** + * Remove all packages and their versions + */ + public async removeAll() { + await fsExtra.emptyDir(this.storageLocation); + } + public dispose() { } diff --git a/src/util.ts b/src/util.ts index 2605091e..48a6f4da 100644 --- a/src/util.ts +++ b/src/util.ts @@ -486,7 +486,7 @@ class Util { /** * Run an action with option for a progress spinner. If `showProgress` is `false` then no progress is shown and instead the action is run directly */ - public async runWithProgress(action: () => PromiseLike, options: Partial & { showProgress?: boolean }) { + public async runWithProgress(options: Partial & { showProgress?: boolean }, action: () => PromiseLike) { //show a progress spinner if configured to do so if (options?.showProgress !== false) { return vscode.window.withProgress({ From a4098125ef4830c89e88e2bb7548a94b11b7def0 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Wed, 18 Sep 2024 14:59:26 -0400 Subject: [PATCH 17/25] Tweak comment --- src/managers/LocalPackageManager.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 1c65c1cb..626f0d6d 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -131,7 +131,12 @@ export class LocalPackageManager { } /** - * The versionInfo of a package. This can be a specific version number, a semver range, a url to a package, or a release channel + * The versionInfo of a package. This can be: + * - specific version number (i.e. `1.0.0`, `2.3.4-alpha.1`) + * - a url to a package (i.e. `https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz`) + * - TODO: a path to a local package (i.e. `file:/path/to/package.tgz`) + * - TODO: a release tag (i.e. `@latest`, `@next`) + * - TODO: a release line (i.e. `insider:lsp-rewrite`) */ export type VersionInfo = string; From 277d21c09c843f8e56ca44605cd6308f8eb49979 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 19 Sep 2024 08:47:02 -0400 Subject: [PATCH 18/25] Better internal package object handling --- package-lock.json | 8 + package.json | 2 + src/managers/LocalPackageManager.spec.ts | 107 ++++++++++-- src/managers/LocalPackageManager.ts | 201 +++++++++++++++++++---- 4 files changed, 270 insertions(+), 48 deletions(-) diff --git a/package-lock.json b/package-lock.json index ac6389d4..123f4aca 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56,6 +56,7 @@ "@types/clone-deep": "^4.0.3", "@types/fs-extra": "^5.0.4", "@types/glob": "^7.1.1", + "@types/lodash": "^4.17.7", "@types/mocha": "^7.0.2", "@types/node": "^20.14.10", "@types/node-ssdp": "^3.3.0", @@ -78,6 +79,7 @@ "eslint": "^8.10.0", "eslint-plugin-github": "^4.3.5", "eslint-plugin-no-only-tests": "^2.6.0", + "lodash": "^4.17.21", "mocha": "^9.1.3", "node-notifier": "^10.0.1", "nyc": "^15.0.0", @@ -1450,6 +1452,12 @@ "integrity": "sha512-dRLjCWHYg4oaA77cxO64oO+7JwCwnIzkZPdrrC71jQmQtlhM556pwKo5bUzqvZndkVbeFLIIi+9TC40JNF5hNQ==", "dev": true }, + "node_modules/@types/lodash": { + "version": "4.17.7", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.17.7.tgz", + "integrity": "sha512-8wTvZawATi/lsmNu10/j2hk1KEP0IvjubqPE3cu1Xz7xfXXt5oCq3SNUz4fMIP4XGF9Ky+Ue2tBA3hcS7LSBlA==", + "dev": true + }, "node_modules/@types/mime": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/@types/mime/-/mime-1.3.5.tgz", diff --git a/package.json b/package.json index 59115190..51476e23 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,7 @@ "@types/clone-deep": "^4.0.3", "@types/fs-extra": "^5.0.4", "@types/glob": "^7.1.1", + "@types/lodash": "^4.17.7", "@types/mocha": "^7.0.2", "@types/node": "^20.14.10", "@types/node-ssdp": "^3.3.0", @@ -120,6 +121,7 @@ "eslint": "^8.10.0", "eslint-plugin-github": "^4.3.5", "eslint-plugin-no-only-tests": "^2.6.0", + "lodash": "^4.17.21", "mocha": "^9.1.3", "node-notifier": "^10.0.1", "nyc": "^15.0.0", diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 11e8eec2..4a9b4907 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -1,9 +1,12 @@ -import type { PackageCatalog } from './LocalPackageManager'; +import type { PackageCatalogPackageInfo } from './LocalPackageManager'; import { LocalPackageManager } from './LocalPackageManager'; import { standardizePath as s } from 'brighterscript'; import * as fsExtra from 'fs-extra'; import { expect } from 'chai'; import { util } from '../util'; +import * as dayjs from 'dayjs'; +import { vscode } from '../mockVscode.spec'; +import * as lodash from 'lodash'; import { createSandbox } from 'sinon'; const sinon = createSandbox(); @@ -16,7 +19,7 @@ describe.only('LocalPackageManager', () => { let manager: LocalPackageManager; beforeEach(() => { - manager = new LocalPackageManager(storageDir); + manager = new LocalPackageManager(storageDir, vscode.context); fsExtra.emptyDirSync(storageDir); sinon.restore(); @@ -41,18 +44,12 @@ describe.only('LocalPackageManager', () => { sinon.restore(); }); - function expectCatalogEquals(expectedCatalog: { packages: Record> }) { + function expectCatalogEquals(expectedCatalog: { packages: Record>> }) { const catalog = fsExtra.readJsonSync(manager['catalogPath']); //remove the `lastUpdated` property because it's not deterministic for (let packageName in catalog.packages) { for (let version in catalog.packages[packageName]) { delete catalog.packages[packageName][version].installDate; - - //coerce all paths to the same dir separator - catalog.packages[packageName][version].dir = s`${catalog.packages[packageName][version].dir}`; - if (expectedCatalog?.packages?.[packageName]?.[version]?.dir) { - expectedCatalog.packages[packageName][version].dir = s`${expectedCatalog.packages[packageName][version].dir}`; - } } } expect(catalog).to.eql(expectedCatalog); @@ -94,15 +91,15 @@ describe.only('LocalPackageManager', () => { packages: { 'is-odd': { '1.0.0': { - dir: `${storageDir}/is-odd/1.0.0` + versionDirName: '1.0.0' }, '2.0.0': { - dir: `${storageDir}/is-odd/2.0.0` + versionDirName: '2.0.0' } }, 'is-even': { '1.0.0': { - dir: `${storageDir}/is-even/1.0.0` + versionDirName: '1.0.0' } } } @@ -133,12 +130,12 @@ describe.only('LocalPackageManager', () => { packages: { 'is-odd': { '1.0.0': { - dir: `${storageDir}/is-odd/1.0.0` + versionDirName: '1.0.0' } }, 'is-even': { '1.0.0': { - dir: `${storageDir}/is-even/1.0.0` + versionDirName: '1.0.0' } } } @@ -149,11 +146,52 @@ describe.only('LocalPackageManager', () => { it('does not crash when removing missing package', async () => { await manager.removePackageVersion('is-odd', '1.0.0'); }); + }); - it('does not crash when packageDir is undefined', async () => { - sinon.stub(manager as any, 'getPackageDir').returns(undefined); - await manager.removePackageVersion('is-odd', '1.0.0'); + describe('withCatalog', () => { + it('loads the default catalog when not supplied', async () => { + + //install a package so the catalog is non-empty + await manager.install('is-odd', '1.0.0'); + await manager.install('is-odd', '2.0.0'); + + //ensure the catalog is populated correctly + expect(manager['getCatalog']().packages['is-odd']['1.0.0'].versionDirName).to.eql('1.0.0'); + expect(manager['getCatalog']().packages['is-odd']['2.0.0'].versionDirName).to.eql('2.0.0'); + + const spy = sinon.spy(manager as any, 'setCatalog'); + + await manager['withCatalog']((catalog) => { + //did it load the correct catalog? + expect(catalog.packages['is-odd']['1.0.0'].versionDirName).to.eql('1.0.0'); + expect(catalog.packages['is-odd']['2.0.0'].versionDirName).to.eql('2.0.0'); + + //delete the entry from the catalog + delete catalog.packages['is-odd']['2.0.0']; + }); + + expect(manager['getCatalog']().packages['is-odd']['1.0.0'].versionDirName).to.eql('1.0.0'); + expect(manager['getCatalog']().packages['is-odd']?.['2.0.0']).to.be.undefined; + + expect(spy.called).to.be.true; + }); + + it('uses the given catalog', async () => { + //install a package so the catalog is non-empty + await manager.install('is-odd', '1.0.0'); + + const actualCatalog = manager['getCatalog'](); + + const spy = sinon.spy(manager as any, 'setCatalog'); + + await manager['withCatalog']((catalog) => { + expect(catalog).to.equal(catalog); + }, actualCatalog); + + //when we provide a catalog, it shouldn't write to disk itself + expect(spy.called).to.be.false; }); + }); describe('removePackage', () => { @@ -192,6 +230,41 @@ describe.only('LocalPackageManager', () => { }); }); + describe('usage', () => { + it('uses a default date when not specified', async () => { + const now = new Date(); + + await manager.setUsage('is-odd', '1.0.0'); + + const usage = manager['getPackageInfo']('is-odd', '1.0.0'); + + expect(usage.lastUsedDate).to.be.within(now, new Date()); + }); + + it('marks a package as used right now', async () => { + await manager.install('is-odd', '1.0.0'); + + const now = new Date(); + const littleAfterNow = dayjs(now).add(1, 'minute').toDate(); + const yesterday = dayjs(now).subtract(1, 'days').toDate(); + + await manager.setUsage('is-odd', '1.0.0', now); + + await manager.deletePackagesOlderThan(yesterday); + + expect( + manager.isInstalled('is-odd', '1.0.0') + ).to.be.true; + + await manager.setUsage('is-odd', '1.0.0', now); + + await manager.deletePackagesOlderThan(littleAfterNow); + expect( + manager.isInstalled('is-odd', '1.0.0') + ).to.be.false; + }); + }); + describe('dispose', () => { it('works', () => { manager.dispose(); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 626f0d6d..4f975d4b 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -2,13 +2,19 @@ import * as fsExtra from 'fs-extra'; import { standardizePath as s } from 'brighterscript'; import { util } from '../util'; +import type { ExtensionContext } from 'vscode'; +import * as lodash from 'lodash'; +import { isPromise } from 'util/types'; + +const USAGE_KEY = 'local-package-usage'; /** * Manages all node_module packages that are installed by this extension */ export class LocalPackageManager { constructor( - public readonly storageLocation: string + public readonly storageLocation: string, + public readonly context: ExtensionContext ) { this.catalogPath = s`${this.storageLocation}/catalog.json`; } @@ -20,7 +26,7 @@ export class LocalPackageManager { */ private getCatalog(): PackageCatalog { //load from disk - return fsExtra.readJsonSync(this.catalogPath, { throws: false }) || {}; + return fsExtra.readJsonSync(this.catalogPath, { throws: false }) ?? {}; } /** @@ -30,23 +36,22 @@ export class LocalPackageManager { fsExtra.outputJsonSync(this.catalogPath, catalog); } - private setCatalogPackageInfo(packageName: string, version: string, info: PackageInfo) { + private setCatalogPackageInfo(packageName: string, version: string, info: PackageCatalogPackageInfo) { const catalog = this.getCatalog(); - catalog.packages ??= {}; - catalog.packages[packageName] ??= {}; - catalog.packages[packageName][version] = info; - this.setCatalog(catalog); - } + lodash.set(catalog, ['packages', packageName, version], info); - private getPackageDir(packageName: string, version: string): string { - return s`${this.storageLocation}/${packageName}/${version}`; + this.setCatalog(catalog); } - public isInstalled(packageName: string, version: string) { - return fsExtra.pathExistsSync( - this.getPackageDir(packageName, version) - ); + /** + * Is the given package installed + * @param packageName name of the package + * @param versionInfo versionInfo of the package + * @returns true if the package is installed, false if not + */ + public isInstalled(packageName: string, versionInfo: string) { + return this.getPackageInfo(packageName, versionInfo).isInstalled; } /** @@ -56,17 +61,18 @@ export class LocalPackageManager { * @returns the absolute path to the installed package */ public async install(packageName: string, version: string): Promise { - const packageDir = this.getPackageDir(packageName, version); + const packageInfo = this.getPackageInfo(packageName, version); //if this package is already installed, skip the install - if (this.isInstalled(packageName, version)) { + if (packageInfo.isInstalled) { return; } + const rootDir = s`${this.storageLocation}/${packageName}/${packageInfo.versionDirName}`; - fsExtra.ensureDirSync(packageDir); + fsExtra.ensureDirSync(rootDir); //write a simple package.json file referencing the version of brighterscript we want - await fsExtra.outputJson(`${packageDir}/package.json`, { + await fsExtra.outputJson(`${rootDir}/package.json`, { name: 'vscode-brighterscript-host', private: true, version: '1.0.0', @@ -77,16 +83,16 @@ export class LocalPackageManager { //install the package await util.spawnNpmAsync(['install'], { - cwd: packageDir + cwd: rootDir }); //update the catalog this.setCatalogPackageInfo(packageName, version, { - dir: packageDir, + versionDirName: packageInfo.versionDirName, installDate: Date.now() }); - return s`${packageDir}/node_modules/${packageName}`; + return s`${rootDir}/node_modules/${packageName}`; } /** @@ -94,15 +100,30 @@ export class LocalPackageManager { * @param packageName name of the package * @param version version of the package to remove */ - public async removePackageVersion(packageName: string, version: VersionInfo) { - const packageDir = this.getPackageDir(packageName, version); - if (packageDir) { - await fsExtra.remove(packageDir); - - const catalog = this.getCatalog(); + public async removePackageVersion(packageName: string, version: VersionInfo, catalog?: PackageCatalog) { + await this.withCatalog(async (catalog) => { + const info = this.getPackageInfo(packageName, version, catalog); + await fsExtra.remove(info.rootDir); delete catalog.packages?.[packageName]?.[version]; + }, catalog); + } + + /** + * Run an action with a given catalog object. If no catalog is provided, the catalog will be loaded from disk and saved back to disk after the action is complete. + * If a catalog is provided, it's assumed the outside caller will handle saving the catalog to disk + */ + private async withCatalog(callback: (catalog: PackageCatalog) => T | PromiseLike, catalog?: PackageCatalog): Promise { + let hasExternalCatalog = !!catalog; + catalog ??= this.getCatalog(); + + const result = await Promise.resolve( + callback(catalog) + ); + + if (!hasExternalCatalog) { this.setCatalog(catalog); } + return result; } /** @@ -125,6 +146,91 @@ export class LocalPackageManager { await fsExtra.emptyDir(this.storageLocation); } + /** + * Get info about this package (regardless of whether it's installed or not). + * If the package is not installed, all + * @param packageName name of the package + * @param versionInfo versionInfo of the package + * @param catalog the catalog object. If not provided, it will be loaded from disk + * @returns + */ + private getPackageInfo(packageName: string, versionInfo: VersionInfo, catalog = this.getCatalog()): PackageInfo { + //TODO derive a better name for some edge cases (like urls or tags) + const versionDirName = versionInfo; + + const rootDir = s`${this.storageLocation}/${packageName}/${versionDirName}`; + const packageDir = s`${rootDir}/node_modules/${packageName}`; + const packageInfo = (catalog.packages?.[packageName]?.[versionInfo] ?? {}) as PackageCatalogPackageInfo; + const lastUseDate = this.context.globalState.get(USAGE_KEY, {})[packageName]?.[versionInfo]; + return { + packageName: packageName, + versionInfo: versionInfo, + rootDir: rootDir, + packageDir: packageDir, + versionDirName: versionDirName, + isInstalled: fsExtra.pathExistsSync(packageDir), + lastUsedDate: lastUseDate ? new Date(lastUseDate) : undefined, + installDate: packageInfo.installDate ? new Date(packageInfo.installDate) : undefined + }; + } + + /** + * Mark a package as being used by the user right now. This can help with determining which packages are safe to remove after a period of time. + * @param packageName the name of the package + * @param version the version of the package + */ + public async setUsage(packageName: string, version: VersionInfo, dateUsed: Date = new Date()) { + const usage = this.context.globalState.get(USAGE_KEY, {}); + lodash.set(usage, [packageName, version], dateUsed.getTime()); + await this.context.globalState.update(USAGE_KEY, usage); + } + + /** + * Delete packages that havent older than the given cutoff date + * @param cutoffDate any package not used since this date will be deleted + */ + public async deletePackagesOlderThan(cutoffDate: Date) { + //get the list of directories from the storage folder (these are our package names) + const packageNames = (await fsExtra.readdir(this.storageLocation)) + .filter(x => x !== 'catalog.json'); + + let onDiskPackages = {}; + + //get every version folder for each package + await Promise.all( + packageNames.map(async (packageName) => { + onDiskPackages[packageName] = {}; + for (const versionDirName of await fsExtra.readdir(s`${this.storageLocation}/${packageName}`)) { + //set to the oldest date possible + onDiskPackages[packageName][versionDirName] = 0; + } + }) + ); + + const catalog = this.getCatalog(); + + //now get the actual usage dates + const usage = this.context.globalState.get(USAGE_KEY, {}); + for (const [packageName, versions] of Object.entries(usage)) { + for (const [version, dateUsed] of Object.entries(versions)) { + const packageInfo = this.getPackageInfo(packageName, version, catalog); + onDiskPackages[packageName][packageInfo.versionDirName] = dateUsed; + } + } + + let cutoffDateMs = cutoffDate.getTime(); + //now delete every directory that's older than our date + for (const [packageName, versions] of Object.entries(onDiskPackages)) { + for (const [versionDirName, lastUsedDate] of Object.entries(versions)) { + if (lastUsedDate < cutoffDateMs) { + await this.removePackageVersion(packageName, versionDirName); + } + } + } + this.setCatalog(catalog); + } + + public dispose() { } @@ -143,15 +249,48 @@ export type VersionInfo = string; export interface PackageCatalog { packages: { [packageName: string]: { - [version: string]: PackageInfo; + [version: string]: PackageCatalogPackageInfo; }; }; } +export interface PackageCatalogPackageInfo { + versionDirName: string; + installDate: number; +} + export interface PackageInfo { /** - * The path to the package on disk + * The name of the package */ - dir: string; - installDate: number; + packageName: string; + /** + * The versionInfo of the package. + */ + versionInfo: VersionInfo; + /** + * The directory where this this package version will be installed (i.e. `${storageDir}/${packageName}/${versionDirName}`) + */ + rootDir: string; + /** + * Directory where this package will actually be located (i.e. `${packageDir}/node_modules/${packageName}`) + */ + packageDir: string; + /** + * The name of the directory representing this version. If versionInfo is a semantic version, we'll use that for the dirName. + * Otherwise, we'll create a unique hash of the versionInfo + */ + versionDirName: string; + /** + * Is this package currently installed + */ + isInstalled: boolean; + /** + * Date this package was installed + */ + installDate: Date; + /** + * Date this package was last used by vscode + */ + lastUsedDate: Date; } From 91fc9943c72805e160e0a9efa90fa92d27a4bc47 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 19 Sep 2024 09:05:44 -0400 Subject: [PATCH 19/25] Add live npm install test --- src/managers/LocalPackageManager.spec.ts | 9 ++++++++- src/managers/LocalPackageManager.ts | 1 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 4a9b4907..88ef66a7 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -6,7 +6,6 @@ import { expect } from 'chai'; import { util } from '../util'; import * as dayjs from 'dayjs'; import { vscode } from '../mockVscode.spec'; -import * as lodash from 'lodash'; import { createSandbox } from 'sinon'; const sinon = createSandbox(); @@ -58,6 +57,14 @@ describe.only('LocalPackageManager', () => { describe('install', function() { this.timeout(10_000); + it('actually works with real npm package', async () => { + //remove the mock npm install + sinon.restore(); + + await manager.install('is-odd', '1.0.0'); + expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; + }); + it('installs a package when missing', async () => { await manager.install('is-odd', '1.0.0'); expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 4f975d4b..8b530b9e 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -4,7 +4,6 @@ import { standardizePath as s } from 'brighterscript'; import { util } from '../util'; import type { ExtensionContext } from 'vscode'; import * as lodash from 'lodash'; -import { isPromise } from 'util/types'; const USAGE_KEY = 'local-package-usage'; From 4147d3137a57d1d5289ff2f9f6ad7e5880b71df0 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 19 Sep 2024 11:08:46 -0400 Subject: [PATCH 20/25] support for non-semver versions --- package-lock.json | 7 +++++ package.json | 1 + src/managers/LocalPackageManager.spec.ts | 20 ++++++++++++ src/managers/LocalPackageManager.ts | 40 ++++++++++++++++++++++-- 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 123f4aca..7e9d30a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,6 +57,7 @@ "@types/fs-extra": "^5.0.4", "@types/glob": "^7.1.1", "@types/lodash": "^4.17.7", + "@types/md5": "^2.3.5", "@types/mocha": "^7.0.2", "@types/node": "^20.14.10", "@types/node-ssdp": "^3.3.0", @@ -1458,6 +1459,12 @@ "integrity": "sha512-8wTvZawATi/lsmNu10/j2hk1KEP0IvjubqPE3cu1Xz7xfXXt5oCq3SNUz4fMIP4XGF9Ky+Ue2tBA3hcS7LSBlA==", "dev": true }, + "node_modules/@types/md5": { + "version": "2.3.5", + "resolved": "https://registry.npmjs.org/@types/md5/-/md5-2.3.5.tgz", + "integrity": "sha512-/i42wjYNgE6wf0j2bcTX6kuowmdL/6PE4IVitMpm2eYKBUuYCprdcWVK+xEF0gcV6ufMCRhtxmReGfc6hIK7Jw==", + "dev": true + }, "node_modules/@types/mime": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/@types/mime/-/mime-1.3.5.tgz", diff --git a/package.json b/package.json index 51476e23..24726284 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "@types/fs-extra": "^5.0.4", "@types/glob": "^7.1.1", "@types/lodash": "^4.17.7", + "@types/md5": "^2.3.5", "@types/mocha": "^7.0.2", "@types/node": "^20.14.10", "@types/node-ssdp": "^3.3.0", diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 88ef66a7..49cae547 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -117,6 +117,26 @@ describe.only('LocalPackageManager', () => { expect(fsExtra.pathExistsSync(`${storageDir}/is-even/1.0.0/node_modules/is-even/package.json`)).to.be.true; }); + // it('installs packages from a URL', async () => { + // const url = 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz'; + // await manager.install('brighterscript', url); + // const info = manager['getPackageInfo']('brighterscript', url); + // expect( + // fsExtra.pathExistsSync(info.packageDir) + // ).to.be.true; + // }); + }); + + describe('getPackageInfo', () => { + it('transforms URLs into a filesystem-safe name', () => { + const info = manager['getPackageInfo']( + 'brighterscript', + 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz' + ); + expect( + info.versionDirName + ).to.match(/^[a-z0-9_]+$/i); + }); }); describe('remove', () => { diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 8b530b9e..5ca65bea 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -4,6 +4,8 @@ import { standardizePath as s } from 'brighterscript'; import { util } from '../util'; import type { ExtensionContext } from 'vscode'; import * as lodash from 'lodash'; +import * as md5 from 'md5'; +import * as semver from 'semver'; const USAGE_KEY = 'local-package-usage'; @@ -145,6 +147,38 @@ export class LocalPackageManager { await fsExtra.emptyDir(this.storageLocation); } + /** + * Create a filesystem-safe name for the given version. This will be used as the directory name for the package version. + * Will also handle collisions with existing directories by appending a number to the end of the directory name if we already have + * a directory with the same name for this package + * @param version + * @returns + */ + private getVersionDirName(packageName: string, version: string, catalog = this.getCatalog()) { + const existingVersionDirName = catalog.packages?.[packageName]?.[version]?.versionDirName; + + //if there's already a directory for this package, return it + if (existingVersionDirName) { + return existingVersionDirName; + } + + //this is a valid semver number, so we can use it as the directory name + if (semver.valid(version)) { + return version; + } else { + + //hash the string to create a unique folder name. There is next to zero possibility these will clash, but we'll handle collisions anyway + const hash = md5(version.trim()); + const existingHashes = Object.values(catalog.packages?.[packageName] ?? {}).map(x => x.versionDirName); + let newHash = hash; + let i = 0; + while (existingHashes.includes(newHash)) { + newHash = hash + i++; + } + return newHash; + } + } + /** * Get info about this package (regardless of whether it's installed or not). * If the package is not installed, all @@ -155,7 +189,7 @@ export class LocalPackageManager { */ private getPackageInfo(packageName: string, versionInfo: VersionInfo, catalog = this.getCatalog()): PackageInfo { //TODO derive a better name for some edge cases (like urls or tags) - const versionDirName = versionInfo; + const versionDirName = this.getVersionDirName(packageName, versionInfo, catalog); const rootDir = s`${this.storageLocation}/${packageName}/${versionDirName}`; const packageDir = s`${rootDir}/node_modules/${packageName}`; @@ -268,7 +302,9 @@ export interface PackageInfo { */ versionInfo: VersionInfo; /** - * The directory where this this package version will be installed (i.e. `${storageDir}/${packageName}/${versionDirName}`) + * The directory where the top-level folder for this package and version will be located. (i.e. `${storageDir}/${packageName}/${versionDirName}`). + * Due to how how we install packages, this will be the root directory for the package which contains a barebones `package.json` file, + * and once installed, will also contain the `node_modules/${packageName}` folder. */ rootDir: string; /** From 1f0f92160863664134ad1787d9e42e78ccd6c89c Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 19 Sep 2024 16:06:35 -0400 Subject: [PATCH 21/25] Refactored LanguageServerManager to better handle bsc version --- src/LanguageServerManager.ts | 308 +++++++++++------------ src/managers/LocalPackageManager.spec.ts | 10 +- src/managers/LocalPackageManager.ts | 105 +++++++- src/util.ts | 9 +- tsconfig.json | 4 +- 5 files changed, 255 insertions(+), 181 deletions(-) diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index a12cb3d0..ca8d1031 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -1,21 +1,9 @@ -import type { - LanguageClientOptions, - ServerOptions, - ExecuteCommandParams, - StateChangeEvent -} from 'vscode-languageclient/node'; -import { - LanguageClient, - State, - TransportKind -} from 'vscode-languageclient/node'; +import type { LanguageClientOptions, ServerOptions, ExecuteCommandParams, StateChangeEvent } from 'vscode-languageclient/node'; +import { LanguageClient, State, TransportKind } from 'vscode-languageclient/node'; import * as vscode from 'vscode'; import * as path from 'path'; import type { Disposable } from 'vscode'; -import { - window, - workspace -} from 'vscode'; +import { window, workspace } from 'vscode'; import { BusyStatus, NotificationName, standardizePath as s } from 'brighterscript'; import { Logger } from '@rokucommunity/logger'; import { CustomCommands, Deferred } from 'brighterscript'; @@ -30,10 +18,8 @@ import { util } from './util'; import { LanguageServerInfoCommand, languageServerInfoCommand } from './commands/LanguageServerInfoCommand'; import * as fsExtra from 'fs-extra'; import { EventEmitter } from 'eventemitter3'; -import * as semver from 'semver'; -import * as md5 from 'md5'; import * as dayjs from 'dayjs'; -import type { LocalPackageManager } from './managers/LocalPackageManager'; +import type { LocalPackageManager, ParsedVersionInfo } from './managers/LocalPackageManager'; /** * Tracks the running/stopped state of the language server. When the lsp crashes, vscode will restart it. After the 5th crash, they'll leave it permanently crashed. @@ -75,10 +61,12 @@ export const LANGUAGE_SERVER_NAME = 'BrighterScript Language Server'; export class LanguageServerManager { constructor() { this.deferred = new Deferred(); + // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires + const version = require('brighterscript/package.json').version; this.embeddedBscInfo = { - path: require.resolve('brighterscript').replace(/[\\\/]dist[\\\/]index.js/i, ''), - // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires - version: require('brighterscript/package.json').version + packageDir: require.resolve('brighterscript').replace(/[\\\/]dist[\\\/]index.js/i, ''), + versionInfo: version, + version: version }; //default to the embedded bsc version this.selectedBscInfo = this.embeddedBscInfo; @@ -113,7 +101,7 @@ export class LanguageServerManager { //anytime the window changes focus, save the current brighterscript version vscode.window.onDidChangeWindowState(async (e) => { - await this.updateBscVersionUsageDate(this.selectedBscInfo.path); + await this.localPackageManager.setUsage('brighterscript', this.selectedBscInfo.versionInfo); }); //in about 5 minutes, clean up any outdated bsc versions (delayed to prevent slower startup times) @@ -186,7 +174,7 @@ export class LanguageServerManager { //give the runner the specific version of bsc to run const args = [ - this.selectedBscInfo.path, + this.selectedBscInfo.packageDir, (this.context.extensionMode === vscode.ExtensionMode.Development).toString() ]; // If the extension is launched in debug mode then the debug server options are used @@ -412,24 +400,22 @@ export class LanguageServerManager { * and if different, re-launch the specific version of the language server' */ public async syncVersionAndTryRun() { - const bsdkPath = await this.getBsdkPath(); + const versionInfo = await this.getBsdkVersionInfo(); //if the path to bsc is different, spin down the old server and start a new one - if (bsdkPath !== this.selectedBscInfo.path) { + if (versionInfo !== this.selectedBscInfo.packageDir) { await this.disableLanguageServer(); } + //ensure the version of the language server is installed and available + //try to load the package version. try { - this.selectedBscInfo = { - path: bsdkPath, - // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires - version: fsExtra.readJsonSync(`${bsdkPath}/package.json`).version - }; + this.selectedBscInfo = await this.ensureBscVersionInstalled(versionInfo); } catch (e) { console.error(e); //fall back to the embedded version, and show a popup - await vscode.window.showErrorMessage(`Can't find language server at "${bsdkPath}". Did you forget to run \`npm install\`? Using embedded version v${this.embeddedBscInfo.version} instead.`); + await vscode.window.showErrorMessage(`Can't find language server for "${versionInfo}". Did you forget to run \`npm install\`? Using embedded version v${this.embeddedBscInfo.version} instead.`); this.selectedBscInfo = this.embeddedBscInfo; } @@ -440,47 +426,59 @@ export class LanguageServerManager { } } + public parseVersionInfo(versionInfo: string, cwd = process.cwd()): ParsedVersionInfo { + if (versionInfo === 'embedded') { + return { + type: 'dir', + value: this.embeddedBscInfo.packageDir + }; + } else { + return this.localPackageManager.parseVersionInfo(versionInfo, cwd); + } + } + /** - * Get the full path to the brighterscript module where the LanguageServer should be run. - * If `brightscript.bsdk` is a version number or a URL, install that version in global storage and return that path. - * if it's a relative path, resolve it to the workspace folder and return that path. + * Get the value for `brightscript.bsdk` from the following locations (in order). First one found wins: + * - use `brightscript.bsdk` value from the current `.code-workspace` file + * - if there is only 1 workspaceFolder with a `brightscript.bsdk` value, use that. + * - if there are multiple workspace folders with `brightscript.bsdk` values, prompt the user to pick which one to use + * - if there are no `brightscript.bsdk` values, use the embedded version + * @returns an absolute path to a directory for the bsdk, or the non-path value (i.e. a URL or a version number) */ - private async getBsdkPath() { - //if there's a bsdk entry in the workspace settings, assume the path is relative to the workspace + private async getBsdkVersionInfo(): Promise { + + //use bsdk entry in the code-workspace file if (this.workspaceConfigIncludesBsdkKey()) { - let bsdk = vscode.workspace.getConfiguration('brightscript', vscode.workspace.workspaceFile).get('bsdk'); - if (bsdk === 'embedded') { - return this.embeddedBscInfo.path; + let result = this.parseVersionInfo( + vscode.workspace.getConfiguration('brightscript', vscode.workspace.workspaceFile).get('bsdk')?.trim?.(), + path.dirname(vscode.workspace.workspaceFile.fsPath) + ); + if (result) { + return result.value; } - let bscPath = await this.ensureBscVersionInstalled(bsdk); - return path.resolve(path.dirname(vscode.workspace.workspaceFile.fsPath), bscPath); } - const folderResults = new Set(); - //look for a bsdk entry in each of the workspace folders - for (const folder of vscode.workspace.workspaceFolders) { - const bsdk = vscode.workspace.getConfiguration('brightscript', folder).get('bsdk'); - if (bsdk) { - if (bsdk === 'embedded') { - folderResults.add(this.embeddedBscInfo.path); - } else { - let bscPath = await this.ensureBscVersionInstalled(bsdk); - folderResults.add( - path.resolve(folder.uri.fsPath, bscPath) - ); - } + //collect `brightscript.bsdk` setting value from each workspaceFolder + const folderResults = vscode.workspace.workspaceFolders.reduce((acc, workspaceFolder) => { + const versionInfo = vscode.workspace.getConfiguration('brightscript', workspaceFolder).get('bsdk'); + const parsed = this.parseVersionInfo(versionInfo, workspaceFolder.uri.fsPath); + if (parsed) { + acc.set(parsed.value, parsed); } - } - const values = [...folderResults.values()]; - //there's no bsdk configuration in folder settings. - if (values.length === 0) { - return this.embeddedBscInfo.path; + return acc; + }, new Map()); + + //no results found, use the embedded version + if (folderResults.size === 0) { + return this.embeddedBscInfo.packageDir; //we have exactly one result. use it - } else if (values.length === 1) { - return values[0]; - } else { + } else if (folderResults.size === 1) { + return [...folderResults.values()][0].value; + //there were multiple versions. make the user pick which to use + } else { + //TODO should we prompt for just these items? return languageServerInfoCommand.selectBrighterScriptVersion(); } } @@ -494,123 +492,64 @@ export class LanguageServerManager { ); } - /** - * Extract some info about the bsc version string - * @param version can be a semantic version, a URL, or a folder path - * @returns - */ - private parseBscVersion(version: string) { - //if this is a URL - if (/^(http|https):\/\//.test(version)) { - //hash the URL to create a unique folder name. There is next to zero possibility these will clash, so the hash should be fine. - return { - folderName: `brighterscript-${md5(version.trim())}`, - packageJsonEntry: version.trim() - }; - - //this is a valid semantic version - } else if (semver.valid(version)) { - return { - folderName: `brighterscript-${version}`, - packageJsonEntry: version - }; - //assume this is a folder path, return undefined - } else { - return undefined; - } - } - /** * Ensure that the specified bsc version is installed in the global storage directory. * @param version * @param retryCount the number of times we should retry before giving up * @returns full path to the root of where the brighterscript module is installed */ - private ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { - let timeout: ReturnType; - - this.ensureBscVersionInstalledPromise = Promise.race([ - //only block for a few minutes, then let the next one try - timeout = util.sleep(3 * 60 * 1000), - this.ensureBscVersionInstalledPromise - ]).then(() => { - return this._ensureBscVersionInstalled(bsdkEntry, retryCount, showProgress); - }).catch((error) => { - console.error(error); - }).finally(() => { - timeout.cancel(); - }); - return this.ensureBscVersionInstalledPromise; - } - private ensureBscVersionInstalledPromise = Promise.resolve(undefined); + @OneAtATime({ timeout: 3 * 60 * 1000 }) + private async ensureBscVersionInstalled(versionInfo: string, retryCount = 1, showProgress = true): Promise { + const parsed = this.parseVersionInfo(versionInfo); - private async _ensureBscVersionInstalled(bsdkEntry: string, retryCount = 1, showProgress = true): Promise { - const { folderName, packageJsonEntry } = this.parseBscVersion(bsdkEntry) ?? {}; - - //assume this is a folder path, return as-is - if (!folderName || !packageJsonEntry) { - return bsdkEntry; + //if this is a directory, use it as-is + if (parsed.type === 'dir') { + return { + packageDir: parsed.value, + version: fsExtra.readJsonSync(s`${parsed.value}/package.json`, { throws: false })?.version ?? parsed.value, + versionInfo: versionInfo + }; } //install this version of brighterscript - return util.runWithProgress({ - title: 'Installing brighterscript language server ' + packageJsonEntry, - location: vscode.ProgressLocation.Notification, - cancellable: false, - //show a progress spinner if configured to do so - showProgress: showProgress && !this.localPackageManager.isInstalled('brighterscript', packageJsonEntry) - }, async () => { - - return this.localPackageManager.install('brighterscript', packageJsonEntry); - }); - } + try { + const packageInfo = await util.runWithProgress({ + title: 'Installing brighterscript language server ' + versionInfo, + location: vscode.ProgressLocation.Notification, + cancellable: false, + //show a progress spinner if configured to do so + showProgress: showProgress && !this.localPackageManager.isInstalled('brighterscript', versionInfo) + }, async () => { + return this.localPackageManager.install('brighterscript', versionInfo); + }); + return { + packageDir: packageInfo.packageDir, + version: packageInfo.version, + versionInfo: versionInfo + }; - /** - * Update the last-used date of a specific instance of brighterscript so we can prevent it from being cleaned up too soon - * @param bscPath - */ - private async updateBscVersionUsageDate(bscPath: string, date = new Date()) { - const usage = this.context.globalState.get('bsc-usage', {}); - usage[s`${bscPath}`] = date.getTime(); - await this.context.globalState.update('bsc-usage', usage); - } + } catch (e) { + if (retryCount > 0) { + console.error('Failed to install brighterscript', versionInfo, e); - /** - * The number of days to retain a cached npm package before deleting it. - */ - private get npmCacheRetentionDays() { - return vscode.workspace.getConfiguration('brightscript')?.get?.('npmCacheRetentionDays', 45) ?? 45; + //if the install failed for some reason, uninstall the package and try again + await this.localPackageManager.uninstall('brighterscript', versionInfo); + return await this.ensureBscVersionInstalled(versionInfo, retryCount - 1, showProgress); + } else { + throw e; + } + } } /** * Delete any brighterscript versions that haven't been used in a while */ - public async deleteOutdatedBscVersions() { - //build a date that represents the cutoff date (i.e. 45 days ago) - const cutoffDate = dayjs().subtract(this.npmCacheRetentionDays, 'days'); - - //get a list of bsc locations that have been recently used - const bscLocationsToKeep = Object.entries( - this.context.globalState.get>('bsc-usage', {}) - - //keep versions that have been used recently - ).filter(([, lastUsedTime]) => { - //if this version was used within the past 45 days, keep it - return dayjs(lastUsedTime).diff(cutoffDate) > 0; - }).map(x => x[0]); - - //get a list of folders in our internal packages directory - const dirs = (await fsExtra.readdir(this.packagesDir)); - - //delete any dir not in the keep list - for (const dir of dirs) { - const fullDir = s`${this.packagesDir}/${dir}/node_modules/brighterscript`; - if (bscLocationsToKeep.includes(fullDir) === false) { - try { - await fsExtra.remove(s`${this.packagesDir}/${dir}`); - } catch { } - } - } + private async deleteOutdatedBscVersions() { + const npmCacheRetentionDays = vscode.workspace.getConfiguration('brightscript')?.get?.('npmCacheRetentionDays', 45) ?? 45; + + //build the cutoff date (i.e. 45 days ago) + const cutoffDate = dayjs().subtract(npmCacheRetentionDays, 'days'); + await this.localPackageManager.deletePackagesNotUsedSince(cutoffDate.toDate()); } } @@ -618,11 +557,50 @@ export const languageServerManager = new LanguageServerManager(); interface BscInfo { /** - * The full path to the brighterscript module + * The full path to the brighterscript module (i.e. the folder where its `package.json` is located + */ + packageDir: string; + /** + * The versionInfo of the brighterscript module. Typically this is a semantic version, but it could be a URL or a folder path. + * Anything that can go inside a `package.json` file is acceptable as well */ - path: string; + versionInfo: string; /** - * The version of the brighterscript module + * The version of the brighterscript module from its package.json. This is displayed in the statusbar */ version: string; } + + +/** + * Force method calls to run one-at-a-time, waiting for the completion of the previous call before running the next. + */ +function OneAtATime(options: { timeout?: number }) { + return function OneAtATime(target: any, propertyKey: string, descriptor: PropertyDescriptor) { + let originalMethod = descriptor.value; + + //wrap the original method + descriptor.value = function value(...args: any[]) { + //ensure the promise structure exists for this call + target.__oneAtATime ??= {}; + target.__oneAtATime[propertyKey] ??= Promise.resolve(); + + const timer = util.sleep(options.timeout > 0 ? options.timeout : Number.MAX_SAFE_INTEGER); + + return Promise.race([ + //race for the last task to resolve + target.__oneAtATime[propertyKey].finally(() => { + timer?.cancel?.(); + }), + //race for the timeout to expire (we give up waiting for the previous task to complete) + timer.then(() => { + //our timer fired before we had a chance to cancel it. Report the error and move on + console.error(`timer expired waiting for the previous ${propertyKey} to complete. Running the next instance`, target); + }) + //now we can move on to the actual task + ]).then(() => { + return originalMethod.apply(this, args); + }); + }; + }; +} diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 49cae547..154bd58a 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -147,7 +147,7 @@ describe.only('LocalPackageManager', () => { manager.install('is-even', '1.0.0') ]); - await manager.removePackageVersion('is-odd', '2.0.0'); + await manager.uninstall('is-odd', '2.0.0'); expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`)).to.be.true; expect(fsExtra.pathExistsSync(`${storageDir}/is-odd/2.0.0/node_modules/is-odd/package.json`)).to.be.false; @@ -171,7 +171,7 @@ describe.only('LocalPackageManager', () => { }); it('does not crash when removing missing package', async () => { - await manager.removePackageVersion('is-odd', '1.0.0'); + await manager.uninstall('is-odd', '1.0.0'); }); }); @@ -277,15 +277,17 @@ describe.only('LocalPackageManager', () => { await manager.setUsage('is-odd', '1.0.0', now); - await manager.deletePackagesOlderThan(yesterday); + await manager.deletePackagesNotUsedSince(yesterday); + //package was not deleted expect( manager.isInstalled('is-odd', '1.0.0') ).to.be.true; await manager.setUsage('is-odd', '1.0.0', now); - await manager.deletePackagesOlderThan(littleAfterNow); + await manager.deletePackagesNotUsedSince(littleAfterNow); + //package was deleted because it was not used since the cutoff date expect( manager.isInstalled('is-odd', '1.0.0') ).to.be.false; diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 5ca65bea..08f3d67b 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -6,6 +6,7 @@ import type { ExtensionContext } from 'vscode'; import * as lodash from 'lodash'; import * as md5 from 'md5'; import * as semver from 'semver'; +import * as path from 'path'; const USAGE_KEY = 'local-package-usage'; @@ -58,11 +59,11 @@ export class LocalPackageManager { /** * Install a package with the given name and version information * @param packageName the name of the package - * @param version the versionInfo of the package. See {versionInfo} for more details + * @param versionInfo the versionInfo of the package. See {versionInfo} for more details * @returns the absolute path to the installed package */ - public async install(packageName: string, version: string): Promise { - const packageInfo = this.getPackageInfo(packageName, version); + public async install(packageName: string, versionInfo: string): Promise { + const packageInfo = this.getPackageInfo(packageName, versionInfo); //if this package is already installed, skip the install if (packageInfo.isInstalled) { @@ -78,7 +79,7 @@ export class LocalPackageManager { private: true, version: '1.0.0', dependencies: { - [packageName]: version + [packageName]: versionInfo } }); @@ -88,12 +89,12 @@ export class LocalPackageManager { }); //update the catalog - this.setCatalogPackageInfo(packageName, version, { + this.setCatalogPackageInfo(packageName, versionInfo, { versionDirName: packageInfo.versionDirName, installDate: Date.now() }); - return s`${rootDir}/node_modules/${packageName}`; + return this.getPackageInfo(packageName, versionInfo); } /** @@ -101,7 +102,7 @@ export class LocalPackageManager { * @param packageName name of the package * @param version version of the package to remove */ - public async removePackageVersion(packageName: string, version: VersionInfo, catalog?: PackageCatalog) { + public async uninstall(packageName: string, version: VersionInfo, catalog?: PackageCatalog) { await this.withCatalog(async (catalog) => { const info = this.getPackageInfo(packageName, version, catalog); await fsExtra.remove(info.rootDir); @@ -201,6 +202,7 @@ export class LocalPackageManager { rootDir: rootDir, packageDir: packageDir, versionDirName: versionDirName, + version: fsExtra.readJsonSync(s`${packageDir}/package.json`, { throws: false })?.version, isInstalled: fsExtra.pathExistsSync(packageDir), lastUsedDate: lastUseDate ? new Date(lastUseDate) : undefined, installDate: packageInfo.installDate ? new Date(packageInfo.installDate) : undefined @@ -219,10 +221,10 @@ export class LocalPackageManager { } /** - * Delete packages that havent older than the given cutoff date + * Delete packages that havent been used since the given cutoff date * @param cutoffDate any package not used since this date will be deleted */ - public async deletePackagesOlderThan(cutoffDate: Date) { + public async deletePackagesNotUsedSince(cutoffDate: Date) { //get the list of directories from the storage folder (these are our package names) const packageNames = (await fsExtra.readdir(this.storageLocation)) .filter(x => x !== 'catalog.json'); @@ -256,13 +258,72 @@ export class LocalPackageManager { for (const [packageName, versions] of Object.entries(onDiskPackages)) { for (const [versionDirName, lastUsedDate] of Object.entries(versions)) { if (lastUsedDate < cutoffDateMs) { - await this.removePackageVersion(packageName, versionDirName); + await this.uninstall(packageName, versionDirName); } } } this.setCatalog(catalog); } + /** + * Parse the versionInfo string into a ParsedVersionInfo object which gives us more details about how to handle it + * @param versionInfo the string to evaluate + * @param cwd a current working directory to use when resolving relative paths + * @returns an object with parsed information about the versionInfo + */ + public parseVersionInfo(versionInfo: string, cwd: string): ParsedVersionInfo { + //is empty string or undefined, return undefined + if (!util.isNonEmptyString(versionInfo)) { + return undefined; + + //is an exact semver value + } else if (semver.valid(versionInfo)) { + return { + type: 'semver-exact', + value: versionInfo + }; + //is a semver range + } else if (semver.validRange(versionInfo)) { + return { + type: 'semver-range', + value: versionInfo + }; + //is a dist tag (like @next, @latest, etc...) + } else if (/^@[a-zA-Z][a-zA-Z0-9-_]*$/.test(versionInfo)) { + return { + type: 'semver-range', + value: versionInfo + }; + + //is a url, return as-is + } else if (/^(http|https):\/\//.test(versionInfo)) { + return { + type: 'url', + value: versionInfo + }; + + //path to a tgz + } else if (/\.tgz$/i.test(versionInfo)) { + return { + type: 'tgz-path', + value: versionInfo + }; + + //an absolute path + } else if (path.isAbsolute(versionInfo)) { + return { + type: 'dir', + value: versionInfo + }; + + //assume relative path, resolve it to the cwd + } else { + return { + type: 'dir', + value: path.resolve(cwd, versionInfo) + }; + } + } public dispose() { @@ -311,6 +372,10 @@ export interface PackageInfo { * Directory where this package will actually be located (i.e. `${packageDir}/node_modules/${packageName}`) */ packageDir: string; + /** + * The version from this package's `package.json` file. Will be `undefined` if unable to read the file + */ + version?: string; /** * The name of the directory representing this version. If versionInfo is a semantic version, we'll use that for the dirName. * Otherwise, we'll create a unique hash of the versionInfo @@ -329,3 +394,23 @@ export interface PackageInfo { */ lastUsedDate: Date; } + +export type ParsedVersionInfo = { + type: 'url'; + value: string; +} | { + type: 'tgz-path'; + value: string; +} | { + type: 'semver-exact'; + value: string; +} | { + type: 'semver-range'; + value: string; +} | { + type: 'dist-tag'; + value: string; +} | { + type: 'dir'; + value: string; +}; diff --git a/src/util.ts b/src/util.ts index 48a6f4da..05b65bd3 100644 --- a/src/util.ts +++ b/src/util.ts @@ -486,7 +486,7 @@ class Util { /** * Run an action with option for a progress spinner. If `showProgress` is `false` then no progress is shown and instead the action is run directly */ - public async runWithProgress(options: Partial & { showProgress?: boolean }, action: () => PromiseLike) { + public async runWithProgress(options: Partial & { showProgress?: boolean }, action: () => PromiseLike): Promise { //show a progress spinner if configured to do so if (options?.showProgress !== false) { return vscode.window.withProgress({ @@ -498,6 +498,13 @@ class Util { return action(); } } + + /** + * Is the value a non-empty string? + */ + public isNonEmptyString(value: any): value is string { + return typeof value === 'string' && value.trim() !== ''; + } } const util = new Util(); diff --git a/tsconfig.json b/tsconfig.json index 566e1385..1d8232b1 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,7 +14,9 @@ "strictNullChecks": false, "noUnusedParameters": false, "allowSyntheticDefaultImports": true, - "forceConsistentCasingInFileNames": true + "forceConsistentCasingInFileNames": true, + "emitDecoratorMetadata": true, + "experimentalDecorators": true }, "include": [ "src/**/*" From 0b0c12cf30d73a6f12fead7b67f03103319cd2e2 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Fri, 20 Sep 2024 10:02:24 -0400 Subject: [PATCH 22/25] Add unit tests for parseVersionInfo and getVersionDirName --- src/managers/LocalPackageManager.spec.ts | 174 ++++++++++++++++++++++- src/managers/LocalPackageManager.ts | 8 +- 2 files changed, 173 insertions(+), 9 deletions(-) diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 154bd58a..4522763c 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -1,3 +1,4 @@ +import { vscode } from '../mockVscode.spec'; import type { PackageCatalogPackageInfo } from './LocalPackageManager'; import { LocalPackageManager } from './LocalPackageManager'; import { standardizePath as s } from 'brighterscript'; @@ -5,7 +6,7 @@ import * as fsExtra from 'fs-extra'; import { expect } from 'chai'; import { util } from '../util'; import * as dayjs from 'dayjs'; -import { vscode } from '../mockVscode.spec'; +import * as md5 from 'md5'; import { createSandbox } from 'sinon'; const sinon = createSandbox(); @@ -14,6 +15,8 @@ const tempDir = s`${cwd}/.tmp`; describe.only('LocalPackageManager', () => { + const packageUrl = 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz'; + const storageDir = s`${tempDir}/storage`; let manager: LocalPackageManager; @@ -129,10 +132,7 @@ describe.only('LocalPackageManager', () => { describe('getPackageInfo', () => { it('transforms URLs into a filesystem-safe name', () => { - const info = manager['getPackageInfo']( - 'brighterscript', - 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz' - ); + const info = manager['getPackageInfo']('brighterscript', packageUrl); expect( info.versionDirName ).to.match(/^[a-z0-9_]+$/i); @@ -299,4 +299,168 @@ describe.only('LocalPackageManager', () => { manager.dispose(); }); }); + + describe('getVersionDirName', () => { + it('fetches the catalog when not supplied', async () => { + expect( + await manager['getVersionDirName']('brighterscript', '1.0.0') + ).to.eql('1.0.0'); + }); + + it('creates a hash', async () => { + expect( + await manager['getVersionDirName']('brighterscript', packageUrl) + ).to.eql(md5(packageUrl)); + }); + + it('uses a pre-existing hash when available', async () => { + const packageUrl2 = `${packageUrl}2`; + //need to do some hackery here to force a hash to already exist (since hash collisions are hard to reproduce...) + await manager.install('brighterscript', packageUrl2); + + await manager['withCatalog']((catalog) => { + //override the hash to be the hash of `packageUrl` + catalog.packages['brighterscript'][packageUrl2].versionDirName = md5(packageUrl); + }); + + //ask for the dir name, it should come back with the hash of the packageUrl + expect( + await manager['getVersionDirName']('brighterscript', packageUrl2) + ).to.eql(md5(packageUrl)); + + //now ask for the dir name, it should come with a number appended to it since that hash already exists + expect( + await manager['getVersionDirName']('brighterscript', packageUrl) + ).to.eql(`${md5(packageUrl)}-1`); + }); + }); + + describe('parseVersionInfo', () => { + it('returns undefined for bad values', () => { + expect( + manager['parseVersionInfo'](undefined, process.cwd()) + ).to.be.undefined; + + expect( + manager['parseVersionInfo'](null, process.cwd()) + ).to.be.undefined; + + expect( + manager['parseVersionInfo']('', process.cwd()) + ).to.be.undefined; + + expect( + manager['parseVersionInfo'](' ', process.cwd()) + ).to.be.undefined; + }); + it('detects valid semver versions', () => { + expect( + manager['parseVersionInfo']('1.0.0', process.cwd()) + ).to.eql({ + value: '1.0.0', + type: 'semver-exact' + }); + + expect( + manager['parseVersionInfo']('1.0.0-alpha.2', process.cwd()) + ).to.eql({ + value: '1.0.0-alpha.2', + type: 'semver-exact' + }); + }); + + it('detects valid semver version ranges', () => { + expect( + manager['parseVersionInfo']('~1.0.0', process.cwd()) + ).to.eql({ + value: '~1.0.0', + type: 'semver-range' + }); + + expect( + manager['parseVersionInfo']('^1.0.0', process.cwd()) + ).to.eql({ + value: '^1.0.0', + type: 'semver-range' + }); + + expect( + manager['parseVersionInfo']('1.2.x', process.cwd()) + ).to.eql({ + value: '1.2.x', + type: 'semver-range' + }); + + expect( + manager['parseVersionInfo']('1.2.0 || >=1.2.2 <1.3.0', process.cwd()) + ).to.eql({ + value: '1.2.0 || >=1.2.2 <1.3.0', + type: 'semver-range' + }); + }); + + it('detects valid dist tags', () => { + expect( + manager['parseVersionInfo']('@next', process.cwd()) + ).to.eql({ + value: '@next', + type: 'dist-tag' + }); + }); + + it('detects valid URLs', () => { + expect( + manager['parseVersionInfo']('https://github.com', process.cwd()) + ).to.eql({ + value: 'https://github.com', + type: 'url' + }); + + expect( + manager['parseVersionInfo'](packageUrl, process.cwd()) + ).to.eql({ + value: packageUrl, + type: 'url' + }); + }); + + it('detects paths to tgz', () => { + expect( + manager['parseVersionInfo']('./something.tgz', process.cwd()) + ).to.eql({ + value: './something.tgz', + type: 'tgz-path' + }); + + expect( + manager['parseVersionInfo'](s`${tempDir}/thing.tgz`, process.cwd()) + ).to.eql({ + value: s`${tempDir}/thing.tgz`, + type: 'tgz-path' + }); + }); + + it('detects paths to directories', () => { + expect( + manager['parseVersionInfo']('./something', s`${process.cwd()}`) + ).to.eql({ + value: s`${process.cwd()}/something`, + type: 'dir' + }); + + expect( + manager['parseVersionInfo']('./something', cwd) + ).to.eql({ + value: s`${cwd}/something`, + type: 'dir' + }); + + expect( + manager['parseVersionInfo'](s`${tempDir}/thing`, process.cwd()) + ).to.eql({ + value: s`${tempDir}/thing`, + type: 'dir' + }); + }); + }); }); diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 08f3d67b..0dd8b768 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -172,9 +172,9 @@ export class LocalPackageManager { const hash = md5(version.trim()); const existingHashes = Object.values(catalog.packages?.[packageName] ?? {}).map(x => x.versionDirName); let newHash = hash; - let i = 0; + let i = 1; while (existingHashes.includes(newHash)) { - newHash = hash + i++; + newHash = `${hash}-${i++}`; } return newHash; } @@ -289,9 +289,9 @@ export class LocalPackageManager { value: versionInfo }; //is a dist tag (like @next, @latest, etc...) - } else if (/^@[a-zA-Z][a-zA-Z0-9-_]*$/.test(versionInfo)) { + } else if (/^@[a-z0-9-_]*$/i.test(versionInfo)) { return { - type: 'semver-range', + type: 'dist-tag', value: versionInfo }; From bca7a87e5e292d5d1b7accbeebcbc15575403697 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Fri, 20 Sep 2024 14:09:07 -0400 Subject: [PATCH 23/25] Fix some broken tests --- src/LanguageServerManager.spec.ts | 149 +++++++++++------------ src/LanguageServerManager.ts | 12 +- src/extension.ts | 3 +- src/managers/LocalPackageManager.spec.ts | 4 +- src/mockVscode.spec.ts | 7 +- 5 files changed, 90 insertions(+), 85 deletions(-) diff --git a/src/LanguageServerManager.spec.ts b/src/LanguageServerManager.spec.ts index 383bc320..c19bcd4d 100644 --- a/src/LanguageServerManager.spec.ts +++ b/src/LanguageServerManager.spec.ts @@ -16,8 +16,8 @@ import { State } from 'vscode-languageclient/node'; import { util } from './util'; -import * as dayjs from 'dayjs'; import { GlobalStateManager } from './GlobalStateManager'; +import { LocalPackageManager } from './managers/LocalPackageManager'; const Module = require('module'); const sinon = createSandbox(); @@ -40,7 +40,10 @@ describe('LanguageServerManager', () => { let languageServerManager: LanguageServerManager; - beforeEach(() => { + beforeEach(function() { + //deleting certain directories take a while + this.timeout(30_000); + languageServerManager = new LanguageServerManager(); languageServerManager['definitionRepository'] = new DefinitionRepository( new DeclarationProvider() @@ -51,6 +54,7 @@ describe('LanguageServerManager', () => { subscriptions: [] } as unknown as ExtensionContext; languageServerManager['globalStateManager'] = new GlobalStateManager(languageServerManager['context']); + languageServerManager['localPackageManager'] = new LocalPackageManager(storageDir, languageServerManager['context']); fsExtra.removeSync(storageDir); (languageServerManager['context'] as any).globalStorageUri = URI.file(storageDir); @@ -181,7 +185,7 @@ describe('LanguageServerManager', () => { }); }); - describe('getBsdkPath', () => { + describe('getBsdkVersionInfo', () => { const embeddedPath = path.resolve(s`${__dirname}/../node_modules/brighterscript`); function setConfig(filePath: string, settings: any) { @@ -197,7 +201,7 @@ describe('LanguageServerManager', () => { it('returns embedded version when not in workspace and no settings exist', async () => { expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql(embeddedPath); }); @@ -206,7 +210,7 @@ describe('LanguageServerManager', () => { fsExtra.outputFileSync(vscode.workspace.workspaceFile.fsPath, ''); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql(embeddedPath); }); @@ -218,7 +222,7 @@ describe('LanguageServerManager', () => { }); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql(embeddedPath); }); @@ -229,7 +233,7 @@ describe('LanguageServerManager', () => { }); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql(embeddedPath); }); @@ -244,7 +248,7 @@ describe('LanguageServerManager', () => { }); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql(embeddedPath); }); @@ -256,7 +260,7 @@ describe('LanguageServerManager', () => { }); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql( path.resolve( s`${tempDir}/relative/path` @@ -275,7 +279,7 @@ describe('LanguageServerManager', () => { }); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql( path.resolve( s`${tempDir}/app1/folder/path` @@ -296,7 +300,7 @@ describe('LanguageServerManager', () => { }); expect( - s(await languageServerManager['getBsdkPath']()) + s(await languageServerManager['getBsdkVersionInfo']()) ).to.eql( path.resolve( s`${tempDir}/app1/folder/path` @@ -329,7 +333,7 @@ describe('LanguageServerManager', () => { uri: URI.file(`${tempDir}/app2`) }); const stub = sinon.stub(languageServerInfoCommand, 'selectBrighterScriptVersion').returns(Promise.resolve(null)); - const bsdkPath = await languageServerManager['getBsdkPath'](); + const bsdkPath = await languageServerManager['getBsdkVersionInfo'](); expect(stub.called).to.be.true; //should get null since that's what the 'selectBrighterScriptVersion' function returns from our stub @@ -342,20 +346,23 @@ describe('LanguageServerManager', () => { this.timeout(20_000); it('installs a bsc version when not present', async () => { + const info = await languageServerManager['ensureBscVersionInstalled']('0.65.0'); + expect(info).to.eql({ + packageDir: s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript`, + versionInfo: '0.65.0', + version: '0.65.0' + }); expect( - await languageServerManager['ensureBscVersionInstalled']('0.65.0') - ).to.eql(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); - expect( - fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`) + fsExtra.pathExistsSync(info.packageDir) ).to.be.true; }); - it('does not run multiple installs for the same version at the same time', async () => { - let spy = sinon.stub(util, 'exec').callsFake(async (command, options) => { + it.skip('does not run multiple installs for the same version at the same time', async () => { + let spy = sinon.stub(util, 'spawnNpmAsync').callsFake(async (command, options) => { //simulate that the bsc code was installed fsExtra.outputFileSync(`${options.cwd}/node_modules/brighterscript/dist/index.js`, ''); //ensure both requests have the opportunity to run at same time - await util.sleep(200); + await util.sleep(1000); }); //request the install multiple times without waiting for them const promises = [ @@ -363,71 +370,84 @@ describe('LanguageServerManager', () => { languageServerManager['ensureBscVersionInstalled']('0.65.0'), languageServerManager['ensureBscVersionInstalled']('0.65.1') ]; + //now wait for them to finish expect( - await Promise.all(promises) + (await Promise.all(promises)).map(x => x.packageDir) ).to.eql([ - s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`, - s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`, - s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript` + s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript`, + s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript`, + s`${storageDir}/brighterscript/0.65.1/node_modules/brighterscript` ]); //the spy should have only been called once for each unique version expect(spy.getCalls().map(x => x.args[1].cwd)).to.eql([ - s`${storageDir}/packages/brighterscript-0.65.0`, - s`${storageDir}/packages/brighterscript-0.65.1` + s`${storageDir}/brighterscript/0.65.0`, + s`${storageDir}/brighterscript/0.65.1` ]); }); - it('reuses the same bsc version when already exists', async () => { - let spy = sinon.spy(util, 'exec'); + it.skip('reuses the same bsc version when already exists', async () => { + let spy = sinon.spy(util, 'spawnNpmAsync'); fsExtra.ensureDirSync( - s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript/dist/index.js` + s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript/dist/index.js` ); expect( await languageServerManager['ensureBscVersionInstalled']('0.65.0') - ).to.eql(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); + ).to.eql({ + packageDir: s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript`, + version: '0.65.0', + versionInfo: '0.65.0' + }); expect( - fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`) + fsExtra.pathExistsSync(s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript`) ).to.be.true; + + //the install should not have been called expect(spy.called).to.be.false; }); it('installs from url', async () => { fsExtra.ensureDirSync( - s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript/dist/index.js` + s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript/dist/index.js` ); expect( await languageServerManager['ensureBscVersionInstalled']( - 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz' + 'https://github.com/rokucommunity/brighterscript/releases/download/v0.67.6/brighterscript-0.67.6.tgz' ) - ).to.eql(s`${storageDir}/packages/brighterscript-028738851c072bf844c10c260d6d2c65/node_modules/brighterscript`); + ).to.eql({ + packageDir: s`${storageDir}/brighterscript/71f52abe1087b924a1ded1e6d8a71022/node_modules/brighterscript`, + version: '0.67.6', + versionInfo: 'https://github.com/rokucommunity/brighterscript/releases/download/v0.67.6/brighterscript-0.67.6.tgz' + }); expect( - fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-028738851c072bf844c10c260d6d2c65/node_modules/brighterscript`) + fsExtra.pathExistsSync(s`${storageDir}/brighterscript/71f52abe1087b924a1ded1e6d8a71022/node_modules/brighterscript`) ).to.be.true; }); it('repairs a broken bsc version', async () => { - let stub = sinon.stub(fsExtra, 'remove'); - fsExtra.ensureDirSync( - s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript` - ); - fsExtra.writeFileSync( - s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript/package.json`, - 'bad json' - ); - expect( - await languageServerManager['ensureBscVersionInstalled']('0.65.1') - ).to.eql(s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`); - expect( - fsExtra.pathExistsSync(s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`) - ).to.be.true; + //mock the actual installation process (since we're handling when it crashes) + let callCount = 0; + sinon.stub(util, 'spawnNpmAsync').callsFake(async (args, options) => { + callCount++; + //fail first time, pass all others + if (callCount === 1) { + throw new Error('failed'); + } + await fsExtra.outputJson(`${options.cwd}/node_modules/brighterscript/package.json`, { + version: '0.65.0' + }); + }); - //make sure we deleted the bad folder + //install the package expect( - s`${stub.getCalls()[0].args[0]}` - ).to.eql(s`${storageDir}/packages/brighterscript-0.65.1`); + await languageServerManager['ensureBscVersionInstalled']('0.65.0') + ).to.eql({ + packageDir: s`${storageDir}/brighterscript/0.65.0/node_modules/brighterscript`, + version: '0.65.0', + versionInfo: '0.65.0' + }); }); }); @@ -449,32 +469,5 @@ describe('LanguageServerManager', () => { await util.sleep(100); expect(stub.called).to.be.true; }); - - it('deletes bsc versions that are older than the specified number of days', async () => { - //create a vew bsc versions - fsExtra.ensureDirSync(`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`); - fsExtra.ensureDirSync(`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`); - fsExtra.ensureDirSync(`${storageDir}/packages/brighterscript-0.65.2/node_modules/brighterscript`); - - //mark the first and third as outdated - await languageServerManager['updateBscVersionUsageDate']( - s`${storageDir}/packages/brighterscript-0.65.0/node_modules/brighterscript`, - dayjs().subtract(46, 'day').toDate() - ); - await languageServerManager['updateBscVersionUsageDate']( - s`${storageDir}/packages/brighterscript-0.65.1/node_modules/brighterscript`, - dayjs().subtract(20, 'day').toDate() - ); - await languageServerManager['updateBscVersionUsageDate']( - s`${storageDir}/packages/brighterscript-0.65.2/node_modules/brighterscript`, - dayjs().subtract(60, 'day').toDate() - ); - - await languageServerManager.deleteOutdatedBscVersions(); - - expect(fsExtra.pathExistsSync(`${storageDir}/packages/brighterscript-0.65.0`)).to.be.false; - expect(fsExtra.pathExistsSync(`${storageDir}/packages/brighterscript-0.65.1`)).to.be.true; - expect(fsExtra.pathExistsSync(`${storageDir}/packages/brighterscript-0.65.2`)).to.be.false; - }); }); }); diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index ca8d1031..762a6d94 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -61,10 +61,10 @@ export const LANGUAGE_SERVER_NAME = 'BrighterScript Language Server'; export class LanguageServerManager { constructor() { this.deferred = new Deferred(); - // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires - const version = require('brighterscript/package.json').version; + const brighterscriptDir = require.resolve('brighterscript').replace(/[\\\/]dist[\\\/]index.js/i, ''); + const version = fsExtra.readJsonSync(`${brighterscriptDir}/package.json`).version; this.embeddedBscInfo = { - packageDir: require.resolve('brighterscript').replace(/[\\\/]dist[\\\/]index.js/i, ''), + packageDir: brighterscriptDir, versionInfo: version, version: version }; @@ -72,7 +72,13 @@ export class LanguageServerManager { this.selectedBscInfo = this.embeddedBscInfo; } + /** + * Information about the embedded brighterscript version + */ public embeddedBscInfo: BscInfo; + /** + * Information about the currently selected brighterscript version (the one that's running right now) + */ public selectedBscInfo: BscInfo; private context: vscode.ExtensionContext; diff --git a/src/extension.ts b/src/extension.ts index c6558b00..1f56665c 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -66,7 +66,8 @@ export class Extension { ); let localPackageManager = new LocalPackageManager( - s`${context.globalStorageUri.fsPath}/packages` + s`${context.globalStorageUri.fsPath}/packages`, + context ); this.telemetryManager.sendStartupEvent(); diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 4522763c..57f72a03 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -13,9 +13,9 @@ const sinon = createSandbox(); const cwd = s`${__dirname}/../../`; const tempDir = s`${cwd}/.tmp`; -describe.only('LocalPackageManager', () => { +describe('LocalPackageManager', () => { - const packageUrl = 'https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.67.5-lsp-refactor.20240806164122.tgz'; + const packageUrl = 'https://github.com/rokucommunity/brighterscript/releases/download/v0.67.6/brighterscript-0.67.6.tgz'; const storageDir = s`${tempDir}/storage`; let manager: LocalPackageManager; diff --git a/src/mockVscode.spec.ts b/src/mockVscode.spec.ts index c99eb680..fd4a5dae 100644 --- a/src/mockVscode.spec.ts +++ b/src/mockVscode.spec.ts @@ -1,6 +1,11 @@ import { EventEmitter } from 'eventemitter3'; import type { Command, Range, TreeDataProvider, TreeItemCollapsibleState, Uri, WorkspaceFolder, ConfigurationScope, ExtensionContext, WorkspaceConfiguration, OutputChannel, QuickPickItem } from 'vscode'; +import URI from 'vscode-uri'; import * as path from 'path'; +import { standardizePath as s } from 'brighterscript'; + +const cwd = s`${__dirname}/../`; +const tempDir = s`${cwd}/.tmp`; //copied from vscode to help with unit tests enum QuickPickItemKind { @@ -112,7 +117,7 @@ export let vscode = { return this._data[key]; } } as any, - globalStorageUri: undefined as Uri, + globalStorageUri: URI.file(tempDir), environmentVariableCollection: {} as any, logUri: undefined as Uri, logPath: '', From c7bf7e5a2470f8a5e8e7243dbb2f785dea47d25f Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Fri, 20 Sep 2024 15:16:51 -0400 Subject: [PATCH 24/25] Handle loading bsc version using version number --- src/LanguageServerManager.ts | 5 ++- src/commands/LanguageServerInfoCommand.ts | 43 +++++++++++------------ src/managers/LocalPackageManager.spec.ts | 10 +++++- src/managers/LocalPackageManager.ts | 9 +++-- src/util.ts | 16 ++++++--- 5 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/LanguageServerManager.ts b/src/LanguageServerManager.ts index 762a6d94..69449d64 100644 --- a/src/LanguageServerManager.ts +++ b/src/LanguageServerManager.ts @@ -133,7 +133,10 @@ export class LanguageServerManager { //dynamically enable or disable the language server based on user settings vscode.workspace.onDidChangeConfiguration(async (configuration) => { - await this.syncVersionAndTryRun(); + //if we've changed the bsdk setting, restart the language server + if (configuration.affectsConfiguration('brightscript.bsdk')) { + await this.syncVersionAndTryRun(); + } }); await this.syncVersionAndTryRun(); } diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index 6b3ba428..484acaa5 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -12,6 +12,7 @@ import { util } from '../util'; import type { LocalPackageManager } from '../managers/LocalPackageManager'; import { standardizePath as s } from 'brighterscript'; import * as dayjs from 'dayjs'; +import type { QuickPickItem } from 'vscode'; dayjs.extend(relativeTime); export class LanguageServerInfoCommand { @@ -71,10 +72,11 @@ export class LanguageServerInfoCommand { await vscode.commands.executeCommand('extension.brightscript.languageServer.restart'); } - private discoverBrighterScriptVersions(workspaceFolders: string[]): BscVersionInfo[] { - const versions: BscVersionInfo[] = [{ + private discoverBrighterScriptVersions(workspaceFolders: string[]): QuickPickItemEnhanced[] { + const versions: QuickPickItemEnhanced[] = [{ label: `Use VSCode's version`, - description: languageServerManager.embeddedBscInfo.version + description: languageServerManager.embeddedBscInfo.version, + value: 'embedded' }]; //look for brighterscript in node_modules from all workspace folders @@ -100,7 +102,8 @@ export class LanguageServerInfoCommand { versions.push({ label: 'Use Workspace Version', description: version, - detail: bscPath.replace(/\\+/g, '/') + detail: bscPath.replace(/\\+/g, '/'), + value: bscPath.replace(/\\+/g, '/') }); } } @@ -147,7 +150,7 @@ export class LanguageServerInfoCommand { * If this changes the user/folder/workspace settings, that will trigger a reload of the language server so there's no need to * call the reload manually */ - public async selectBrighterScriptVersion() { + public async selectBrighterScriptVersion(): Promise { const quickPickItems = this.discoverBrighterScriptVersions( vscode.workspace.workspaceFolders.map(x => this.getWorkspaceOrFolderPath(x.uri.fsPath)) ); @@ -161,35 +164,33 @@ export class LanguageServerInfoCommand { description: '', detail: '', command: async () => { - let versionsFromNpm = (await versionsFromNpmPromise).map(x => ({ + let versionsFromNpm: QuickPickItemEnhanced[] = (await versionsFromNpmPromise).map(x => ({ label: x.version, - detail: x.version, + value: x.version, description: dayjs(x.date).fromNow(true) + ' ago' })); return await vscode.window.showQuickPick(versionsFromNpm, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; } } as any); - let selection = await vscode.window.showQuickPick(quickPickItems, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; + let selection: QuickPickItemEnhanced = await vscode.window.showQuickPick(quickPickItems, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; //if the selection has a command, run it before continuing; selection = await selection?.command?.() ?? selection; if (selection) { const config = vscode.workspace.getConfiguration('brightscript'); - //quickly clear the setting, then set it again so we are guaranteed to trigger a change event - await config.update('bsdk', undefined); - - //if the user picked "use embedded version", then remove the setting - if (quickPickItems.indexOf(selection) === 0) { - //setting to undefined means "remove" - await config.update('bsdk', 'embedded'); - return 'embedded'; + const currentValue = config.get('bsdk') ?? 'embedded'; + + //if the user chose the same value that's already there, just restart the language server + if (selection.value === currentValue) { + await this.restartLanguageServer(); + //set the new value } else { //save this to workspace/folder settings (vscode automatically decides if it goes into the code-workspace settings or the folder settings) - await config.update('bsdk', selection.detail); - return selection.detail; + await config.update('bsdk', selection.value); } + return selection.value; } } @@ -203,10 +204,6 @@ export class LanguageServerInfoCommand { } } -interface BscVersionInfo { - label: string; - description: string; - detail?: string; -} +type QuickPickItemEnhanced = QuickPickItem & { value: string; command?: () => Promise }; export const languageServerInfoCommand = new LanguageServerInfoCommand(); diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index 57f72a03..cb976b90 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -82,7 +82,15 @@ describe('LocalPackageManager', () => { customKey: 'test' }); - await manager.install('is-odd', '1.0.0'); + expect( + await manager.install('is-odd', '1.0.0') + ).to.include({ + packageDir: s`${storageDir}/is-odd/1.0.0/node_modules/is-odd`, + packageName: 'is-odd', + rootDir: s`${storageDir}/is-odd/1.0.0`, + versionDirName: '1.0.0', + versionInfo: '1.0.0' + }); expect( fsExtra.readJsonSync(`${storageDir}/is-odd/1.0.0/node_modules/is-odd/package.json`).customKey diff --git a/src/managers/LocalPackageManager.ts b/src/managers/LocalPackageManager.ts index 0dd8b768..5ea9bf1c 100644 --- a/src/managers/LocalPackageManager.ts +++ b/src/managers/LocalPackageManager.ts @@ -67,14 +67,13 @@ export class LocalPackageManager { //if this package is already installed, skip the install if (packageInfo.isInstalled) { - return; + return packageInfo; } - const rootDir = s`${this.storageLocation}/${packageName}/${packageInfo.versionDirName}`; - fsExtra.ensureDirSync(rootDir); + fsExtra.ensureDirSync(packageInfo.rootDir); //write a simple package.json file referencing the version of brighterscript we want - await fsExtra.outputJson(`${rootDir}/package.json`, { + await fsExtra.outputJson(`${packageInfo.rootDir}/package.json`, { name: 'vscode-brighterscript-host', private: true, version: '1.0.0', @@ -85,7 +84,7 @@ export class LocalPackageManager { //install the package await util.spawnNpmAsync(['install'], { - cwd: rootDir + cwd: packageInfo.rootDir }); //update the catalog diff --git a/src/util.ts b/src/util.ts index 05b65bd3..79c4d3c9 100644 --- a/src/util.ts +++ b/src/util.ts @@ -461,11 +461,17 @@ class Util { spawnNpmAsync(args: Array, options?: childProcess.SpawnOptions) { //filter out undefined args args = args.filter(arg => arg !== undefined); - return this.spawnAsync( - this.isWindowsPlatform() ? 'npm.cmd' : 'npm', - args, - options - ); + + if (this.isWindowsPlatform()) { + return this.spawnAsync('npm.cmd', args, { + ...options, + shell: true, + detached: false, + windowsHide: true + }); + } else { + return this.spawnAsync('npm', args, options); + } } /** From 758c2a8a3e893c1ab8aefe44f19409fa730d78e0 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Fri, 20 Sep 2024 16:14:34 -0400 Subject: [PATCH 25/25] Split mainline releases and prereleases --- .../LanguageServerInfoCommand.spec.ts | 33 ++++++++++------ src/commands/LanguageServerInfoCommand.ts | 38 +++++++++++++++---- src/managers/LocalPackageManager.spec.ts | 12 +++--- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/src/commands/LanguageServerInfoCommand.spec.ts b/src/commands/LanguageServerInfoCommand.spec.ts index 5e389d8f..511dd72b 100644 --- a/src/commands/LanguageServerInfoCommand.spec.ts +++ b/src/commands/LanguageServerInfoCommand.spec.ts @@ -73,7 +73,8 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }]); }); @@ -91,7 +92,8 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }]); }); @@ -101,11 +103,13 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }, { label: `Use Workspace Version`, description: '1.2.3', - detail: 'node_modules/brighterscript' + detail: 'node_modules/brighterscript', + value: 'node_modules/brighterscript' }]); }); @@ -115,11 +119,13 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }, { label: `Use Workspace Version`, description: '1.2.3', - detail: 'node_modules/brighterscript' + detail: 'node_modules/brighterscript', + value: 'node_modules/brighterscript' }]); writePackage('2.3.4'); @@ -127,11 +133,13 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }, { label: `Use Workspace Version`, description: '2.3.4', - detail: 'node_modules/brighterscript' + detail: 'node_modules/brighterscript', + value: 'node_modules/brighterscript' }]); }); @@ -141,11 +149,13 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }, { label: `Use Workspace Version`, description: '1.2.3', - detail: 'node_modules/brighterscript' + detail: 'node_modules/brighterscript', + value: 'node_modules/brighterscript' }]); fsExtra.removeSync(`${tempDir}/node_modules`); @@ -153,7 +163,8 @@ describe('LanguageServerInfoCommand', () => { command['discoverBrighterScriptVersions']([tempDir]) ).to.eql([{ label: `Use VSCode's version`, - description: embeddedBscVersion + description: embeddedBscVersion, + value: 'embedded' }]); }); }); diff --git a/src/commands/LanguageServerInfoCommand.ts b/src/commands/LanguageServerInfoCommand.ts index 484acaa5..0ee61aaf 100644 --- a/src/commands/LanguageServerInfoCommand.ts +++ b/src/commands/LanguageServerInfoCommand.ts @@ -9,16 +9,21 @@ import { VscodeCommand } from './VscodeCommand'; import URI from 'vscode-uri'; import * as relativeTime from 'dayjs/plugin/relativeTime'; import { util } from '../util'; -import type { LocalPackageManager } from '../managers/LocalPackageManager'; +import { type LocalPackageManager } from '../managers/LocalPackageManager'; +import * as semver from 'semver'; import { standardizePath as s } from 'brighterscript'; -import * as dayjs from 'dayjs'; import type { QuickPickItem } from 'vscode'; +import * as dayjs from 'dayjs'; dayjs.extend(relativeTime); export class LanguageServerInfoCommand { public static commandName = 'extension.brightscript.languageServer.info'; + public localPackageManager: LocalPackageManager; + public register(context: vscode.ExtensionContext, localPackageManager: LocalPackageManager) { + this.localPackageManager = localPackageManager; + context.subscriptions.push(vscode.commands.registerCommand(LanguageServerInfoCommand.commandName, async () => { const commands = [{ label: `Change Selected BrighterScript Version`, @@ -164,11 +169,30 @@ export class LanguageServerInfoCommand { description: '', detail: '', command: async () => { - let versionsFromNpm: QuickPickItemEnhanced[] = (await versionsFromNpmPromise).map(x => ({ - label: x.version, - value: x.version, - description: dayjs(x.date).fromNow(true) + ' ago' - })); + let versionsFromNpm: QuickPickItemEnhanced[] = (await versionsFromNpmPromise).filter(x => !semver.prerelease(x.version)).map(x => { + return { + label: x.version, + value: x.version, + description: `${dayjs(x.date).fromNow(true)} ago` + }; + }); + return await vscode.window.showQuickPick(versionsFromNpm, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; + } + } as any); + + //get the full list of versions from npm + quickPickItems.push({ + label: '$(package) Install from npm (insider builds)', + description: '', + detail: '', + command: async () => { + let versionsFromNpm: QuickPickItemEnhanced[] = (await versionsFromNpmPromise).filter(x => semver.prerelease(x.version)).map(x => { + return { + label: x.version, + value: x.version, + description: `${dayjs(x.date).fromNow(true)} ago` + }; + }); return await vscode.window.showQuickPick(versionsFromNpm, { placeHolder: `Select the BrighterScript version used for BrightScript and BrighterScript language features` }) as any; } } as any); diff --git a/src/managers/LocalPackageManager.spec.ts b/src/managers/LocalPackageManager.spec.ts index cb976b90..f4da7cc4 100644 --- a/src/managers/LocalPackageManager.spec.ts +++ b/src/managers/LocalPackageManager.spec.ts @@ -309,15 +309,15 @@ describe('LocalPackageManager', () => { }); describe('getVersionDirName', () => { - it('fetches the catalog when not supplied', async () => { + it('fetches the catalog when not supplied', () => { expect( - await manager['getVersionDirName']('brighterscript', '1.0.0') + manager['getVersionDirName']('brighterscript', '1.0.0') ).to.eql('1.0.0'); }); - it('creates a hash', async () => { + it('creates a hash', () => { expect( - await manager['getVersionDirName']('brighterscript', packageUrl) + manager['getVersionDirName']('brighterscript', packageUrl) ).to.eql(md5(packageUrl)); }); @@ -333,12 +333,12 @@ describe('LocalPackageManager', () => { //ask for the dir name, it should come back with the hash of the packageUrl expect( - await manager['getVersionDirName']('brighterscript', packageUrl2) + manager['getVersionDirName']('brighterscript', packageUrl2) ).to.eql(md5(packageUrl)); //now ask for the dir name, it should come with a number appended to it since that hash already exists expect( - await manager['getVersionDirName']('brighterscript', packageUrl) + manager['getVersionDirName']('brighterscript', packageUrl) ).to.eql(`${md5(packageUrl)}-1`); }); });