Conversation
These queries live next to the C++ QL tests they check, so that `codeql test run --consistency-queries` can find them without an internal checkout. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`lint.py --format-only` gives the upcoming `just format` verb a way to reformat without failing on pre-existing lint findings. codegen.sh looked up its runfiles via `external/ql+`, which only resolves in a main-repository layout. `../ql+` works from both, so codegen keeps working when the repository is consumed as a bazel dependency. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Introduces a small set of verbs (`test`, `build`, `generate`, `format`, `lint`) that work the same from anywhere in the tree, so that contributors do not have to remember a different incantation per language. Running a verb from the root forwards it to whichever justfile actually implements it for the given paths; running it in a language directory uses that language's definition directly. Everything language-specific stays in the per-language justfiles added next; this commit only provides the vocabulary they share: - `misc/just/forward.just` and `forward_command.py` resolve a verb plus a set of paths to the justfiles that implement it, grouping paths per justfile. - `misc/just/lib.just` exposes `_codeql_test`, `_language_tests` and `_integration_test` for the per-language justfiles to build on. - `codeql_test_run.py` turns test flags into a `codeql test run` invocation, resolving `RAM_PER_THREAD`/`CPUS` from arguments, environment, then platform defaults. - `misc/just/defs.just` holds the settings and generic helpers, including the internal-checkout detection that lets the same justfiles work in both repos. Arguments are passed around as just lists (`set lists`), so values containing spaces survive intact rather than being re-split by the helpers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Each language declares its own test flags, consistency queries and build steps, so that `just test`, `just build` and friends do the right thing wherever they are run from. The flag sets are transcribed from the internal CI definitions they replace, so behaviour is unchanged. `unified` gets the same treatment as the other languages, including the consistency queries that were previously not run anywhere. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pointing a verb at a directory that merely contains implementations used to fail, so `just test cpp` was an error and reaching a suite required either naming it in full or hand-writing an aggregate recipe. When nothing at or above an argument implements the verb, look below it instead. Justfiles are enumerated with `git ls-files`, which is two orders of magnitude faster than walking a checkout with build outputs in it, and probed in parallel with `just --dump`. That dump also replaces the previous trick of recognising a forwarder by a string in its stderr: a recipe that depends on `_forward` does not implement the verb, whichever repository it lives in. Upward search still wins, so naming a recipe after a verb now decides what that verb means for the whole subtree. `unified` was doing exactly that and would have hidden its own QL tests, so its bazel entry point goes back to being called `extractor-tests`. Directories that only make sense when named explicitly say so with `explicit_verbs`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Running a whole language suite means building a CodeQL CLI and waiting a long time, which is not something `just test <anything above it>` should decide to do on the user's behalf. Integration tests and the Kotlin CI shards already opted out for the same reason; the suites themselves are the bigger case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolution used to stop at the first justfile found walking up, and only looked downwards when that found nothing. That assumed a recipe named after a verb covers everything beneath it, which is not how these are written: `rust` formats Rust sources while `rust/ql` formats QL, so `just format rust` silently skipped the QL files. Both directions are now searched and every distinct recipe runs. Recipes reached through `import` are the same job rather than a new one, so they are recognised as already covered and run once. A cross-cutting recipe placed high up therefore composes with the ones below it rather than shadowing them, which is the point: formatting Bazel files repository-wide should add to what each directory does with its own sources, not replace it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`format` pasted a `find` command substitution straight into the shell, which split the result on whitespace. Hundreds of query files live under directories such as `Best Practices`, so `just format cpp` handed the formatter a nonexistent `./src/Best` and died. Arguments given on the command line were torn apart the same way, which defeats the point of `set lists`. Collecting the files in a helper rather than with `find` also avoids two portability traps: `find` on Windows is an unrelated program, and the full file list is well past the command line length limit there, so it has to be batched. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Directories opting out of discovery were only named when a verb found nothing at all. When it did find something, `just test .` looked like it had covered the tree while quietly leaving eleven test suites alone. Report them whenever they are passed over. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`language-tests-386` was dropped when the Go justfile was ported, leaving two generated workflows invoking a recipe that no longer existed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Naming the directories a verb passed over was reported the same way as a verb matching nothing at all, on stderr and marked as an error. A run that did everything asked of it then looked like a failure. Report it as part of the account of what ran instead, and keep the error for the case where nothing matched. List one directory per line, as a verb aimed at a repository root passes over dozens, and name the invocation that failed, as by then a verb may have fanned out widely. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bazel files are identified by name rather than extension, and the tree of checked-in generated ones has to stay out of any sweep over them. Matching globs against the file name covers both the old extensions and those names, and exclusions keep the generated files out. Absolute names are an option because a command run through `bazel run` starts in the runfiles directory, where a relative name means nothing. Splitting on the last `--` lets such a command carry one of its own. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A repository root forwards every verb, and a recipe written beside the import replaces the imported one, so it had no name left to implement a verb under: adding `format` to the root broke `just format cpp` outright. Some work belongs to no single directory though, and the root is where it should live. Such a justfile now spells its own implementation `_root_<verb>`, which the forwarder looks for whenever the plain name turns out to be the forwarder's. Taking an argument, it composes with what is found below rather than shadowing it, so a verb aimed at a subdirectory still reaches only that subdirectory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
These are spread across the whole tree rather than gathered under a language, so they are the root's to format, and `_root_format` keeps a run aimed at a subdirectory to the bazel files under it. The buildifier bazel target cannot be driven directly: the wrapper it generates ignores the paths given to it and always sweeps the workspace. Running the binary instead means supplying the exclusion of the checked-in generated files ourselves, which buildifier has no flag for. Being a dev dependency, the target only resolves in a build rooted here; inside the internal repository its own buildifier target covers these files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`codeql query format` will only name the files it rewrites if it also names every file it leaves alone, so asking which files changed meant thousands of lines to find them in, and bazel was similarly talkative about building the formatter it was about to run. The file runner can now be told which lines of a command's output to hide, so the formatter is asked for everything and the lines about untouched files are dropped. It is a denylist rather than a pick of what to keep, so errors and anything unforeseen still come through, and the command's exit code is passed on unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Building the internal distribution printed its whole log every time, so any command that needed one first said several dozen lines about unzipping a JDK before saying the one thing it was asked to say. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Guarding this to standalone checkouts made it useless, as that is not where the work happens. It was guarded because the target here is a bazel dev dependency, unreachable from a build rooted in the internal repository; but both repositories depend on the buildifier binary, each as the root module of its own checkout, so asking for it directly resolves either way. What differs is which bazel to ask and from where. The internal workspace encloses this one, and this one is itself a bazel module, so a nested invocation would take the enclosing checkout for something it is not; the file runner can now be told which directory to run from, which is also what its absolute file names were already for. Rewritten files are now named, as the QL formatter does, leaving out the accounting buildifier gives for those it did not rewrite. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Needing a CodeQL CLI is not a reason to hide a suite, as every QL recipe here needs one and the rest stay discoverable. Being slow is the whole of it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A file named `BUILD.<something>` that is not `BUILD.bazel` is not a bazel file: bazel knows `BUILD` and `WORKSPACE` by name and the rest by extension, so `BUILD.windows.tpl` and its kind are templates, holding placeholders that no formatter can parse. Matching them failed every format whose scope contained one, which the internal repository has and this one does not. Formatting now also asks bazel from the root of the checkout the files belong to, rather than from the enclosing one when there is one. The buildifier behind it is a dependency of whichever checkout is the root, so which one asks decides which version formats, and the files of a repository are best formatted by the version it pins and skipped by the list of generated files it keeps. Building goes the other way, as a target there needs the enclosing workspace to resolve at all, so the two no longer share a helper. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The exclusions are one justfile variable, and a root defining its own bazel formatting has more than one directory its generators write to. Reading them the way the file name patterns are already read costs nothing and saves spelling the option twice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A command spanning a line break left the sentence for it to rewrap, so formatting the directory always came back with a change, and the rewrap broke out of the list it was in. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Asking bazel from this repository's root was half of formatting its own files and not another's: the paths came from the verb rather than from the root, so a checkout enclosing this one had its files formatted here, by whichever buildifier version this repository happens to pin. The two are not interchangeable, differing in the fixes they apply, so files came out formatted by a version other than the one their own repository would use on them. Bounding the files to the root leaves each repository formatting what it owns, and a verb spanning both is answered once by each, every root implementing the verb for itself. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…it is on A single argument is capped far below the whole command line, at 128KB against 2MB on Linux, and a command that passes its arguments on through a shell arrives as one of them. Sizing batches by the line alone let a large enough tree build one argument over that cap, which fails as an `execv` error from whatever did the handing on, naming neither this file nor the files it was given. Also says how an exclusion is matched, as it is against the path the walk built rather than the one on the command line, and the natural way to name a directory only matches when the walk starts above it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Forwarding, Rust generation, formatting paths, and Kotlin environment handling contain unresolved functional issues.
Get a fresh assessment by requesting another Copilot review.
Review tier: Balanced
Findings: 2
Open (5)
Resolve relative arguments against the working directory · New Do not pass a directory to the zero-argument generate recipe · New Pass an actually empty Kotlin diagnostic limit · New Preserve forwarded paths in the Prettier recipe · New Do not pass a directory to the zero-argument generate recipe · New
What changed in this PR
Introduces shared just verbs and forwarding infrastructure for consistent language-specific build, test, format, lint, and generation workflows.
Changes:
- Adds verb forwarding, formatting, build, and CodeQL test helpers.
- Adds language and component-level recipes.
- Adds C++ consistency queries and supporting Rust workflow updates.
| File | Description |
|---|---|
unified/swift-syntax-rs/justfile |
Adds Bazel tests. |
unified/ql/test/justfile |
Configures QL tests. |
unified/ql/justfile |
Adds QL formatting. |
unified/justfile |
Adds build and test recipes. |
unified/extractor/justfile |
Adds extractor tests. |
swift/ql/test/justfile |
Configures QL tests. |
swift/ql/justfile |
Adds QL formatting. |
swift/ql/integration-tests/justfile |
Adds integration tests. |
swift/justfile |
Adds Swift verbs. |
rust/ql/test/justfile |
Configures QL tests. |
rust/ql/justfile |
Adds QL formatting. |
rust/ql/integration-tests/justfile |
Adds integration tests. |
rust/lint.py |
Separates formatting from linting. |
rust/justfile |
Adds Rust verbs. |
rust/codegen/codegen.sh |
Updates runfiles lookup. |
ruby/ql/test/justfile |
Configures QL tests. |
ruby/ql/justfile |
Adds QL formatting. |
ruby/ql/integration-tests/justfile |
Adds integration tests. |
ruby/justfile |
Adds Ruby verbs. |
python/ql/test/justfile |
Configures QL tests. |
python/ql/justfile |
Defines Python test environments. |
python/ql/integration-tests/justfile |
Adds integration tests. |
python/justfile |
Adds versioned test suites. |
misc/just/semmle-code-stub.just |
Stubs standalone configuration. |
misc/just/run_on_files.py |
Adds portable batched file execution. |
misc/just/README.md |
Documents verb forwarding. |
misc/just/lib.just |
Defines shared test helpers. |
misc/just/language_tests.py |
Launches complete language suites. |
misc/just/justfile |
Adds infrastructure formatting. |
misc/just/forward.just |
Defines forwarded verbs and aliases. |
misc/just/forward_command.py |
Implements recipe discovery and dispatch. |
misc/just/format.just |
Defines shared formatters. |
misc/just/defs.just |
Defines common settings and helpers. |
misc/just/codeql_test_run.py |
Configures CodeQL test execution. |
misc/just/build.just |
Defines shared build helpers. |
misc/codegen/justfile |
Adds codegen tests and formatting. |
misc/bazel/buildifier/BUILD.bazel |
Exposes the buildifier binary. |
lib.just |
Exposes shared infrastructure. |
justfile |
Adds repository-level forwarding. |
javascript/ql/test/justfile |
Configures QL tests. |
javascript/ql/justfile |
Adds QL formatting. |
javascript/ql/integration-tests/justfile |
Adds integration tests. |
javascript/justfile |
Adds JavaScript verbs. |
java/ql/test/justfile |
Configures Java QL tests. |
java/ql/test-kotlin2/justfile |
Configures Kotlin 2 tests. |
java/ql/test-kotlin1/justfile |
Configures Kotlin 1 tests. |
java/ql/justfile |
Adds QL formatting. |
java/ql/integration-tests/justfile |
Adds integration tests. |
java/justfile |
Adds Java builds. |
go/ql/test/justfile |
Configures QL tests. |
go/ql/justfile |
Adds QL formatting. |
go/ql/integration-tests/justfile |
Adds integration tests. |
go/justfile |
Adds Go test variants. |
csharp/ql/test/justfile |
Configures QL tests. |
csharp/ql/justfile |
Adds QL formatting. |
csharp/ql/integration-tests/justfile |
Adds integration tests. |
csharp/justfile |
Adds C# verbs. |
cpp/ql/test/justfile |
Configures QL tests. |
cpp/ql/justfile |
Adds QL formatting. |
cpp/ql/integration-tests/justfile |
Adds integration tests. |
cpp/ql/consistency-queries/variablesWithoutTypes.ql |
Checks variable types. |
cpp/ql/consistency-queries/variableDeclarationsWithoutTypes.ql |
Checks declaration types. |
cpp/ql/consistency-queries/unusedLocations.ql |
Checks unused locations. |
cpp/ql/consistency-queries/qlpack.yml |
Defines the query pack. |
cpp/ql/consistency-queries/nullInToString.ql |
Checks null string representations. |
cpp/ql/consistency-queries/badLocations.ql |
Checks malformed locations. |
cpp/justfile |
Adds C++ verbs. |
actions/ql/test/justfile |
Configures QL tests. |
actions/ql/justfile |
Adds QL formatting. |
actions/ql/integration-tests/justfile |
Adds integration tests. |
actions/justfile |
Adds Actions verbs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The upward search walked `Path(arg).parents`, which is empty for the default argument `.`, so a verb reached through just's fallback from a nested directory saw only what was below it. `just format` inside a language directory silently skipped the root's bazel formatting. Resolving the argument first makes `format .` from a directory agree with naming that directory from the root, which is what already happened for an absolute argument. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Naming the directory sent the forwarder looking for a `generate` that takes one, and rust's takes none, so the integration tests stopped before they started. Without the argument, just's fallback reaches that recipe directly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The space was a workaround for the old encoding, where a value that was set but empty did not survive being split out of a whitespace-separated blob. Lists make the intent writable, and the two Kotlin shard suites already spell it this way. The extractor reads the limit with `toIntOrNull`, so neither spelling ever parsed; this is about saying what was meant. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The recipe took the argument but also let just change directory into its own, so a file named below it was looked for twice over. Interpolating the argument raw split paths containing spaces as well. The shared formatters already avoid both. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`[no-cd]` is load-bearing and does not look it: the forwarder reaches a recipe above its argument with `--justfile`, which otherwise runs it from that justfile's directory, so the default `.` would quietly mean the whole repository. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
Author
Rerun has been triggered: 1 restarted 🚀 |
The shape is easy to extend by adding a body, which is where a relative argument stops meaning the caller's directory. Cheaper to say so where the pattern is taught than to leave the next one to find out. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CI requires every pack in this repository to set it, and the ten other consistency-queries packs already do. The internal repository does not check this, so the pack arrived without it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The comment claimed the recipe skipped what `//misc/bazel/buildifier` skips, but that target also excludes `.git`, and this did not. A branch name is a file, so `just format .` in an ordinary clone could hand a ref called `WORKSPACE` or anything `.bzl` to buildifier in fix mode. It does not bite in a worktree, where `.git` is a file rather than a directory, which is why it went unnoticed. Run the binary through the alias that exists for it too, so both entry points name a target in the same file. The two exclusion lists cannot be collapsed: the canonical target formats the whole workspace and so cannot take a path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…n detail `set allow-duplicate-variables` lets a consuming root replace any of them, and `just` cannot warn about an assignment that no longer overrides anything: renaming one leaves the root parsing, listing and passing CI while silently falling back to the value here, losing only the variable that moved. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The underscore that keeps these variables out of `just --list` keeps them out of `--variables` and `--evaluate` too, so the only introspection that could reveal an override that no longer overrides anything does not show them. Asked by name they answer, which is the check worth reaching for, with the caveat that it sees a rename rather than a change of meaning. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or 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
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.


