Allow synthetic variant analysis packs to handle ${workspace}
#1774
Conversation
Also, we're no longer using the rush package manager, so delete that from gitignore.
|
Of course... |
aeisenberg
force-pushed
the
aeisenberg/fix-mrva-packs
branch
from
9129e05 to
647a718
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
...ons/ql-vscode/src/vscode-tests/cli-integration/remote-queries/remote-queries-manager.test.ts
Fixed
Show fixed
Hide fixed
aeisenberg
force-pushed
the
aeisenberg/fix-mrva-packs
branch
7 times, most recently
from
8de6ae0 to
0f66afe
Compare
|
I think I found out why the CLI tests are failing locally. I made a change to https://github.com/aeisenberg/vscode-codeql/blob/aeisenberg/fix-mrva-packs/extensions/ql-vscode/src/vscode-tests/test-config.ts#L110 to add a timeout. Also, the |
|
@angelapwen would you mind taking another look? I think the tests are passing now. I had to do a bit of magic to get the tests working. I'll explain better in the PR description. |
| codeql/javascript-all: '*' | ||
| codeql/javascript-all: '${workspace}' |
There was a problem hiding this comment.
I don't know these tests well, but do you think you could add a comment here to explain that the '${workspace}' bit is important to ensure tests pass? Not sure that it's clear from the name of the test/directory.
| @@ -122,3 +125,34 @@ export async function cleanDatabases(databaseManager: DatabaseManager) { | |||
| await commands.executeCommand("codeQLDatabases.removeDatabase", item); | |||
| } | |||
| } | |||
|
|
|||
| export async function fixWorkspaceReferences( | |||
There was a problem hiding this comment.
Also wonder if you could add a comment here to describe why someone would use this
| return undefined; | ||
| } | ||
|
|
||
| export async function restoreWorkspaceReferences( |
There was a problem hiding this comment.
Same as above (comment description)
`${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.
aeisenberg
force-pushed
the
aeisenberg/fix-mrva-packs
branch
from
0f66afe to
24bbd51
Compare
${workspace}references are new in CLI version 2.11.3. These mean thatthe 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.I wanted to ensure the tests exercise this feature, so I altered one of our test qlpacks to use the
${workspace}reference. This will work on CLI v2.11.3 and later. For earlier versions, I needed to change that reference to a*reference.I also fixed a problem I was having locally where the CLI tests were timing out. By bumping the timeout to 20s in the
testConfigHelper, I got around that error.Checklist
ready-for-doc-reviewlabel there.