Skip to content

Commit 24bbd51

Browse files
committed
Allow synthetic variant analysis packs to handle ${workspace}
`${workspace}` references are new in CLI version 2.11.3. These mean that the version depended upon in a pack must be the version available in the current codeql workspace. When generating a variant analysis pack, however, we copy the target query and generate a synthetic pack with the original dependencies. This breaks workspace references since the synthetic pack is no longer in the same workspace. A simple workaround is to replace `${workspace}` with `*` references.
1 parent 4eb4652 commit 24bbd51

File tree

8 files changed

+153
-22
lines changed

8 files changed

+153
-22
lines changed

extensions/ql-vscode/src/cli.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,6 +1605,11 @@ export class CliVersionConstraint {
16051605
*/
16061606
public static CLI_VERSION_WITH_NEW_QUERY_SERVER = new SemVer("2.11.1");
16071607

1608+
/**
1609+
* CLI version that supports `${workspace}` references in qlpack.yml files.
1610+
*/
1611+
public static CLI_VERSION_WITH_WORKSPACE_RFERENCES = new SemVer("2.11.3");
1612+
16081613
constructor(private readonly cli: CodeQLCliServer) {
16091614
/**/
16101615
}
@@ -1734,4 +1739,10 @@ export class CliVersionConstraint {
17341739
CliVersionConstraint.CLI_VERSION_WITH_NEW_QUERY_SERVER,
17351740
);
17361741
}
1742+
1743+
async supportsWorkspaceReferences() {
1744+
return this.isVersionAtLeast(
1745+
CliVersionConstraint.CLI_VERSION_WITH_WORKSPACE_RFERENCES,
1746+
);
1747+
}
17371748
}

