Skip to content

Just: introduce common "verbs" #19978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Just: introduce common "verbs" #19978

wants to merge 25 commits into from

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Jul 4, 2025

This introduces verbs (build, test, format, lint, generate) that individual parts of the project can implement, and some common functionality that can be used to that effect.

The core of the functionality is given by forwarding. The idea is that:

  • if you are in the directory where a verb is implemented, you will get that as per standard just behaviour (possibly using fallback).
  • if on the other hand you are beneath it, and you run something like just test ql/rust/ql/test/{a,b}, then a forwarder script finds a common justfile to all the positional arguments passed there, and then retries calling just test from there. If test is implemented beneath that (in that case, it is in rust/ql/test), it uses that recipe.
  • if that doesn't work, the forwarder will still try to see if the recipe can be run on the targets separately. So just build ql/rust ql/java, or just test ql/rust/ql/test/some/test ql/rust/ql/integration-test/some/other/test will also work, with recipes run sequentially.

Another point is how launching QL tests can be tweaked:

  • by default, the corresponding CLI is built from the internal repo (nothing is done if working in codeql standalone), and no additional database or consistency checks are made
  • --codeql=built can be passed to skip the build step (if no changes were made to the CLI/extractors). This is consistent with the same pytest option
  • you can add the additional checks that CI does with --all-checks or the + abbreviation. These additional checks are configured in justfiles per language.

Some caveats:

  • passing arguments with spaces generally doesn't work, although setting arguments with spaces in justfiles (for the base arguments) is supported using escaping as in \\
  • when running different recipes for the same verb, non-positional arguments need to be supported by all recipes involved. For example this will work ok for --learn or --codeql options in language and integration tests

While I made the effort to use typescript for the supporting scripts like is the case for the current just things we use, I'm reconsidering whether that is a good idea:

  • there is a short delay while the script is compiled before startup. It is quite short, but still noticeable, and the whole forwarding architecture doesn't help. This delay becomes really annoying on Windows
  • the standard node library seems a bit lacking with respect to what is available in vanilla python (and we can't really use third party packages
  • for example, python offers out of the box nice argument parsing, os.path.commonpath (that I had to implement in TS), and shlex.split which solves the space problems more elegantly and robustly
  • on the other hand, making sure the correct python interpreter is used might be tricky (while for TS we just to npx tsx@... which is pretty light-weight). On the other hand, we do something like that for pytest already (using uv).

But this can be discussed later.

@github-actions github-actions bot added Rust Pull requests that update Rust code Swift labels Jul 4, 2025
@github-actions github-actions bot added the Java label Jul 7, 2025
redsun82 added 13 commits July 8, 2025 10:52
This allows to mix different verb implementations in a single
invocation, for example:
```
just build ql/rust ql/java
just test ql/rust/ql/test/some/test ql/rust/ql/integrartion-test/some other
```
If a common justfile recipe is found, it is used for all arguments in
one go. If on the other hand no common justfile recipe is found, each
argument is processed separately in sequence.

This does require that any flags passed are compatible with all recipes
involved (like is the case for `--learn` or `--codeql=built` for
language and integration tests).
@redsun82 redsun82 marked this pull request as ready for review July 9, 2025 15:05
@Copilot Copilot AI review requested due to automatic review settings July 9, 2025 15:05
@redsun82 redsun82 requested review from a team as code owners July 9, 2025 15:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java Rust Pull requests that update Rust code Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant