From fb63ec7db0a2e3354a6bcb72ebab7abb2ca145a0 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 11 Jan 2024 16:57:42 -0500 Subject: [PATCH 01/17] Consume `codeql version` JSON output for feature capabilities --- .../ql-vscode/src/codeql-cli/cli-command.ts | 24 +++++-- .../ql-vscode/src/codeql-cli/cli-version.ts | 34 ++++++++-- extensions/ql-vscode/src/codeql-cli/cli.ts | 68 ++++++++++++++----- .../ql-vscode/src/codeql-cli/distribution.ts | 20 +++--- extensions/ql-vscode/src/extension.ts | 13 ++-- 5 files changed, 119 insertions(+), 40 deletions(-) diff --git a/extensions/ql-vscode/src/codeql-cli/cli-command.ts b/extensions/ql-vscode/src/codeql-cli/cli-command.ts index 1b4ab75f9..b211793e1 100644 --- a/extensions/ql-vscode/src/codeql-cli/cli-command.ts +++ b/extensions/ql-vscode/src/codeql-cli/cli-command.ts @@ -3,7 +3,10 @@ import { promisify } from "util"; import type { BaseLogger } from "../common/logging"; import type { ProgressReporter } from "../common/logging/vscode"; -import { getChildProcessErrorMessage } from "../common/helpers-pure"; +import { + getChildProcessErrorMessage, + getErrorMessage, +} from "../common/helpers-pure"; /** * Flags to pass to all cli commands. @@ -11,26 +14,27 @@ import { getChildProcessErrorMessage } from "../common/helpers-pure"; export const LOGGING_FLAGS = ["-v", "--log-to-stderr"]; /** - * Runs a CodeQL CLI command without invoking the CLI server, returning the output as a string. + * Runs a CodeQL CLI command without invoking the CLI server, deserializing the output as JSON. * @param codeQlPath The path to the CLI. * @param command The `codeql` command to be run, provided as an array of command/subcommand names. * @param commandArgs The arguments to pass to the `codeql` command. * @param description Description of the action being run, to be shown in log and error messages. * @param logger Logger to write command log messages, e.g. to an output channel. * @param progressReporter Used to output progress messages, e.g. to the status bar. - * @returns The contents of the command's stdout, if the command succeeded. + * @returns A JSON object parsed from the contents of the command's stdout, if the command succeeded. */ -export async function runCodeQlCliCommand( +export async function runJsonCodeQlCliCommand( codeQlPath: string, command: string[], commandArgs: string[], description: string, logger: BaseLogger, progressReporter?: ProgressReporter, -): Promise { +): Promise { // Add logging arguments first, in case commandArgs contains positional parameters. const args = command.concat(LOGGING_FLAGS).concat(commandArgs); const argsString = args.join(" "); + let stdout: string; try { if (progressReporter !== undefined) { progressReporter.report({ message: description }); @@ -41,10 +45,18 @@ export async function runCodeQlCliCommand( const result = await promisify(execFile)(codeQlPath, args); void logger.log(result.stderr); void logger.log("CLI command succeeded."); - return result.stdout; + stdout = result.stdout; } catch (err) { throw new Error( `${description} failed: ${getChildProcessErrorMessage(err)}`, ); } + + try { + return JSON.parse(stdout) as OutputType; + } catch (err) { + throw new Error( + `Parsing output of ${description} failed: ${getErrorMessage(err)}`, + ); + } } diff --git a/extensions/ql-vscode/src/codeql-cli/cli-version.ts b/extensions/ql-vscode/src/codeql-cli/cli-version.ts index 17e995d02..975ed7d79 100644 --- a/extensions/ql-vscode/src/codeql-cli/cli-version.ts +++ b/extensions/ql-vscode/src/codeql-cli/cli-version.ts @@ -1,25 +1,49 @@ import type { SemVer } from "semver"; import { parse } from "semver"; -import { runCodeQlCliCommand } from "./cli-command"; +import { runJsonCodeQlCliCommand } from "./cli-command"; import type { Logger } from "../common/logging"; import { getErrorMessage } from "../common/helpers-pure"; +interface VersionResult { + version: string; + features: CliFeatures | undefined; +} + +export interface CliFeatures { + featuresInVersionResult?: boolean; + mrvaPackCreate?: boolean; +} + +export interface VersionAndFeatures { + version: SemVer; + features: CliFeatures; +} + /** * Get the version of a CodeQL CLI. */ export async function getCodeQlCliVersion( codeQlPath: string, logger: Logger, -): Promise { +): Promise { try { - const output: string = await runCodeQlCliCommand( + const output: VersionResult = await runJsonCodeQlCliCommand( codeQlPath, ["version"], - ["--format=terse"], + ["--format=json"], "Checking CodeQL version", logger, ); - return parse(output.trim()) || undefined; + + const version = parse(output.version.trim()) || undefined; + if (version === undefined) { + return undefined; + } + + return { + version, + features: output.features ?? {}, + }; } catch (e) { // Failed to run the version command. This might happen if the cli version is _really_ old, or it is corrupted. // Either way, we can't determine compatibility. diff --git a/extensions/ql-vscode/src/codeql-cli/cli.ts b/extensions/ql-vscode/src/codeql-cli/cli.ts index e04e012ea..1db2fa0ca 100644 --- a/extensions/ql-vscode/src/codeql-cli/cli.ts +++ b/extensions/ql-vscode/src/codeql-cli/cli.ts @@ -34,6 +34,7 @@ import { QueryLanguage } from "../common/query-language"; import { LINE_ENDINGS, splitStreamAtSeparators } from "../common/split-stream"; import type { Position } from "../query-server/messages"; import { LOGGING_FLAGS } from "./cli-command"; +import type { CliFeatures, VersionAndFeatures } from "./cli-version"; /** * The version of the SARIF format that we are using. @@ -193,7 +194,9 @@ type OnLineCallback = ( line: string, ) => Promise | string | undefined; -type VersionChangedListener = (newVersion: SemVer | undefined) => void; +type VersionChangedListener = ( + newVersionAndFeatures: VersionAndFeatures | undefined, +) => void; /** * This class manages a cli server started by `codeql execute cli-server` to @@ -211,8 +214,8 @@ export class CodeQLCliServer implements Disposable { /** A buffer with a single null byte. */ nullBuffer: Buffer; - /** Version of current cli, lazily computed by the `getVersion()` method */ - private _version: SemVer | undefined; + /** Version of current cli and its supported features, lazily computed by the `getVersion()` method */ + private _versionAndFeatures: VersionAndFeatures | undefined; private _versionChangedListeners: VersionChangedListener[] = []; @@ -288,7 +291,7 @@ export class CodeQLCliServer implements Disposable { const callback = (): void => { try { this.killProcessIfRunning(); - this._version = undefined; + this._versionAndFeatures = undefined; this._supportedLanguages = undefined; } finally { this.runNext(); @@ -1417,6 +1420,27 @@ export class CodeQLCliServer implements Disposable { ); } + public async packCreate( + dir: string, + workspaceFolders: string[], + outputPath: string, + moreOptions: string[], + ): Promise { + const args = [ + "--output", + outputPath, + dir, + ...moreOptions, + ...this.getAdditionalPacksArg(workspaceFolders), + ]; + + return this.runJsonCodeQlCliCommandWithAuthentication( + ["pack", "create"], + args, + "Creating pack", + ); + } + async packBundle( dir: string, workspaceFolders: string[], @@ -1481,27 +1505,35 @@ export class CodeQLCliServer implements Disposable { ); } - public async getVersion() { - if (!this._version) { + public async getVersion(): Promise { + return (await this.getVersionAndFeatures()).version; + } + + public async getFeatures(): Promise { + return (await this.getVersionAndFeatures()).features; + } + + public async getVersionAndFeatures(): Promise { + if (!this._versionAndFeatures) { try { - const newVersion = await this.refreshVersion(); - this._version = newVersion; + const newVersionAndFeatures = await this.refreshVersion(); + this._versionAndFeatures = newVersionAndFeatures; this._versionChangedListeners.forEach((listener) => - listener(newVersion), + listener(newVersionAndFeatures), ); // this._version is only undefined upon config change, so we reset CLI-based context key only when necessary. await this.app.commands.execute( "setContext", "codeql.supportsQuickEvalCount", - newVersion.compare( + newVersionAndFeatures.version.compare( CliVersionConstraint.CLI_VERSION_WITH_QUICK_EVAL_COUNT, ) >= 0, ); await this.app.commands.execute( "setContext", "codeql.supportsTrimCache", - newVersion.compare( + newVersionAndFeatures.version.compare( CliVersionConstraint.CLI_VERSION_WITH_TRIM_CACHE, ) >= 0, ); @@ -1512,23 +1544,23 @@ export class CodeQLCliServer implements Disposable { throw e; } } - return this._version; + return this._versionAndFeatures; } public addVersionChangedListener(listener: VersionChangedListener) { - if (this._version) { - listener(this._version); + if (this._versionAndFeatures) { + listener(this._versionAndFeatures); } this._versionChangedListeners.push(listener); } - private async refreshVersion() { + private async refreshVersion(): Promise { const distribution = await this.distributionProvider.getDistribution(); switch (distribution.kind) { case FindDistributionResultKind.CompatibleDistribution: // eslint-disable-next-line no-fallthrough -- Intentional fallthrough case FindDistributionResultKind.IncompatibleDistribution: - return distribution.version; + return distribution.versionAndFeatures; default: // We should not get here because if no distributions are available, then @@ -1745,4 +1777,8 @@ export class CliVersionConstraint { CliVersionConstraint.CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA, ); } + + async supportsMrvaPackCreate(): Promise { + return (await this.cli.getFeatures()).mrvaPackCreate === true; + } } diff --git a/extensions/ql-vscode/src/codeql-cli/distribution.ts b/extensions/ql-vscode/src/codeql-cli/distribution.ts index 108fb4e01..e69fca721 100644 --- a/extensions/ql-vscode/src/codeql-cli/distribution.ts +++ b/extensions/ql-vscode/src/codeql-cli/distribution.ts @@ -1,11 +1,11 @@ import { createWriteStream, mkdtemp, pathExists, remove } from "fs-extra"; import { tmpdir } from "os"; import { delimiter, dirname, join } from "path"; -import type { SemVer } from "semver"; import { Range, satisfies } from "semver"; import type { Event, ExtensionContext } from "vscode"; import type { DistributionConfig } from "../config"; import { extLogger } from "../common/logging/vscode"; +import type { VersionAndFeatures } from "./cli-version"; import { getCodeQlCliVersion } from "./cli-version"; import type { ProgressCallback } from "../common/vscode/progress"; import { reportStreamProgress } from "../common/vscode/progress"; @@ -88,11 +88,11 @@ export class DistributionManager implements DistributionProvider { kind: FindDistributionResultKind.NoDistribution, }; } - const version = await getCodeQlCliVersion( + const versionAndFeatures = await getCodeQlCliVersion( distribution.codeQlPath, extLogger, ); - if (version === undefined) { + if (versionAndFeatures === undefined) { return { distribution, kind: FindDistributionResultKind.UnknownCompatibilityDistribution, @@ -119,17 +119,21 @@ export class DistributionManager implements DistributionProvider { distribution.kind !== DistributionKind.ExtensionManaged || this.config.includePrerelease; - if (!satisfies(version, this.versionRange, { includePrerelease })) { + if ( + !satisfies(versionAndFeatures.version, this.versionRange, { + includePrerelease, + }) + ) { return { distribution, kind: FindDistributionResultKind.IncompatibleDistribution, - version, + versionAndFeatures, }; } return { distribution, kind: FindDistributionResultKind.CompatibleDistribution, - version, + versionAndFeatures, }; } @@ -599,7 +603,7 @@ interface DistributionResult { interface CompatibleDistributionResult extends DistributionResult { kind: FindDistributionResultKind.CompatibleDistribution; - version: SemVer; + versionAndFeatures: VersionAndFeatures; } interface UnknownCompatibilityDistributionResult extends DistributionResult { @@ -608,7 +612,7 @@ interface UnknownCompatibilityDistributionResult extends DistributionResult { interface IncompatibleDistributionResult extends DistributionResult { kind: FindDistributionResultKind.IncompatibleDistribution; - version: SemVer; + versionAndFeatures: VersionAndFeatures; } interface NoDistributionResult { diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 58be99fad..ecc4cda54 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -418,7 +418,7 @@ export async function activate( codeQlExtension.variantAnalysisManager, ); codeQlExtension.cliServer.addVersionChangedListener((ver) => { - telemetryListener.cliVersion = ver; + telemetryListener.cliVersion = ver?.version; }); let unsupportedWarningShown = false; @@ -431,13 +431,16 @@ export async function activate( return; } - if (CliVersionConstraint.OLDEST_SUPPORTED_CLI_VERSION.compare(ver) < 0) { + if ( + CliVersionConstraint.OLDEST_SUPPORTED_CLI_VERSION.compare(ver.version) < + 0 + ) { return; } void showAndLogWarningMessage( extLogger, - `You are using an unsupported version of the CodeQL CLI (${ver}). ` + + `You are using an unsupported version of the CodeQL CLI (${ver.version}). ` + `The minimum supported version is ${CliVersionConstraint.OLDEST_SUPPORTED_CLI_VERSION}. ` + `Please upgrade to a newer version of the CodeQL CLI.`, ); @@ -592,7 +595,7 @@ async function getDistributionDisplayingDistributionWarnings( switch (result.kind) { case FindDistributionResultKind.CompatibleDistribution: void extLogger.log( - `Found compatible version of CodeQL CLI (version ${result.version.raw})`, + `Found compatible version of CodeQL CLI (version ${result.versionAndFeatures.version.raw})`, ); break; case FindDistributionResultKind.IncompatibleDistribution: { @@ -612,7 +615,7 @@ async function getDistributionDisplayingDistributionWarnings( void showAndLogWarningMessage( extLogger, - `The current version of the CodeQL CLI (${result.version.raw}) ` + + `The current version of the CodeQL CLI (${result.versionAndFeatures.version.raw}) ` + `is incompatible with this extension. ${fixGuidanceMessage}`, ); break; From 29d8c65b5957c6ef1516173669a8074dfff66d0a Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 11 Jan 2024 17:43:42 -0500 Subject: [PATCH 02/17] Use `codeql pack create --mrva` if available --- .../src/variant-analysis/run-remote-query.ts | 217 +++++++++++------- 1 file changed, 138 insertions(+), 79 deletions(-) diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index 421dc54e7..b38e20aa9 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -3,6 +3,7 @@ import { Uri, window } from "vscode"; import { relative, join, sep, dirname, parse, basename } from "path"; import { dump, load } from "js-yaml"; import { copy, writeFile, readFile, mkdirp } from "fs-extra"; +import type { DirectoryResult } from "tmp-promise"; import { dir, tmpName } from "tmp-promise"; import { tmpDir } from "../tmp-dir"; import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders"; @@ -58,21 +59,53 @@ interface GeneratedQueryPack { async function generateQueryPack( cliServer: CodeQLCliServer, queryFile: string, - queryPackDir: string, + tmpDir: RemoteQueryTempDir, ): Promise { const originalPackRoot = await findPackRoot(queryFile); const packRelativePath = relative(originalPackRoot, queryFile); - const targetQueryFileName = join(queryPackDir, packRelativePath); const workspaceFolders = getOnDiskWorkspaceFolders(); + const extensionPacks = await getExtensionPacksToInject( + cliServer, + workspaceFolders, + ); - let language: QueryLanguage | undefined; + const mustSynthesizePack = + (await getQlPackPath(originalPackRoot)) === undefined; + const cliSupportsMrvaPackCreate = + await cliServer.cliConstraints.supportsMrvaPackCreate(); - // Check if the query is already in a query pack. - // If so, copy the entire query pack to the temporary directory. - // Otherwise, copy only the query file to the temporary directory - // and generate a synthetic query pack. - if (await getQlPackPath(originalPackRoot)) { - // don't include ql files. We only want the queryFile to be copied. + const language: QueryLanguage | undefined = mustSynthesizePack + ? await askForLanguage(cliServer) // open popup to ask for language if not already hardcoded + : await findLanguage(cliServer, Uri.file(queryFile)); + if (!language) { + throw new UserCancellationException("Could not determine language."); + } + + let queryPackDir: string; + let precompilationOpts: string[]; + let needsInstall: boolean; + if (mustSynthesizePack) { + // This section applies whether or not the CLI supports MRVA pack creation directly. + + queryPackDir = tmpDir.queryPackDir; + + // Synthesize a query pack for the query. + // copy only the query file to the query pack directory + // and generate a synthetic query pack + await createNewQueryPack( + queryFile, + queryPackDir, + language, + packRelativePath, + ); + // Clear the cliServer cache so that the previous qlpack text is purged from the CLI. + await cliServer.clearCache(); + + // Install packs, since we just synthesized a dependency on the language's standard library. + needsInstall = true; + } else if (!cliSupportsMrvaPackCreate) { + // We need to copy the query pack to a temporary directory and then fix it up to work with MRVA. + queryPackDir = tmpDir.queryPackDir; await copyExistingQueryPack( cliServer, originalPackRoot, @@ -81,61 +114,64 @@ async function generateQueryPack( packRelativePath, ); - language = await findLanguage(cliServer, Uri.file(targetQueryFileName)); + // We should already have all the dependencies available, but these older versions of the CLI + // have a bug where they will not search `--additional-packs` during validation in `codeql pack bundle`. + // Installing the packs will ensure that any extension packs get put in the right place. + needsInstall = true; } else { - // open popup to ask for language if not already hardcoded - language = await askForLanguage(cliServer); - - // copy only the query file to the query pack directory - // and generate a synthetic query pack - await createNewQueryPack( - queryFile, - queryPackDir, - targetQueryFileName, - language, - packRelativePath, - ); - } - if (!language) { - throw new UserCancellationException("Could not determine language."); + // The CLI supports creating a MRVA query pack directly from the source pack. + queryPackDir = originalPackRoot; + // We expect any dependencies to be available already. + needsInstall = false; } - // Clear the cliServer cache so that the previous qlpack text is purged from the CLI. - await cliServer.clearCache(); + if (needsInstall) { + // Install the dependencies of the synthesized query pack. + await cliServer.packInstall(queryPackDir, { + workspaceFolders, + }); - let precompilationOpts: string[] = []; - if (await cliServer.cliConstraints.usesGlobalCompilationCache()) { - precompilationOpts = ["--qlx"]; - } else { - const ccache = join(originalPackRoot, ".cache"); - precompilationOpts = [ - "--qlx", - "--no-default-compilation-cache", - `--compilation-cache=${ccache}`, - ]; + // Clear the CLI cache so that the most recent qlpack lock file is used. + await cliServer.clearCache(); } - if (await cliServer.useExtensionPacks()) { - await injectExtensionPacks(cliServer, queryPackDir, workspaceFolders); - } - - await cliServer.packInstall(queryPackDir, { - workspaceFolders, - }); - // Clear the CLI cache so that the most recent qlpack lock file is used. await cliServer.clearCache(); + if (cliSupportsMrvaPackCreate) { + precompilationOpts = [ + "--mrva", + "--query", + join(queryPackDir, packRelativePath), + // We need to specify the extension packs as dependencies so that they are included in the MRVA pack. + // The version range doesn't matter, since they'll always be found by source lookup. + ...extensionPacks.map((p) => `--extension-pack=${p}@*`), + ]; + } else { + if (await cliServer.cliConstraints.usesGlobalCompilationCache()) { + precompilationOpts = ["--qlx"]; + } else { + const ccache = join(originalPackRoot, ".cache"); + precompilationOpts = [ + "--qlx", + "--no-default-compilation-cache", + `--compilation-cache=${ccache}`, + ]; + } - const bundlePath = await getPackedBundlePath(queryPackDir); + if (extensionPacks.length > 0) { + await addExtensionPacksAsDependencies(queryPackDir, extensionPacks); + } + } + + const bundlePath = tmpDir.bundleFile; void extLogger.log( `Compiling and bundling query pack from ${queryPackDir} to ${bundlePath}. (This may take a while.)`, ); - await cliServer.packBundle( - queryPackDir, - workspaceFolders, - bundlePath, - precompilationOpts, - ); + await cliServer.packBundle(queryPackDir, workspaceFolders, bundlePath, [ + "--pack-path", + tmpDir.compiledPackDir, + ...precompilationOpts, + ]); const base64Pack = (await readFile(bundlePath)).toString("base64"); return { base64Pack, @@ -146,11 +182,11 @@ async function generateQueryPack( async function createNewQueryPack( queryFile: string, queryPackDir: string, - targetQueryFileName: string, language: string | undefined, packRelativePath: string, ) { void extLogger.log(`Copying ${queryFile} to ${queryPackDir}`); + const targetQueryFileName = join(queryPackDir, packRelativePath); await copy(queryFile, targetQueryFileName); void extLogger.log("Generating synthetic query pack"); const syntheticQueryPack = { @@ -242,19 +278,28 @@ function isFileSystemRoot(dir: string): boolean { return pathObj.root === dir && pathObj.base === ""; } -async function createRemoteQueriesTempDirectory() { +interface RemoteQueryTempDir { + remoteQueryDir: DirectoryResult; + queryPackDir: string; + compiledPackDir: string; + bundleFile: string; +} + +async function createRemoteQueriesTempDirectory(): Promise { const remoteQueryDir = await dir({ dir: tmpDir.name, unsafeCleanup: true, }); const queryPackDir = join(remoteQueryDir.path, "query-pack"); await mkdirp(queryPackDir); - return { remoteQueryDir, queryPackDir }; + const compiledPackDir = join(remoteQueryDir.path, "compiled-pack"); + const bundleFile = await getPackedBundlePath(tmpDir.name); + return { remoteQueryDir, queryPackDir, compiledPackDir, bundleFile }; } -async function getPackedBundlePath(queryPackDir: string) { +async function getPackedBundlePath(remoteQueryDir: string): Promise { return tmpName({ - dir: dirname(queryPackDir), + dir: remoteQueryDir, postfix: "generated.tgz", prefix: "qlpack", }); @@ -314,15 +359,14 @@ export async function prepareRemoteQueryRun( throw new UserCancellationException("Cancelled"); } - const { remoteQueryDir, queryPackDir } = - await createRemoteQueriesTempDirectory(); + const tempDir = await createRemoteQueriesTempDirectory(); let pack: GeneratedQueryPack; try { - pack = await generateQueryPack(cliServer, queryFile, queryPackDir); + pack = await generateQueryPack(cliServer, queryFile, tempDir); } finally { - await remoteQueryDir.cleanup(); + await tempDir.remoteQueryDir.cleanup(); } const { base64Pack, language } = pack; @@ -389,11 +433,38 @@ async function fixPackFile( await writeFile(packPath, dump(qlpack)); } -async function injectExtensionPacks( +async function getExtensionPacksToInject( cliServer: CodeQLCliServer, - queryPackDir: string, workspaceFolders: string[], -) { +): Promise { + const result: string[] = []; + if (await cliServer.useExtensionPacks()) { + const extensionPacks = await cliServer.resolveQlpacks( + workspaceFolders, + true, + ); + Object.entries(extensionPacks).forEach(([name, paths]) => { + // We are guaranteed that there is at least one path found for each extension pack. + // If there are multiple paths, then we have a problem. This means that there is + // ambiguity in which path to use. This is an error. + if (paths.length > 1) { + throw new Error( + `Multiple versions of extension pack '${name}' found: ${paths.join( + ", ", + )}`, + ); + } + result.push(name); + }); + } + + return result; +} + +async function addExtensionPacksAsDependencies( + queryPackDir: string, + extensionPacks: string[], +): Promise { const qlpackFile = await getQlPackPath(queryPackDir); if (!qlpackFile) { throw new Error( @@ -402,24 +473,13 @@ async function injectExtensionPacks( )} file in '${queryPackDir}'`, ); } + const syntheticQueryPack = load( await readFile(qlpackFile, "utf8"), ) as QlPackFile; const dependencies = syntheticQueryPack.dependencies ?? {}; - - const extensionPacks = await cliServer.resolveQlpacks(workspaceFolders, true); - Object.entries(extensionPacks).forEach(([name, paths]) => { - // We are guaranteed that there is at least one path found for each extension pack. - // If there are multiple paths, then we have a problem. This means that there is - // ambiguity in which path to use. This is an error. - if (paths.length > 1) { - throw new Error( - `Multiple versions of extension pack '${name}' found: ${paths.join( - ", ", - )}`, - ); - } + extensionPacks.forEach((name) => { // Add this extension pack as a dependency. It doesn't matter which // version we specify, since we are guaranteed that the extension pack // is resolved from source at the given path. @@ -429,7 +489,6 @@ async function injectExtensionPacks( syntheticQueryPack.dependencies = dependencies; await writeFile(qlpackFile, dump(syntheticQueryPack)); - await cliServer.clearCache(); } function updateDefaultSuite(qlpack: QlPackFile, packRelativePath: string) { @@ -534,7 +593,7 @@ async function getControllerRepoFromApi( } } -function removeWorkspaceRefs(qlpack: QlPackFile) { +export function removeWorkspaceRefs(qlpack: QlPackFile) { if (!qlpack.dependencies) { return; } From bdc94a3b234d84d9f3350da8cc8c4930b28bdbc5 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 11 Jan 2024 18:05:00 -0500 Subject: [PATCH 03/17] Remove unused export --- extensions/ql-vscode/src/variant-analysis/run-remote-query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index b38e20aa9..0b1f4f127 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -593,7 +593,7 @@ async function getControllerRepoFromApi( } } -export function removeWorkspaceRefs(qlpack: QlPackFile) { +function removeWorkspaceRefs(qlpack: QlPackFile) { if (!qlpack.dependencies) { return; } From 2ccd99fc5bb746084bd70002ab5b373b741e0352 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 11 Jan 2024 18:40:03 -0500 Subject: [PATCH 04/17] Don't use `${workspace}` in test pack --- .../vscode-tests/cli-integration/data-remote-qlpack/qlpack.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/data-remote-qlpack/qlpack.yml b/extensions/ql-vscode/test/vscode-tests/cli-integration/data-remote-qlpack/qlpack.yml index 1b3f20eee..6558fe72c 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/data-remote-qlpack/qlpack.yml +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/data-remote-qlpack/qlpack.yml @@ -1,4 +1,4 @@ name: github/remote-query-pack version: 0.0.0 dependencies: - codeql/javascript-all: ${workspace} + codeql/javascript-all: "*" From 2aacea417644bf937c7d7053399a0c10c178241c Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 11 Jan 2024 18:58:44 -0500 Subject: [PATCH 05/17] Fix test expectation --- .../variant-analysis/variant-analysis-manager.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index 2c854f9ea..47603e83d 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -364,9 +364,9 @@ describe("Variant Analysis Manager", () => { // Assume the first dependency to check is the core library. if (dependenciesToCheck.length > 0) { - expect(qlpackContents.dependencies?.[dependenciesToCheck[0]]).toEqual( - "*", - ); + expect( + qlpackContents.dependencies?.[dependenciesToCheck[0]], + ).not.toEqual("${workspace}"); } const qlpackLockContents = load( packFS.fileContents("codeql-pack.lock.yml").toString("utf-8"), From 9c42c6a851f1371136aa7c507121e274d8fae2b2 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 12 Jan 2024 09:30:13 -0500 Subject: [PATCH 06/17] Allow CodeQL checkout to be named `ql` This makes it easier for cross-repo development with `semmle-code`, where the `codeql` repo is in a submodule in a directory named `ql`. --- extensions/ql-vscode/test/vscode-tests/cli.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli.ts b/extensions/ql-vscode/test/vscode-tests/cli.ts index e875c44ea..c1110dfe8 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli.ts @@ -6,7 +6,10 @@ import { workspace } from "vscode"; */ function hasCodeQL() { const folders = workspace.workspaceFolders; - return !!folders?.some((folder) => folder.uri.path.endsWith("/codeql")); + return !!folders?.some( + (folder) => + folder.uri.path.endsWith("/codeql") || folder.uri.path.endsWith("/ql"), + ); } // describeWithCodeQL will be equal to describe if the CodeQL libraries are From e03c2b132c7f3f3e697528d8973bcae7dcaeb8c0 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 12 Jan 2024 10:42:34 -0500 Subject: [PATCH 07/17] Custom message for assertions about files existing in packs --- .../variant-analysis-manager.test.ts | 43 +++++++++++++++++-- .../utils/bundled-pack-helpers.ts | 2 +- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index 47603e83d..a7536016a 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -20,10 +20,47 @@ import { ExtensionApp } from "../../../../src/common/vscode/vscode-app"; import { DbConfigStore } from "../../../../src/databases/config/db-config-store"; import { mockedQuickPickItem } from "../../utils/mocking.helpers"; import { QueryLanguage } from "../../../../src/common/query-language"; +import type { QueryPackFS } from "../../utils/bundled-pack-helpers"; import { readBundledPack } from "../../utils/bundled-pack-helpers"; import { load } from "js-yaml"; import type { ExtensionPackMetadata } from "../../../../src/model-editor/extension-pack-metadata"; import type { QlPackLockFile } from "../../../../src/packaging/qlpack-lock-file"; +import { expect } from "@jest/globals"; +import type { ExpectationResult } from "expect"; + +/** + * Custom Jest matcher to check if a file exists in a query pack. + */ +function toExistInPack( + this: jest.MatcherContext, + actual: unknown, + packFS: QueryPackFS, +): ExpectationResult { + if (typeof actual !== "string") { + throw new TypeError("Expected actual value to be a string."); + } + + const pass = packFS.fileExists(actual); + if (pass) { + return { + pass: true, + message: () => `expected ${actual} not to exist in pack`, + }; + } else { + return { + pass: false, + message: () => `expected ${actual} to exist in pack`, + }; + } +} + +expect.extend({ toExistInPack }); + +declare module "expect" { + interface Matchers { + toExistInPack(packFS: QueryPackFS): R; + } +} describe("Variant Analysis Manager", () => { let cli: CodeQLCliServer; @@ -331,14 +368,14 @@ describe("Variant Analysis Manager", () => { const packFS = await readBundledPack(request.query.pack); filesThatExist.forEach((file) => { - expect(packFS.fileExists(file)).toBe(true); + expect(file).toExistInPack(packFS); }); qlxFilesThatExist.forEach((file) => { - expect(packFS.fileExists(file)).toBe(true); + expect(file).toExistInPack(packFS); }); filesThatDoNotExist.forEach((file) => { - expect(packFS.fileExists(file)).toBe(false); + expect(file).not.toExistInPack(packFS); }); expect( diff --git a/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts b/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts index a49130c40..32c34cbec 100644 --- a/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts +++ b/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts @@ -4,7 +4,7 @@ import { extract as tar_extract } from "tar-stream"; import { pipeline } from "stream/promises"; import { createGunzip } from "zlib"; -interface QueryPackFS { +export interface QueryPackFS { fileExists: (name: string) => boolean; fileContents: (name: string) => Buffer; directoryContents: (name: string) => string[]; From 61933c34d52c9bb8ad1248d8e4a694a9961a0ecb Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 12 Jan 2024 12:41:36 -0500 Subject: [PATCH 08/17] Dump pack contents when expected file not found --- .../variant-analysis/variant-analysis-manager.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index a7536016a..3e8e9869f 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -41,6 +41,7 @@ function toExistInPack( } const pass = packFS.fileExists(actual); + const files = packFS.allFiles().join("\n"); if (pass) { return { pass: true, @@ -49,7 +50,8 @@ function toExistInPack( } else { return { pass: false, - message: () => `expected ${actual} to exist in pack`, + message: () => + `expected ${actual} to exist in pack. The following files were found in the pack:\n${files}`, }; } } From 6d308f8688b9e60df65f46fa810484707c444d3d Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 12 Jan 2024 12:44:16 -0500 Subject: [PATCH 09/17] Add missing change --- .../ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts b/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts index 32c34cbec..f572537bf 100644 --- a/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts +++ b/extensions/ql-vscode/test/vscode-tests/utils/bundled-pack-helpers.ts @@ -8,6 +8,7 @@ export interface QueryPackFS { fileExists: (name: string) => boolean; fileContents: (name: string) => Buffer; directoryContents: (name: string) => string[]; + allFiles: () => string[]; } export async function readBundledPack( @@ -82,5 +83,8 @@ export async function readBundledPack( ) .map((dir) => dir.substring(name.length + 1)); }, + allFiles: (): string[] => { + return Object.keys(files); + }, }; } From 9d2190a88d08fc4595d8b3c7e462178b10b23c45 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 12 Jan 2024 13:03:21 -0500 Subject: [PATCH 10/17] Add text if pack is empty --- .../variant-analysis/variant-analysis-manager.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index 3e8e9869f..564b42a85 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -41,7 +41,8 @@ function toExistInPack( } const pass = packFS.fileExists(actual); - const files = packFS.allFiles().join("\n"); + const files = packFS.allFiles(); + const filesString = files.length > 0 ? files.join("\n") : ""; if (pass) { return { pass: true, @@ -51,7 +52,7 @@ function toExistInPack( return { pass: false, message: () => - `expected ${actual} to exist in pack. The following files were found in the pack:\n${files}`, + `expected ${actual} to exist in pack.\nThe following files were found in the pack:\n${filesString}`, }; } } From ee73639720235cf010f397c1eeaa32ca591a81fc Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Sat, 13 Jan 2024 14:30:01 -0500 Subject: [PATCH 11/17] Expand short paths before calling `codeql pack bundle` --- .../ql-vscode/src/common/short-paths.ts | 111 ++++++++++++++++++ .../src/variant-analysis/run-remote-query.ts | 9 +- 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 extensions/ql-vscode/src/common/short-paths.ts diff --git a/extensions/ql-vscode/src/common/short-paths.ts b/extensions/ql-vscode/src/common/short-paths.ts new file mode 100644 index 000000000..27eac829c --- /dev/null +++ b/extensions/ql-vscode/src/common/short-paths.ts @@ -0,0 +1,111 @@ +import { platform } from "os"; +import { basename, dirname, join, normalize, resolve } from "path"; +import { lstat, readdir } from "fs/promises"; +import { extLogger } from "./logging/vscode"; + +async function log(message: string): Promise { + await extLogger.log(message); +} + +/** + * Expand a single short path component + * @param dir The absolute path of the directory containing the short path component. + * @param shortBase The shot path component to expand. + * @returns The expanded path component. + */ +async function expandShortPathComponent( + dir: string, + shortBase: string, +): Promise { + await log(`Expanding short path component: ${shortBase}`); + + const fullPath = join(dir, shortBase); + + // Use `lstat` instead of `stat` to avoid following symlinks. + const stats = await lstat(fullPath, { bigint: true }); + if (stats.dev === BigInt(0) || stats.ino === BigInt(0)) { + // No inode info, so we won't be able to find this in the directory listing. + await log(`No inode info available. Skipping.`); + return shortBase; + } + await log(`dev/inode: ${stats.dev}/${stats.ino}`); + + try { + // Enumerate the children of the parent directory, and try to find one with the same dev/inode. + const children = await readdir(dir); + for (const child of children) { + await log(`considering child: ${child}`); + try { + const childStats = await lstat(join(dir, child), { bigint: true }); + await log(`child dev/inode: ${childStats.dev}/${childStats.ino}`); + if (childStats.dev === stats.dev && childStats.ino === stats.ino) { + // Found a match. + await log(`Found a match: ${child}`); + return child; + } + } catch (e) { + // Can't read stats for the child, so skip it. + await log(`Error reading stats for child: ${e}`); + } + } + } catch (e) { + // Can't read the directory, so we won't be able to find this in the directory listing. + await log(`Error reading directory: ${e}`); + return shortBase; + } + + await log(`No match found. Returning original.`); + return shortBase; +} + +/** + * Expand the short path components in a path, including those in ancestor directories. + * @param shortPath The path to expand. + * @returns The expanded path. + */ +async function expandShortPathRecursive(shortPath: string): Promise { + const shortBase = basename(shortPath); + if (shortBase.length === 0) { + // We've reached the root. + return shortPath; + } + + const dir = await expandShortPathRecursive(dirname(shortPath)); + await log(`dir: ${dir}`); + await log(`base: ${shortBase}`); + if (shortBase.indexOf("~") < 0) { + // This component doesn't have a short name, so just append it to the (long) parent. + await log(`Component is not a short name`); + return join(dir, shortBase); + } + + // This component looks like it has a short name, so try to expand it. + const longBase = await expandShortPathComponent(dir, shortBase); + return join(dir, longBase); +} + +/** + * Expands a path that potentially contains 8.3 short names (e.g. "C:\PROGRA~1" instead of "C:\Program Files"). + * @param shortPath The path to expand. + * @returns A normalized, absolute path, with any short components expanded. + */ +export async function expandShortPaths(shortPath: string): Promise { + const absoluteShortPath = normalize(resolve(shortPath)); + if (platform() !== "win32") { + // POSIX doesn't have short paths. + return absoluteShortPath; + } + + await log(`Expanding short paths in: ${absoluteShortPath}`); + // A quick check to see if there might be any short components. + // There might be a case where a short component doesn't contain a `~`, but if there is, I haven't + // found it. + // This may find long components that happen to have a '~', but that's OK. + if (absoluteShortPath.indexOf("~") < 0) { + // No short components to expand. + await log(`Skipping due to no short components`); + return absoluteShortPath; + } + + return await expandShortPathRecursive(absoluteShortPath); +} diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index 0b1f4f127..8938649a4 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -38,6 +38,7 @@ import type { QueryLanguage } from "../common/query-language"; import { tryGetQueryMetadata } from "../codeql-cli/query-metadata"; import { askForLanguage, findLanguage } from "../codeql-cli/query-language"; import type { QlPackFile } from "../packaging/qlpack-file"; +import { expandShortPaths } from "../common/short-paths"; /** * Well-known names for the query pack used by the server. @@ -286,10 +287,16 @@ interface RemoteQueryTempDir { } async function createRemoteQueriesTempDirectory(): Promise { - const remoteQueryDir = await dir({ + const shortRemoteQueryDir = await dir({ dir: tmpDir.name, unsafeCleanup: true, }); + // Expand 8.3 filenames here to work around a CLI bug where `codeql pack bundle` produces an empty + // archive if the pack path contains any 8.3 components. + const remoteQueryDir = { + ...shortRemoteQueryDir, + dir: expandShortPaths(shortRemoteQueryDir.path), + }; const queryPackDir = join(remoteQueryDir.path, "query-pack"); await mkdirp(queryPackDir); const compiledPackDir = join(remoteQueryDir.path, "compiled-pack"); From b16aeb38872f1aac586e288111f6e13a6e344ab8 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Sun, 14 Jan 2024 10:17:35 -0500 Subject: [PATCH 12/17] Fix property name, and expand another path --- .../ql-vscode/src/variant-analysis/run-remote-query.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index 8938649a4..5f0dea969 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -295,12 +295,14 @@ async function createRemoteQueriesTempDirectory(): Promise { // archive if the pack path contains any 8.3 components. const remoteQueryDir = { ...shortRemoteQueryDir, - dir: expandShortPaths(shortRemoteQueryDir.path), + path: await expandShortPaths(shortRemoteQueryDir.path), }; const queryPackDir = join(remoteQueryDir.path, "query-pack"); await mkdirp(queryPackDir); const compiledPackDir = join(remoteQueryDir.path, "compiled-pack"); - const bundleFile = await getPackedBundlePath(tmpDir.name); + const bundleFile = await expandShortPaths( + await getPackedBundlePath(tmpDir.name), + ); return { remoteQueryDir, queryPackDir, compiledPackDir, bundleFile }; } From f53826c09db5088aeaa25b41ad9f435e4baf84cf Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Sun, 14 Jan 2024 10:20:55 -0500 Subject: [PATCH 13/17] Better dependency version assertions --- .../variant-analysis-manager.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index 564b42a85..ca1287ea1 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -404,9 +404,17 @@ describe("Variant Analysis Manager", () => { // Assume the first dependency to check is the core library. if (dependenciesToCheck.length > 0) { - expect( - qlpackContents.dependencies?.[dependenciesToCheck[0]], - ).not.toEqual("${workspace}"); + const dependencyVersion = + qlpackContents.dependencies?.[dependenciesToCheck[0]]; + + // There should be a version specified. + expect(dependencyVersion).toBeDefined(); + + // Any `${workspace}` placeholder should have been replaced. + // The actual version might be `*` (for the legacy code path where we replace workspace + // references with `*`) or a specific version (for the new code path where the CLI does all + // the work). + expect(dependencyVersion).not.toEqual("${workspace}"); } const qlpackLockContents = load( packFS.fileContents("codeql-pack.lock.yml").toString("utf-8"), From 55306c9a93b4d8200dd1f63c88ee6bea93b0aaba Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 17 Jan 2024 12:47:06 -0500 Subject: [PATCH 14/17] Fix PR feedback --- extensions/ql-vscode/src/codeql-cli/cli.ts | 21 -------- .../ql-vscode/src/common/short-paths.ts | 54 ++++++++++--------- .../src/variant-analysis/run-remote-query.ts | 3 +- 3 files changed, 32 insertions(+), 46 deletions(-) diff --git a/extensions/ql-vscode/src/codeql-cli/cli.ts b/extensions/ql-vscode/src/codeql-cli/cli.ts index 1db2fa0ca..a3be391a4 100644 --- a/extensions/ql-vscode/src/codeql-cli/cli.ts +++ b/extensions/ql-vscode/src/codeql-cli/cli.ts @@ -1420,27 +1420,6 @@ export class CodeQLCliServer implements Disposable { ); } - public async packCreate( - dir: string, - workspaceFolders: string[], - outputPath: string, - moreOptions: string[], - ): Promise { - const args = [ - "--output", - outputPath, - dir, - ...moreOptions, - ...this.getAdditionalPacksArg(workspaceFolders), - ]; - - return this.runJsonCodeQlCliCommandWithAuthentication( - ["pack", "create"], - args, - "Creating pack", - ); - } - async packBundle( dir: string, workspaceFolders: string[], diff --git a/extensions/ql-vscode/src/common/short-paths.ts b/extensions/ql-vscode/src/common/short-paths.ts index 27eac829c..805075c99 100644 --- a/extensions/ql-vscode/src/common/short-paths.ts +++ b/extensions/ql-vscode/src/common/short-paths.ts @@ -1,11 +1,7 @@ import { platform } from "os"; import { basename, dirname, join, normalize, resolve } from "path"; import { lstat, readdir } from "fs/promises"; -import { extLogger } from "./logging/vscode"; - -async function log(message: string): Promise { - await extLogger.log(message); -} +import type { BaseLogger } from "./logging"; /** * Expand a single short path component @@ -16,8 +12,9 @@ async function log(message: string): Promise { async function expandShortPathComponent( dir: string, shortBase: string, + logger: BaseLogger, ): Promise { - await log(`Expanding short path component: ${shortBase}`); + void logger.log(`Expanding short path component: ${shortBase}`); const fullPath = join(dir, shortBase); @@ -25,36 +22,36 @@ async function expandShortPathComponent( const stats = await lstat(fullPath, { bigint: true }); if (stats.dev === BigInt(0) || stats.ino === BigInt(0)) { // No inode info, so we won't be able to find this in the directory listing. - await log(`No inode info available. Skipping.`); + void logger.log(`No inode info available. Skipping.`); return shortBase; } - await log(`dev/inode: ${stats.dev}/${stats.ino}`); + void logger.log(`dev/inode: ${stats.dev}/${stats.ino}`); try { // Enumerate the children of the parent directory, and try to find one with the same dev/inode. const children = await readdir(dir); for (const child of children) { - await log(`considering child: ${child}`); + void logger.log(`considering child: ${child}`); try { const childStats = await lstat(join(dir, child), { bigint: true }); - await log(`child dev/inode: ${childStats.dev}/${childStats.ino}`); + void logger.log(`child dev/inode: ${childStats.dev}/${childStats.ino}`); if (childStats.dev === stats.dev && childStats.ino === stats.ino) { // Found a match. - await log(`Found a match: ${child}`); + void logger.log(`Found a match: ${child}`); return child; } } catch (e) { // Can't read stats for the child, so skip it. - await log(`Error reading stats for child: ${e}`); + void logger.log(`Error reading stats for child: ${e}`); } } } catch (e) { // Can't read the directory, so we won't be able to find this in the directory listing. - await log(`Error reading directory: ${e}`); + void logger.log(`Error reading directory: ${e}`); return shortBase; } - await log(`No match found. Returning original.`); + void logger.log(`No match found. Returning original.`); return shortBase; } @@ -63,49 +60,58 @@ async function expandShortPathComponent( * @param shortPath The path to expand. * @returns The expanded path. */ -async function expandShortPathRecursive(shortPath: string): Promise { +async function expandShortPathRecursive( + shortPath: string, + logger: BaseLogger, +): Promise { const shortBase = basename(shortPath); if (shortBase.length === 0) { // We've reached the root. return shortPath; } - const dir = await expandShortPathRecursive(dirname(shortPath)); - await log(`dir: ${dir}`); - await log(`base: ${shortBase}`); + const dir = await expandShortPathRecursive(dirname(shortPath), logger); + void logger.log(`dir: ${dir}`); + void logger.log(`base: ${shortBase}`); if (shortBase.indexOf("~") < 0) { // This component doesn't have a short name, so just append it to the (long) parent. - await log(`Component is not a short name`); + void logger.log(`Component is not a short name`); return join(dir, shortBase); } // This component looks like it has a short name, so try to expand it. - const longBase = await expandShortPathComponent(dir, shortBase); + const longBase = await expandShortPathComponent(dir, shortBase, logger); return join(dir, longBase); } /** * Expands a path that potentially contains 8.3 short names (e.g. "C:\PROGRA~1" instead of "C:\Program Files"). + * + * See https://en.wikipedia.org/wiki/8.3_filename if you're not familiar with Windows 8.3 short names. + * * @param shortPath The path to expand. * @returns A normalized, absolute path, with any short components expanded. */ -export async function expandShortPaths(shortPath: string): Promise { +export async function expandShortPaths( + shortPath: string, + logger: BaseLogger, +): Promise { const absoluteShortPath = normalize(resolve(shortPath)); if (platform() !== "win32") { // POSIX doesn't have short paths. return absoluteShortPath; } - await log(`Expanding short paths in: ${absoluteShortPath}`); + void logger.log(`Expanding short paths in: ${absoluteShortPath}`); // A quick check to see if there might be any short components. // There might be a case where a short component doesn't contain a `~`, but if there is, I haven't // found it. // This may find long components that happen to have a '~', but that's OK. if (absoluteShortPath.indexOf("~") < 0) { // No short components to expand. - await log(`Skipping due to no short components`); + void logger.log(`Skipping due to no short components`); return absoluteShortPath; } - return await expandShortPathRecursive(absoluteShortPath); + return await expandShortPathRecursive(absoluteShortPath, logger); } diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index 5f0dea969..13caced7a 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -295,13 +295,14 @@ async function createRemoteQueriesTempDirectory(): Promise { // archive if the pack path contains any 8.3 components. const remoteQueryDir = { ...shortRemoteQueryDir, - path: await expandShortPaths(shortRemoteQueryDir.path), + path: await expandShortPaths(shortRemoteQueryDir.path, extLogger), }; const queryPackDir = join(remoteQueryDir.path, "query-pack"); await mkdirp(queryPackDir); const compiledPackDir = join(remoteQueryDir.path, "compiled-pack"); const bundleFile = await expandShortPaths( await getPackedBundlePath(tmpDir.name), + extLogger, ); return { remoteQueryDir, queryPackDir, compiledPackDir, bundleFile }; } From 1d275985d0aa010ad3a841865927ba98d9746db4 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 17 Jan 2024 12:49:20 -0500 Subject: [PATCH 15/17] Fix PR feedback --- .../ql-vscode/src/common/short-paths.ts | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/extensions/ql-vscode/src/common/short-paths.ts b/extensions/ql-vscode/src/common/short-paths.ts index 805075c99..838dac031 100644 --- a/extensions/ql-vscode/src/common/short-paths.ts +++ b/extensions/ql-vscode/src/common/short-paths.ts @@ -3,6 +3,38 @@ import { basename, dirname, join, normalize, resolve } from "path"; import { lstat, readdir } from "fs/promises"; import type { BaseLogger } from "./logging"; +/** + * Expands a path that potentially contains 8.3 short names (e.g. "C:\PROGRA~1" instead of "C:\Program Files"). + * + * See https://en.wikipedia.org/wiki/8.3_filename if you're not familiar with Windows 8.3 short names. + * + * @param shortPath The path to expand. + * @returns A normalized, absolute path, with any short components expanded. + */ +export async function expandShortPaths( + shortPath: string, + logger: BaseLogger, +): Promise { + const absoluteShortPath = normalize(resolve(shortPath)); + if (platform() !== "win32") { + // POSIX doesn't have short paths. + return absoluteShortPath; + } + + void logger.log(`Expanding short paths in: ${absoluteShortPath}`); + // A quick check to see if there might be any short components. + // There might be a case where a short component doesn't contain a `~`, but if there is, I haven't + // found it. + // This may find long components that happen to have a '~', but that's OK. + if (absoluteShortPath.indexOf("~") < 0) { + // No short components to expand. + void logger.log(`Skipping due to no short components`); + return absoluteShortPath; + } + + return await expandShortPathRecursive(absoluteShortPath, logger); +} + /** * Expand a single short path component * @param dir The absolute path of the directory containing the short path component. @@ -83,35 +115,3 @@ async function expandShortPathRecursive( const longBase = await expandShortPathComponent(dir, shortBase, logger); return join(dir, longBase); } - -/** - * Expands a path that potentially contains 8.3 short names (e.g. "C:\PROGRA~1" instead of "C:\Program Files"). - * - * See https://en.wikipedia.org/wiki/8.3_filename if you're not familiar with Windows 8.3 short names. - * - * @param shortPath The path to expand. - * @returns A normalized, absolute path, with any short components expanded. - */ -export async function expandShortPaths( - shortPath: string, - logger: BaseLogger, -): Promise { - const absoluteShortPath = normalize(resolve(shortPath)); - if (platform() !== "win32") { - // POSIX doesn't have short paths. - return absoluteShortPath; - } - - void logger.log(`Expanding short paths in: ${absoluteShortPath}`); - // A quick check to see if there might be any short components. - // There might be a case where a short component doesn't contain a `~`, but if there is, I haven't - // found it. - // This may find long components that happen to have a '~', but that's OK. - if (absoluteShortPath.indexOf("~") < 0) { - // No short components to expand. - void logger.log(`Skipping due to no short components`); - return absoluteShortPath; - } - - return await expandShortPathRecursive(absoluteShortPath, logger); -} From 65fbb6bca8cd179569acfeff3a4eed041ce1c8ed Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 17 Jan 2024 14:20:14 -0500 Subject: [PATCH 16/17] Fix PR feedback --- extensions/ql-vscode/src/codeql-cli/cli.ts | 22 ++++++--- .../src/variant-analysis/run-remote-query.ts | 20 ++++----- .../test/matchers/toExistInCodeQLPack.ts | 43 ++++++++++++++++++ .../variant-analysis-manager.test.ts | 45 ++----------------- extensions/ql-vscode/test/vscode-tests/cli.ts | 9 ++-- 5 files changed, 78 insertions(+), 61 deletions(-) create mode 100644 extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts diff --git a/extensions/ql-vscode/src/codeql-cli/cli.ts b/extensions/ql-vscode/src/codeql-cli/cli.ts index a3be391a4..5f3ebab70 100644 --- a/extensions/ql-vscode/src/codeql-cli/cli.ts +++ b/extensions/ql-vscode/src/codeql-cli/cli.ts @@ -1420,16 +1420,28 @@ export class CodeQLCliServer implements Disposable { ); } + /** + * Compile a CodeQL pack and bundle it into a single file. + * + * @param sourcePackDir The directory of the input CodeQL pack. + * @param workspaceFolders The workspace folders to search for additional packs. + * @param outputBundleFile The path to the output bundle file. + * @param outputPackDir The directory to contain the unbundled output pack. + * @param moreOptions Additional options to be passed to `codeql pack bundle`. + */ async packBundle( - dir: string, + sourcePackDir: string, workspaceFolders: string[], - outputPath: string, + outputBundleFile: string, + outputPackDir: string, moreOptions: string[], ): Promise { const args = [ "-o", - outputPath, - dir, + outputBundleFile, + sourcePackDir, + "--pack-path", + outputPackDir, ...moreOptions, ...this.getAdditionalPacksArg(workspaceFolders), ]; @@ -1492,7 +1504,7 @@ export class CodeQLCliServer implements Disposable { return (await this.getVersionAndFeatures()).features; } - public async getVersionAndFeatures(): Promise { + private async getVersionAndFeatures(): Promise { if (!this._versionAndFeatures) { try { const newVersionAndFeatures = await this.refreshVersion(); diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index 13caced7a..85a40552a 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -79,11 +79,10 @@ async function generateQueryPack( ? await askForLanguage(cliServer) // open popup to ask for language if not already hardcoded : await findLanguage(cliServer, Uri.file(queryFile)); if (!language) { - throw new UserCancellationException("Could not determine language."); + throw new UserCancellationException("Could not determine language"); } let queryPackDir: string; - let precompilationOpts: string[]; let needsInstall: boolean; if (mustSynthesizePack) { // This section applies whether or not the CLI supports MRVA pack creation directly. @@ -136,8 +135,7 @@ async function generateQueryPack( await cliServer.clearCache(); } - // Clear the CLI cache so that the most recent qlpack lock file is used. - await cliServer.clearCache(); + let precompilationOpts: string[]; if (cliSupportsMrvaPackCreate) { precompilationOpts = [ "--mrva", @@ -151,11 +149,11 @@ async function generateQueryPack( if (await cliServer.cliConstraints.usesGlobalCompilationCache()) { precompilationOpts = ["--qlx"]; } else { - const ccache = join(originalPackRoot, ".cache"); + const cache = join(originalPackRoot, ".cache"); precompilationOpts = [ "--qlx", "--no-default-compilation-cache", - `--compilation-cache=${ccache}`, + `--compilation-cache=${cache}`, ]; } @@ -168,11 +166,13 @@ async function generateQueryPack( void extLogger.log( `Compiling and bundling query pack from ${queryPackDir} to ${bundlePath}. (This may take a while.)`, ); - await cliServer.packBundle(queryPackDir, workspaceFolders, bundlePath, [ - "--pack-path", + await cliServer.packBundle( + queryPackDir, + workspaceFolders, + bundlePath, tmpDir.compiledPackDir, - ...precompilationOpts, - ]); + precompilationOpts, + ); const base64Pack = (await readFile(bundlePath)).toString("base64"); return { base64Pack, diff --git a/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts b/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts new file mode 100644 index 000000000..5021c6bdb --- /dev/null +++ b/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts @@ -0,0 +1,43 @@ +import { expect } from "@jest/globals"; +import type { ExpectationResult } from "expect"; +import type { QueryPackFS } from "../vscode-tests/utils/bundled-pack-helpers"; +import { EOL } from "os"; + +/** + * Custom Jest matcher to check if a file exists in a query pack. + */ +function toExistInPack( + this: jest.MatcherContext, + actual: unknown, + packFS: QueryPackFS, +): ExpectationResult { + if (typeof actual !== "string") { + throw new TypeError( + `Expected actual value to be a string. Found ${typeof actual}`, + ); + } + + const pass = packFS.fileExists(actual); + if (pass) { + return { + pass: true, + message: () => `expected ${actual} not to exist in pack`, + }; + } else { + const files = packFS.allFiles(); + const filesString = files.length > 0 ? files.join(EOL) : ""; + return { + pass: false, + message: () => + `expected ${actual} to exist in pack.\nThe following files were found in the pack:\n${filesString}`, + }; + } +} + +expect.extend({ toExistInPack }); + +declare module "expect" { + interface Matchers { + toExistInCodeQLPack(packFS: QueryPackFS): R; + } +} diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index ca1287ea1..ee2bc14b6 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -20,50 +20,11 @@ import { ExtensionApp } from "../../../../src/common/vscode/vscode-app"; import { DbConfigStore } from "../../../../src/databases/config/db-config-store"; import { mockedQuickPickItem } from "../../utils/mocking.helpers"; import { QueryLanguage } from "../../../../src/common/query-language"; -import type { QueryPackFS } from "../../utils/bundled-pack-helpers"; import { readBundledPack } from "../../utils/bundled-pack-helpers"; import { load } from "js-yaml"; import type { ExtensionPackMetadata } from "../../../../src/model-editor/extension-pack-metadata"; import type { QlPackLockFile } from "../../../../src/packaging/qlpack-lock-file"; import { expect } from "@jest/globals"; -import type { ExpectationResult } from "expect"; - -/** - * Custom Jest matcher to check if a file exists in a query pack. - */ -function toExistInPack( - this: jest.MatcherContext, - actual: unknown, - packFS: QueryPackFS, -): ExpectationResult { - if (typeof actual !== "string") { - throw new TypeError("Expected actual value to be a string."); - } - - const pass = packFS.fileExists(actual); - const files = packFS.allFiles(); - const filesString = files.length > 0 ? files.join("\n") : ""; - if (pass) { - return { - pass: true, - message: () => `expected ${actual} not to exist in pack`, - }; - } else { - return { - pass: false, - message: () => - `expected ${actual} to exist in pack.\nThe following files were found in the pack:\n${filesString}`, - }; - } -} - -expect.extend({ toExistInPack }); - -declare module "expect" { - interface Matchers { - toExistInPack(packFS: QueryPackFS): R; - } -} describe("Variant Analysis Manager", () => { let cli: CodeQLCliServer; @@ -371,14 +332,14 @@ describe("Variant Analysis Manager", () => { const packFS = await readBundledPack(request.query.pack); filesThatExist.forEach((file) => { - expect(file).toExistInPack(packFS); + expect(file).toExistInCodeQLPack(packFS); }); qlxFilesThatExist.forEach((file) => { - expect(file).toExistInPack(packFS); + expect(file).toExistInCodeQLPack(packFS); }); filesThatDoNotExist.forEach((file) => { - expect(file).not.toExistInPack(packFS); + expect(file).not.toExistInCodeQLPack(packFS); }); expect( diff --git a/extensions/ql-vscode/test/vscode-tests/cli.ts b/extensions/ql-vscode/test/vscode-tests/cli.ts index c1110dfe8..8ed3a9b4f 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli.ts @@ -1,3 +1,4 @@ +import { dirname } from "path"; import { workspace } from "vscode"; /** @@ -6,10 +7,10 @@ import { workspace } from "vscode"; */ function hasCodeQL() { const folders = workspace.workspaceFolders; - return !!folders?.some( - (folder) => - folder.uri.path.endsWith("/codeql") || folder.uri.path.endsWith("/ql"), - ); + return !!folders?.some((folder) => { + const name = dirname(folder.uri.fsPath); + return name === "codeql" || name === "ql"; + }); } // describeWithCodeQL will be equal to describe if the CodeQL libraries are From d7e555248140f5b50bcc0fc5295df0c03c9e69c4 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 17 Jan 2024 17:48:44 -0500 Subject: [PATCH 17/17] Fix broken test code --- .../test/matchers/toExistInCodeQLPack.ts | 29 ++++++++++++------- .../variant-analysis-manager.test.ts | 3 +- extensions/ql-vscode/test/vscode-tests/cli.ts | 4 +-- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts b/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts index 5021c6bdb..622c41200 100644 --- a/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts +++ b/extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts @@ -1,16 +1,16 @@ import { expect } from "@jest/globals"; -import type { ExpectationResult } from "expect"; +import type { MatcherFunction } from "expect"; import type { QueryPackFS } from "../vscode-tests/utils/bundled-pack-helpers"; import { EOL } from "os"; /** * Custom Jest matcher to check if a file exists in a query pack. */ -function toExistInPack( - this: jest.MatcherContext, - actual: unknown, - packFS: QueryPackFS, -): ExpectationResult { +// eslint-disable-next-line func-style -- We need to set the type of this function +const toExistInCodeQLPack: MatcherFunction<[packFS: QueryPackFS]> = function ( + actual, + packFS, +) { if (typeof actual !== "string") { throw new TypeError( `Expected actual value to be a string. Found ${typeof actual}`, @@ -32,12 +32,19 @@ function toExistInPack( `expected ${actual} to exist in pack.\nThe following files were found in the pack:\n${filesString}`, }; } -} +}; -expect.extend({ toExistInPack }); +expect.extend({ toExistInCodeQLPack }); -declare module "expect" { - interface Matchers { - toExistInCodeQLPack(packFS: QueryPackFS): R; +declare global { + // eslint-disable-next-line @typescript-eslint/no-namespace -- We need to extend this global declaration + namespace jest { + interface AsymmetricMatchers { + toExistInCodeQLPack(packFS: QueryPackFS): void; + } + + interface Matchers { + toExistInCodeQLPack(packFS: QueryPackFS): R; + } } } diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index ee2bc14b6..e3eb13d4f 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -24,7 +24,8 @@ import { readBundledPack } from "../../utils/bundled-pack-helpers"; import { load } from "js-yaml"; import type { ExtensionPackMetadata } from "../../../../src/model-editor/extension-pack-metadata"; import type { QlPackLockFile } from "../../../../src/packaging/qlpack-lock-file"; -import { expect } from "@jest/globals"; +//import { expect } from "@jest/globals"; +import "../../../matchers/toExistInCodeQLPack"; describe("Variant Analysis Manager", () => { let cli: CodeQLCliServer; diff --git a/extensions/ql-vscode/test/vscode-tests/cli.ts b/extensions/ql-vscode/test/vscode-tests/cli.ts index 8ed3a9b4f..9dca8eaf1 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli.ts @@ -1,4 +1,4 @@ -import { dirname } from "path"; +import { basename } from "path"; import { workspace } from "vscode"; /** @@ -8,7 +8,7 @@ import { workspace } from "vscode"; function hasCodeQL() { const folders = workspace.workspaceFolders; return !!folders?.some((folder) => { - const name = dirname(folder.uri.fsPath); + const name = basename(folder.uri.fsPath); return name === "codeql" || name === "ql"; }); }