diff --git a/.github/workflows/python-tooling.yml b/.github/workflows/python-tooling.yml index a3ad9900ea47..51ca49533cc3 100644 --- a/.github/workflows/python-tooling.yml +++ b/.github/workflows/python-tooling.yml @@ -5,9 +5,10 @@ on: paths: - "misc/bazel/**" - "misc/codegen/**" + - "misc/just/**" - "misc/scripts/models-as-data/*.py" - "*.bazel*" - - .github/workflows/codegen.yml + - .github/workflows/python-tooling.yml - .pre-commit-config.yaml branches: - main @@ -33,3 +34,7 @@ jobs: shell: bash run: | bazel test //misc/codegen/... + - name: Run just tooling tests + shell: bash + run: | + bazel test //misc/just/... diff --git a/actions/justfile b/actions/justfile new file mode 100644 index 000000000000..b96eb20dfe26 --- /dev/null +++ b/actions/justfile @@ -0,0 +1,9 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "actions") + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) diff --git a/actions/ql/integration-tests/justfile b/actions/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/actions/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/actions/ql/justfile b/actions/ql/justfile new file mode 100644 index 000000000000..e613c25b52c1 --- /dev/null +++ b/actions/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := "" diff --git a/actions/ql/test/justfile b/actions/ql/test/justfile new file mode 100644 index 000000000000..0c6841ebb405 --- /dev/null +++ b/actions/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks + +[no-cd] +test *ARGS=".": (_codeql_test "actions" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/cpp/justfile b/cpp/justfile new file mode 100644 index 000000000000..0fc87260ebec --- /dev/null +++ b/cpp/justfile @@ -0,0 +1,10 @@ +import '../lib.just' +import? '../../cpp-coding-standards.just' + +[group('build')] +build: (_build_dist "cpp") + +roots := [source_dir() / 'ql/test', SEMMLE_CODE / 'semmlecode-cpp-tests'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) diff --git a/cpp/ql/consistency-queries/badLocations.ql b/cpp/ql/consistency-queries/badLocations.ql new file mode 100644 index 000000000000..385d3d92fe6a --- /dev/null +++ b/cpp/ql/consistency-queries/badLocations.ql @@ -0,0 +1,9 @@ +import cpp + +// Locations should either be :0:0:0:0 locations (UnknownLocation, or +// a whole file), or all 4 fields should be positive. +from Location l +where + [l.getStartLine(), l.getEndLine(), l.getStartColumn(), l.getEndColumn()] != 0 and + [l.getStartLine(), l.getEndLine(), l.getStartColumn(), l.getEndColumn()] < 1 +select l diff --git a/cpp/ql/consistency-queries/nullInToString.ql b/cpp/ql/consistency-queries/nullInToString.ql new file mode 100644 index 000000000000..4a6385b519ab --- /dev/null +++ b/cpp/ql/consistency-queries/nullInToString.ql @@ -0,0 +1,5 @@ +import cpp + +from Element e +where e.toString().matches("%(null)%") +select e diff --git a/cpp/ql/consistency-queries/qlpack.yml b/cpp/ql/consistency-queries/qlpack.yml new file mode 100644 index 000000000000..303f2271be12 --- /dev/null +++ b/cpp/ql/consistency-queries/qlpack.yml @@ -0,0 +1,6 @@ +name: codeql/cpp-consistency-queries +groups: [cpp, test, consistency-queries] +dependencies: + codeql/cpp-all: ${workspace} +extractor: cpp +warnOnImplicitThis: true diff --git a/cpp/ql/consistency-queries/unusedLocations.ql b/cpp/ql/consistency-queries/unusedLocations.ql new file mode 100644 index 000000000000..875c60ba3251 --- /dev/null +++ b/cpp/ql/consistency-queries/unusedLocations.ql @@ -0,0 +1,10 @@ +import cpp + +from Location l +where + not any(Element e).getLocation() = l and + not any(LambdaCapture lc).getLocation() = l and + not any(MacroAccess ma).getActualLocation() = l and + not any(NamespaceDeclarationEntry nde).getBodyLocation() = l and + not any(XmlLocatable xml).getLocation() = l +select l diff --git a/cpp/ql/consistency-queries/variableDeclarationsWithoutTypes.ql b/cpp/ql/consistency-queries/variableDeclarationsWithoutTypes.ql new file mode 100644 index 000000000000..2573d660defd --- /dev/null +++ b/cpp/ql/consistency-queries/variableDeclarationsWithoutTypes.ql @@ -0,0 +1,5 @@ +import cpp + +from VariableDeclarationEntry i +where not exists(i.getType()) +select i diff --git a/cpp/ql/consistency-queries/variablesWithoutTypes.ql b/cpp/ql/consistency-queries/variablesWithoutTypes.ql new file mode 100644 index 000000000000..d004c175abd0 --- /dev/null +++ b/cpp/ql/consistency-queries/variablesWithoutTypes.ql @@ -0,0 +1,5 @@ +import cpp + +from Variable i +where not exists(i.getType()) +select i diff --git a/cpp/ql/integration-tests/justfile b/cpp/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/cpp/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/cpp/ql/justfile b/cpp/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/cpp/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/cpp/ql/test/justfile b/cpp/ql/test/justfile new file mode 100644 index 000000000000..bc32c4f8d970 --- /dev/null +++ b/cpp/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := ['--include-location-in-star'] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-unused-labels', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "cpp" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/csharp/justfile b/csharp/justfile new file mode 100644 index 000000000000..6f99dd8703e0 --- /dev/null +++ b/csharp/justfile @@ -0,0 +1,9 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "csharp") + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) diff --git a/csharp/ql/integration-tests/justfile b/csharp/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/csharp/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/csharp/ql/justfile b/csharp/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/csharp/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/csharp/ql/test/justfile b/csharp/ql/test/justfile new file mode 100644 index 000000000000..5c25af2e049e --- /dev/null +++ b/csharp/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-repeated-labels', '--check-redefined-labels', '--additional-packs=ql', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "csharp" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/go/justfile b/go/justfile new file mode 100644 index 000000000000..fa4c18266af6 --- /dev/null +++ b/go/justfile @@ -0,0 +1,17 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "go") + +roots := [source_dir() / 'ql/test'] + +# The `IncorrectIntegerConversion` query treats `math.MaxInt`/`math.MaxUint` differently on 32- and +# 64-bit targets, so we run its test under `GOARCH=386` as well. `GOOS=linux` because +# `GOOS=darwin GOARCH=386` is no longer supported. +roots_386 := [source_dir() / 'ql/test/query-tests/Security/CWE-681/IncorrectIntegerConversion.qlref'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) + +[group('test')] +language-tests-386 *EXTRA_ARGS: (_language_tests (roots_386 ++ ['GOOS=linux', 'GOARCH=386'] ++ EXTRA_ARGS)) diff --git a/go/ql/integration-tests/justfile b/go/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/go/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/go/ql/justfile b/go/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/go/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/go/ql/test/justfile b/go/ql/test/justfile new file mode 100644 index 000000000000..c79e9a543f55 --- /dev/null +++ b/go/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-unused-labels', '--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "go" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/java/justfile b/java/justfile new file mode 100644 index 000000000000..aba4ba7b21dd --- /dev/null +++ b/java/justfile @@ -0,0 +1,4 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "java") diff --git a/java/ql/integration-tests/justfile b/java/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/java/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/java/ql/justfile b/java/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/java/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/java/ql/test-kotlin1/justfile b/java/ql/test-kotlin1/justfile new file mode 100644 index 000000000000..fc29900a05c7 --- /dev/null +++ b/java/ql/test-kotlin1/justfile @@ -0,0 +1,14 @@ +import "../justfile" + +# These are CI shards of the Kotlin language tests, too long to run by accident, +# so a verb coming from above passes over them and they run only when this +# directory is named. +explicit_verbs := ['test'] + +# Kotlin tests may fail the diags.ql consistency test if the diagnostic limit is set. +base_flags := ['CODEQL_EXTRACTOR_KOTLIN_DIAGNOSTIC_LIMIT='] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "java" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/java/ql/test-kotlin2/justfile b/java/ql/test-kotlin2/justfile new file mode 100644 index 000000000000..4ef773c27ecd --- /dev/null +++ b/java/ql/test-kotlin2/justfile @@ -0,0 +1,14 @@ +import "../justfile" + +# These are CI shards of the Kotlin language tests, too long to run by accident, +# so a verb coming from above passes over them and they run only when this +# directory is named. +explicit_verbs := ['test'] + +# Kotlin tests may fail the diags.ql consistency test if the diagnostic limit is set. +base_flags := ['CODEQL_EXTRACTOR_KOTLIN_DIAGNOSTIC_LIMIT=', 'CODEQL_KOTLIN_LEGACY_TEST_EXTRACTION_KOTLIN2=true'] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "java" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/java/ql/test/justfile b/java/ql/test/justfile new file mode 100644 index 000000000000..f2b6774fa41c --- /dev/null +++ b/java/ql/test/justfile @@ -0,0 +1,12 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +# Kotlin tests may fail the diags.ql consistency test if the diagnostic limit is set. +base_flags := ['CODEQL_EXTRACTOR_KOTLIN_DIAGNOSTIC_LIMIT='] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "java" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/javascript/justfile b/javascript/justfile new file mode 100644 index 000000000000..769847a380d1 --- /dev/null +++ b/javascript/justfile @@ -0,0 +1,9 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "javascript") + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) diff --git a/javascript/ql/integration-tests/justfile b/javascript/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/javascript/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/javascript/ql/justfile b/javascript/ql/justfile new file mode 100644 index 000000000000..e613c25b52c1 --- /dev/null +++ b/javascript/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := "" diff --git a/javascript/ql/test/justfile b/javascript/ql/test/justfile new file mode 100644 index 000000000000..37ea1df3c257 --- /dev/null +++ b/javascript/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks + +[no-cd] +test *ARGS=".": (_codeql_test "javascript" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/justfile b/justfile new file mode 100644 index 000000000000..6fe875f6facb --- /dev/null +++ b/justfile @@ -0,0 +1,9 @@ +# see misc/just/README.md for an overview + +import 'lib.just' +import 'misc/just/forward.just' + +# bazel files live all over the repository rather than under any one language, so they +# are formatted from here. `format` itself is the forwarder, hence `_root_`; see +# misc/just/README.md. +_root_format *ARGS=".": (_format_bazel ARGS) diff --git a/lib.just b/lib.just new file mode 100644 index 000000000000..0ddd926bcda5 --- /dev/null +++ b/lib.just @@ -0,0 +1 @@ +import "misc/just/lib.just" diff --git a/misc/codegen/justfile b/misc/codegen/justfile new file mode 100644 index 000000000000..bd632b74817b --- /dev/null +++ b/misc/codegen/justfile @@ -0,0 +1,5 @@ +import "../just/lib.just" + +test *ARGS: (_bazel (['test', '@codeql//misc/codegen/...'] ++ ARGS)) + +format *ARGS=".": (_format_py ARGS) diff --git a/misc/just/BUILD.bazel b/misc/just/BUILD.bazel new file mode 100644 index 000000000000..f94804c9fcd0 --- /dev/null +++ b/misc/just/BUILD.bazel @@ -0,0 +1,27 @@ +load("@rules_python//python:defs.bzl", "py_library", "py_test") + +py_library( + name = "tooling", + srcs = [ + "codeql_test_run.py", + "forward_command.py", + "language_tests.py", + "run_on_files.py", + ], + imports = ["."], + visibility = ["//visibility:public"], +) + +[ + py_test( + name = src[:-len(".py")], + size = "small", + srcs = [src], + deps = [":tooling"], + ) + for src in glob(["test_*.py"]) +] + +test_suite( + name = "test", +) diff --git a/misc/just/README.md b/misc/just/README.md new file mode 100644 index 000000000000..9dc2693e36b1 --- /dev/null +++ b/misc/just/README.md @@ -0,0 +1,227 @@ +This directory contains an infrastructure for [`just`](https://github.com/casey/just) +recipes that can be used throughout this and the internal repository. In particular we +have common 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. + +`just` 1.53 or newer is required. Recipes forward argument lists rather than encoding +them as whitespace separated strings, and every piece of that arrived in that one +release: `set lists`, list literals, the `++` operator, and variadic parameters that pass +their elements on one at a time rather than space-joined. Lists are still unstable, at +1.58 as much as at 1.53, which is what `set unstable` in `defs.just` is for. An older +`just` stops at the first list literal it parses, complaining about an unexpected `[` and +saying nothing about versions; only `defs.just` read on its own gives the clearer +`unknown setting` naming `lists`. + +# Forwarding + +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 above it, and you run something like + `just test ql/rust/ql/test/{a,b}`, then a forwarder script finds a common justfile + implementing the verb for all the positional arguments passed there, and then retries + calling `just test` from there. So if `test` is implemented beneath that (in that case, + it is in `rust/ql/test`), it uses that recipe. +- even if there isn't a recipe that is common to all the positional arguments, the + forwarder will still group the arguments in batches using the same recipe. So + `just build ql/rust ql/java`, or + `just test ql/rust/ql/test/some/language/test ql/rust/ql/integration-test/some/integration/test` + will also work, with corresponding recipes run sequentially. +- finally, the forwarder also looks _below_ each argument, so that `just test ql/cpp` + runs the tests defined underneath it. The argument only says where to look in this + case, so each recipe found is run on its own directory rather than being passed the + argument. Several may be found, in which case they run sequentially: + `just format ql/cpp` formats everything under `ql/cpp` that knows how to format + itself. + +Both directions are searched, and every distinct recipe found runs. This matters because +a verb higher up is usually doing a different job from one further down rather than a +broader version of it: `rust` formats Rust sources while `rust/ql` formats QL, so +`just format rust` has to do both. A recipe that only arrived through `import` is the +same job, though, and runs once. + +A repository root forwards every verb, which leaves it no way to answer one itself: a +recipe written next to the `import` overrides the imported one and takes the forwarder's +place, so `just format cpp` would stop finding anything. The root spells its own +implementation `_root_` instead, and the forwarder picks that up wherever the +plain name turns out to be the forwarder's own: + +```just +import 'misc/just/forward.just' + +_root_format *ARGS=".": (_format_bazel ARGS) +``` + +This is for work that belongs to no single directory. bazel files are the case in hand: +they sit throughout the tree rather than under any one language, so formatting them is +the root's job, and taking the argument keeps `just format cpp` to the bazel files under +`cpp`. + +Note that this one delegates rather than doing the work itself. The forwarder reaches a +recipe above its argument with `--justfile`, which runs it from the directory of the +justfile defining it unless the recipe is `[no-cd]`. A `_root_` that grows a body +therefore reads its default `.` as the whole repository rather than the directory the +caller is in, so one that does its own work needs `[no-cd]` itself. + +Being a recipe like any other, a `_root_` is inherited by a justfile importing the +one defining it. It runs once either way, as the two spellings are the same recipe. A +root that defines its own instead replaces it, and then both run, each over the files of +the repository that defines it: bazel formatting asks bazel from the root of the checkout +the files belong to, so that a repository formats its own files with its own pin. + +Nothing in a justfile says which of the two happened, so they are told apart by comparing +the recipes. A root whose own copy is identical to the one it would otherwise inherit, +down to the comment above it, is therefore taken for the inherited one and runs once. +Copy such a recipe to start from if it helps, but leave its comment behind. + +That last part is arranged by variables rather than by recipes. `set +allow-duplicate-variables` in `defs.just` lets an importing justfile assign a variable +defined here and have its value win, which is how a consuming root points the bazel +formatter at its own workspace, its own buildifier and its own exclusions. The leading +underscore says these are not meant to be run, not that they are private: any of them a +root might reasonably want to redirect is an interface between the two repositories. + +Renaming one is therefore a breaking change that nothing reports. `just` has no notion +of an assignment that fails to override, so a root assigning the old name keeps parsing, +keeps listing, keeps passing CI, and silently reverts to the value here. Worse, a root +overriding several loses only the renamed one, leaving a half-applied configuration: +total failure would land in a state someone designed, while partial failure lands in one +nobody has ever seen. + +An override also freezes what it replaces. A root assigns the whole value, so an +exclusion or a name added here later never reaches one, and `just` offers no way to +append: a root writing `_bazel_excluded := _bazel_excluded + ",mine"` is told the +variable is defined in terms of itself. This runs the opposite way from a rename, where +the override stops applying and the value here wins. Here the override keeps applying +exactly as written, and the roots that never see the addition are the ones that cared +enough about the setting to redirect it. Adding to one of these values is therefore a +change to make on both sides at once. + +Neither shows up anywhere, because the underscore that keeps these out of `just --list` +keeps them out of `--variables` and a bare `--evaluate` as well. Asked by name they do +answer, which is how a root checks that an override of its own still overrides anything: + +```sh +just --evaluate _bazel_excluded # what mine is now +just --justfile /justfile --evaluate _bazel_excluded # what it would be +``` + +A name that has gone says so rather than reporting an empty value. Read as two values +rather than as two names, the same pair also shows a freeze: a pattern appearing only +under what it would be is one this repository added and the override never received. +Expect differences both ways, since a root that overrode a value usually added something +of its own, and only the missing half is a bug. Rename freely, but say so when handing +the change over. + +A directory that only makes sense when named explicitly can opt out of being found from +above: + +```just +explicit_verbs := ['test'] +``` + +This only affects the downward search. Running the verb from inside that directory, or +naming the directory on the command line, keeps working. A verb that passed over such a +directory says so and names it, so that a command covering a tree does not look like it +covered more than it did. That listing is part of the account of what ran and leaves the +exit status alone; only a verb that matched nothing at all fails. + +The QL test suites use this: `test` on a language runs the whole suite, which takes a +long time, so that has to be asked for by name. Integration tests and the sharded +Kotlin suites that CI runs opt out for the same reason. What is left discoverable from +above is what is cheap enough to run without meaning to. + +Being an ordinary variable, `explicit_verbs` is inherited by justfiles importing one +that sets it. That is normally what is wanted, as importing a suite's justfile means +being the same kind of suite, down to the reason for naming it explicitly. An importer +that disagrees can reassign it, and its own value wins: + +```just +import '../some/suite/justfile' + +explicit_verbs := [] +``` + +Duplicate variables are allowed throughout (see `defs.just`), so this is silent in both +directions: assigning `explicit_verbs` without realising one was inherited overrides it +without complaint, which can put a heavy suite back within reach of a verb aimed at a +parent directory. + +Justfiles are found through `git`, so a newly written one needs to be either tracked or +untracked-but-not-ignored to be picked up. + +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, and + correspond to all the additional checks that CI adds (but that a dev might not want to + run by default). Checks a root passes unconditionally are not offers: what it offers + is what `--all-checks` can enable + +Test arguments are passed around as `just` lists (`set lists`), so they reach the +underlying runner already split and arguments containing spaces survive intact. + +One caveat: when a verb ends up running several recipes, non-positional arguments need +to be understood by all of them. That is fine when they speak the same language, as +`--learn` or `--codeql` do across QL and integration tests. It is not when they do not: +a broad `just test .` reaches bazel and pytest suites alike, and a flag meant for one of +them will fail on the other. It fails rather than being quietly ignored, so the answer +is to aim the verb at something narrower. + +# Command separators + +Commands are echoed between rules that span the terminal. The formatters echo a summary +rather than the line that runs: they name the formatter and what it is told to do, and +leave out the wrapper that collects the files, the patterns it walks and the flags that +only shape output. A leading `-> ` marks a line as that summary. `just -n` prints what +actually runs, which it does whether or not the recipe is `@`-quiet. + +Nothing is lost by this, as a banner has never been something to paste: the echoed line +carried an unexpanded `"$@"`, which matches no file in another shell, so pasting one +formatted nothing and exited 0. + +What a formatter says for itself is filtered down to what happened, as several name every +file they considered and most of them were left alone. One such line is kept on purpose: +buildifier's count of the warnings it could not fix, which is the only notice of them, +since it reports no detail in fix mode and exits 0 whether or not any remain. That tally +grows with the tree and is mostly lint about docstrings. Finding it tiresome is a reason +to configure what buildifier lints, never to widen the filter back over it, which would +take the warnings worth having along with the rest. + +Measuring the width means a `shell()` call, and that runs on every parse, so the result +is exported as `JUST_CMD_RULE` and an inherited value is preferred to measuring again. + +Inheritance crosses processes, which is what forwarding creates: a child per justfile +reached, each of them measuring nothing. A `mod` spawns no process, so a module measures +for itself, and the count is one per `mod` reached, however deeply nested, plus one for +the file itself. That is cheap, and modules agree anyway since they share a terminal, +but it is worth knowing before counting measurements. Presetting `JUST_CMD_RULE` skips +all of them, and is also how to fix the width in CI or in a recording — it is the whole +separator and ends up in a shell script, so every line of it has to be a comment, or it +runs. + +With no terminal to ask — a pipe, a log, a shell without `stty` — it falls back to a +fixed 57 columns, so logs and CI output are the same width every time. That is one +branch rather than a platform test: `just` runs `sh` everywhere, so Windows takes +whichever arm fits rather than a path of its own. + +# Tests + +The scripts here have tests, run by `just test misc/just`, by `bazel test +//misc/just/...`, or by CI on any change under this directory. They are plain +`unittest`: this is the layer that runs other tooling, so it should not need a package +manager to check itself. + +They exist for the class of bug that leaves no trace. An exclusion that stops excluding, +a verb that stops being found, an argument that arrives split in two — each of those +still exits zero, and the only symptom is work quietly not done. So the tests come in +pairs: one asserting the quiet outcome, and one positive control asserting the same +setup can produce the loud one. Without the second, a passing first test is also what a +test that runs nothing at all looks like. diff --git a/misc/just/build.just b/misc/just/build.just new file mode 100644 index 000000000000..efb85aaa7e81 --- /dev/null +++ b/misc/just/build.just @@ -0,0 +1,27 @@ +# Helper build recipes + +import "defs.just" + +# Build the given language-specific CLI distribution +_build_dist LANGUAGE: _require_semmle_code (_maybe_build_dist LANGUAGE) + +# Build the language-specific distribution if we are in an internal repository checkout +# Otherwise, do nothing +# +# The install log is worth reading only when the install fails, and printing it +# regardless buried whatever was actually asked for underneath it. Note that bazel is +# not quietened any further than that here: unlike an error, a failing test's log is not +# something `--ui_event_filters` can let back through, and a build this long is one to +# see the progress of. +[no-exit-message] +_maybe_build_dist LANGUAGE: (_if_in_semmle_code (f'cd "$SEMMLE_CODE"; tools/bazel test //language-packs:intree-{{ LANGUAGE }}-as-test --test_output=errors') '# using codeql from PATH, if any') + +# Call bazel. Uses our official bazel wrapper if we are in an internal repository checkout +[no-cd] +[no-exit-message] +_bazel *ARGS: (_if_in_semmle_code 'cd "$SEMMLE_CODE"; tools/bazel' 'bazel' ARGS) + +# Call sembuild (requires an internal repository checkout) +[no-cd] +[no-exit-message] +_sembuild *ARGS: (_run_in_semmle_code (['./build'] ++ ARGS)) diff --git a/misc/just/codeql_test_run.py b/misc/just/codeql_test_run.py new file mode 100755 index 000000000000..a4d70b5a260f --- /dev/null +++ b/misc/just/codeql_test_run.py @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +"""Run CodeQL tests with appropriate configuration. + +Called from just recipes as: + python3 codeql_test_run.py LANGUAGE [ARG...] + +Arguments are already split by `just` (see `set lists`), so each one is taken verbatim. +`--extra-check=FLAG` offers FLAG as a check to run, and `--all-checks` (or its `+` +abbreviation) turns the offered ones on. Per-language justfiles supply the offers and +the caller supplies the switch, so the two are separate options rather than one. +""" + +import argparse +import os +import re +import subprocess +import sys +import shutil +from pathlib import Path + +JUST = os.environ.get("JUST_EXECUTABLE", "just") +ERROR = os.environ.get("JUST_ERROR", "error: ") +CMD_BEGIN = os.environ.get("CMD_BEGIN", "") +CMD_END = os.environ.get("CMD_END", "") +SEMMLE_CODE = os.environ.get("SEMMLE_CODE") + +ENV_RE = re.compile(r"(^[A-Z_][A-Z_0-9]*)=(.*)$") + + +def invoke(invocation, *, cwd=None, log_prefix=""): + prefix = f"{log_prefix} " if log_prefix else "" + print(f"{CMD_BEGIN}{prefix}{' '.join(map(str, invocation))}{CMD_END}") + try: + subprocess.run(invocation, check=True, cwd=cwd) + except subprocess.CalledProcessError as e: + return e.returncode + return 0 + + +def error(message): + print(f"{ERROR}{message}", file=sys.stderr) + raise SystemExit(1) + + +class _Parser(argparse.ArgumentParser): + """An `argparse` parser that fails the way the rest of this script does. + + The default reports to `stderr` in its own format and exits 2, which would arrive + in a `just` banner unprefixed and alongside a usage line naming this script rather + than the recipe the caller actually typed. + """ + + def error(self, message): + error(message) + + +def build_parser(): + # `+` can be an option string only because it is also a prefix character. `-h` and + # `--help` are left unclaimed so that they reach `codeql test run`. + parser = _Parser(add_help=False, allow_abbrev=False, prefix_chars="-+") + parser.add_argument("language") + parser.add_argument("--codeql", default="build" if SEMMLE_CODE else "host") + parser.add_argument("--extra-check", action="append", dest="extra_checks") + parser.add_argument("--all-checks", "+", action="store_true", dest="all") + parser.set_defaults(extra_checks=[], tests=[], flags=[], env={}) + return parser + + +def parse_arguments(): + """Sort a command line into the kinds that are handled differently. + + `argparse` owns the options this script acts on itself. Everything else belongs to + `codeql test run` and has to survive untouched, which is what `parse_known_args` + hands back, and what is sorted by shape here: a test path and a `CPUS=4` are both + positionals, told apart only by how they look. + """ + p = build_parser() + args, rest = p.parse_known_args() + if args.codeql in ("build", "built") and not SEMMLE_CODE: + p.error( + "Using `--codeql=build` or `--codeql=built` requires working " + "with the internal repository" + ) + + for arg in rest: + if arg.startswith("-"): + args.flags.append(arg) + elif m := ENV_RE.match(arg): + k, v = m.groups() + args.env[k] = v + else: + args.tests.append(arg) + return args + + +def resolve_codeql(args: argparse.Namespace) -> Path: + suffix = ".exe" if sys.platform == "win32" else "" + match args.codeql: + case "built" | "build": + return Path( + SEMMLE_CODE, + "target", + "intree", + f"codeql-{args.language}", + "codeql" + suffix, + ) + case "host": + codeql = shutil.which("codeql" + suffix) + if not codeql: + error("CodeQL executable not found in PATH") + return Path(codeql) + case _: + codeql = Path(args.codeql) + if codeql.is_dir(): + codeql /= "codeql" + suffix + return codeql + + +def main(): + args = parse_arguments() + + if args.all: + # Apply what the language offered by parsing it alongside everything else, so an + # offered check lands exactly where the same flag typed by hand would. + sys.argv[1:1] = args.extra_checks + args = parse_arguments() + + if not args.tests: + args.tests.append(".") + + os.environ.update(args.env) + + # Resolve these only once all arguments are known, so that a `RAM_PER_THREAD=` test + # argument can lower the default on memory-heavy suites. + default_ram = 3000 if sys.platform == "linux" else 2048 + ram_per_thread = int(os.environ.get("RAM_PER_THREAD") or default_ram) + cpus = int(os.environ.get("CPUS") or os.cpu_count() or 1) + args.flags[:0] = [f"--ram={ram_per_thread * cpus}", f"-j{cpus}"] + + if args.codeql == "build": + if ret := invoke([JUST, args.language, "build"], cwd=SEMMLE_CODE): + return ret + + if args.codeql != "host": + # Disable the default implicit config file, but keep an explicit one. + # Same behavior wrt --codeql as the integration test runner. + os.environ.setdefault("CODEQL_CONFIG_FILE", ".") + + codeql = resolve_codeql(args) + + if not codeql.exists(): + error(f"CodeQL executable not found: {codeql}") + + return invoke( + [codeql, "test", "run", *args.flags, "--", *args.tests], + log_prefix=" ".join(f"{k}={v}" for k, v in args.env.items()), + ) + + +if __name__ == "__main__": + try: + sys.exit(main()) + except KeyboardInterrupt: + sys.exit(128 + 2) diff --git a/misc/just/defs.just b/misc/just/defs.just new file mode 100644 index 000000000000..16a9c18c0aa3 --- /dev/null +++ b/misc/just/defs.just @@ -0,0 +1,101 @@ +# The first definition of a variable wins, not the last, so this order is what lets the +# internal file set `SEMMLE_CODE` while the stub is the fallback when it is absent. +# Swapped, the stub wins even where the real file exists and that checkout then behaves +# as if it were external. Nothing here can catch that: with the file absent, as it +# always is in this repository, both orders evaluate the same. +import? '../../../semmle-code.just' # internal repo just file, if present +import 'semmle-code-stub.just' + +# `set lists` is what lets recipes forward argument lists without encoding them as +# whitespace separated strings. It is still unstable as of just 1.58. +set unstable +set lists +set fallback +set allow-duplicate-recipes +set allow-duplicate-variables + +export PATH_SEP := if os() == "windows" { ";" } else { ":" } +export JUST_EXECUTABLE := just_executable() + +# MSYS2 rewrites arguments that look like paths as it hands them to a native program, +# which mangles bazel's `//target` labels. Set once here rather than at each bazel call: +# a call that forgets it works everywhere except Windows, which is where nobody looks. +# The price of setting it once is that it covers every command a recipe runs, so a tool +# that wants Windows path conversion has to ask for it back. +export MSYS2_ARG_CONV_EXCL := if os() == "windows" { "*" } else { "" } + +error := f'{{ style("error") }}error{{ NORMAL }}: ' + +# The separator line, sized to the terminal. +# +# `stty` rather than `tput`: it needs no terminfo, so it survives `TERM=dumb`, and reads +# the terminal from stdin, so it answers when stdout is piped. The arithmetic is in the +# shell because just has no integers. +# +# With no terminal the value is non-numeric and this falls back to a fixed 57 columns. +# That is a branch, not a platform test: just runs `sh` everywhere. +# +# `JUST_CMD_RULE` is the whole separator and lands in a shell script, so a hand-set value +# has to be a shell comment, every line of it, or it runs. Left unchecked: setting it at +# all means being able to run commands already, so there is nothing to defend. +# +# Exported and preferred over measuring: `shell()` runs on every parse, so a forwarded +# verb would otherwise re-measure in every child it spawns, and `if` is lazy in its +# branches. Inheritance needs a process, so a `mod` measures for itself; presetting +# `JUST_CMD_RULE` skips measuring entirely. +_given_horizontal_rule := env('JUST_CMD_RULE', '') + +_horizontal_rule := if _given_horizontal_rule == '' { shell(''' + w=$(stty size 2>/dev/null | cut -d" " -f2) + case "$w" in '' | *[!0-9]*) w=57 ;; esac + printf "%*s" $w "" | tr " " '#' +''') } else { _given_horizontal_rule } + +export JUST_CMD_RULE := _horizontal_rule + +cmd_sep := "\n" + _horizontal_rule + "\n" +export CMD_BEGIN := style("command") + cmd_sep +export CMD_END := cmd_sep + NORMAL +export JUST_ERROR := error + +py := "python3" + +default_db_checks := ['--check-databases', '--check-diff-informed', '--fail-on-trap-errors'] + +[no-exit-message] +@_require_semmle_code: + {{ if SEMMLE_CODE == "" { f''' + echo "{{ error }}running this recipe requires doing so from an internal repository checkout" >&2 + exit 1 + ''' } else { "" } }} + +[no-cd] +_run +ARGS: + {{ cmd_sep }}{{ ARGS }}{{ cmd_sep }} + +[no-cd] +_run_in DIR +ARGS: + {{ cmd_sep }}cd "{{ DIR }}"; {{ ARGS }}{{ cmd_sep }} + +[no-cd] +_run_in_semmle_code +ARGS: _require_semmle_code (_run_in "$SEMMLE_CODE" ARGS) + +[no-cd] +[no-exit-message] +[positional-arguments] +@_just +ARGS: + echo "-> just $@" + "{{ JUST_EXECUTABLE }}" "$@" + +[no-cd] +[positional-arguments] +@_if_not_on_ci_just +ARGS: + if [ "${GITHUB_ACTIONS:-}" != "true" ]; then \ + echo "-> just $@"; \ + "$JUST_EXECUTABLE" "$@"; \ + fi + +[no-cd] +[no-exit-message] +_if_in_semmle_code THEN ELSE *ARGS: + {{ cmd_sep }}{{ if SEMMLE_CODE != "" { THEN } else { ELSE } }} {{ ARGS }}{{ cmd_sep }} diff --git a/misc/just/format.just b/misc/just/format.just new file mode 100644 index 000000000000..8175a84bbb6d --- /dev/null +++ b/misc/just/format.just @@ -0,0 +1,74 @@ +import "build.just" + +# These formatters take files rather than directories, so `run_on_files.py` collects them +# and passes them positionally, which is what lets paths contain spaces. Both name every +# file they were given, so the lines saying nothing happened are dropped: for buildifier +# that is an empty warning tally, and a non-zero one is left alone, being the only notice +# of what it could not fix now that the exit code is 0 either way. +# +# The collector announces the run, rather than each recipe saying what it is about to do. +# A recipe reaches that line knowing neither whether any file matched nor which of the +# paths it was handed hold one, and both are worth waiting for: a path whose files are all +# excluded is the ordinary case here, as a repository formatting its own bazel files is +# handed a nested repository's path too. The price of letting the collector speak is that +# it names the command as it is really spelled, flags that only shape output and all. It +# is a report rather than something to paste in any case, the collecting being the point; +# `just -n` prints what actually runs, `@` or no `@`. `_format_py` echoes its own, having +# no collector to defer to and nothing to decide. +# +# `[no-cd]` keeps a relative argument meaning the caller's directory: the forwarder passes +# `--justfile`, so without it the default `.` would silently become the whole repository. + +_ql_formatter := (if SEMMLE_CODE != "" { '"$SEMMLE_CODE/target/intree/codeql-nolang/codeql"' } else { "codeql" }) + " query format --in-place" + +[no-cd] +[no-exit-message] +[positional-arguments] +@_format_ql +ARGS: (_maybe_build_dist "nolang") + {{ py }} "{{ source_dir() }}/run_on_files.py" --drop '^Formatting ' --drop '^No change for ' "*.ql,*.qll" {{ _ql_formatter }} -v -- "$@" + +_py_formatter := if SEMMLE_CODE != "" { "uv run black" } else { "black" } + +[no-cd] +[no-exit-message] +[positional-arguments] +@_format_py *ARGS=".": + echo "$CMD_BEGIN-> "{{ quote(_py_formatter) }}" $*$CMD_END" >&2 + {{ _py_formatter }} "$@" + +_cpp_formatter := (if SEMMLE_CODE != "" { "uv run clang-format" } else { "clang-format" }) + " -i" + +[no-cd] +[no-exit-message] +[positional-arguments] +@_format_cpp *ARGS=".": + {{ py }} "{{ source_dir() }}/run_on_files.py" "*.h,*.cpp" {{ _cpp_formatter }} -- "$@" + +# The `buildifier` target formats a whole workspace and so cannot take a path, so its +# binary is driven directly. bazel is asked from this repository's root even when it sits +# inside another, and the files are bounded to it, so each repository formats its own with +# the version it pins. Building needs the enclosing workspace instead, hence `_bazel`. +_bazel_workspace := quote(parent_directory(parent_directory(source_dir()))) + +_bazel_formatter := (if SEMMLE_CODE != "" { '"$SEMMLE_CODE/tools/bazel"' } else { "bazel" }) + " run @buildifier_prebuilt//:buildifier" + +# Kept out of the formatter, unlike the other three: bazel's own flags have to sit between +# the target and `--`, so no single string spans both sides of it. +_bazel_args := "-mode=fix -lint=fix" + +# Keep both in step with the `buildifier` target. The names are what it recognises walking a +# workspace, a check it skips for paths handed to it, so anything extra here is rewritten +# regardless. Being wider is deliberate: what it recognises differs between versions, and +# each repository pins its own, so only a list covering every one of them can be shared. A +# directory of one file per candidate name, formatted with `-r`, says what a version takes. +# Exclusions match the path as walked and its absolute form, so one that has to hold +# however the path was reached is anchored absolutely. +_bazel_names := "BUILD,WORKSPACE,*.bazel,*.bzl,*.sky,*.star" + +_bazel_excluded := ".git/*,*/.git/*,*misc/bazel/3rdparty/*_deps/*" + +[no-cd] +[no-exit-message] +[positional-arguments] +@_format_bazel *ARGS=".": + {{ py }} "{{ source_dir() }}/run_on_files.py" --absolute --chdir {{ _bazel_workspace }} --within {{ _bazel_workspace }} --drop ': applied fixes, 0 warnings left$' --exclude '{{ _bazel_excluded }}' "{{ _bazel_names }}" {{ _bazel_formatter }} --noshow_progress --ui_event_filters=,+error,+fail -- {{ _bazel_args }} -v -- "$@" diff --git a/misc/just/forward.just b/misc/just/forward.just new file mode 100644 index 000000000000..571f5e31a806 --- /dev/null +++ b/misc/just/forward.just @@ -0,0 +1,30 @@ +# Common verbs +# See README.md in this directory for an overview. + +import "lib.just" + +# Verbs are recipe names, so each one needs its own recipe. They all delegate to the +# same forwarder, which decides where the verb is actually implemented. + +[no-cd] +[no-exit-message] +[positional-arguments] +@_forward VERB *ARGS: + {{ py }} "{{ source_dir() }}/forward_command.py" "$@" + +alias t := test +alias b := build +alias g := generate +alias gen := generate +alias f := format +alias l := lint + +test *ARGS: (_forward "test" ARGS) + +build *ARGS: (_forward "build" ARGS) + +generate *ARGS: (_forward "generate" ARGS) + +lint *ARGS: (_forward "lint" ARGS) + +format *ARGS: (_forward "format" ARGS) diff --git a/misc/just/forward_command.py b/misc/just/forward_command.py new file mode 100644 index 000000000000..300e1a0a8263 --- /dev/null +++ b/misc/just/forward_command.py @@ -0,0 +1,428 @@ +#!/usr/bin/env python3 +"""Forward a common verb to the justfiles that implement it. + +Verbs like `test`, `build` and `format` are spelled the same everywhere, but what they +mean is defined per language, next to the code they act on. This finds the justfiles +implementing a verb for each of its arguments and runs every one of them, so that +`just test rust` or `just format .` work without a central list of who implements what. + +Justfiles are looked for in both directions from an argument: + +- above it, where a recipe is passed the argument itself, as that says what to act on +- below it, where a recipe is passed its own directory, as there the argument only said + where to look + +Every distinct recipe found this way runs. Recipes are compared by value, so one reached +through `import` is 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. + +Two things keep the search useful: recipes delegating back here are skipped, so it never +settles on a forwarder, and a directory can set `explicit_verbs` to stay out of reach of +a verb aimed at one of its parents. See README.md for the whole picture. + +Called from just recipes as: + python3 forward_command.py COMMAND [ARGS...] +""" + +import json +import os +import re +import subprocess +import sys +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +JUST = os.environ.get("JUST_EXECUTABLE", "just") +ERROR = os.environ.get("JUST_ERROR", "") + +# Recipes that delegate to this one do not implement a verb, they pass it on. Skipping +# them is what stops the search from settling on a forwarder, be it this one or the root +# justfile of a nested repository. +FORWARD_RECIPE = "_forward" + +# A justfile that forwards a verb has already spent the plain name on the forwarder, so +# it names its own implementation of that verb `_root_`. This is how a repository +# root gets to answer a verb for itself while still dispatching it everywhere else. +ROOT_PREFIX = "_root_" + +# Justfiles may list verbs that must be spelled out explicitly instead of being picked +# up by a verb aimed at one of their parent directories. +EXPLICIT_VERBS = "explicit_verbs" + +PROBE_WORKERS = 16 + + +def error(message): + # Anything already reported on stdout belongs before this, and the two streams are + # buffered differently when they are not both a terminal. + sys.stdout.flush() + print(f"{ERROR}{message}", file=sys.stderr) + + +def get_just_context(justfile, recipe, flags, positional_args): + """Get the (cwd, args) for invoking just with the given justfile.""" + if ( + len(positional_args) == 1 + and justfile == Path(positional_args[0]) / "justfile" + ): + # If there's only one positional argument and it matches the justfile + # path, suppress arguments so e.g. `just build ql/rust` becomes + # `just build` in the `ql/rust` directory + return positional_args[0], [recipe, *flags] + else: + return None, ["--justfile", str(justfile), recipe, *flags, *positional_args] + + +def dump_justfile(justfile): + """Parse a justfile with `just`, returning its JSON dump or an error message.""" + result = subprocess.run( + [JUST, "--dump", "--dump-format", "json", "--justfile", str(justfile)], + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + ) + if result.returncode != 0: + return None, result.stderr.strip() + return json.loads(result.stdout), None + + +def list_value(assignments, name): + """Read a list literal assignment from a justfile dump.""" + value = assignments.get(name, {}).get("value") + # A literal list is dumped as ["list", element...]. Anything else is an expression + # that cannot be evaluated without running just, and counts as absent. + if isinstance(value, list) and value[:1] == ["list"]: + return value[1:] + return [] + + +def is_variadic(recipe): + parameters = recipe["parameters"] + return bool(parameters and parameters[-1]["kind"] in ("star", "plus")) + + +def accepts(recipe, argc): + """Check whether a recipe can be called with a given number of arguments.""" + parameters = recipe["parameters"] + required = sum( + 1 + for parameter in parameters + if parameter["default"] is None and parameter["kind"] != "star" + ) + return required <= argc and (is_variadic(recipe) or argc <= len(parameters)) + + +def implements(dump, command, argc): + """Return the recipe a justfile runs for a command, if it has a usable one.""" + recipes = dump["recipes"] + # An alias dumps as an object rather than as its target, so the name has to be read + # out of it. Resolved once and used throughout: a verb reached by an alias is the + # verb, so the justfile's own answer to it is named after the target too. + alias = dump["aliases"].get(command) + name = alias["target"] if alias else command + recipe = recipes.get(name) + if recipe is None or recipe["private"]: + return None + if any( + dependency["recipe"] == FORWARD_RECIPE for dependency in recipe["dependencies"] + ): + # Here the plain name is the forwarder's own, so it says nothing about what this + # directory does. A justfile that both forwards and answers the command itself + # spells its own answer `_root_`, the one name the two can share. + recipe = recipes.get(f"{ROOT_PREFIX}{name}") + if recipe is None: + return None + return recipe if accepts(recipe, argc) else None + + +def opts_out(dump, command): + """Whether a justfile asks to be named rather than found by a command.""" + return command in list_value(dump["assignments"], EXPLICIT_VERBS) + + +def dump_all(justfiles): + """Parse justfiles in parallel, reporting the ones that cannot be read.""" + with ThreadPoolExecutor(PROBE_WORKERS) as executor: + dumps = list(executor.map(dump_justfile, justfiles)) + parsed = [] + failed = False + for justfile, (dump, failure) in zip(justfiles, dumps): + if dump is None: + failed = True + error(f"could not read {justfile}:\n{failure}") + else: + parsed.append((justfile, dump)) + return parsed, failed + + +def git(directory, *args): + """Run a git command in a directory, returning its output lines.""" + result = subprocess.run( + ["git", "-C", directory, *args], + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + ) + if result.returncode != 0: + error(f"`git {' '.join(args)}` failed in {directory}:\n{result.stderr.strip()}") + return [], True + return result.stdout.splitlines(), False + + +def submodules(directory): + """List the initialised submodules under a directory.""" + toplevel, failed = git(directory, "rev-parse", "--show-toplevel") + if not toplevel or not (Path(toplevel[0]) / ".gitmodules").exists(): + return [], failed + lines, config_failed = git( + toplevel[0], "config", "--file", ".gitmodules", "--get-regexp", r"\.path$" + ) + paths = [Path(toplevel[0]) / line.split(" ", 1)[1] for line in lines] + within = Path(directory).resolve() + return [ + Path(directory) / os.path.relpath(path, within) + for path in paths + # An uninitialised submodule is an empty directory, with nothing to run. + if path.is_relative_to(within) and (path / ".git").exists() + ], failed or config_failed + + +def find_justfiles(directory): + """List every justfile under a directory. + + Submodules are listed separately, as `git ls-files` can either recurse into them or + report untracked files, but not both, and a justfile that has just been written is + worth finding. + """ + justfiles = set() + repositories, failed = submodules(directory) + for repository in [directory, *repositories]: + lines, git_failed = git( + repository, + "ls-files", + "--cached", + "--others", + "--exclude-standard", + "--", + "justfile", + "*/justfile", + ) + failed = failed or git_failed + justfiles.update(Path(repository) / line for line in lines) + return justfiles, failed + + +def invocation_path(path, *, like): + """Spell an absolute path like the user spelled the argument.""" + if Path(like).is_absolute(): + return path + return Path(os.path.relpath(path, Path.cwd())) + + +def find_justfiles_above(command, arg): + """Search up the directory tree for justfiles implementing the command. + + All of them are collected rather than just the nearest, because a recipe higher up + is often doing a different job from one further down rather than a broader version + of it. Returns (justfile, recipe) pairs, nearest first. + """ + directory = Path(arg).resolve() + candidates = [ + invocation_path(p / "justfile", like=arg) + for p in [directory, *directory.parents] + if (p / "justfile").exists() + ] + found = [] + seen = [] + parsed, failed = dump_all(candidates) + for justfile, dump in parsed: + # A justfile sitting exactly on the argument is called without it, as the + # argument would only repeat where it already is. + argc = 0 if justfile.parent.resolve() == directory else 1 + recipe = implements(dump, command, argc) + # These justfiles are nested, so a recipe that was seen already is one this + # one merely imported, and the nearest spelling of it has been taken. + # + # Two repositories that each define a root recipe are not that case: the text + # can match while the workspace, the tool it runs and the paths it excludes all + # differ, so they have to stay apart. Nothing here says so. They are told apart + # only by whatever the two happened not to write identically, which today is a + # doc comment on one of them, so dropping a field from this comparison silently + # discards an invocation unless a real discriminator arrives in the same change. + # `TestFindJustfilesAbove` holds both shapes. + if recipe is not None and recipe not in seen: + seen.append(recipe) + found.append((justfile, recipe)) + return found, failed + + +def find_justfiles_below(command, directory, covered=()): + """Search down a directory for justfiles implementing the command. + + A justfile is skipped when the recipe it would run is one an enclosing directory + already contributes, which is what `import` makes happen: the recipe is the same + job, so running it once is enough. `covered` holds the recipes already found above + the directory. + + Returns the justfiles to run and, separately, the ones that implement the command + but ask to be named rather than found. + """ + # The justfile at `directory` is covered by the search above it. + justfiles, failed = find_justfiles(directory) + candidates = sorted(justfiles - {Path(directory) / "justfile"}) + matches = [] + opted_out = [] + parsed, dump_failed = dump_all(candidates) + failed = failed or dump_failed + for justfile, dump in parsed: + recipe = implements(dump, command, 0) + if recipe is None: + continue + if opts_out(dump, command): + opted_out.append(justfile) + else: + matches.append((justfile, recipe)) + contributed = {Path(directory): list(covered)} + found = [] + # Shallowest first, so that an enclosing justfile is always decided before the ones + # it may account for. + for justfile, recipe in sorted(matches, key=lambda match: len(match[0].parts)): + if any(recipe in contributed.get(p, []) for p in justfile.parent.parents): + continue + contributed.setdefault(justfile.parent, []).append(recipe) + found.append((justfile, recipe)) + return sorted(found, key=lambda match: match[0]), sorted(opted_out), failed + + +def resolve(command, arg): + """Find the justfiles implementing a command for an argument. + + Returns a list of (justfile, argument, recipe) triples, from both above and below + the argument. One found above gets the argument itself, as that selects what to act + on. One found below gets its own directory instead, as there the argument only said + where to look. Justfiles below that asked to be named are returned separately. + """ + above, failed = find_justfiles_above(command, arg) + resolved = [(justfile, arg, recipe) for justfile, recipe in above] + opted_out = [] + if os.path.isdir(arg): + below, opted_out, below_failed = find_justfiles_below( + command, arg, [recipe for _, recipe in above] + ) + failed = failed or below_failed + resolved += [ + (justfile, str(justfile.parent), recipe) for justfile, recipe in below + ] + return resolved, opted_out, failed + + +def report_opted_out(command, justfiles, *, ran): + """Name the justfiles a command passed over because they ask to be named. + + Worth saying even when other recipes did run, as otherwise a command that looks + like it covered a whole directory quietly left parts of it alone. That case is + informational and goes to stdout with the rest of the account of what ran: the + command did what was asked of it. Only matching nothing at all is an error. + """ + if not justfiles: + return + directories = sorted(str(jf.parent) for jf in set(justfiles)) + # One per line: there can be dozens, and a single wrapped line is unreadable. + listed = "\n".join(f" {directory}" for directory in directories) + message = f"not run, as {command} must name these explicitly:\n{listed}" + if ran: + print(message) + else: + error(message) + + +def invoke_just(cwd, args): + """Run just with the given arguments.""" + # This process' stdout is block-buffered off a terminal, while the child writes to the + # same descriptor at once: without this the account lands after what it describes. + sys.stdout.flush() + try: + subprocess.run([JUST, *args], check=True, cwd=cwd) + except subprocess.CalledProcessError as e: + return e.returncode + return 0 + + +def invocation_argument_groups(recipe, pos_args): + """Split arguments into what the recipe can be called with at once. + + Every recipe reachable from a verb today is variadic or takes none, so this only + ever yields one group. It is here because the arguments are paths and running the + verb once per path is what a fixed-arity recipe would mean, where passing them + together would fail on arity alone and say nothing useful about why. + """ + if is_variadic(recipe) or len(pos_args) <= 1: + return [pos_args] + return [[arg] for arg in pos_args] + + +def forward(cmd, args): + """Forward a command to language-specific justfiles.""" + is_non_positional = re.compile(r"^(-.*|\+|[A-Z_][A-Z_0-9]*=.*)$") + flags = [arg for arg in args if is_non_positional.match(arg)] + positional_args = [arg for arg in args if not is_non_positional.match(arg)] + + justfiles = {} + opted_out = [] + resolution_failed = False + for arg in positional_args or ["."]: + resolved, skipped, failed = resolve(cmd, arg) + opted_out += skipped + resolution_failed = resolution_failed or failed + if not resolved: + # A candidate that could not be read is reported below rather than here: + # saying nothing matched would blame the argument for a broken justfile. + if failed: + continue + error(f"No justfile found for {cmd} on {arg}") + report_opted_out(cmd, skipped, ran=False) + return 1 + for justfile, justfile_arg, recipe in resolved: + justfiles.setdefault(justfile, (recipe, []))[1].append(justfile_arg) + + if resolution_failed: + report_opted_out(cmd, opted_out, ran=False) + return 1 + + invocations = [] + for justfile, (recipe, pos_args) in justfiles.items(): + # An argument standing for the whole directory subsumes any more specific one + # that ended up on the same justfile. + whole_directory = str(justfile.parent) + if whole_directory in pos_args: + pos_args = [whole_directory] + for group in invocation_argument_groups(recipe, pos_args): + cwd, just_args = get_just_context(justfile, recipe["name"], flags, group) + prefix = f"cd {cwd}; " if cwd else "" + print(f"-> {prefix}just {' '.join(just_args)}") + invocations.append((cwd, just_args)) + + report_opted_out(cmd, opted_out, ran=True) + + for cwd, just_args in invocations: + if invoke_just(cwd, just_args) != 0: + # Say which one, as a verb can fan out over a great many directories. + where = f" in {cwd}" if cwd else "" + error(f"{cmd} failed{where}: just {' '.join(just_args)}") + return 1 + return 0 + + +def main(): + argv = sys.argv[1:] + if not argv: + error("No command provided") + return 1 + return forward(argv[0], argv[1:]) + + +if __name__ == "__main__": + try: + sys.exit(main()) + except KeyboardInterrupt: + sys.exit(128 + 2) diff --git a/misc/just/justfile b/misc/just/justfile new file mode 100644 index 000000000000..29f9659fe863 --- /dev/null +++ b/misc/just/justfile @@ -0,0 +1,15 @@ +import "defs.just" + +[no-cd] +[positional-arguments] +format *ARGS=".": + npx prettier --write "$@" + +# Test the tooling in this directory. +# Arguments are `unittest` ones, so a test file or a dotted test name rather than a +# directory. A path given stays valid as this does not change directory: `PYTHONPATH` is +# what lets the tests import the modules they are about, wherever they are run from. +[no-cd] +[positional-arguments] +@test *ARGS=['discover', '-s', source_dir()]: + PYTHONPATH="{{ source_dir() }}" {{ py }} -m unittest "$@" diff --git a/misc/just/language_tests.py b/misc/just/language_tests.py new file mode 100755 index 000000000000..ab6d4bd5a148 --- /dev/null +++ b/misc/just/language_tests.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +"""Run a whole language test suite for CI. + +Called from just recipes as: + python3 language_tests.py ROOT [ARG...] + +Arguments are already split by `just` (see `set lists`). Only the first locates a +justfile: its `test` recipe is run once, and every argument after it is handed to that +one recipe rather than visited in turn. Roots wanting different `test` recipes therefore +cannot be run together. +""" + +import os +import subprocess +import sys +from pathlib import Path + + +def main(): + argv = sys.argv[1:] + if not argv: + print("Usage: language_tests.py ROOT [ARG...]", file=sys.stderr) + return 1 + + semmle_code = Path(os.environ["SEMMLE_CODE"]) + # Test roots are absolute, as justfiles build them from `source_dir()`. We run from + # the internal checkout, so relativize them there to keep command lines readable. + # Anything else (flags, environment assignments, relative paths) is passed verbatim. + args = [ + os.path.relpath(arg, semmle_code) if os.path.isabs(arg) else arg for arg in argv + ] + + just = os.environ.get("JUST_EXECUTABLE", "just") + + # Find the nearest justfile at or above the first root + justfile_dir = Path(args[0]) + while not (semmle_code / justfile_dir / "justfile").exists(): + parent = justfile_dir.parent + if parent == justfile_dir: + print(f"No justfile found above {args[0]}", file=sys.stderr) + return 1 + justfile_dir = parent + + invocation = [ + just, + "--justfile", + str(justfile_dir / "justfile"), + "test", + "--all-checks", + "--codeql=built", + *args, + ] + + print(f"-> just {' '.join(invocation[1:])}") + try: + subprocess.run(invocation, check=True, cwd=semmle_code) + except subprocess.CalledProcessError as e: + return e.returncode + return 0 + + +if __name__ == "__main__": + try: + sys.exit(main()) + except KeyboardInterrupt: + sys.exit(128 + 2) diff --git a/misc/just/lib.just b/misc/just/lib.just new file mode 100644 index 000000000000..84389e84b1d3 --- /dev/null +++ b/misc/just/lib.just @@ -0,0 +1,31 @@ +# Helper recipes + +import "build.just" +import "format.just" + +# Run language tests for LANGUAGE. +# +# `--extra-check=` offers a check without running it, and `--all-checks` or `+` runs the +# offered ones. Per-language justfiles supply the offers, which is how the extra checks +# CI runs stay next to the language they belong to. +[no-cd] +[no-exit-message] +[positional-arguments] +@_codeql_test LANGUAGE *ARGS: + {{ py }} "{{ source_dir() }}/codeql_test_run.py" "$@" + +# Run a whole language test suite. The first argument must be a test root. This is +# intended to be called by CI +[no-cd] +[no-exit-message] +[positional-arguments] +@_language_tests *ARGS: _require_semmle_code + {{ py }} "{{ source_dir() }}/language_tests.py" "$@" + +# Run integration tests. Requires an internal repository checkout +[no-cd] +[no-exit-message] +[positional-arguments] +@_integration_test *ARGS: _require_semmle_code + echo "$CMD_BEGIN-> $SEMMLE_CODE/tools/pytest --codeql=build-as-test $*$CMD_END" >&2 + "$SEMMLE_CODE/tools/pytest" --codeql=build-as-test "$@" diff --git a/misc/just/run_on_files.py b/misc/just/run_on_files.py new file mode 100644 index 000000000000..dd77fcb7db2d --- /dev/null +++ b/misc/just/run_on_files.py @@ -0,0 +1,266 @@ +"""Run a command on the files matching the given patterns below the given paths. + +This is a portable `find ... -name -exec {} +`. It exists +because `find` is an unrelated program on Windows, and because a shell command +substitution splits the file names it produces on whitespace, which mangles the many +paths in this repository that contain spaces. + +The command is run once per batch of file names rather than once per file, and the +batches are sized so that no single command line runs into a length limit. Nothing is +run at all when no file matches, and nothing is announced either, so silence means that +nothing here matched rather than that nothing was there: a path that does not exist is +refused instead, naming one being an assertion that it does. +""" + +import argparse +import os +import re +import subprocess +import sys +from fnmatch import fnmatch +from pathlib import Path + +def batch_limit(): + """How many characters of file names to put on one command line. + + Windows caps a whole command line at 32767 characters. Elsewhere the cap is + `ARG_MAX`, which the environment is counted against as well, so that is taken off + along with some slack. This is worth doing rather than assuming the tightest of the + two: `ARG_MAX` is 2MB on Linux, which turns the couple of thousand QL files of a + language into a single invocation rather than several. + + A single argument is capped far lower than the whole line, at 128KB on Linux, and a + command that hands its arguments on through a shell arrives as one of them. Batches + are kept below that too, as the resulting failure is reported by whatever did the + handing on rather than by anything naming this file. + """ + if sys.platform == "win32": + return 30000 + single_argument = 100000 + try: + arg_max = os.sysconf("SC_ARG_MAX") + except (ValueError, OSError): + return 30000 + environment = sum(len(name) + len(value) + 2 for name, value in os.environ.items()) + return max(4096, min(arg_max - environment - 4096, single_argument)) + + +def files_under(paths, patterns, excludes=(), absolute=False, within=None): + """Collect the files matching one of the patterns at or below each path. + + Patterns are matched against the file name, as bazel files are identified by name + rather than by extension. Exclusions are matched against the whole path instead, + which is how a directory of generated files is left alone. Both the path the walk + built and its resolved form are tried, as the walk only ever extends the path it + was given: walking `cpp` from inside a directory builds nothing naming that + directory, so an exclusion naming it could never match. An exclusion that has to + hold however the path was reached is therefore anchored absolutely, and resolving + is what makes that spelling hold for a path reached through a symbolic link too. + + A `within` directory bounds the result to the files below it, for a command that + answers for one project and may be handed a path reaching outside it. + + Symbolic links are not followed, which is what keeps the `bazel-*` convenience + links out of the walk. + + Returns the file names, and the given paths that yielded one. The two differ + whenever a path is excluded or simply holds nothing matching, and telling them + apart is what lets the banner name the paths being acted on rather than the paths + that were asked about. + """ + boundary = Path(within).resolve() if within else None + + def wanted(path): + # The name decides most files and costs nothing, so it is asked first: resolving + # is a system call, and is only owed for a file that could still be collected. + if not any(fnmatch(path.name, p) for p in patterns): + return False + if boundary is None and not excludes: + return True + resolved = path.resolve() + if boundary is not None and not resolved.is_relative_to(boundary): + return False + # A relative pattern is anchored at the start, so it cannot match the resolved + # spelling: trying both only ever gives a pattern the reach it was written with. + spellings = (str(path), str(resolved)) + return not any( + fnmatch(spelling, e) for e in excludes for spelling in spellings + ) + + def collect(path): + if path.is_file(): + return {path} if wanted(path) else set() + return { + file + for directory, _, names in os.walk(path) + for file in map(Path(directory).joinpath, names) + if wanted(file) + } + + # Kept per path rather than in one set, as which path a file came from is not a + # question the collected names can be asked afterwards without resolving each of + # them again. A file reached by two paths still only appears once below. + contributed = {} + for given in paths: + collected = collect(Path(given)) + if collected: + # Keyed by the spelling that was given rather than a normalised one, as + # this goes back to whoever wrote it and is theirs to recognise. + contributed[str(given)] = collected + files = sorted( + os.path.abspath(file) if absolute else str(file) + for file in set().union(*contributed.values()) + ) + return files, list(contributed) + + +def batched(files, limit): + """Split file names into groups that each fit on one command line.""" + batch, length = [], 0 + for file in files: + if batch and length + len(file) + 1 > limit: + yield batch + batch, length = [], 0 + batch.append(file) + length += len(file) + 1 + if batch: + yield batch + + +def comma_separated(value): + """Split an option value listing several patterns. + + Patterns tend to come in groups, and a justfile passes them as one variable, so they + are spelled as one argument here rather than repeated. Repeating the option works + too, which is what lets a list be extended rather than restated. + """ + return value.split(",") + + +def parse_args(): + """Work out what to run, on which files, and what to hide of what it says.""" + parser = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + usage="%(prog)s [option...] [,...] " + " [...] -- [...]", + ) + parser.add_argument( + "--exclude", + action="extend", + default=[], + type=comma_separated, + metavar="[,...]", + help="leave out files whose path matches, repeatable", + ) + parser.add_argument( + "--absolute", + action="store_true", + help="pass absolute file names, needed when the command runs elsewhere", + ) + parser.add_argument( + "--chdir", + metavar="", + help="run the command from here, for one that must be run from a project root", + ) + parser.add_argument( + "--within", + metavar="", + help="leave out files outside this directory, for a command answering for one " + "project that may be handed a path reaching beyond it", + ) + parser.add_argument( + "--drop", + action="append", + default=[], + metavar="", + help="hide matching lines of the command's output, repeatable", + ) + parser.add_argument( + "patterns", + metavar="[,...]", + type=comma_separated, + help="what to match file names against", + ) + parser.add_argument( + "rest", + nargs=argparse.REMAINDER, + metavar=" [...] -- [...]", + help="the command, then the paths to search, separated by the last `--` so " + "that the command may contain one of its own", + ) + args = parser.parse_args() + if "--" not in args.rest: + parser.error("the paths must be separated from the command by `--`") + separator = len(args.rest) - 1 - args.rest[::-1].index("--") + args.command, args.paths = args.rest[:separator], args.rest[separator + 1 :] + if not args.command: + parser.error("no command given") + missing = [path for path in args.paths if not os.path.exists(path)] + if missing: + parser.error("no such path: " + ", ".join(missing)) + return args + + +def run(command, drops, chdir=None): + """Run the command, hiding the lines of its output that were asked to be hidden. + + Told nothing to hide, the command keeps this process' own output streams, so that + it can do as it likes with them. Otherwise its diagnostics are read a line at a + time and passed on as they arrive, which is what keeps a long run's progress + visible. Only what was named is hidden, so an unforeseen message still gets out. + + Note that these tools report on their progress over standard error rather than + standard output, which is left alone here. + """ + if not drops: + return subprocess.run(command, cwd=chdir).returncode + hidden = re.compile("|".join(drops)) + process = subprocess.Popen( + command, cwd=chdir, stderr=subprocess.PIPE, text=True, bufsize=1 + ) + for line in process.stderr: + if not hidden.search(line): + sys.stderr.write(line) + sys.stderr.flush() + return process.wait() + + +def banner(command, paths): + """Announce a command over the paths it turned out to have something to do in. + + Only the paths that yielded a file are named: one whose files were all excluded is + not being acted on, and naming it claims work that is not about to happen. The file + names are left out, there being thousands of them and the paths being what was + asked for. + + So this is a report rather than something to paste, the collecting being the whole + point. `just -n` prints what really runs. + + `CMD_BEGIN` and `CMD_END` are the rules the justfiles put around a command; with + neither set this is a plain line. + """ + begin = os.environ.get("CMD_BEGIN", "") + end = os.environ.get("CMD_END", "") + return f"{begin}-> {' '.join(command)} -- {' '.join(paths)}{end}" + + +def main(): + args = parse_args() + files, contributing = files_under( + args.paths, args.patterns, args.exclude, args.absolute, args.within + ) + if files: + # Neither half of this is known where the caller would have to say it: whether + # anything is going to run at all, and which of the paths it named hold any of + # it. + print(banner(args.command, contributing), file=sys.stderr, flush=True) + limit = batch_limit() - sum(len(argument) + 1 for argument in args.command) + status = 0 + for batch in batched(files, limit): + status = run([*args.command, *batch], args.drop, args.chdir) or status + return status + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/misc/just/semmle-code-stub.just b/misc/just/semmle-code-stub.just new file mode 100644 index 000000000000..14733ffb648e --- /dev/null +++ b/misc/just/semmle-code-stub.just @@ -0,0 +1 @@ +export SEMMLE_CODE := "" diff --git a/misc/just/test_codeql_test_run.py b/misc/just/test_codeql_test_run.py new file mode 100644 index 000000000000..acb88ac11db6 --- /dev/null +++ b/misc/just/test_codeql_test_run.py @@ -0,0 +1,223 @@ +#!/usr/bin/env python3 +"""Tests for `codeql_test_run.py`. + +Sorting arguments is the whole of what this file decides, and it decides it by looking +at each one: a word is a test, a `-` is a flag, `NAME=value` is an environment +assignment. Several of these pin down that an argument arrives whole, spaces and all, +which is what taking them as a list rather than re-splitting a string bought. + +Sorting and resolving are tested at different levels because they happen at different +levels. `parse_arguments` only sorts; a setting is not resolved until `main` has merged +the assignments into the environment, so anything about precedence is observed there. +""" + +import os +import sys +import unittest +from unittest import mock + +import codeql_test_run + + +def sorted_args(*argv, semmle_code=None): + """Sort a command line, supplying the language that always precedes it.""" + with ( + mock.patch.object(codeql_test_run, "SEMMLE_CODE", semmle_code), + mock.patch.object(sys, "argv", ["codeql_test_run.py", "alanguage", *argv]), + ): + return codeql_test_run.parse_arguments() + + +def run_main(*argv, environ=None): + """Run `main` with the executable and the child process stubbed out. + + Returns the flags and tests handed to `codeql test run`, and the environment the + child would have been given. + """ + codeql = mock.MagicMock() + codeql.exists.return_value = True + with ( + mock.patch.object(codeql_test_run, "SEMMLE_CODE", None), + mock.patch.object(sys, "argv", ["codeql_test_run.py", "alanguage", *argv]), + mock.patch.dict(os.environ, environ or {}, clear=True), + mock.patch.object(codeql_test_run, "resolve_codeql", return_value=codeql), + mock.patch.object(codeql_test_run, "invoke", return_value=0) as invoke, + ): + codeql_test_run.main() + (invocation,) = invoke.call_args.args + separator = invocation.index("--") + # Past the executable and `test run`, up to the separator `main` adds itself. + return invocation[3:separator], invocation[separator + 1 :], dict(os.environ) + + +def flags(*argv, environ=None): + return run_main(*argv, environ=environ)[0] + + +def paths(*argv, environ=None): + return run_main(*argv, environ=environ)[1] + + +def child_environ(*argv, environ=None): + return run_main(*argv, environ=environ)[2] + + +class TestParseArgs(unittest.TestCase): + def test_a_plain_word_is_a_test(self): + self.assertEqual(sorted_args("ql/test/Foo").tests, ["ql/test/Foo"]) + + def test_a_dash_is_a_flag(self): + self.assertEqual( + sorted_args("--fail-on-trap-errors").flags, ["--fail-on-trap-errors"] + ) + + def test_an_uppercase_assignment_is_an_environment_variable(self): + self.assertEqual(sorted_args("CPUS=4").env, {"CPUS": "4"}) + + def test_a_lowercase_assignment_is_a_test(self): + # Only shouting counts, so a path that happens to contain `=` stays a path. + self.assertEqual(sorted_args("dir/a=b").tests, ["dir/a=b"]) + + def test_codeql_selects_the_executable(self): + # `built` is rejected outright without an internal checkout to build in, so + # this says what it means to sort the option, not to act on it. + args = sorted_args("--codeql=built", semmle_code="/somewhere") + self.assertEqual(args.codeql, "built") + + def test_the_last_codeql_wins(self): + args = sorted_args("--codeql=host", "--codeql=built", semmle_code="/somewhere") + self.assertEqual(args.codeql, "built") + + def test_all_checks_is_asked_for_by_either_spelling(self): + self.assertTrue(sorted_args("--all-checks").all) + self.assertTrue(sorted_args("+").all) + + def test_an_extra_check_is_held_back_until_it_is_asked_for(self): + held = sorted_args("--extra-check=--check-databases") + self.assertEqual(held.extra_checks, ["--check-databases"]) + # Held back means held back: it is not a flag until `--all-checks` arrives. + self.assertEqual(held.flags, []) + self.assertFalse(held.all) + + def test_a_double_dash_hands_everything_after_it_to_codeql(self): + # Standard `--`: past it, an option is the caller's business and not ours. The + # separator itself is ours, though, so it is not passed on as well. + args = sorted_args("--", "--codeql=built") + self.assertEqual(args.codeql, "host") + self.assertIn("--codeql=built", args.flags) + self.assertNotIn("--", args.flags) + + def test_a_test_path_containing_a_space_stays_one_argument(self): + self.assertEqual(sorted_args("some dir/test").tests, ["some dir/test"]) + + def test_an_assignment_whose_value_contains_a_space_stays_whole(self): + """The value is the point, and a split one used to arrive as three characters. + + An argument list carries this; a whitespace-separated string cannot, as there + is nothing left in it to tell a separator from part of a value. + """ + self.assertEqual(sorted_args("EXTRA=a b").env, {"EXTRA": "a b"}) + + def test_sorts_a_whole_command_line_at_once(self): + args = sorted_args( + "-j2", "CPUS=4", "ql/test", "+", "--extra-check=--check-diff" + ) + self.assertEqual(args.flags, ["-j2"]) + self.assertEqual(args.env, {"CPUS": "4"}) + self.assertEqual(args.tests, ["ql/test"]) + self.assertEqual(args.extra_checks, ["--check-diff"]) + self.assertTrue(args.all) + + +class TestOfferedChecks(unittest.TestCase): + """What a root offers and what `--all-checks` enables are separate things. + + These go through `main` because that is where the two meet. `--all-checks` is + injected on every language test run rather than typed, so it means "enable whatever + this root offers" and not "I want more coverage": a root offering nothing has to + stay runnable through it. + """ + + def test_an_offered_check_is_applied_when_asked_for(self): + applied = flags("--extra-check=--check-databases", "--all-checks") + self.assertIn("--check-databases", applied) + + def test_an_offered_check_stays_held_back_until_it_is(self): + self.assertNotIn("--check-databases", flags("--extra-check=--check-databases")) + + def test_asking_for_checks_a_root_offers_none_of_enables_nothing(self): + self.assertEqual(paths("--all-checks", "some/test"), ["some/test"]) + self.assertEqual(flags("--all-checks", "some/test"), flags("some/test")) + + def test_a_check_passed_unconditionally_is_not_an_offer(self): + # A root can mean to run a check always rather than put it behind the flag. That + # is a flag like any other here, so it neither becomes an offer nor is withheld + # until the offers are asked for. + always = flags("--check-databases", "some/test") + self.assertIn("--check-databases", always) + self.assertEqual( + flags("--check-databases", "--all-checks", "some/test"), always + ) + + +class TestSettings(unittest.TestCase): + """`RAM_PER_THREAD` and `CPUS` are read back after assignments are applied. + + Resolution is what these pin down, so they go through `main`: an assignment and an + inherited variable only meet once `main` has merged them. + """ + + def test_prefers_a_test_argument(self): + self.assertIn("-j4", flags("CPUS=4", environ={"CPUS": "8"})) + + def test_falls_back_to_the_environment(self): + self.assertIn("-j8", flags(environ={"CPUS": "8"})) + + def test_falls_back_to_the_default(self): + self.assertIn(f"-j{os.cpu_count()}", flags()) + + def test_the_last_assignment_wins(self): + self.assertIn("-j2", flags("CPUS=4", "CPUS=2")) + + def test_an_empty_value_falls_back_to_the_default(self): + """An empty value does not override, so a later one erases an earlier setting. + + Compared against a run that never mentions the setting, so this pins the + behaviour without restating what the default happens to be. It is about + resolution only: see below for what the child is given. + """ + self.assertEqual(flags("CPUS=4", "CPUS="), flags()) + + def test_an_empty_assignment_still_reaches_the_child(self): + """Falling back to the default is not the same as the assignment being dropped. + + `RAM_PER_THREAD` and `CPUS` are read back out of the environment, so an empty + one reads as unset and the default stands. Every assignment is exported either + way, so the child sees the variable set and empty rather than absent, and a + variable this script does not read has no other behaviour to fall back to. + """ + self.assertEqual(child_environ("CPUS=")["CPUS"], "") + self.assertNotIn("CPUS", child_environ()) + + def test_an_assignment_reaches_the_child(self): + self.assertEqual(child_environ("EXTRA=a b")["EXTRA"], "a b") + + def test_ram_is_per_thread(self): + self.assertIn("--ram=200", flags("CPUS=2", "RAM_PER_THREAD=100")) + + +class TestDefaults(unittest.TestCase): + def test_the_current_directory_is_the_default_test(self): + self.assertEqual(paths(), ["."]) + + def test_a_named_test_replaces_the_default(self): + self.assertEqual(paths("ql/test/Foo"), ["ql/test/Foo"]) + + def test_an_offered_check_becomes_a_flag_once_asked_for(self): + self.assertIn( + "--check-databases", flags("--extra-check=--check-databases", "+") + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/misc/just/test_forward_command.py b/misc/just/test_forward_command.py new file mode 100644 index 000000000000..512423389773 --- /dev/null +++ b/misc/just/test_forward_command.py @@ -0,0 +1,553 @@ +#!/usr/bin/env python3 +"""Tests for `forward_command.py`. + +These cover the deciding rather than the running: which justfile answers a verb, with +how many arguments, and which ones ask to be passed over. All of that is read out of +`just --dump`, so the shapes below were taken from what `just` really emits rather than +imagined. That is checked rather than claimed: the last class here dumps a real justfile +and holds the fixtures against it, because a hand-written shape is otherwise only as +good as the day it was written, and agrees with itself long after `just` has moved on. +""" + +import json +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +import forward_command + + +def parameter(name, kind="singular", default=None): + return {"name": name, "kind": kind, "default": default} + + +def recipe(name, parameters=(), dependencies=(), private=False, doc=None): + return { + "name": name, + "private": private, + "doc": doc, + "parameters": list(parameters), + "dependencies": [{"recipe": dependency} for dependency in dependencies], + } + + +def alias(name, target): + return {"attributes": [], "name": name, "target": target} + + +def dump(*recipes, aliases=None, assignments=None): + return { + "recipes": {recipe["name"]: recipe for recipe in recipes}, + "aliases": aliases or {}, + "assignments": assignments or {}, + } + + +def forwarding(command): + """The pair a justfile has when it both forwards a verb and answers it itself.""" + return dump( + recipe( + command, + [parameter("ARGS", kind="star")], + dependencies=[forward_command.FORWARD_RECIPE], + ), + recipe(f"{forward_command.ROOT_PREFIX}{command}", [parameter("ARGS", "star")]), + ) + + +class TestAccepts(unittest.TestCase): + def accepts(self, parameters, argc): + return forward_command.accepts(recipe("r", parameters), argc) + + def test_a_recipe_without_parameters_takes_none(self): + self.assertTrue(self.accepts([], 0)) + self.assertFalse(self.accepts([], 1)) + + def test_a_parameter_without_a_default_must_be_given(self): + self.assertFalse(self.accepts([parameter("X")], 0)) + self.assertTrue(self.accepts([parameter("X")], 1)) + self.assertFalse(self.accepts([parameter("X")], 2)) + + def test_a_defaulted_parameter_may_be_left_out(self): + defaulted = [parameter("X", default=".")] + self.assertTrue(self.accepts(defaulted, 0)) + self.assertTrue(self.accepts(defaulted, 1)) + self.assertFalse(self.accepts(defaulted, 2)) + + def test_a_star_parameter_takes_any_number(self): + star = [parameter("ARGS", kind="star")] + for argc in (0, 1, 7): + self.assertTrue(self.accepts(star, argc), argc) + + def test_a_plus_parameter_takes_at_least_one(self): + plus = [parameter("ARGS", kind="plus")] + self.assertFalse(self.accepts(plus, 0)) + self.assertTrue(self.accepts(plus, 1)) + self.assertTrue(self.accepts(plus, 7)) + + +class TestImplements(unittest.TestCase): + def test_finds_the_recipe_named_after_the_command(self): + found = forward_command.implements(dump(recipe("test")), "test", 0) + self.assertEqual(found["name"], "test") + + def test_follows_an_alias(self): + found = forward_command.implements( + dump(recipe("test"), aliases={"t": alias("t", "test")}), "t", 0 + ) + self.assertEqual(found["name"], "test") + + def test_follows_an_alias_into_a_forwarding_justfile(self): + # The recipe named after the alias forwards, so the answer is the root one -- + # and that is named after the verb, which is the target rather than the alias. + # Reaching here needs the alias to be a verb's own name, the only spelling the + # forwarder ever passes. + found = forward_command.implements( + dump( + recipe("build", dependencies=[forward_command.FORWARD_RECIPE]), + recipe(f"{forward_command.ROOT_PREFIX}build"), + aliases={"format": alias("format", "build")}, + ), + "format", + 0, + ) + self.assertEqual(found["name"], f"{forward_command.ROOT_PREFIX}build") + + def test_does_not_settle_on_a_root_recipe_named_after_the_alias(self): + # `_root_format` exists in most repository roots, so looking the alias up + # unresolved finds a real recipe rather than nothing: the wrong directory's + # answer, run in earnest. The version of this without one returns None, which + # is indistinguishable from a justfile that does not implement the verb at all. + found = forward_command.implements( + dump( + recipe("build", dependencies=[forward_command.FORWARD_RECIPE]), + recipe(f"{forward_command.ROOT_PREFIX}build"), + recipe(f"{forward_command.ROOT_PREFIX}format"), + aliases={"format": alias("format", "build")}, + ), + "format", + 0, + ) + self.assertEqual(found["name"], f"{forward_command.ROOT_PREFIX}build") + + def test_passes_over_a_private_recipe(self): + self.assertIsNone( + forward_command.implements(dump(recipe("test", private=True)), "test", 0) + ) + + def test_passes_over_a_recipe_that_cannot_take_the_arguments(self): + self.assertIsNone(forward_command.implements(dump(recipe("test")), "test", 1)) + + def test_takes_the_root_recipe_when_the_plain_name_forwards(self): + """A forwarder's own name says nothing about what its directory does. + + Settling on it would make the search find itself, so the one name the two can + share is `_root_`. + """ + found = forward_command.implements(forwarding("format"), "format", 1) + self.assertEqual(found["name"], "_root_format") + + def test_finds_nothing_when_a_forwarder_has_no_recipe_of_its_own(self): + forwarder = dump( + recipe( + "format", + [parameter("ARGS", kind="star")], + dependencies=[forward_command.FORWARD_RECIPE], + ) + ) + self.assertIsNone(forward_command.implements(forwarder, "format", 1)) + + +class TestListValue(unittest.TestCase): + def read(self, value): + return forward_command.list_value({"explicit_verbs": {"value": value}}, "explicit_verbs") + + def test_reads_a_list_literal(self): + self.assertEqual(self.read(["list", "test", "build"]), ["test", "build"]) + + def test_reads_an_empty_list_literal(self): + self.assertEqual(self.read(["list"]), []) + + def test_counts_an_expression_as_absent(self): + # What `['a'] ++ ['b']` dumps as. Evaluating it would mean running just. + concatenation = ["list-concatenate", ["list", "a"], ["list", "b"]] + self.assertEqual(self.read(concatenation), []) + + def test_counts_a_plain_string_as_absent(self): + self.assertEqual(self.read("test"), []) + + def test_counts_an_unset_name_as_absent(self): + self.assertEqual(forward_command.list_value({}, "explicit_verbs"), []) + + +class TestOptsOut(unittest.TestCase): + def test_a_listed_verb_asks_to_be_named(self): + listed = dump(assignments={"explicit_verbs": {"value": ["list", "test"]}}) + self.assertTrue(forward_command.opts_out(listed, "test")) + self.assertFalse(forward_command.opts_out(listed, "build")) + + def test_listing_nothing_opts_out_of_nothing(self): + self.assertFalse(forward_command.opts_out(dump(), "test")) + + +class TestGetJustContext(unittest.TestCase): + def test_a_justfile_sitting_on_the_argument_is_run_from_there(self): + # `just build ql/rust` becomes `just build` inside `ql/rust`, as repeating the + # directory it is already in says nothing. + cwd, args = forward_command.get_just_context( + Path("ql/rust/justfile"), "build", [], ["ql/rust"] + ) + self.assertEqual(cwd, "ql/rust") + self.assertEqual(args, ["build"]) + + def test_flags_survive_being_run_from_there(self): + cwd, args = forward_command.get_just_context( + Path("ql/rust/justfile"), "test", ["--all-checks"], ["ql/rust"] + ) + self.assertEqual(cwd, "ql/rust") + self.assertEqual(args, ["test", "--all-checks"]) + + def test_anything_else_names_the_justfile_and_keeps_the_arguments(self): + cwd, args = forward_command.get_just_context( + Path("ql/justfile"), "build", ["-x"], ["ql/rust"] + ) + self.assertIsNone(cwd) + self.assertEqual( + args, ["--justfile", str(Path("ql/justfile")), "build", "-x", "ql/rust"] + ) + + def test_two_arguments_are_both_kept(self): + cwd, args = forward_command.get_just_context( + Path("ql/justfile"), "format", [], ["ql/rust", "ql/cpp"] + ) + self.assertIsNone(cwd) + self.assertEqual(args[-2:], ["ql/rust", "ql/cpp"]) + + +class TestInvocationPath(unittest.TestCase): + def test_spells_a_path_relatively_when_the_argument_was(self): + found = Path.cwd() / "sub" / "justfile" + self.assertEqual( + forward_command.invocation_path(found, like="sub"), Path("sub/justfile") + ) + + def test_leaves_a_path_absolute_when_the_argument_was(self): + found = Path.cwd() / "sub" / "justfile" + self.assertEqual( + forward_command.invocation_path(found, like=str(Path.cwd())), found + ) + + +class TestFindJustfilesAbove(unittest.TestCase): + """The two shapes a consuming root can have, both of which the README promises. + + A root that imports the justfile defining a `_root_` inherits it, and the verb + is then reached twice under two spellings of one recipe, which has to run once. A + root that defines its own replaces it, and both have to run, each over the files of + the repository defining it. No dump says which of the two happened, so they are told + apart by comparing the recipes themselves, and the fields that carry the difference + are whatever the two repositories happened not to write identically. + + That makes a verbatim copy of one root recipe into another indistinguishable from + inheritance, comment included, and a comment is the part of a recipe most likely to + survive the paste that produces this. + """ + + def setUp(self): + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + root = Path(directory.name).resolve() + self.outer = root / "justfile" + self.inner = root / "inner" / "justfile" + self.argument = root / "inner" / "below" + self.argument.mkdir(parents=True) + for justfile in (self.outer, self.inner): + justfile.write_text("") + + def implementing(self, doc=None, dependencies=()): + """A justfile that forwards `format` and answers it itself.""" + return dump( + recipe( + "format", + [parameter("ARGS", "star")], + dependencies=[forward_command.FORWARD_RECIPE], + ), + recipe( + f"{forward_command.ROOT_PREFIX}format", + [parameter("ARGS", "star")], + dependencies=dependencies, + doc=doc, + ), + ) + + def found(self, outer, inner): + dumps = {self.outer: outer, self.inner: inner} + with mock.patch.object( + forward_command, + "dump_justfile", + side_effect=lambda justfile: (dumps[Path(justfile)], None), + ): + found, failed = forward_command.find_justfiles_above( + "format", str(self.argument) + ) + self.assertFalse(failed) + return found + + def test_a_recipe_reached_under_two_spellings_runs_once(self): + found = self.found(self.implementing(), self.implementing()) + self.assertEqual([justfile for justfile, _ in found], [self.inner]) + + def test_two_roots_differing_only_in_their_comment_both_run(self): + found = self.found(self.implementing(), self.implementing(doc="From here.")) + self.assertEqual([justfile for justfile, _ in found], [self.inner, self.outer]) + + def test_two_roots_sharing_a_comment_still_both_run_if_they_do_different_work(self): + found = self.found( + self.implementing(doc="From here.", dependencies=["_format_other"]), + self.implementing(doc="From here."), + ) + self.assertEqual([justfile for justfile, _ in found], [self.inner, self.outer]) + + +class TestDiscoveryFailures(unittest.TestCase): + def test_dump_all_reports_every_failure(self): + justfiles = [Path("one/justfile"), Path("two/justfile"), Path("three/justfile")] + dumps = [(None, "bad one"), (dump(), None), (None, "bad three")] + with mock.patch.object( + forward_command, "dump_justfile", side_effect=dumps + ), mock.patch.object(forward_command, "error") as report: + parsed, failed = forward_command.dump_all(justfiles) + self.assertTrue(failed) + self.assertEqual(parsed, [(Path("two/justfile"), dump())]) + self.assertEqual(report.call_count, 2) + + def test_git_failure_is_reported_to_callers(self): + result = subprocess.CompletedProcess( + ["git"], 128, stdout="", stderr="fatal: not a repository" + ) + with mock.patch.object( + subprocess, "run", return_value=result + ), mock.patch.object(forward_command, "error"): + lines, failed = forward_command.git(".", "ls-files") + self.assertEqual(lines, []) + self.assertTrue(failed) + + def test_forward_fails_without_running_after_resolution_failure(self): + justfile = Path("pkg/justfile") + resolved = [(justfile, "pkg", recipe("test", [parameter("ARGS", "star")]))] + with mock.patch.object( + forward_command, "resolve", return_value=(resolved, [], True) + ), mock.patch.object(forward_command, "invoke_just") as invoke: + self.assertEqual(forward_command.forward("test", ["pkg"]), 1) + invoke.assert_not_called() + + +class TestForwardInvocations(unittest.TestCase): + def test_accumulates_variadic_recipe_arguments(self): + justfile = Path("pkg/justfile") + test_recipe = recipe("test", [parameter("ARGS", "star")]) + + def resolve(command, arg): + return [(justfile, arg, test_recipe)], [], False + + with mock.patch.object( + forward_command, "resolve", side_effect=resolve + ), mock.patch.object(forward_command, "invoke_just", return_value=0) as invoke: + self.assertEqual(forward_command.forward("test", ["pkg/a", "pkg/b"]), 0) + + invoke.assert_called_once_with( + None, ["--justfile", "pkg/justfile", "test", "pkg/a", "pkg/b"] + ) + + def test_splits_non_variadic_recipe_arguments(self): + justfile = Path("pkg/justfile") + test_recipe = recipe("test", [parameter("ARG")]) + + def resolve(command, arg): + return [(justfile, arg, test_recipe)], [], False + + with mock.patch.object( + forward_command, "resolve", side_effect=resolve + ), mock.patch.object(forward_command, "invoke_just", return_value=0) as invoke: + self.assertEqual(forward_command.forward("test", ["pkg/a", "pkg/b"]), 0) + + self.assertEqual( + invoke.call_args_list, + [ + mock.call(None, ["--justfile", "pkg/justfile", "test", "pkg/a"]), + mock.call(None, ["--justfile", "pkg/justfile", "test", "pkg/b"]), + ], + ) + + +# Resolve the same binary `forward_command` will run: it honours JUST_EXECUTABLE, so a +# pinned `just` would otherwise have these fixtures checked against a different binary +# than the code uses. `which` covers both spellings, returning an explicit path as given +# and looking a bare name up on PATH. +JUST = shutil.which(forward_command.JUST) + +# The justfile below uses every construct the fixtures above model, so that a dump of it +# can be checked against them. +CONSTRUCTS = """ +set unstable +set lists + +alias t := test + +explicit_verbs := ['test'] + +test *ARGS='.': _helper + echo {{ ARGS }} + +# A comment above a recipe becomes its doc. +build X Y='y': + echo {{ X }} {{ Y }} + +lint +ARGS: + echo {{ ARGS }} + +[private] +_helper: + echo helper +""" + + +@unittest.skipUnless(JUST, "needs `just` on PATH") +class TestFixturesStillMatchJust(unittest.TestCase): + """Check the fixtures above against what `just` really dumps. + + Everything else here reads a shape written by hand, which is only as good as the + day it was written: `just` changed how it dumps an alias once already, and the test + covering aliases went on passing against the shape that had gone away. A fixture + cannot notice that on its own, so this asks the real thing. + + Only the fields the code reads are compared. `just` is free to dump more, and a + test that failed whenever it did would be noise rather than a warning. + """ + + @classmethod + def setUpClass(cls): + with tempfile.TemporaryDirectory() as directory: + justfile = Path(directory) / "justfile" + justfile.write_text(CONSTRUCTS) + dumped = subprocess.run( + [JUST, "--justfile", str(justfile), "--dump", "--dump-format", "json"], + capture_output=True, + text=True, + check=True, + ) + cls.dump = json.loads(dumped.stdout) + + def test_a_dump_is_read_by_the_keys_the_fixtures_use(self): + self.assertLessEqual(set(dump()), set(self.dump)) + + def test_an_alias_names_its_target(self): + real = self.dump["aliases"]["t"] + self.assertEqual(set(alias("t", "test")), set(real)) + self.assertEqual(real["target"], "test") + + def test_a_recipe_is_read_by_the_keys_the_fixtures_use(self): + self.assertLessEqual(set(recipe("test")), set(self.dump["recipes"]["test"])) + + def test_a_comment_above_a_recipe_is_the_doc_the_comparison_reads(self): + # Two root recipes doing different jobs are often told apart by this alone. + recipes = self.dump["recipes"] + self.assertEqual( + recipes["build"]["doc"], "A comment above a recipe becomes its doc." + ) + self.assertIsNone(recipes["test"]["doc"]) + + def test_a_private_recipe_says_so(self): + self.assertIs(self.dump["recipes"]["_helper"]["private"], True) + self.assertIs(self.dump["recipes"]["test"]["private"], False) + + def test_a_dependency_names_its_recipe(self): + dependencies = self.dump["recipes"]["test"]["dependencies"] + self.assertEqual([d["recipe"] for d in dependencies], ["_helper"]) + + def test_parameters_keep_the_kinds_and_defaults_accepts_reads(self): + kinds = { + name: [(p["kind"], p["default"]) for p in recipe["parameters"]] + for name, recipe in self.dump["recipes"].items() + } + self.assertEqual(kinds["test"], [("star", ".")]) + self.assertEqual(kinds["build"], [("singular", None), ("singular", "y")]) + self.assertEqual(kinds["lint"], [("plus", None)]) + self.assertEqual(kinds["_helper"], []) + + def test_a_list_assignment_is_the_shape_list_value_unwraps(self): + self.assertEqual( + forward_command.list_value(self.dump["assignments"], "explicit_verbs"), + ["test"], + ) + + +@unittest.skipUnless(JUST, "needs `just` on PATH") +class TestImportingDoesNotChangeARecipe(unittest.TestCase): + """Ask `just` for the premise the deduplication rests on rather than assuming it. + + A root importing the justfile that defines a `_root_` reaches one recipe under + two spellings, and it is told apart from two repositories each defining their own by + comparing the recipes whole. That comparison reads every field `just` emits, not the + few the rest of this file models, so a release adding a per-recipe field that varies + between justfiles -- a source path, a line number, anything saying where a recipe was + written -- would stop the two spellings comparing equal and run an inherited recipe + twice. + + Such a field is precisely the discriminator this code would otherwise want, so it + would arrive looking like a feature. `namepath`, the closest thing to one today, is + a module path rather than a file one, and so reads identically for two top-level + recipes. The fixtures above cannot see any of this: they are compared as subsets, + which is right for reading a field by name and blind to a field nobody thought to + model. Nor would a repository notice, since defining a recipe of one's own only + moves it further from the imported one: the shape that would start running twice is + the one built here and kept nowhere else. + """ + + def recipes(self, justfile): + dumped = subprocess.run( + [JUST, "--justfile", str(justfile), "--dump", "--dump-format", "json"], + capture_output=True, + text=True, + check=True, + ) + return json.loads(dumped.stdout)["recipes"] + + def setUp(self): + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + root = Path(directory.name) + (root / "inner").mkdir() + self.inner = root / "inner" / "justfile" + self.inner.write_text("# Shared.\n_root_format:\n echo shared\n") + self.outer = root / "justfile" + + def test_an_imported_recipe_is_the_one_it_came_from(self): + self.outer.write_text("import 'inner/justfile'\n") + self.assertEqual( + self.recipes(self.outer)["_root_format"], + self.recipes(self.inner)["_root_format"], + ) + + def test_a_root_defining_its_own_is_not(self): + # Same body, so only the comment separates them: the narrowest the difference + # between the two shapes ever gets. + self.outer.write_text( + "set allow-duplicate-recipes\n" + "import 'inner/justfile'\n" + "\n" + "# Mine.\n" + "_root_format:\n" + " echo shared\n" + ) + self.assertNotEqual( + self.recipes(self.outer)["_root_format"], + self.recipes(self.inner)["_root_format"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/misc/just/test_language_tests.py b/misc/just/test_language_tests.py new file mode 100644 index 000000000000..b2e93a28306c --- /dev/null +++ b/misc/just/test_language_tests.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +"""Tests for `language_tests.py`. + +`just` is stubbed out here: what this file decides is the invocation, and running it +needs a built CLI and a whole test suite. The invocation is also where the interesting +property lives, as an argument has to reach the suite exactly as it was written. +""" + +import contextlib +import io +import os +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +import language_tests + + +class TestMain(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + # Resolved once: macOS puts temporary directories behind a symbolic link, and + # an unresolved root would not be a prefix of the paths built from it. + self.semmle_code = Path(temporary.name).resolve() + self.suite = self.semmle_code / "ql" / "rust" / "ql" / "test" + self.suite.mkdir(parents=True) + (self.semmle_code / "ql" / "rust" / "justfile").touch() + + def run_main(self, *argv, environment=None, side_effect=None): + """Run `main` with `just` stubbed out, returning its status and that stub.""" + env = {"SEMMLE_CODE": str(self.semmle_code), "JUST_EXECUTABLE": "just"} + env.update(environment or {}) + printed = io.StringIO() + with ( + mock.patch.object( + language_tests.sys, "argv", ["language_tests.py", *argv] + ), + mock.patch.dict(os.environ, env), + mock.patch.object( + language_tests.subprocess, "run", side_effect=side_effect + ) as run, + contextlib.redirect_stdout(printed), + # The messages it writes here are expected by the tests below, and reading + # them among the results would suggest something had gone wrong. + contextlib.redirect_stderr(io.StringIO()), + ): + status = language_tests.main() + return status, run, printed.getvalue() + + def invocation(self, *argv, **kwargs): + status, run, _ = self.run_main(*argv, **kwargs) + self.assertEqual(status, 0) + return list(run.call_args.args[0]) + + def test_relativizes_an_absolute_root_against_the_checkout(self): + # Roots are absolute because a justfile builds them from `source_dir()`, and + # the command line is read by people. + self.assertEqual( + self.invocation(str(self.suite))[-1], + os.path.join("ql", "rust", "ql", "test"), + ) + + def test_finds_the_nearest_justfile_above_the_root(self): + invocation = self.invocation(str(self.suite)) + self.assertEqual( + invocation[invocation.index("--justfile") + 1], + str(Path("ql/rust/justfile")), + ) + + def test_asks_for_the_checks_ci_wants(self): + invocation = self.invocation(str(self.suite)) + self.assertIn("--all-checks", invocation) + self.assertIn("--codeql=built", invocation) + + def test_runs_from_the_checkout(self): + _, run, _ = self.run_main(str(self.suite)) + self.assertEqual(run.call_args.kwargs["cwd"], self.semmle_code) + + def test_an_argument_containing_a_space_stays_one_argument(self): + """The reason these arrive as a list rather than one string to re-split. + + Splitting on whitespace made this reach the suite as two arguments, and a value + that was only whitespace reached it as its own separators. + """ + self.assertIn("EXTRA=a b", self.invocation(str(self.suite), "EXTRA=a b")) + + def test_a_relative_argument_is_passed_verbatim(self): + self.assertIn("--fail-fast", self.invocation(str(self.suite), "--fail-fast")) + + def test_keeps_the_arguments_in_the_order_they_were_given(self): + invocation = self.invocation(str(self.suite), "CPUS=2", "--verbose") + self.assertEqual(invocation[-3:], [os.path.join("ql", "rust", "ql", "test"), "CPUS=2", "--verbose"]) + + def test_uses_the_just_it_was_given(self): + invocation = self.invocation( + str(self.suite), environment={"JUST_EXECUTABLE": "/opt/just"} + ) + self.assertEqual(invocation[0], "/opt/just") + + def test_says_what_it_is_about_to_run(self): + _, _, printed = self.run_main(str(self.suite)) + self.assertIn("-> just", printed) + + def test_needs_a_root(self): + status, run, _ = self.run_main() + self.assertEqual(status, 1) + run.assert_not_called() + + def test_reports_a_root_with_no_justfile_above_it(self): + orphan = self.semmle_code / "elsewhere" + orphan.mkdir() + status, run, _ = self.run_main(str(orphan)) + self.assertEqual(status, 1) + run.assert_not_called() + + def test_passes_on_the_status_of_a_failing_suite(self): + failure = subprocess.CalledProcessError(3, "just") + status, _, _ = self.run_main(str(self.suite), side_effect=failure) + self.assertEqual(status, 3) + + +if __name__ == "__main__": + unittest.main() diff --git a/misc/just/test_run_on_files.py b/misc/just/test_run_on_files.py new file mode 100644 index 000000000000..ce2517547c06 --- /dev/null +++ b/misc/just/test_run_on_files.py @@ -0,0 +1,415 @@ +#!/usr/bin/env python3 +"""Tests for `run_on_files.py`. + +Two of these guard properties that cost nothing to break and say nothing when broken. +An exclusion that stops excluding does not fail: it formats files it was told to leave +alone, with a tool the other repository did not choose, and the only way to notice is +to go looking. These are that noticing, done once and kept. + +Each of those two carries a positive control, as the assertion they make is that a +collection is empty, and an empty collection is also what a mistyped pattern, a wrong +directory or a walk that never ran produce. +""" + +import contextlib +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +import run_on_files + +HERE = Path(__file__).resolve().parent + +# Print each argument on its own line, so that a test can tell one argument containing a +# space from two arguments. +ECHO_ARGUMENTS = "import sys\nfor a in sys.argv[1:]: print(a)" +ECHO_TO_STDERR = "import sys\nfor a in sys.argv[1:]: print(a, file=sys.stderr)" + + +def can_symlink(): + """Whether this process may create symbolic links, which Windows restricts.""" + with tempfile.TemporaryDirectory() as directory: + try: + os.symlink(directory, Path(directory) / "link") + return True + except (OSError, NotImplementedError): + return False + + +CAN_SYMLINK = can_symlink() + + +@contextlib.contextmanager +def working_directory(directory): + previous = os.getcwd() + os.chdir(directory) + try: + yield + finally: + os.chdir(previous) + + +class TemporaryTree(unittest.TestCase): + """A scratch tree of empty files, named by `files`.""" + + files = () + + def setUp(self): + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + # Resolved once here: macOS puts temporary directories behind a symbolic link, + # and these tests compare against absolute paths. + self.root = Path(temporary.name).resolve() + for name in self.files: + path = self.root / name + path.parent.mkdir(parents=True, exist_ok=True) + path.touch() + + +class TestFilesUnder(TemporaryTree): + files = ( + "outer/BUILD", + "outer/BUILD.bazel", + "outer/notes.md", + "outer/nested/BUILD.bazel", + "outer/nested/deep/BUILD.bazel", + ) + + def collect(self, paths, patterns, *args, **kwargs): + files, _ = run_on_files.files_under(paths, patterns, *args, **kwargs) + return files + + def test_matches_the_whole_name_rather_than_an_extension(self): + # A bazel file may be called `BUILD`, with no extension to match on. + with working_directory(self.root / "outer"): + self.assertEqual(self.collect(["."], ["BUILD"]), ["BUILD"]) + + def test_leaves_out_a_file_no_pattern_names(self): + with working_directory(self.root): + found = self.collect(["outer"], ["*.bazel"]) + self.assertNotIn(os.path.join("outer", "notes.md"), found) + + def test_walks_the_whole_tree_below_a_directory(self): + with working_directory(self.root): + found = self.collect(["outer"], ["*.bazel"]) + self.assertEqual( + found, + [ + os.path.join("outer", "BUILD.bazel"), + os.path.join("outer", "nested", "BUILD.bazel"), + os.path.join("outer", "nested", "deep", "BUILD.bazel"), + ], + ) + + def test_takes_a_file_as_well_as_a_directory(self): + with working_directory(self.root): + path = os.path.join("outer", "BUILD.bazel") + self.assertEqual(self.collect([path], ["*.bazel"]), [path]) + + def test_leaves_out_a_file_named_directly_that_no_pattern_matches(self): + with working_directory(self.root): + self.assertEqual(self.collect([os.path.join("outer", "notes.md")], ["*.bazel"]), []) + + def test_excludes_are_matched_against_the_path_not_the_name(self): + with working_directory(self.root): + found = self.collect(["outer"], ["*.bazel"], ["*/nested/*"]) + self.assertEqual(found, [os.path.join("outer", "BUILD.bazel")]) + + def test_absolute_exclusion_holds_for_a_path_walked_from_inside(self): + """The walk only ever extends the path it was given. + + Walking `nested` from inside `outer` builds no path naming `outer`, so an + exclusion spelled relative to the enclosing directory cannot match it. One + anchored absolutely has to, which is what a repository relies on to stay out of + a nested one that is being formatted from within. + """ + with working_directory(self.root / "outer"): + # Positive control: there is something here to exclude, so an empty result + # below means the exclusion worked rather than that the walk found nothing. + self.assertEqual(len(self.collect(["nested"], ["*.bazel"])), 2) + self.assertEqual( + self.collect(["nested"], ["*.bazel"], [f"{self.root / 'outer'}/*"]), + [], + ) + + @unittest.skipUnless(CAN_SYMLINK, "symbolic links need a privilege on Windows") + def test_absolute_exclusion_holds_for_a_path_reached_through_a_link(self): + """An absolute path may still be spelled through a symbolic link. + + Joining it with the working directory leaves that spelling alone, so only + resolving it makes an absolutely anchored exclusion hold here too. + """ + link = self.root / "link" + link.symlink_to(self.root / "outer", target_is_directory=True) + reached_through_link = str(link / "nested") + # Positive control, as above. + self.assertEqual(len(self.collect([reached_through_link], ["*.bazel"])), 2) + self.assertEqual( + self.collect([reached_through_link], ["*.bazel"], [f"{self.root / 'outer'}/*"]), + [], + ) + + def test_a_relative_exclusion_still_holds_for_a_relative_walk(self): + # Trying both spellings must not cost a pattern the reach it was written with. + with working_directory(self.root): + self.assertEqual(self.collect(["outer"], ["*.bazel"], ["outer/*"]), []) + + def test_within_leaves_out_what_lies_beyond_it(self): + with working_directory(self.root): + self.assertEqual( + self.collect(["outer"], ["*.bazel"], within=str(self.root / "outer" / "nested")), + [ + os.path.join("outer", "nested", "BUILD.bazel"), + os.path.join("outer", "nested", "deep", "BUILD.bazel"), + ], + ) + + @unittest.skipUnless(CAN_SYMLINK, "symbolic links need a privilege on Windows") + def test_does_not_walk_through_a_symbolic_link(self): + # This is what keeps the `bazel-*` convenience links out of a walk, which would + # otherwise reach the whole output tree. + (self.root / "outer" / "bazel-out").symlink_to( + self.root / "outer" / "nested", target_is_directory=True + ) + with working_directory(self.root): + found = self.collect(["outer"], ["*.bazel"]) + self.assertFalse([path for path in found if "bazel-out" in path]) + + def test_absolute_asks_for_absolute_names(self): + with working_directory(self.root): + found = self.collect(["outer"], ["BUILD"], absolute=True) + self.assertEqual(found, [str(self.root / "outer" / "BUILD")]) + + def test_a_file_is_collected_once_however_many_paths_reach_it(self): + with working_directory(self.root): + found = self.collect(["outer", os.path.join("outer", "BUILD.bazel")], ["*.bazel"]) + self.assertEqual(len(found), len(set(found))) + + +class TestContributingPaths(TemporaryTree): + """The second half of what a walk learns, and the reason the banner is honest. + + A path that was asked about is not the same as a path being acted on. Naming the + first is what made a run over a repository and a nested one report the nested path + twice, once from an invocation that had excluded every file in it. + """ + + files = ( + "outer/BUILD.bazel", + "outer/nested/BUILD.bazel", + "outer/barren/notes.md", + ) + + def contributing(self, paths, patterns, *args, **kwargs): + _, contributing = run_on_files.files_under(paths, patterns, *args, **kwargs) + return contributing + + def test_names_only_a_path_that_yielded_a_file(self): + with working_directory(self.root): + self.assertEqual( + self.contributing( + [os.path.join("outer", "nested"), os.path.join("outer", "barren")], + ["*.bazel"], + ), + [os.path.join("outer", "nested")], + ) + + def test_leaves_out_a_path_whose_files_were_all_excluded(self): + with working_directory(self.root): + self.assertEqual( + self.contributing( + ["outer"], + ["*.bazel"], + [f"{self.root / 'outer'}/*"], + ), + [], + ) + + def test_keeps_the_spelling_the_path_was_given_in(self): + # It goes back to whoever wrote it, so it has to be recognisable as theirs. + with working_directory(self.root): + self.assertEqual(self.contributing(["./outer"], ["*.bazel"]), ["./outer"]) + + def test_names_both_paths_that_reach_the_same_file(self): + # The file is collected once, but each path did have something in it. + with working_directory(self.root): + paths = ["outer", os.path.join("outer", "nested")] + files, contributing = run_on_files.files_under(paths, ["*.bazel"]) + self.assertEqual(contributing, paths) + self.assertEqual(len(files), 2) + + +class TestBanner(unittest.TestCase): + def test_names_the_command_and_the_paths_it_has_something_to_do_in(self): + # The bare form has to be asked for rather than assumed: `just` exports these + # two, so run through the `test` recipe the ambient environment is not empty, + # and a test that reads it would pass or fail by how it was started. + with mock.patch.dict(os.environ): + os.environ.pop("CMD_BEGIN", None) + os.environ.pop("CMD_END", None) + self.assertEqual( + run_on_files.banner(["clang-format", "-i"], ["cpp", "swift"]), + "-> clang-format -i -- cpp swift", + ) + + def test_is_wrapped_in_the_rules_the_justfiles_set(self): + with mock.patch.dict( + os.environ, {"CMD_BEGIN": "", "CMD_END": ""} + ): + self.assertEqual( + run_on_files.banner(["black"], ["."]), "-> black -- ." + ) + + +class TestBatched(unittest.TestCase): + def test_keeps_everything_in_one_batch_when_it_fits(self): + self.assertEqual(list(run_on_files.batched(["a", "b"], 100)), [["a", "b"]]) + + def test_splits_once_the_limit_is_reached(self): + batches = list(run_on_files.batched(["aaa", "bbb", "ccc"], 8)) + self.assertEqual(batches, [["aaa", "bbb"], ["ccc"]]) + + def test_loses_no_file_and_keeps_their_order(self): + files = [f"file{n}" for n in range(50)] + batched = [file for batch in run_on_files.batched(files, 20) for file in batch] + self.assertEqual(batched, files) + + def test_yields_nothing_for_no_files(self): + self.assertEqual(list(run_on_files.batched([], 100)), []) + + def test_still_yields_a_file_longer_than_the_limit(self): + # Dropping it would be silent, and the command is a better place for the + # complaint than a batch that never happens. + long = "x" * 200 + self.assertEqual(list(run_on_files.batched([long], 10)), [[long]]) + + +class TestBatchLimit(unittest.TestCase): + def test_leaves_room_for_the_environment_and_a_single_argument(self): + limit = run_on_files.batch_limit() + self.assertGreaterEqual(limit, 4096) + # A single argument is capped far lower than the whole command line, and a + # command handing its arguments on through a shell arrives as one of them. + self.assertLessEqual(limit, 100000) + + +class TestCommaSeparated(unittest.TestCase): + def test_splits_a_group_given_as_one_argument(self): + self.assertEqual(run_on_files.comma_separated("a,b,c"), ["a", "b", "c"]) + + def test_leaves_a_single_pattern_alone(self): + self.assertEqual(run_on_files.comma_separated("a"), ["a"]) + + +class TestCommandLine(TemporaryTree): + files = ( + "project/BUILD.bazel", + "project/with space/BUILD.bazel", + "project/notes.md", + "elsewhere/BUILD.bazel", + ) + + def run_script(self, *arguments, cwd=None): + return subprocess.run( + [sys.executable, str(HERE / "run_on_files.py"), *arguments], + cwd=cwd or self.root, + capture_output=True, + text=True, + ) + + def echo(self, script=ECHO_ARGUMENTS): + return [sys.executable, "-c", script] + + def test_passes_a_name_containing_a_space_as_one_argument(self): + """The reason this program exists rather than a shell command substitution.""" + result = self.run_script("*.bazel", *self.echo(), "--", "project") + self.assertEqual(result.returncode, 0, result.stderr) + printed = result.stdout.splitlines() + self.assertIn(os.path.join("project", "with space", "BUILD.bazel"), printed) + + def test_announces_the_command_once_a_file_has_matched(self): + result = self.run_script("*.bazel", *self.echo(), "--", "project") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("-> ", result.stderr) + self.assertIn(" -- project", result.stderr) + + def test_the_banner_leaves_out_a_path_that_contributed_nothing(self): + """What a repository formatting its own files over a nested one used to say. + + Both paths were named while one of them had every file excluded, so the run + read as covering ground it had already declined to touch. + """ + result = self.run_script( + "--exclude", + f"{self.root / 'project'}/*", + "*.bazel", + *self.echo(), + "--", + "elsewhere", + "project", + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(" -- elsewhere", result.stderr) + self.assertNotIn("project", result.stderr) + + def test_says_nothing_at_all_when_no_file_matches(self): + """Silence has to mean that nothing matched, not that nothing was looked at. + + The banner is the whole point: announced before the collection, it would claim + a formatter ran over a directory it never opened. + """ + result = self.run_script("*.nomatch", *self.echo(), "--", "project") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout, "") + self.assertEqual(result.stderr, "") + + def test_refuses_a_path_that_does_not_exist(self): + # Naming a path is an assertion that it is there, so this is not the silent case + # above: an unexpanded glob would otherwise look exactly like nothing to do. + result = self.run_script("*.bazel", *self.echo(), "--", "absent") + self.assertNotEqual(result.returncode, 0) + self.assertIn("no such path: absent", result.stderr) + + def test_needs_the_paths_separated_from_the_command(self): + result = self.run_script("*.bazel", *self.echo(), "project") + self.assertNotEqual(result.returncode, 0) + self.assertIn("--", result.stderr) + + def test_needs_a_command(self): + # Two separators: argparse takes the first for its own end-of-options marker + # when nothing precedes it, so one alone leaves no separator to find and is + # reported as the missing one above. + result = self.run_script("*.bazel", "--", "--", "project") + self.assertNotEqual(result.returncode, 0) + self.assertIn("no command given", result.stderr) + + def test_separates_on_the_last_dash_dash_so_a_command_may_contain_one(self): + result = self.run_script("*.bazel", *self.echo(), "--", "--", "project") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("--", result.stdout.splitlines()) + + def test_drops_the_lines_it_was_told_to_drop(self): + result = self.run_script( + "--drop", + "notes", + "*.bazel,*.md", + *self.echo(ECHO_TO_STDERR), + "--", + "project", + ) + self.assertEqual(result.returncode, 0, result.stdout) + self.assertNotIn("notes.md", result.stderr) + self.assertIn("BUILD.bazel", result.stderr) + + def test_reports_the_command_failing(self): + failing = [sys.executable, "-c", "import sys; sys.exit(3)"] + result = self.run_script("*.bazel", *failing, "--", "project") + self.assertEqual(result.returncode, 3) + + +if __name__ == "__main__": + unittest.main() diff --git a/python/justfile b/python/justfile new file mode 100644 index 000000000000..dec897cdf672 --- /dev/null +++ b/python/justfile @@ -0,0 +1,26 @@ +import '../lib.just' +import 'ql/justfile' + +[group('build')] +build: (_build_dist "python") + +# Long filename needed for extractor tests (too long for Git on Windows) +[no-cd] +@_ensure_long_filename: _require_semmle_code + #!/usr/bin/env bash + longfile="$SEMMLE_CODE/ql/python/ql/test/extractor-tests/long_path/really_rather_too_long_for_windows_path_length/with_unecessarily_longwinded_and_verbose_sub_folder/extremely_long_module_name_with_lots_of_digits_at_the_end_000000000000000000000000000000000000000000000000000000000000000000/test0000000000000000000000000000000000000000000000000000000.py" + mkdir -p "$(dirname "$longfile")" + touch "$longfile" + +_tests := source_dir() / 'ql/test' + +_shared_roots := [_tests / 'library-tests', _tests / 'query-tests', _tests / 'extractor-tests', _tests / 'experimental'] + +roots_2 := _shared_roots ++ [_tests / '2'] +roots_3 := _shared_roots ++ [_tests / 'modelling', _tests / '3'] + +[group('test')] +language-tests-2 *EXTRA_ARGS: _ensure_long_filename (_language_tests (roots_2 ++ _v2_env ++ EXTRA_ARGS)) + +[group('test')] +language-tests-3 *EXTRA_ARGS: _ensure_long_filename (_language_tests (roots_3 ++ _v3_env ++ EXTRA_ARGS)) diff --git a/python/ql/integration-tests/justfile b/python/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/python/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/python/ql/justfile b/python/ql/justfile new file mode 100644 index 000000000000..45ed8d733cff --- /dev/null +++ b/python/ql/justfile @@ -0,0 +1,12 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" + +python_version := env("python_version", "3") + +_v2_env := ['CODEQL_EXTRACTOR_PYTHON_ANALYSIS_VERSION=2', 'CODEQL_PYTHON_LEGACY_TEST_EXTRACTION_VERSION=2'] +_v3_env := ['CODEQL_PYTHON_LEGACY_TEST_EXTRACTION_VERSION=3'] +_python_env := if python_version == "2" { _v2_env } else { _v3_env } diff --git a/python/ql/test/justfile b/python/ql/test/justfile new file mode 100644 index 000000000000..c02c78ba8241 --- /dev/null +++ b/python/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := _python_env + +all_checks := default_db_checks ++ ['--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "python" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/ruby/justfile b/ruby/justfile new file mode 100644 index 000000000000..b9cc748f169f --- /dev/null +++ b/ruby/justfile @@ -0,0 +1,9 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "ruby") + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) diff --git a/ruby/ql/integration-tests/justfile b/ruby/ql/integration-tests/justfile new file mode 100644 index 000000000000..8392c2b3ba40 --- /dev/null +++ b/ruby/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_integration_test ARGS) diff --git a/ruby/ql/justfile b/ruby/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/ruby/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/ruby/ql/test/justfile b/ruby/ql/test/justfile new file mode 100644 index 000000000000..ab79be8d890e --- /dev/null +++ b/ruby/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks ++ ['--check-undefined-labels', '--check-unused-labels', '--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "ruby" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/rust/codegen/codegen.sh b/rust/codegen/codegen.sh index 2d415009aed8..726ff138db78 100755 --- a/rust/codegen/codegen.sh +++ b/rust/codegen/codegen.sh @@ -2,7 +2,7 @@ set -eu -source misc/bazel/runfiles.sh 2>/dev/null || source external/ql+/misc/bazel/runfiles.sh +source misc/bazel/runfiles.sh 2>/dev/null || source ../ql+/misc/bazel/runfiles.sh ast_generator="$(rlocation "$1")" grammar_file="$(rlocation "$2")" diff --git a/rust/justfile b/rust/justfile new file mode 100644 index 000000000000..9877da7b9f1f --- /dev/null +++ b/rust/justfile @@ -0,0 +1,17 @@ +import '../lib.just' + +install: (_bazel ['run', '@codeql//rust:install']) + +[group('build')] +build: (_if_not_on_ci_just ['generate', source_dir()]) (_build_dist "rust") + +generate: (_bazel ['run', '@codeql//rust/codegen']) + +lint: (_run_in source_dir() ['python3', 'lint.py']) + +format: (_run_in source_dir() ['python3', 'lint.py', '--format-only']) + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) diff --git a/rust/lint.py b/rust/lint.py index 600a888649e9..3ace1667a464 100755 --- a/rust/lint.py +++ b/rust/lint.py @@ -4,6 +4,15 @@ import pathlib import shutil import sys +import argparse + + +def options(): + parser = argparse.ArgumentParser(description="lint rust language pack code") + parser.add_argument( + "--format-only", action="store_true", help="Only apply formatting" + ) + return parser.parse_args() def tool(name): @@ -12,27 +21,33 @@ def tool(name): return ret -this_dir = pathlib.Path(__file__).resolve().parent +def main(): + args = options() + this_dir = pathlib.Path(__file__).resolve().parent + + cargo = tool("cargo") + bazel = tool("bazel") -cargo = tool("cargo") -bazel = tool("bazel") + runs = [] -runs = [] + def run(tool, args, *, cwd=this_dir): + print("+", tool, args) + runs.append(subprocess.run([tool] + args.split(), cwd=cwd)) -def run(tool, args, *, cwd=this_dir): - print("+", tool, args) - runs.append(subprocess.run([tool] + args.split(), cwd=cwd)) + # make sure bazel-provided sources are put in tree for `cargo` to work with them + run(bazel, "run ast-generator:inject-sources") + run(cargo, "fmt --all --quiet") + if not args.format_only: + for manifest in this_dir.rglob("Cargo.toml"): + if not manifest.is_relative_to(this_dir / "ql") and not manifest.is_relative_to(this_dir / "integration-tests"): + run(cargo, + "clippy --fix --allow-dirty --allow-staged --quiet -- -D warnings", + cwd=manifest.parent) -# make sure bazel-provided sources are put in tree for `cargo` to work with them -run(bazel, "run ast-generator:inject-sources") -run(cargo, "fmt --all --quiet") + return max(r.returncode for r in runs) -for manifest in this_dir.rglob("Cargo.toml"): - if not manifest.is_relative_to(this_dir / "ql") and not manifest.is_relative_to(this_dir / "integration-tests"): - run(cargo, - "clippy --fix --allow-dirty --allow-staged --quiet -- -D warnings", - cwd=manifest.parent) -sys.exit(max(r.returncode for r in runs)) +if __name__ == "__main__": + sys.exit(main()) diff --git a/rust/ql/integration-tests/justfile b/rust/ql/integration-tests/justfile new file mode 100644 index 000000000000..2ee4b833c128 --- /dev/null +++ b/rust/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_if_not_on_ci_just ['generate']) (_integration_test ARGS) diff --git a/rust/ql/justfile b/rust/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/rust/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/rust/ql/test/justfile b/rust/ql/test/justfile new file mode 100644 index 000000000000..8b6133008e8a --- /dev/null +++ b/rust/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks ++ ['--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "rust" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/swift/justfile b/swift/justfile new file mode 100644 index 000000000000..b565923cae08 --- /dev/null +++ b/swift/justfile @@ -0,0 +1,18 @@ +import '../lib.just' + +install: (_bazel ['run', '@codeql//swift:install']) + +[group('build')] +build: (_build_dist "swift") + +generate: (_bazel ['run', '@codeql//swift/codegen']) + +format *ARGS=".": (_format_cpp ARGS) + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) + +[group('test')] +extra-tests: (_sembuild "target/test/check-queries-swift") (_sembuild "target/test/check-db-upgrades-swift") (_sembuild "target/test/check-db-downgrades-swift") diff --git a/swift/ql/integration-tests/justfile b/swift/ql/integration-tests/justfile new file mode 100644 index 000000000000..097faf5baebf --- /dev/null +++ b/swift/ql/integration-tests/justfile @@ -0,0 +1,8 @@ +import "../../../lib.just" + +# Integration tests are slow and need an internal checkout, so they only run when +# asked for by name. +explicit_verbs := ['test'] + +[no-cd] +test *ARGS=".": (_just "generate") (_integration_test ARGS) diff --git a/swift/ql/justfile b/swift/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/swift/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/swift/ql/test/justfile b/swift/ql/test/justfile new file mode 100644 index 000000000000..f396305ed282 --- /dev/null +++ b/swift/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks ++ ['--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "swift" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/unified/extractor/justfile b/unified/extractor/justfile new file mode 100644 index 000000000000..f6a6417ed867 --- /dev/null +++ b/unified/extractor/justfile @@ -0,0 +1,4 @@ +import '../../lib.just' + +[group('test')] +test *BAZEL_ARGS: (_bazel (['test', '--build_tests_only', '@codeql//unified/extractor/...'] ++ BAZEL_ARGS)) diff --git a/unified/justfile b/unified/justfile new file mode 100644 index 000000000000..610ec84901c0 --- /dev/null +++ b/unified/justfile @@ -0,0 +1,12 @@ +import '../lib.just' + +[group('build')] +build: (_build_dist "unified") + +roots := [source_dir() / 'ql/test'] + +[group('test')] +language-tests *EXTRA_ARGS: (_language_tests (roots ++ EXTRA_ARGS)) + +[group('test')] +extractor-tests *BAZEL_ARGS: (_bazel (['test', '--build_tests_only', '@codeql//unified/...'] ++ BAZEL_ARGS)) diff --git a/unified/ql/justfile b/unified/ql/justfile new file mode 100644 index 000000000000..ff0e4c1090f3 --- /dev/null +++ b/unified/ql/justfile @@ -0,0 +1,6 @@ +import "../../lib.just" + +[no-cd] +format *ARGS=".": (_format_ql ARGS) + +consistency_queries := source_dir() / "consistency-queries" diff --git a/unified/ql/test/justfile b/unified/ql/test/justfile new file mode 100644 index 000000000000..4a097e0b1d65 --- /dev/null +++ b/unified/ql/test/justfile @@ -0,0 +1,11 @@ +import "../justfile" + +# A whole language test suite is slow, so it only runs when asked for by name. +explicit_verbs := ['test'] + +base_flags := [] + +all_checks := default_db_checks ++ ['--check-repeated-labels', '--check-redefined-labels', '--check-use-before-definition', '--consistency-queries=' + consistency_queries] + +[no-cd] +test *ARGS=".": (_codeql_test "unified" (base_flags ++ prepend('--extra-check=', all_checks) ++ ARGS)) diff --git a/unified/swift-syntax-rs/justfile b/unified/swift-syntax-rs/justfile new file mode 100644 index 000000000000..021bb0e9e75e --- /dev/null +++ b/unified/swift-syntax-rs/justfile @@ -0,0 +1,4 @@ +import '../../lib.just' + +[group('test')] +test *BAZEL_ARGS: (_bazel (['test', '--build_tests_only', '@codeql//unified/swift-syntax-rs/...'] ++ BAZEL_ARGS))