Contextual queries: Support running when the library pack is in the package cache #1735
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ache If the library pack containing the AST query does not have a lock file, it is likely to be in the package cache, not a checkout of the CodeQL repo. In this case, use `codeql pack resolve-dependencies` to create a temporary lock file, and `codeql pack install` to install the dependencies of this library pack. This allows the CLI to resolve the library path and dependencies for the AST query before running it.
aeisenberg
reviewed
Nov 8, 2022
| const packContents = await this.cli.packPacklist(query, false); | ||
| const packFilePath = packContents.find((p) => ['codeql-pack.yml', 'qlpack.yml'].includes(path.basename(p))); | ||
| if (packFilePath === undefined) { | ||
| // Should not happen; we already resolved this query. |
There was a problem hiding this comment.
Is this always true? What happens if you try to run a query in a directory that's outside of a qlpack?
There was a problem hiding this comment.
Not generally true, but I hope it is always true for our own contextual queries.
Clear the CLI server's pack cache before installing packs, to avoid race conditions where the new lock file is not detected during query running. Adjust some helper methods.
Shared by the AST viewer, jump to def, and find references contextual queries. This allows contextual queries to have their dependencies resolved and be run whether the library pack is in the workspace or in the package cache.
|
Thanks! Addressed comments and refactored to cover AST viewer, find references, and jump to definition. |
adityasharad
changed the title
aeisenberg
reviewed
Nov 9, 2022
| const tempLockFilePath = path.resolve(packPath, 'codeql-pack.lock.yml'); | ||
| void logger.log(`Deleting temporary package lock file at ${tempLockFilePath}`); | ||
| // It's fine if the file doesn't exist. | ||
| await fs.promises.rm(path.resolve(packPath, 'codeql-pack.lock.yml'), { force: true }); |
There was a problem hiding this comment.
Minor: this should work.
Suggested change
| await fs.promises.rm(path.resolve(packPath, 'codeql-pack.lock.yml'), { force: true }); | |
| await fs.rm(path.resolve(packPath, 'codeql-pack.lock.yml'), { force: true }); |
There was a problem hiding this comment.
The compiler is not happy with this, and for some reason I don't see an async version of rm in the installed type definitions. Going to keep it for now but happy to replace later if we get it working.
aeisenberg
approved these changes
Nov 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the library pack containing the AST query does not have a lock file, it is likely to be in the package cache, not
a checkout of the CodeQL repo.
In this case, use
codeql pack resolve-dependenciesto create a temporary lock file, andcodeql pack installto install the dependencies of this library pack.
This allows the CLI to resolve the library path and dependencies for the AST query before running it, in the use case where the standard library packs are installed in the package cache rather than present in the workspace. Otherwise, in the absence of a lock file,
codeql resolve library-pathfinds the standard library pack that contains the AST query, but not thecodeql/ssashared pack that this library pack depends on.Reviewer notes
codeqlrepo is present as a workspace folder, containing library packs with lockfiles, and the AST queries are obtained from there.Checklist
ready-for-doc-reviewlabel there.