This introduces common "verbs" (
build,test,format,lint,generate) that individual parts of the project can implement, and the shared infrastructure inmisc/just/behind them.This supersedes #19978, which had accumulated too much history to review. Same content, recreated as a readable commit series on top of
main, plus the fixes listed below.Forwarding
A verb is spelled the same everywhere, but what it means is defined per language, next to the code it acts on. The forwarder finds the justfiles implementing a verb for each of its arguments, so there is no central list of who implements what.
Justfiles are looked for in both directions from an argument:
Every distinct recipe found this way runs. Recipes are compared by value, so one reached through
importis recognised as the same job and runs once, while a cross-cutting recipe higher up composes with the more specific ones below instead of hiding them. Arguments are grouped by the recipe they resolve to, sojust build rust javaworks.Two things keep the search useful:
explicit_verbsto stay out of reach of a verb aimed at one of its parents. QL test suites use this:just test .no longer sweeps whole language suites, which take a long time. A verb that passes over such a directory says so and names it._root_<verb>. That is how the root formats bazel files, which belong to no single language, whilejust format cppstays withincpp.Running QL tests
codeqlstandalone--all-checks, abbreviated+, adds the extra checks CI runs, configured per language--codeql=builtskips the build step, consistent with the same pytest optionWhat changed since #19978
The reason for recreating rather than squashing:
set lists(casey/just#1988, now fixed) replaces the whitespace-separated string blobs that the old version used to encode recipe arguments, along with the re-splitting in the Python helpers. That encoding could not carry an argument faithfully: one containing a space was silently split in two, and a value that was set but empty could not be written at all, which is why the Kotlin tests spelledCODEQL_EXTRACTOR_KOTLIN_DIAGNOSTIC_LIMITwith a literal space, to leave something for the split to find.Bugs fixed along the way:
go language-tests-386is restoredRAM_PER_THREADand the nolang search path are passed throughCaveats
When one verb runs several recipes, any non-positional argument has to be understood by all of them. This works for options like
--learnor--codeql, which the shared test definitions all accept.Companion PR
The internal repo companion PR ports all languages to this infrastructure and updates CI accordingly.