extensions/ql-vscode/src/remote-queries/run-remote-query.ts

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
import { ProgressCallback, UserCancellationException } from "../commandRunner";
2222
import { RequestError } from "@octokit/types/dist-types";
2323
import { QueryMetadata } from "../pure/interface-types";
24-
import { REPO_REGEX } from "../pure/helpers-pure";
24+
import { getErrorMessage, REPO_REGEX } from "../pure/helpers-pure";
2525
import * as ghApiClient from "./gh-api/gh-api-client";
2626
import {
2727
getRepositorySelection,
@@ -99,6 +99,8 @@ async function generateQueryPack(
9999

100100
void logger.log(`Copied ${copiedCount} files to ${queryPackDir}`);
101101

102+
await fixPackFile(queryPackDir, packRelativePath);
103+
102104
language = await findLanguage(cliServer, Uri.file(targetQueryFileName));
103105
} else {
104106
// open popup to ask for language if not already hardcoded
@@ -115,6 +117,7 @@ async function generateQueryPack(
115117
dependencies: {
116118
[`codeql/${language}-all`]: "*",
117119
},
120+
defaultSuite: generateDefaultSuite(packRelativePath),
118121
};
119122
await fs.writeFile(
120123
path.join(queryPackDir, "qlpack.yml"),
@@ -125,8 +128,6 @@ async function generateQueryPack(
125128
throw new UserCancellationException("Could not determine language.");
126129
}
127130

128-
await ensureNameAndSuite(queryPackDir, packRelativePath);
129-
130131
// Clear the cliServer cache so that the previous qlpack text is purged from the CLI.
131132
await cliServer.clearCache();
132133

@@ -298,33 +299,48 @@ export async function prepareRemoteQueryRun(
298299
}
299300

300301
/**
301-
* Updates the default suite of the query pack. This is used to ensure
302-
* only the specified query is run.
302+
* Fixes the qlpack.yml file to be correct in the context of the MRVA request.
303303
*
304-
* Also, ensure the query pack name is set to the name expected by the server.
304+
* Performs the following fixes:
305+
*
306+
* - Updates the default suite of the query pack. This is used to ensure
307+
* only the specified query is run.
308+
* - Ensures the query pack name is set to the name expected by the server.
309+
* - Removes any `${workspace}` version references from the qlpack.yml file. Converts them
310+
* to `*` versions.
305311
*
306312
* @param queryPackDir The directory containing the query pack
307313
* @param packRelativePath The relative path to the query pack from the root of the query pack
308314
*/
309-
async function ensureNameAndSuite(
315+
async function fixPackFile(
310316
queryPackDir: string,
311317
packRelativePath: string,
312318
): Promise<void> {
313319
const packPath = path.join(queryPackDir, "qlpack.yml");
314320
const qlpack = yaml.load(await fs.readFile(packPath, "utf8")) as QlPack;
315-
delete qlpack.defaultSuiteFile;
316321

322+
// update pack name
317323
qlpack.name = QUERY_PACK_NAME;
318324

319-
qlpack.defaultSuite = [
325+
// update default suite
326+
delete qlpack.defaultSuiteFile;
327+
qlpack.defaultSuite = generateDefaultSuite(packRelativePath);
328+
329+
// remove any ${workspace} version references
330+
removeWorkspaceRefs(qlpack);
331+
332+
await fs.writeFile(packPath, yaml.dump(qlpack));
333+
}
334+
335+
function generateDefaultSuite(packRelativePath: string) {
336+
return [
320337
{
321338
description: "Query suite for variant analysis",
322339
},
323340
{
324341
query: packRelativePath.replace(/\\/g, "/"),
325342
},
326343
];
327-
await fs.writeFile(packPath, yaml.dump(qlpack));
328344
}
329345

330346
export function getQueryName(
@@ -385,13 +401,22 @@ export async function getControllerRepo(
385401
fullName: controllerRepo.full_name,
386402
private: controllerRepo.private,
387403
};
388-
} catch (e: any) {
404+
} catch (e) {
389405
if ((e as RequestError).status === 404) {
390406
throw new Error(`Controller repository "${owner}/${repo}" not found`);
391407
} else {
392408
throw new Error(
393-
`Error getting controller repository "${owner}/${repo}": ${e.message}`,
409+
`Error getting controller repository "${owner}/${repo}": ${getErrorMessage(
410+
e,
411+
)}`,
394412
);
395413
}
396414
}
397415
}
416+
export function removeWorkspaceRefs(qlpack: QlPack) {
417+
for (const [key, value] of Object.entries(qlpack.dependencies || {})) {
418+
if (value === "${workspace}") {
419+
qlpack.dependencies[key] = "*";
420+
}
421+
}
422+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
name: github/remote-query-pack
22
version: 0.0.0
33
dependencies:
4+
# The workspace reference will be removed before creating the MRVA pack.
45
codeql/javascript-all: '*'
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
name: github/remote-query-pack
22
version: 0.0.0
33
dependencies:
4-
codeql/javascript-all: '*'
4+
codeql/javascript-all: '${workspace}'

extensions/ql-vscode/src/vscode-tests/cli-integration/global.helper.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as path from "path";
22
import * as tmp from "tmp";
3+
import * as yaml from "js-yaml";
34
import * as fs from "fs-extra";
45
import fetch from "node-fetch";
56

@@ -9,6 +10,8 @@ import { CodeQLExtensionInterface } from "../../extension";
910
import { DatabaseManager } from "../../databases";
1011
import { getTestSetting } from "../test-config";
1112
import { CUSTOM_CODEQL_PATH_SETTING } from "../../config";
13+
import { CodeQLCliServer } from "../../cli";
14+
import { removeWorkspaceRefs } from "../../remote-queries/run-remote-query";
1215

1316
// This file contains helpers shared between actual tests.
1417

@@ -122,3 +125,52 @@ export async function cleanDatabases(databaseManager: DatabaseManager) {
122125
await commands.executeCommand("codeQLDatabases.removeDatabase", item);
123126
}
124127
}
128+
129+
/**
130+
* Conditionally removes `${workspace}` references from a qlpack.yml file.
131+
* CLI versions earlier than 2.11.3 do not support `${workspace}` references in the dependencies block.
132+
* If workspace references are removed, the qlpack.yml file is re-written to disk
133+
* without the `${workspace}` references and the original dependencies are returned.
134+
*
135+
* @param qlpackFileWithWorkspaceRefs The qlpack.yml file with workspace refs
136+
* @param cli The cli to use to check version constraints
137+
* @returns The original dependencies with workspace refs, or undefined if the CLI version supports workspace refs and no changes were made
138+
*/
139+
export async function fixWorkspaceReferences(
140+
qlpackFileWithWorkspaceRefs: string,
141+
cli: CodeQLCliServer,
142+
): Promise<Record<string, string> | undefined> {
143+
if (!(await cli.cliConstraints.supportsWorkspaceReferences())) {
144+
// remove the workspace references from the qlpack
145+
const qlpack = yaml.load(
146+
fs.readFileSync(qlpackFileWithWorkspaceRefs, "utf8"),
147+
);
148+
const originalDeps = { ...qlpack.dependencies };
149+
removeWorkspaceRefs(qlpack);
150+
fs.writeFileSync(qlpackFileWithWorkspaceRefs, yaml.dump(qlpack));
151+
return originalDeps;
152+
}
153+
return undefined;
154+
}
155+
156+
/**
157+
* Restores the original dependencies with `${workspace}` refs to a qlpack.yml file.
158+
* See `fixWorkspaceReferences` for more details.
159+
*
160+
* @param qlpackFileWithWorkspaceRefs The qlpack.yml file to restore workspace refs
161+
* @param originalDeps the original dependencies with workspace refs or undefined if
162+
* no changes were made.
163+
*/
164+
export async function restoreWorkspaceReferences(
165+
qlpackFileWithWorkspaceRefs: string,
166+
originalDeps?: Record<string, string>,
167+
) {
168+
if (!originalDeps) {
169+
return;
170+
}
171+
const qlpack = yaml.load(
172+
fs.readFileSync(qlpackFileWithWorkspaceRefs, "utf8"),
173+
);
174+
qlpack.dependencies = originalDeps;
175+
fs.writeFileSync(qlpackFileWithWorkspaceRefs, yaml.dump(qlpack));
176+
}

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/remote-queries-manager.test.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,21 @@ import { RemoteQueriesSubmission } from "../../../remote-queries/shared/remote-q
3030
import { readBundledPack } from "../../utils/bundled-pack-helpers";
3131
import { RemoteQueriesManager } from "../../../remote-queries/remote-queries-manager";
3232
import { Credentials } from "../../../authentication";
33+
import {
34+
fixWorkspaceReferences,
35+
restoreWorkspaceReferences,
36+
} from "../global.helper";
3337

3438
describe("Remote queries", function () {
3539
const baseDir = path.join(
3640
__dirname,
3741
"../../../../src/vscode-tests/cli-integration",
3842
);
3943

44+
const qlpackFileWithWorkspaceRefs = getFile(
45+
"data-remote-qlpack/qlpack.yml",
46+
).fsPath;
47+
4048
let sandbox: sinon.SinonSandbox;
4149

4250
// up to 3 minutes per test
@@ -51,6 +59,8 @@ describe("Remote queries", function () {
5159
let logger: any;
5260
let remoteQueriesManager: RemoteQueriesManager;
5361

62+
let originalDeps: Record<string, string> | undefined;
63+
5464
// use `function` so we have access to `this`
5565
beforeEach(async function () {
5666
sandbox = sinon.createSandbox();
@@ -69,13 +79,6 @@ describe("Remote queries", function () {
6979
}
7080

7181
ctx = createMockExtensionContext();
72-
logger = new OutputChannelLogger("test-logger");
73-
remoteQueriesManager = new RemoteQueriesManager(
74-
ctx,
75-
cli,
76-
"fake-storage-dir",
77-
logger,
78-
);
7982

8083
if (!(await cli.cliConstraints.supportsRemoteQueries())) {
8184
console.log(
@@ -84,6 +87,14 @@ describe("Remote queries", function () {
8487
this.skip();
8588
}
8689

90+
logger = new OutputChannelLogger("test-logger");
91+
remoteQueriesManager = new RemoteQueriesManager(
92+
ctx,
93+
cli,
94+
"fake-storage-dir",
95+
logger,
96+
);
97+
8798
cancellationTokenSource = new CancellationTokenSource();
8899

89100
progress = sandbox.spy();
@@ -120,10 +131,17 @@ describe("Remote queries", function () {
120131
}),
121132
} as unknown as Credentials;
122133
sandbox.stub(Credentials, "initialize").resolves(mockCredentials);
134+
135+
// Only new version support `${workspace}` in qlpack.yml
136+
originalDeps = await fixWorkspaceReferences(
137+
qlpackFileWithWorkspaceRefs,
138+
cli,
139+
);
123140
});
124141

125142
afterEach(async () => {
126143
sandbox.restore();
144+
await restoreWorkspaceReferences(qlpackFileWithWorkspaceRefs, originalDeps);
127145
});
128146

129147
describe("runRemoteQuery", () => {

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ import * as path from "path";
2424

2525
import { VariantAnalysisManager } from "../../../remote-queries/variant-analysis-manager";
2626
import { CliVersionConstraint, CodeQLCliServer } from "../../../cli";
27-
import { storagePath } from "../global.helper";
27+
import {
28+
fixWorkspaceReferences,
29+
restoreWorkspaceReferences,
30+
storagePath,
31+
} from "../global.helper";
2832
import { VariantAnalysisResultsManager } from "../../../remote-queries/variant-analysis-results-manager";
2933
import { createMockVariantAnalysis } from "../../factories/remote-queries/shared/variant-analysis";
3034
import * as VariantAnalysisModule from "../../../remote-queries/shared/variant-analysis";
@@ -66,6 +70,7 @@ describe("Variant Analysis Manager", async function () {
6670
let getVariantAnalysisRepoStub: sinon.SinonStub;
6771
let getVariantAnalysisRepoResultStub: sinon.SinonStub;
6872
let variantAnalysisResultsManager: VariantAnalysisResultsManager;
73+
let originalDeps: Record<string, string> | undefined;
6974

7075
beforeEach(async () => {
7176
sandbox = sinon.createSandbox();
@@ -126,6 +131,10 @@ describe("Variant Analysis Manager", async function () {
126131
__dirname,
127132
"../../../../src/vscode-tests/cli-integration",
128133
);
134+
const qlpackFileWithWorkspaceRefs = getFile(
135+
"data-remote-qlpack/qlpack.yml",
136+
).fsPath;
137+
129138
function getFile(file: string): Uri {
130139
return Uri.file(path.join(baseDir, file));
131140
}
@@ -177,6 +186,19 @@ describe("Variant Analysis Manager", async function () {
177186
await setRemoteRepositoryLists({
178187
"vscode-codeql": ["github/vscode-codeql"],
179188
});
189+
190+
// Only new version support `${workspace}` in qlpack.yml
191+
originalDeps = await fixWorkspaceReferences(
192+
qlpackFileWithWorkspaceRefs,
193+
cli,
194+
);
195+
});
196+
197+
afterEach(async () => {
198+
await restoreWorkspaceReferences(
199+
qlpackFileWithWorkspaceRefs,
200+
originalDeps,
201+
);
180202
});
181203

182204
it("should run a variant analysis that is part of a qlpack", async () => {

extensions/ql-vscode/src/vscode-tests/test-config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ export const getTestSetting = (
106106
};
107107

108108
export const testConfigHelper = async (mocha: Mocha) => {
109+
// Allow extra time to read settings. Sometimes this can time out.
110+
mocha.timeout(20000);
111+
109112
// Read in all current settings
110113
await Promise.all(TEST_SETTINGS.map((setting) => setting.initialSetup()));
111-
112114
mocha.rootHooks({
113115
async beforeEach() {
114116
// Reset the settings to their initial values before each test

0 commit comments

Comments
 (0)