diff --git a/.github/workflows/certify.yml b/.github/workflows/certify.yml index 0c391c5ec..0f919809a 100644 --- a/.github/workflows/certify.yml +++ b/.github/workflows/certify.yml @@ -416,3 +416,126 @@ jobs: # mean the scanner moved under us rather than a new CVE landing. - name: govulncheck run: go run golang.org/x/vuln/cmd/govulncheck@v1.7.0 ./... + + # THE NIGHTLY NEGATIVE-CONTROL TIER (issue #563). Every negative control this + # tree defines runs on every pull request, in ci.yml, one job per group, and + # every one of those jobs fits the owner's rule for CI that runs per commit: + # one to two minutes. A control that cannot fit that rule ON ITS OWN has no + # place on the pull request and still has to run, so it runs here, on the + # schedule this file already carries. + # + # The plan is the same file the pull-request leg reads, + # make/negative-controls.json, and each group in it names its tier and says + # why. `tools/negativecontrols` refuses a group that names neither tier, so a + # control cannot leave the pull request without landing on this workflow, and + # the package's own test reads both files to prove each one runs the plan + # rather than a typed list. + negative-controls-nightly-matrix: + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + matrix: ${{ steps.plan.outputs.matrix }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.26' + cache: false + + - id: plan + name: the nightly tier, as a matrix + run: | + matrix=$(go run ./tools/negativecontrols matrix nightly) + echo "$matrix" + echo "matrix=$matrix" >> "$GITHUB_OUTPUT" + + negative-controls-nightly: + name: negative controls, nightly (${{ matrix.name }}) + needs: negative-controls-nightly-matrix + runs-on: ubuntu-latest + timeout-minutes: 60 + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.negative-controls-nightly-matrix.outputs.matrix) }} + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # All six siblings in every group, exactly as ci.yml's leg clones them: a + # control reaches for a runtime through the generated code it sabotages, + # and which one that is belongs to the control rather than to the group. + - name: Check out the serialize runtimes (pinned releases) + run: | + cd .. + git clone --quiet --depth 1 --branch "$SERIALIZE_TAG" https://github.com/mas-bandwidth/serialize.git serialize + git clone --quiet --depth 1 --branch "$SERIALIZE_C_TAG" https://github.com/mas-bandwidth/serialize.c.git serialize.c + git clone --quiet --depth 1 --branch "$SERIALIZE_GO_TAG" https://github.com/mas-bandwidth/serialize.go.git serialize.go + git clone --quiet --depth 1 --branch "$SERIALIZE_RS_TAG" https://github.com/mas-bandwidth/serialize.rs.git serialize.rs + git clone --quiet --depth 1 --branch "$SERIALIZE_CS_TAG" https://github.com/mas-bandwidth/serialize.cs.git serialize.cs + git clone --quiet --depth 1 --branch "$SERIALIZE_JS_TAG" https://github.com/mas-bandwidth/serialize.js.git serialize.js + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.26' + cache: false + + # One step per toolchain, each keyed on the row's field for it, the same + # way ci.yml's leg and the conformance matrix key theirs. A base row names + # none of them and installs none of them. + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # the stable BRANCH head; the SHA freezes the action, not the compiler, and rustup resolves stable at run time + if: matrix.rust != '' + + - name: read the .NET SDK pin + if: matrix.dotnet != '' + run: echo "DOTNET_SDK_PIN=$(cat .github/dotnet-version)" >> "$GITHUB_ENV" + + - uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 + if: matrix.dotnet != '' + with: + dotnet-version: ${{ env.DOTNET_SDK_PIN }} + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + if: matrix.node != '' + with: + node-version: ${{ matrix.node }} + + - uses: dart-lang/setup-dart@6afc89df92d6eb3834022f73cd65adc8cdfcb92d # v1.8.1 + if: matrix.dart != '' + with: + sdk: ${{ matrix.dart }} + + - uses: actions/setup-java@dd06d9cba3e5552c54d9f8ea23572deb30010f7c # v6.0.0 + if: matrix.java != '' + with: + distribution: 'temurin' + java-version: ${{ matrix.java }} + + - uses: erlef/setup-beam@54075bcc5e249e4758d363f27d099f55d843f124 # v1.24.1 + if: matrix.otp != '' + with: + otp-version: ${{ matrix.otp }} + elixir-version: ${{ matrix.elixir }} + + - name: build bin/schema and the generated tree once + run: make -j"$(nproc)" bin/schema build/tables-generated/.stamp + + # ONE make invocation for the whole group, with -k so a control that + # refuses does not hide the ones behind it, and the same log scan ci.yml + # runs: a control's own refusal lines are the sentence a reader needs, and + # this pulls them out of a log thousands of compiler lines long. + - name: every negative control in the ${{ matrix.name }} group + run: | + set -o pipefail + targets=$(go run ./tools/negativecontrols targets "${{ matrix.name }}") + echo "$targets" | tr ' ' '\n' + status=0 + # shellcheck disable=SC2086 + make -k $targets \ + RUSTUP_BIN=/usr/bin DART=dart JAVA=java JAVAC=javac ELIXIR=elixir MIX=mix NODE=node \ + 2>&1 | tee controls.log || status=1 + if grep -qE '^NEGATIVE CONTROL( FAILED)?:' controls.log; then + echo "::error::a negative control refused. Either its sabotage no longer patches the line it aims at, or the gate it watches stayed green under the sabotage. Both are stop-the-line: the control is watching nothing until it is repaired." + grep -nE '^NEGATIVE CONTROL( FAILED)?:' controls.log + status=1 + fi + exit $status diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2cdf3c56d..a39bc4e29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,19 +9,36 @@ name: CI # 1:49, msvc 1:27, windows 0:49, conformance 0:21 to 0:41 per leg, go-test # 0:37, lint 0:37, vuln 0:30, shape-gate 0:22, generated 0:16, cpp-lock 0:06. # +# THE RULE IS PER JOB, AND EVERY JOB IN THIS FILE IS HELD TO IT. The +# negative-control leg below is a matrix, and a matrix row is a job, so each +# row is cut to fit the same two minutes rather than the leg as a whole being +# cut to fit it. A control that cannot fit the rule ON ITS OWN is not made to +# fit by grouping, so it does not run here at all: it runs on the nightly tier +# in certify.yml, and make/negative-controls.json records which tier every +# group is in and why. `tools/negativecontrols` refuses a group that is in +# neither, so nothing can fall between the two files. +# # `big-endian` IS THE BINDING CONSTRAINT, and it is worth naming rather than -# rounding away: 87 s of its 109 is `make tables-big-endian-negative`, which -# cross-compiles the tables battery for s390x, runs it under qemu, and then -# compiles it a SECOND time for the negative control. Building its six +# rounding away: 87 s of its 109 is `make tables-big-endian-negative-control`, +# which cross-compiles the tables battery for s390x, runs it under qemu, and +# then compiles it a SECOND time for the negative control. Building its six # binaries in parallel bought 14 s of that; the rest is the control's own two # compiles, which are serial by construction. The next job that wants a place # on this gate takes the budget from somewhere, and that is where to look. +# NOTE, 2026-09-07: that job measures 4:15 to 4:26 on main (runs 34111624059, +# 34111228369, 34110320928), which is over the rule and is not this leg's +# doing. It wants the same treatment the negative controls have here, and +# issue #684 owns giving it that. Its control is the reason +# tables-big-endian-negative-control sits in the plan's exclusion list rather +# than in a group: this job runs it on every pull request already. # # CERTIFICATION IS certify.yml: the inline gates, the full `make test` chain on -# both OSes, every `tables--release` target. It runs on every push to -# main, nightly, and on demand. Nothing is checked less than before — the split -# moves WHEN a certification instrument runs, not whether. A regression only -# certification can see fails loudly on main minutes after the merge. +# both OSes, every `tables--release` target. It runs nightly and on +# demand, and never per push (#669): a thirty-minute suite is not per-commit +# CI. Nothing is checked less than before, because the split moves WHEN a +# certification instrument runs, not whether. A regression only certification +# can see fails loudly at the next nightly, or the same hour when somebody +# dispatches that file on main after a landing near the hot paths. # # The line between the two files is the question a job answers. An ITERATION # gate answers "is this diff right" — it can go red on a commit, so a PR is @@ -383,7 +400,7 @@ jobs: # bin/schema and the generated tree, which make builds once. The runner # has four cores and the leg was spending them one at a time. - name: the big-endian leg, and its negative control - run: make -j"$(nproc)" tables-big-endian-negative BE_CXX=s390x-linux-gnu-g++-13 BE_CC=s390x-linux-gnu-gcc-13 + run: make -j"$(nproc)" tables-big-endian-negative-control BE_CXX=s390x-linux-gnu-g++-13 BE_CC=s390x-linux-gnu-gcc-13 # And the GO table leg on the same target. Go cross-compiles, so the # conformance driver is one env var away from a big-endian binary, and @@ -729,3 +746,204 @@ jobs: exit 1 fi echo "no locked paths touched — clean" + + # EVERY NEGATIVE CONTROL, ON THE PULL REQUEST (issue #563). + # + # A negative control proves a gate is watching: it breaks what the gate + # watches and requires the gate to go red. Each one already refuses when its + # sabotage patches nothing, so a control whose sed pattern has drifted off its + # target line says so out loud. That refusal only helps somebody who reads it, + # and until this leg the only reader was `make test`, which lives in + # certify.yml and runs AFTER a merge. Twice on 2026-09-05 main went red on a + # merge whose pull-request run was 20 of 20 green, and one of the two was + # exactly this: #562 rewrote the line the maps fit control's pattern aimed at, + # the sabotage patched nothing, and the control refused on main. + # + # THE TARGET LIST IS ENUMERATED, NOT TYPED. `tools/negativecontrols` reads the + # Makefile and every file the Makefile includes, and holds what it finds + # against make/negative-controls.json. A control this tree defines is in a + # group here or in that file's exclusion list with a reason, and the tool's + # test, which `go-test` runs as part of `go test ./...` and which the matrix + # job below runs again on its own, goes red on any other outcome. So a + # control added tomorrow cannot be left out of this leg by forgetting a line + # in a workflow. + # + # GROUPED BY TOOLCHAIN FIRST AND BY COST SECOND, one job per group. Most + # controls need Go, a host C and C++ compiler and the C/C++ serialize siblings + # and nothing else; the rest are per-port and need the port's runtime, + # installed here the same way the conformance jobs above install it, keyed on + # the row's field rather than on a language name. Inside a toolchain the + # groups are families that share a compile, cut where the owner's rule cuts + # them: a group runs in one `make -k` invocation and that invocation has to + # fit inside two minutes with the job's own 25 s of checkout, sibling clones + # and first build already spent. + # + # MEASURED, 2026-09-07 on the author's machine (arm64 macOS, one target per + # make invocation, off one warm build) and then on the runner, on this + # branch's own pull-request runs. 203 controls, 40 pull-request groups and + # one nightly group. `go run ./tools/negativecontrols check` prints the + # count, so it is one command rather than a number to keep in step by hand. + # + # The base toolchain carries 134 of them and takes twenty-eight jobs rather + # than one: the map gate alone is 17 controls at 4 s each on the author's + # machine and about 2x that on the runner, and the conformance driver's two + # sabotage rebuilds are 45 s and 47 s of runner time on their own. The + # tolerant-wire family is one control per job: each rebuilds the compiler + # under a source overlay and then fuzzes the sabotaged wire, 55 s to 80 s + # each, so no two of them fit together. + # + # THE FIRST CUT WAS MEASURED AND CUT AGAIN. On run 34115675206 every group + # was inside the rule but eight of them sat between 100 s and 111 s, which is + # a rule met by luck rather than by design, so those eight are the twelve + # groups they are now. The per-group numbers live in + # make/negative-controls.json's `why` lines, next to the targets they are + # about. + # + # ONE CONTROL DOES NOT FIT THE RULE AT ANY GROUP SIZE, and it runs nightly + # instead: tables-message-form-negative-control drives 49 sabotage rows one + # submake each at 124 s, against a 120 s rule. It is the `nightly` group in + # the plan, certify.yml runs that tier on the schedule it carries, and + # `tools/negativecontrols` refuses a group that is in neither tier, so a + # control cannot leave this file without landing on that one. The message + # form's other blades stay here. The group keeps its name at one target, + # because it is where the next control that outgrows the rule lands. + # + # AN UMBRELLA TARGET IS NOT A CONTROL. tables-wire-fuzz-retain-negative-control + # carries no recipe: it names the retain oracle and the retain class leaves, + # which the wire-fuzz-message-oracle and wire-fuzz-retain-class rows run + # between them, so it sits in the plan's exclusion list beside the other + # umbrellas rather than in a job that would run one of those leaves twice. + negative-controls-matrix: + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + matrix: ${{ steps.plan.outputs.matrix }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.26' + cache: false + + # The enumeration gate, first and on its own, so a control that nothing + # runs is named here rather than discovered by its absence from a log. + - name: every negative control the makefiles define is in the plan + run: go run ./tools/negativecontrols check + + - id: plan + name: the plan, as a matrix + run: | + matrix=$(go run ./tools/negativecontrols matrix) + echo "$matrix" + echo "matrix=$matrix" >> "$GITHUB_OUTPUT" + + negative-controls: + name: negative controls (${{ matrix.name }}) + needs: negative-controls-matrix + runs-on: ubuntu-latest + # THE RULE, ENFORCING ITSELF. Every row here is cut to 120 s of control + # time and pays about 25 s of checkout, sibling clones and first build on + # top, so five minutes is generous and a row that reaches it is a row that + # left the rule. A ceiling wide enough to hide that is not a timeout. + timeout-minutes: 5 + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.negative-controls-matrix.outputs.matrix) }} + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # All six siblings in every group. A control reaches for a runtime through + # the generated code it sabotages, and which one that is belongs to the + # control rather than to the group; six shallow clones cost seconds and + # remove the whole class of "the group forgot a sibling". + - name: Check out the serialize runtimes (pinned releases) + run: | + cd .. + git clone --quiet --depth 1 --branch "$SERIALIZE_TAG" https://github.com/mas-bandwidth/serialize.git serialize + git clone --quiet --depth 1 --branch "$SERIALIZE_C_TAG" https://github.com/mas-bandwidth/serialize.c.git serialize.c + git clone --quiet --depth 1 --branch "$SERIALIZE_GO_TAG" https://github.com/mas-bandwidth/serialize.go.git serialize.go + git clone --quiet --depth 1 --branch "$SERIALIZE_RS_TAG" https://github.com/mas-bandwidth/serialize.rs.git serialize.rs + git clone --quiet --depth 1 --branch "$SERIALIZE_CS_TAG" https://github.com/mas-bandwidth/serialize.cs.git serialize.cs + git clone --quiet --depth 1 --branch "$SERIALIZE_JS_TAG" https://github.com/mas-bandwidth/serialize.js.git serialize.js + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.26' + cache: false + + # One step per toolchain, each keyed on the row's field for it, exactly as + # the conformance matrix above does. A base row names none of them and + # installs none of them. + - uses: dtolnay/rust-toolchain@6bed0761d98439e5a578e2877258200ad565ba87 # the stable BRANCH head; the SHA freezes the action, not the compiler, and rustup resolves stable at run time + if: matrix.rust != '' + + - name: read the .NET SDK pin + if: matrix.dotnet != '' + run: echo "DOTNET_SDK_PIN=$(cat .github/dotnet-version)" >> "$GITHUB_ENV" + + - uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 + if: matrix.dotnet != '' + with: + dotnet-version: ${{ env.DOTNET_SDK_PIN }} + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + if: matrix.node != '' + with: + node-version: ${{ matrix.node }} + + - uses: dart-lang/setup-dart@6afc89df92d6eb3834022f73cd65adc8cdfcb92d # v1.8.1 + if: matrix.dart != '' + with: + sdk: ${{ matrix.dart }} + + - uses: actions/setup-java@dd06d9cba3e5552c54d9f8ea23572deb30010f7c # v6.0.0 + if: matrix.java != '' + with: + distribution: 'temurin' + java-version: ${{ matrix.java }} + + - uses: erlef/setup-beam@54075bcc5e249e4758d363f27d099f55d843f124 # v1.24.1 + if: matrix.otp != '' + with: + otp-version: ${{ matrix.otp }} + elixir-version: ${{ matrix.elixir }} + + # The compiler and the generated tree ONCE, in parallel, before any + # control runs. Every control in the group hangs off these two, and + # building them here keeps the build cost out of the first control's + # timing and out of its log. + - name: build bin/schema and the generated tree once + run: make -j"$(nproc)" bin/schema build/tables-generated/.stamp + + # ONE make invocation for the whole group, with -k so a control that + # refuses does not hide the ones behind it: every control in the group + # runs, and every failure is in one log. `-k` still exits nonzero. + # + # The exit status is the first gate. The log scan is the second, and it is + # not redundant: a control's own refusal lines are the sentence a reader + # needs (a sabotage that patched nothing, a gate that stayed green), and + # this pulls them out of a log that is thousands of compiler lines long + # and puts them in the run's annotations. + # + # The overrides are certify.yml's: RUSTUP_BIN defaults to a homebrew keg + # on the author's machine, and DART, JAVA/JAVAC, ELIXIR/MIX and NODE + # default to the repo-local dist/ toolchains, while on CI the setup steps + # above put the pinned versions on PATH. They are harmless in a group that + # runs no control needing them. + - name: every negative control in the ${{ matrix.name }} group + run: | + set -o pipefail + targets=$(go run ./tools/negativecontrols targets "${{ matrix.name }}") + echo "$targets" | tr ' ' '\n' + status=0 + # shellcheck disable=SC2086 + make -k $targets \ + RUSTUP_BIN=/usr/bin DART=dart JAVA=java JAVAC=javac ELIXIR=elixir MIX=mix NODE=node \ + 2>&1 | tee controls.log || status=1 + if grep -qE '^NEGATIVE CONTROL( FAILED)?:' controls.log; then + echo "::error::a negative control refused. Either its sabotage no longer patches the line it aims at, or the gate it watches stayed green under the sabotage. Both are stop-the-line: the control is watching nothing until it is repaired." + grep -nE '^NEGATIVE CONTROL( FAILED)?:' controls.log + status=1 + fi + exit $status diff --git a/Makefile b/Makefile index 07e3c3af0..aac8590fa 100644 --- a/Makefile +++ b/Makefile @@ -1583,8 +1583,16 @@ endif # The sabotaged compiler reaches the build through `go build -overlay`, so no # tracked file is ever written to — the same mechanism the big-endian negative # control uses, and for the same reason. +# +# THE C# STAMP IS A PREREQUISITE, as it is on the padding and pitch controls +# beside it. The C# half below overrides BlockGeneratedDir alone, so the +# project's BlockHomeGeneratedDir keeps its default and the blockhome sources +# have to be on disk; without them the build fails on an undefined namespace +# and the control refuses, correctly, that C# went red but not on the layout +# check. Inside `make test` an earlier leg had already generated them, which is +# why the omission stayed invisible until every control ran on its own. .PHONY: tables-block-layout-model-negative-control -tables-block-layout-model-negative-control: bin/schema +tables-block-layout-model-negative-control: bin/schema build/tables-generated-cs/.stamp @mkdir -p build @sed 's|ml.Fields = append(ml.Fields, FieldLayout{Field: f, Offset: start, Size: offset - start, Align: fieldAlign})|ml.Fields = append(ml.Fields, FieldLayout{Field: f, Offset: start + 8, Size: offset - start, Align: fieldAlign}) // SABOTAGED: one field, moved|' \ ir/blocklayout.go > build/blocklayout-moved.gotext @@ -1840,8 +1848,14 @@ tables-json-clamp-prefix-negative-control: bin/schema test/tables/json_clamp_pre # It sabotages the C++ driver into claiming absence for every instance whose # unit is pointered, which is exactly what a port legitimately does, and # requires the harness to go RED on the reference leg. +# The Go leg is a PREREQUISITE, not an assumption: the second half below runs +# the harness over a substituted registry naming the Go driver, and that driver +# execs build/conformance-go. Within `make test` the Go leg is already built by +# the time this runs, which is why the omission stayed invisible; run this +# target on its own and it fails on a missing binary rather than on its own +# question. .PHONY: conformance-negative-control-absent -conformance-negative-control-absent: build/conformance-harness build/tables-generated/.stamp +conformance-negative-control-absent: build/conformance-harness build/tables-generated/.stamp build/conformance-go @mkdir -p build @sed -e 's|if ( variable != NULL )$$|if ( variable != NULL \&\& !spill( out, f[1] + ".absent", "", 0 ) ) { return 1; } /* SABOTAGED */\n if ( variable != NULL ) { continue; }\n if ( false )|' \ test/conformance/cpp/main.cpp > build/conformance-cpp-absent.cpp @@ -2790,8 +2804,8 @@ tables-cook-endian: bin/schema # tracked file is ever written to: an interrupt cannot leave a sabotaged # working tree, and a parallel `make -j` cannot compile the sabotage into # something else. -.PHONY: tables-big-endian-negative -tables-big-endian-negative: tables-big-endian +.PHONY: tables-big-endian-negative-control +tables-big-endian-negative-control: tables-big-endian @mkdir -p build @sed 's|void put16( uint16_t v ) { uint8_t b\[2\] = { uint8_t( v ), uint8_t( v >> 8 ) }; raw( b, 2 ); }|void put16( uint16_t v ) { raw( \&v, 2 ); } // SABOTAGED: host order|' \ internal/codegen/cpptable/cpptable.go > build/cpptable-host-order.gotext @@ -2914,8 +2928,8 @@ tables-hostile-values: build/schema_test_hostile build/schema_test_hostile_asan # `+`, which RFC 8259 does not — and the gate must go red, because a tree the # manifest says is REFUSED starts packing. Same overlay mechanism as the wire # negative control: no tracked file is ever written to. -.PHONY: tables-hostile-negative -tables-hostile-negative: tables-hostile-values +.PHONY: tables-hostile-negative-control +tables-hostile-negative-control: tables-hostile-values @mkdir -p build @sed "s/in.text\[in.pos\] == '-' {/in.text[in.pos] == '-' || in.text[in.pos] == '+' { \/\/ SABOTAGED/" \ internal/tabletext/read.go > build/read-sabotaged.gotext @@ -2944,8 +2958,8 @@ tables-hostile-negative: tables-hostile-values # `go build -overlay`, so no tracked file is ever written to: an interrupt in # the middle of this target cannot leave a sabotaged working tree, and a # parallel `make -j` cannot compile the sabotage into something else. -.PHONY: tables-pack-negative -tables-pack-negative: tables-pack +.PHONY: tables-pack-negative-control +tables-pack-negative-control: tables-pack @mkdir -p build @sed 's/if !fv\.Present {/if true { \/\/ SABOTAGED: a present ?T elides/' \ internal/tablewire/encode.go > build/encode-sabotaged.gotext @@ -3167,9 +3181,9 @@ test: toolchain build/schema_test build/schema_test_guard build/schema_test_tabl $(MAKE) tables-runtime-home-negative-control $(MAKE) tables-block-inline-array-negative-control $(MAKE) tables-pack - $(MAKE) tables-pack-negative + $(MAKE) tables-pack-negative-control $(MAKE) tables-hostile-values - $(MAKE) tables-hostile-negative + $(MAKE) tables-hostile-negative-control ./build/schema_test_random ./build/schema_test_ludicrous ./build/schema_test_bench diff --git a/compiler/porting_test.go b/compiler/porting_test.go index 4fe29b2fa..55cc755c4 100644 --- a/compiler/porting_test.go +++ b/compiler/porting_test.go @@ -602,10 +602,10 @@ func TestPortingRegisterGateGoesRed(t *testing.T) { // still yields the target, a workflow comment yields nothing, and a leg // registered through a list variable in an included file is reached from // `test` — while one registered in no list is not - if got := makeTargetsAfter(` -j"$(nproc)" tables-big-endian-negative BE_CXX=g++ `); len(got) != 1 || got[0] != "tables-big-endian-negative" { + if got := makeTargetsAfter(` -j"$(nproc)" tables-big-endian-negative-control BE_CXX=g++ `); len(got) != 1 || got[0] != "tables-big-endian-negative-control" { t.Errorf("makeTargetsAfter read %q, want the one target", got) } - if got := workflowMakeTargets([]string{"# 87 s of its 109 is `make tables-big-endian-negative`\n"}); len(got) != 0 { + if got := workflowMakeTargets([]string{"# 87 s of its 109 is `make tables-big-endian-negative-control`\n"}); len(got) != 0 { t.Errorf("a workflow comment counted as a root: %q", got) } exists, reached := makefileReach([]string{ diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 5bd10173e..d4de99eba 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -105,6 +105,94 @@ merging, not just a green pull request. One more gate is not CI at all. A pull request is not merged until its authors have signed the [Contributor Assignment Agreement](#the-contributor-assignment-agreement). +## Negative controls + +A negative control breaks what a gate watches and requires the gate to go red. +It is how this repository proves a gate is watching something rather than +passing over an empty set. `go run ./tools/negativecontrols check` prints how +many there are, which is the only count that cannot go stale. Every one refuses +when its sabotage patches nothing, so a control whose `sed` pattern has drifted +off the line it aims at says so instead of reading as a pass. + +**Every negative control runs on every pull request**, in the `negative +controls ()` jobs, with the exceptions named below. A group is a set +of controls that share a toolchain and a compile and run in one `make -k` +invocation, so one refusal does not hide the ones behind it, and the jobs run +in parallel. The `base` groups need Go, a host C and C++ compiler and the +C/C++ serialize siblings; the `cs`, `js`, `dart`, `java`, `elixir` and `rust` +groups each add the one SDK their controls need, and `block-fuzz` needs two. + +**The rule that decides where a control runs is the owner's rule for CI that +runs per commit: one minute, two at the most.** It is a rule about a job, and +a matrix row is a job, so each group is cut to fit two minutes on the runner +rather than the leg as a whole being cut to fit. That is why the map gate runs +as four groups, why the tolerant-wire family runs one control per job (each of +those rebuilds the compiler under a source overlay and then fuzzes the +sabotaged wire, which costs 55 to 75 seconds), and why there are more groups +than toolchains. + +A control that does not fit the rule **on its own** is not made to fit by +grouping, so it runs nightly instead, in the `nightly` tier that `certify.yml` +runs on the schedule it already carries. Today that is one control: +`tables-message-form-negative-control`, which drives 49 sabotage rows one +submake each and takes 124 seconds. The message form's other blades stay on +the pull request. Each group in `make/negative-controls.json` names its tier in a +`when` field and says why in a `why` field, and `tools/negativecontrols` +refuses a group that names neither tier, so a control cannot leave the pull +request without landing on the nightly. + +The other exceptions are the `excluded` list, and each carries a reason the +test requires to be non-empty. Four of them are umbrella targets with no recipe +of their own, whose leaves already run in groups: +`packet-arm-defaults-negative-controls`, `tables-maps-negative-controls`, +`tables-wire-fuzz-negative-control` and `tables-wire-fuzz-retain-negative-control`. +One is a parameterized worker with no sabotage of its own, +`tables-message-form-one-negative-control`. The last is +`tables-big-endian-negative-control`, which the `big-endian` job already runs +on every pull request: it cross-compiles the tables battery for s390x, and a +second cross-compile on this leg proves nothing the first does not. + +**The target list is enumerated, not typed.** `tools/negativecontrols` reads +the Makefile and every file the Makefile includes, collects each explicit +target whose name carries `negative-control`, and holds that set against +`make/negative-controls.json`, which is the plan the leg's matrix comes from. +`go test ./tools/negativecontrols/` fails on any difference in either +direction: a control the makefiles define and the plan does not carry, and a +control the plan names and no makefile defines. + +**So the name is the convention, and `-negative-control` is the spelling.** A +control called `-negative` is invisible to the enumerator, which is a +control in no group, in no exclusion and on no workflow, with every test here +green. A marked head the reader cannot resolve to names, one written through a +variable or as a pattern rule, is refused rather than dropped, for the same +reason. + +The same package's tests hold two more joins nothing else in the tree holds: +the two legs are parsed as YAML, and each has to expand `${{ fromJSON(...) }}` +over the matrix job's output rather than a hand-typed include list; and the +toolchain versions in `make/negative-controls.json` have to match +`test/conformance//ci.json`, so a runtime bump moves both or neither. + +So adding a negative control costs one line in `make/negative-controls.json`, +in the group whose toolchain it needs and whose job still fits the rule with it +added, and forgetting that line is a red test rather than a control that runs +nowhere. A control that cannot run on either tier goes in the same file's +`excluded` list with a reason, which the test requires to be non-empty. Nothing +leaves the leg silently. + +Locally: + +```bash +go run ./tools/negativecontrols list # every control, and the file that defines it +go run ./tools/negativecontrols check # the plan against the makefiles +go run ./tools/negativecontrols matrix nightly # the groups the nightly tier runs +make -k $(go run ./tools/negativecontrols targets base) +``` + +The whole set is about thirteen minutes of machine time measured one target at +a time, and no single group is more than a minute of that, so running the group +your change touches before opening a pull request is cheap. + ## Changing generated output Any change to a backend's emitted code will move the goldens, and that is diff --git a/docs/PORTING.md b/docs/PORTING.md index 66a9c2f93..461efb743 100644 --- a/docs/PORTING.md +++ b/docs/PORTING.md @@ -460,7 +460,7 @@ and `:501-502`. **Measured effect.** The s390x battery (`tables-big-endian`, `conformance-big-endian`) is green under emulation. -**Negative control.** `tables-big-endian-negative` puts one store back to +**Negative control.** `tables-big-endian-negative-control` puts one store back to host order and requires red on the target while green on the host. **Targets:** none @@ -1092,7 +1092,7 @@ refuse, under s390x emulation); `tables-java-order`; **Measured effect.** Structural. -**Negative control.** `tables-big-endian-negative`; +**Negative control.** `tables-big-endian-negative-control`; `conformance-negative-control-c-foreign` neuters the byte swap and requires both foreign rows red with `cook` and `block` green. diff --git a/make/negative-controls.json b/make/negative-controls.json new file mode 100644 index 000000000..00e5197aa --- /dev/null +++ b/make/negative-controls.json @@ -0,0 +1,529 @@ +{ + "groups": [ + { + "name": "base", + "when": "pull-request", + "why": "the front-end and projection checks, the doc, view and rename controls, the JSON and block refusers, and the float-NaN widening control beside them: Go, the host C and C++ compilers and the C/C++ serialize siblings, and nothing else. Measured 28 s over 24 controls.", + "targets": [ + "check-enum-bound-negative-control", + "check-zero-range-negative-control", + "projection-union-arm-order-negative-control", + "projection-variant-order-negative-control", + "projection-wire-law-negative-control", + "tables-block-const-negative-control", + "tables-block-fill-refuser-negative-control", + "tables-block-race-negative-control", + "tables-clamp-limits-negative-control", + "tables-doctags-negative-controls", + "tables-float-nan-negative-control", + "tables-json-clamp-prefix-negative-control", + "tables-json-keyed-dup-negative-control", + "tables-json-list-walk-negative-controls", + "tables-json-negative-control", + "tables-retain-fixed-class-negative-control", + "tables-retain-message-form-negative-control", + "tables-runtime-home-negative-control", + "tables-view-containment-negative-control", + "tables-view-negative-controls", + "tables-was-negative-control", + "tables-wasrows-negative-control", + "tables-zero-cost-negative-control", + "toolchain-negative-control" + ] + }, + { + "name": "base-packet", + "when": "pull-request", + "why": "the four packet families on the host toolchain, and the wide-scalar controls beside them: each is one small compile off the generated tree, and they share it. Measured 42 s over 22 controls.", + "targets": [ + "packet-arm-defaults-c-negative-control", + "packet-arm-defaults-cpp-negative-control", + "packet-arm-defaults-go-negative-control", + "packet-defaults-c-negative-control", + "packet-defaults-go-negative-control", + "packet-utf8-c-negative-control", + "packet-utf8-go-negative-control", + "packet-void-c-read-negative-control", + "packet-void-c-write-negative-control", + "packet-void-cpp-read-negative-control", + "packet-void-cpp-write-negative-control", + "packet-wide-c-negative-control", + "packet-wide-go-negative-control", + "wide-no-align-negative-control", + "wide-surrogate-negative-control", + "wide-table-byte-length-negative-control", + "wide-table-clamp-negative-control", + "wide-table-odd-length-negative-control", + "wide-table-surrogate-negative-control", + "wide-table-zero-unit-negative-control", + "wide-terminator-negative-control", + "wide-utf8-read-negative-control" + ] + }, + { + "name": "base-conformance", + "when": "pull-request", + "why": "the conformance harness's own controls. The first of them rebuilds the C++ reference driver under a sabotage, which is 45 s of this group on the runner and the reason the absence control sits in a group of its own. Measured 22 s over 7 controls.", + "targets": [ + "conformance-negative-control", + "conformance-negative-control-block-dump", + "conformance-negative-control-c", + "conformance-negative-control-c-foreign", + "conformance-negative-control-go", + "conformance-negative-control-go-walk", + "conformance-negative-control-reference-surface" + ] + }, + { + "name": "base-conformance-absence", + "when": "pull-request", + "why": "one control, and it is alone because it costs 47 s on the runner: it rebuilds the C++ reference driver under a sabotage the way the group above does, and the two together do not fit the owner's rule in one job. Measured 15 s over 1 control.", + "targets": [ + "conformance-negative-control-absent" + ] + }, + { + "name": "base-maps", + "when": "pull-request", + "why": "the map gate's builder and shape controls, and the cook checker's walk over a map. The family ran as two jobs at 80 s and 74 s of control time on run 34115675206, which is inside the rule and too close to it, so it runs as four. Measured 20 s over 5 controls.", + "targets": [ + "tables-maps-ascending-negative-control", + "tables-maps-cap-negative-control", + "tables-maps-clamp-negative-control", + "tables-maps-cook-check-negative-control", + "tables-maps-dead-entry-negative-control" + ] + }, + { + "name": "base-maps-depth", + "when": "pull-request", + "why": "the map gate's depth, duplicate, entry-node and fit controls. Measured 19 s over 4 controls.", + "targets": [ + "tables-maps-depth-negative-control", + "tables-maps-duplicate-negative-control", + "tables-maps-entry-node-negative-control", + "tables-maps-fit-negative-control" + ] + }, + { + "name": "base-maps-keys", + "when": "pull-request", + "why": "the map gate's four key controls: the domain, the identity, the kind and the length. Measured 19 s over 4 controls.", + "targets": [ + "tables-maps-key-domain-negative-control", + "tables-maps-key-identity-negative-control", + "tables-maps-key-kind-negative-control", + "tables-maps-key-length-negative-control" + ] + }, + { + "name": "base-maps-order", + "when": "pull-request", + "why": "the map gate's placement, ordering and reset controls. Measured 19 s over 5 controls.", + "targets": [ + "tables-maps-place-failure-negative-control", + "tables-maps-sort-negative-control", + "tables-maps-text-order-negative-control", + "tables-maps-unreached-negative-control", + "tables-maps-value-reset-negative-control" + ] + }, + { + "name": "base-lists", + "when": "pull-request", + "why": "the list gate, whole, with the umbrella target that names every leaf: make builds each of them once in one invocation, so the umbrella costs this group nothing and stays on the leg. The cook tool's list control rides here for the same corpus. Measured 35 s over 14 controls.", + "targets": [ + "tables-lists-allocation-negative-control", + "tables-lists-clamp-negative-control", + "tables-lists-cook-check-negative-control", + "tables-lists-dead-element-negative-control", + "tables-lists-depth-negative-control", + "tables-lists-element-kind-negative-control", + "tables-lists-fit-negative-control", + "tables-lists-negative-controls", + "tables-lists-order-negative-control", + "tables-lists-preorder-negative-control", + "tables-lists-shared-negative-control", + "tables-lists-tool-cook-negative-control", + "tables-lists-unreached-negative-control", + "tables-lists-walk-order-negative-control" + ] + }, + { + "name": "base-arms", + "when": "pull-request", + "why": "the union-arm traversal gate, whole, with the umbrella target that names every leaf: make builds each of them once in one invocation, so the umbrella costs this group nothing and stays on the leg. Measured 16 s over 8 controls.", + "targets": [ + "tables-arms-array-framing-negative-control", + "tables-arms-cook-arm-negative-control", + "tables-arms-cook-check-negative-control", + "tables-arms-list-edge-negative-control", + "tables-arms-negative-controls", + "tables-arms-nested-union-negative-control", + "tables-arms-reachable-negative-control", + "tables-arms-slot-index-negative-control" + ] + }, + { + "name": "base-shared-node", + "when": "pull-request", + "why": "the shared-node control and the two Go fuzz controls, which read the same walk the arm gate does and are 82 s of control time beside it on run 34115675206. Measured 17 s over 3 controls.", + "targets": [ + "tables-go-fuzz-extent-negative-control", + "tables-go-fuzz-maximum-negative-control", + "tables-shared-node-negative-control" + ] + }, + { + "name": "base-cook", + "when": "pull-request", + "why": "the cook's controls, the blob span and read hooks, and the C leg's fuzz, soak and keyed refusal: one cooked corpus, built once, that every one of them reads. Measured 34 s over 11 controls.", + "targets": [ + "tables-blob-read-hooks-negative-control", + "tables-blob-span-negative-control", + "tables-c-fuzz-negative-control", + "tables-c-keyed-none-refusal-negative-control", + "tables-c-soak-negative-control", + "tables-cook-fuzz-negative-control", + "tables-cook-open-lengths-negative-control", + "tables-cook-open-root-negative-control", + "tables-cook-open-walk-negative-control", + "tables-cook-write-hooks-negative-control", + "tables-cook-write-negative-control" + ] + }, + { + "name": "base-keyed", + "when": "pull-request", + "why": "the keyed-collection controls. The iteration control is the long one. Measured 17 s over 3 controls.", + "targets": [ + "tables-keyed-iteration-negative-control", + "tables-keyed-max-refusal-negative-control", + "tables-keyed-none-refusal-negative-control" + ] + }, + { + "name": "base-keyed-shift", + "when": "pull-request", + "why": "the keyed shift control and the flat-wire one, which are 76 s of control time beside the group above on run 34115675206. Measured 12 s over 2 controls.", + "targets": [ + "tables-flat-wire-negative-control", + "tables-keyed-shift-negative-control" + ] + }, + { + "name": "base-pack", + "when": "pull-request", + "why": "the pack golden's framing control: elide a present `?T` in the Go encoder and the byte comparison against the C++ Save has to go red. Measured 7 s off one warm build.", + "targets": [ + "tables-pack-negative-control" + ] + }, + { + "name": "base-hostile", + "when": "pull-request", + "why": "the JSON number grammar's hostile-value control, alone. Its floor is the SANITIZED build of the hostile-value driver, which no other control shares, and the two pack controls in one job were 81 s of control time in a 113 s job on run 34120798793, the tightest job in the leg. Measured 14 s off one warm build.", + "targets": [ + "tables-hostile-negative-control" + ] + }, + { + "name": "message-form", + "when": "pull-request", + "why": "the message form's blades: the count, the emitter, the text content rule, the blob one, and the form-2 retain capture. The one control of this family that drives 49 sabotage rows one submake each does not fit the owner's rule alone, so it runs nightly. Measured 16 s over 5 controls.", + "targets": [ + "tables-message-form-blob-negative-control", + "tables-message-form-count-negative-control", + "tables-message-form-emitter-negative-control", + "tables-message-form-retain-negative-control", + "tables-message-form-text-negative-control" + ] + }, + { + "name": "wire-fuzz", + "when": "pull-request", + "why": "the tolerant-wire oracle and node-type controls, which read a pinned vector rather than rebuilding the compiler. The five oracles ran as one job at 82 s of control time on run 34115675206, most of it the fuzz driver they all build, so they run as two. Measured 11 s over 2 controls.", + "targets": [ + "tables-wire-fuzz-node-type-negative-control", + "tables-wire-fuzz-oracle-negative-control" + ] + }, + { + "name": "wire-fuzz-message-oracle", + "when": "pull-request", + "why": "the tolerant-wire oracles for the message form's text and blob records, and the retain one. Measured 3 s over 3 controls.", + "targets": [ + "tables-wire-fuzz-message-blob-oracle-negative-control", + "tables-wire-fuzz-message-text-oracle-negative-control", + "tables-wire-fuzz-retain-oracle-negative-control" + ] + }, + { + "name": "wire-fuzz-length", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 23 s on the author's machine and 59 s on the runner.", + "targets": [ + "tables-wire-fuzz-length-negative-control" + ] + }, + { + "name": "wire-fuzz-index", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 24 s on the author's machine and 62 s on the runner.", + "targets": [ + "tables-wire-fuzz-index-negative-control" + ] + }, + { + "name": "wire-fuzz-arm-terminator", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 23 s on the author's machine and 59 s on the runner.", + "targets": [ + "tables-wire-fuzz-arm-terminator-negative-control" + ] + }, + { + "name": "wire-fuzz-arm-width", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 30 s on the author's machine and 73 s on the runner, the longest of the family.", + "targets": [ + "tables-wire-fuzz-arm-width-negative-control" + ] + }, + { + "name": "wire-fuzz-blob-node", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 22 s on the author's machine and 58 s on the runner.", + "targets": [ + "tables-wire-fuzz-blob-node-negative-control" + ] + }, + { + "name": "wire-fuzz-wide-text", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 28 s on the author's machine and 75 s on the runner.", + "targets": [ + "tables-wire-fuzz-wide-text-negative-control" + ] + }, + { + "name": "wire-fuzz-message-text", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 22 s on the author's machine.", + "targets": [ + "tables-wire-fuzz-message-text-leg-negative-control" + ] + }, + { + "name": "wire-fuzz-message-blob", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 22 s on the author's machine.", + "targets": [ + "tables-wire-fuzz-message-blob-leg-negative-control" + ] + }, + { + "name": "wire-fuzz-retain-class", + "when": "pull-request", + "why": "one control per job. Each of these rebuilds the compiler under a source overlay and then runs the fuzzer over the sabotaged wire, which is 55 s to 75 s on the runner, so two of them in one job miss the owner's rule. Measured 29 s on the author's machine.", + "targets": [ + "tables-wire-fuzz-retain-class-negative-control" + ] + }, + { + "name": "block-fuzz", + "when": "pull-request", + "why": "the block fuzz extent oracle, which sabotages the C++, C# and Rust block emitters together and then requires all three fuzzers to go red, so one job needs both the .NET SDK and cargo. Measured 48 s on the runner.", + "targets": [ + "tables-block-fuzz-extent-negative-control" + ], + "rust": "stable", + "dotnet": "pin" + }, + { + "name": "block-fuzz-maximum", + "when": "pull-request", + "why": "the block fuzz maximum oracle, the extent oracle's twin, in a job of its own because the two together are 81 s of control time on top of two SDK installs. Measured 33 s on the runner.", + "targets": [ + "tables-block-fuzz-maximum-negative-control" + ], + "rust": "stable", + "dotnet": "pin" + }, + { + "name": "cs", + "when": "pull-request", + "why": "the C# port's conformance leg and its four packet families. Measured 13 s over 5 controls, on a host carrying no such SDK, so that is a floor and not the job's cost: it is the work up to the point the SDK is first reached.", + "targets": [ + "conformance-negative-control-cs", + "packet-arm-defaults-cs-negative-control", + "packet-defaults-cs-negative-control", + "packet-utf8-cs-negative-control", + "packet-wide-cs-negative-control" + ], + "dotnet": "pin" + }, + { + "name": "cs-cook", + "when": "pull-request", + "why": "the cook-open gates whose instrument is the C# reader, and the two block controls that only compile. Measured 24 s on the runner.", + "targets": [ + "tables-block-home-negative-control", + "tables-block-inline-array-negative-control", + "tables-cook-open-cs-lengths-negative-control", + "tables-cook-open-cs-root-negative-control", + "tables-cook-open-cs-walk-negative-control" + ], + "dotnet": "pin" + }, + { + "name": "cs-block-layout", + "when": "pull-request", + "why": "the block layout model control, alone: it moves an offset in the compiler's model and requires both the C++ static_asserts and the C# once-run layout check to go red, which is 39 s on the runner.", + "targets": [ + "tables-block-layout-model-negative-control" + ], + "dotnet": "pin" + }, + { + "name": "cs-block-pitch", + "when": "pull-request", + "why": "the block padding and pitch controls, whose instrument is the same C# layout check as the group above and which cost 31 s and 29 s on the runner.", + "targets": [ + "tables-block-padding-negative-control", + "tables-block-pitch-negative-control" + ], + "dotnet": "pin" + }, + { + "name": "rust", + "when": "pull-request", + "why": "the Rust port's name gate and its four packet families. Measured 3 s over 5 controls, on a host carrying no such SDK, so that is a floor and not the job's cost.", + "targets": [ + "packet-arm-defaults-rust-negative-control", + "packet-defaults-rust-negative-control", + "packet-utf8-rust-negative-control", + "packet-wide-rust-negative-control", + "tables-rust-names-negative-control" + ], + "rust": "stable" + }, + { + "name": "rust-alloc", + "when": "pull-request", + "why": "the Rust allocation audit's control, alone, because its prerequisite is a conformance run. That prerequisite is `conformance-rust`, the rust leg and the derived manifest the control reads, so this row installs one SDK where the bare `conformance` target's nine legs want all seven. Measured 5 s on a host carrying no such SDK.", + "targets": [ + "tables-rust-alloc-negative-control" + ], + "rust": "stable" + }, + { + "name": "js", + "when": "pull-request", + "why": "the JavaScript port's controls, the packet arm-default oracle's two JS legs, and the three newer packet families. Measured 9 s over 13 controls, on a host carrying no such SDK, so that is a floor and not the job's cost.", + "targets": [ + "conformance-negative-control-js", + "packet-arm-defaults-js-flat-negative-control", + "packet-arm-defaults-js-runtime-negative-control", + "packet-defaults-js-negative-control", + "packet-utf8-js-negative-control", + "packet-wide-js-negative-control", + "tables-js-accessor-negative-control", + "tables-js-alloc-negative-control", + "tables-js-fuzz-negative-control", + "tables-js-json-differential-negative-control", + "tables-js-keyed-negative-control", + "tables-js-runtime-home-negative-control", + "tables-js-slot-negative-control" + ], + "node": "20" + }, + { + "name": "dart", + "when": "pull-request", + "why": "the Dart port's controls and its four packet families. Measured 5 s over 10 controls, on a host carrying no such SDK, so that is a floor and not the job's cost.", + "targets": [ + "conformance-negative-control-dart", + "packet-arm-defaults-dart-negative-control", + "packet-defaults-dart-negative-control", + "packet-utf8-dart-negative-control", + "packet-wide-dart-negative-control", + "tables-dart-alloc-negative-control", + "tables-dart-fuzz-negative-control", + "tables-dart-names-negative-control", + "tables-dart-soak-negative-control", + "tables-dart-standalone-negative-control" + ], + "dart": "3.13.2" + }, + { + "name": "java", + "when": "pull-request", + "why": "the Java port's controls and its four packet families. Measured 6 s over 10 controls, on a host carrying no such SDK, so that is a floor and not the job's cost.", + "targets": [ + "conformance-negative-control-java", + "conformance-negative-control-java-block", + "packet-arm-defaults-java-negative-control", + "packet-defaults-java-negative-control", + "packet-utf8-java-negative-control", + "packet-wide-java-negative-control", + "tables-java-alloc-negative-control", + "tables-java-cook-extent-negative-control", + "tables-java-fuzz-negative-control", + "tables-java-soak-negative-control" + ], + "java": "21" + }, + { + "name": "elixir", + "when": "pull-request", + "why": "the Elixir port's controls and its three packet families. Measured 1 s over 8 controls, on a host carrying no such SDK, so that is a floor and not the job's cost.", + "targets": [ + "conformance-negative-control-elixir", + "packet-defaults-elixir-negative-control", + "packet-utf8-elixir-negative-control", + "packet-wide-elixir-negative-control", + "tables-elixir-alloc-negative-control", + "tables-elixir-block-lead-negative-control", + "tables-elixir-fuzz-negative-control", + "tables-elixir-soak-negative-control" + ], + "otp": "29.0.5", + "elixir": "1.20.4" + }, + { + "name": "nightly", + "when": "nightly", + "why": "the one control that does not fit the owner's rule for CI that runs per commit, on its own, at any group size: tables-message-form-negative-control drives 49 sabotage rows one submake each, 124 s on the runner where the rule is 120. The message form's four blades run on the pull request, so what moves nightly is this one pass and nothing else. The group stays a group even at one target, because it is the place a control lands the day it outgrows the rule. certify.yml runs this tier on the schedule it already carries.", + "targets": [ + "tables-message-form-negative-control" + ] + } + ], + "excluded": [ + { + "target": "packet-arm-defaults-negative-controls", + "reason": "an alias that re-invokes make once per language for the nine packet-arm-defaults--negative-control targets, every one of which this leg already runs in its own toolchain group. Running the alias too would need all seven SDKs on one runner to prove a second time what seven parallel jobs prove once." + }, + { + "target": "tables-message-form-one-negative-control", + "reason": "a parameterized worker with no sabotage of its own: it reads SABOTAGE, SABOTAGE_FILE and SABOTAGE_TEST from the command line, and tables-message-form-negative-control, which the nightly tier runs, drives it once for each of its 49 sabotage rows. Invoked bare it has nothing to break." + }, + { + "target": "tables-maps-negative-controls", + "reason": "an umbrella over the seventeen map-gate leaves, which the base-maps and base-maps-keys groups run between them. The family is too long for one job under the owner's rule, and an umbrella in either half would re-run the other half's eight or nine controls to prove a second time what the other job proves once." + }, + { + "target": "tables-wire-fuzz-negative-control", + "reason": "an umbrella over the twelve tolerant-wire leaves, which the wire-fuzz groups run between them. Each leaf rebuilds the compiler under an overlay and costs 55 s to 75 s on the runner, so they run one per job; the umbrella in any one of them would re-run all twelve and take the job to six minutes." + }, + { + "target": "tables-wire-fuzz-retain-negative-control", + "reason": "an umbrella over the two retain leaves, tables-wire-fuzz-retain-oracle-negative-control and tables-wire-fuzz-retain-class-negative-control, which the wire-fuzz-message-oracle and wire-fuzz-retain-class groups run between them. It carries no recipe of its own, so the umbrella in either group re-runs the other group's leaf to prove a second time what that job proves once." + }, + { + "target": "tables-big-endian-negative-control", + "reason": "the body of ci.yml's big-endian job, which runs it on every pull request already: the job cross-compiles the tables battery for s390x, runs it under qemu and then compiles it a second time for this control. Issue #684 owns moving that job, so the control is named here rather than placed in a group, and this leg does not cross-compile it a second time." + } + ] +} diff --git a/make/rust.mk b/make/rust.mk index a6caffaae..c6bbb16d3 100644 --- a/make/rust.mk +++ b/make/rust.mk @@ -201,6 +201,23 @@ tables-rust-soak: conformance tables-rust-alloc-audit: conformance ./build/conformance-rust build/conformance/manifest.txt alloc-audit +# THE RUST LEG OF THE MATRIX, ALONE, and the derived manifest a run leaves +# behind. `conformance` builds and runs all nine legs, so it needs every pinned +# SDK on the box; the harness derives the manifest before it runs any driver, +# and the rust driver is the only one this file's instruments read. So a target +# that wants the rust driver and the manifest asks for exactly that. +# +# The soak and the audit keep `conformance` as their prerequisite deliberately, +# because a NUMBER measured over a corpus whose matrix is red is a number about +# a defect. The negative control below is not a number: it asks whether the +# gate fires under one planted allocation, and that answer does not change with +# the other eight legs' verdicts. Which is what lets it ride the pull request +# in one job carrying one SDK, inside the owner's one-to-two-minute rule, +# rather than one job carrying seven. +.PHONY: conformance-rust +conformance-rust: build/conformance-harness build/conformance-rust + $(CONFORMANCE_ENV) ./build/conformance-harness run --only rust + # ITS NEGATIVE CONTROL, and the soak's. A gate that has never fired proves # nothing, and the LIVE-BYTE gate could not fire on this class at all: live # bytes answer "does this leak", and a path that allocates and frees the same @@ -208,7 +225,7 @@ tables-rust-alloc-audit: conformance # makes. SOAK_SABOTAGE puts ONE allocation per iteration inside the measured # region and both gates must go red on it. .PHONY: tables-rust-alloc-negative-control -tables-rust-alloc-negative-control: conformance +tables-rust-alloc-negative-control: conformance-rust @if SOAK_SABOTAGE=1 ./build/conformance-rust build/conformance/manifest.txt alloc-audit \ > build/rust-alloc-control.log 2>&1; then \ echo "NEGATIVE CONTROL FAILED: the allocation audit stayed green with one allocation per iteration"; \ diff --git a/tools/negativecontrols/main.go b/tools/negativecontrols/main.go new file mode 100644 index 000000000..f9ef96dbd --- /dev/null +++ b/tools/negativecontrols/main.go @@ -0,0 +1,132 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +const usage = `usage: go run ./tools/negativecontrols + + list every negative-control target the Makefile and its includes define + check hold the manifest against the Makefile; exit nonzero on a difference + matrix one tier's groups as a GitHub Actions matrix, one line of JSON: + matrix [pull-request|nightly], pull-request when no tier is named + targets the make targets of one group, space separated: targets +` + +func main() { + if len(os.Args) < 2 { + fmt.Fprint(os.Stderr, usage) + os.Exit(2) + } + root, err := repoRoot() + if err != nil { + fail(err) + } + switch os.Args[1] { + case "list": + defs, err := enumerate(root) + if err != nil { + fail(err) + } + for _, d := range defs { + fmt.Printf("%s\t%s\n", d.Target, d.File) + } + case "check": + defs, err := enumerate(root) + if err != nil { + fail(err) + } + m, err := loadManifest(root) + if err != nil { + fail(err) + } + missing, stale, err := reconcile(defs, m) + if err != nil { + fail(err) + } + bad := false + for _, t := range missing { + fmt.Fprintf(os.Stderr, "NEGATIVE CONTROL UNCOVERED: %s is defined and the pull-request leg does not run it; add it to a group in %s, or to the exclusion list with a reason\n", t, manifestPath) + bad = true + } + for _, t := range stale { + fmt.Fprintf(os.Stderr, "NEGATIVE CONTROL STALE: %s is named in %s and no makefile defines it\n", t, manifestPath) + bad = true + } + for _, e := range m.Skipped { + if strings.TrimSpace(e.Reason) == "" { + fmt.Fprintf(os.Stderr, "NEGATIVE CONTROL EXCLUDED WITHOUT A REASON: %s\n", e.Target) + bad = true + } + } + for _, g := range m.tiers() { + fmt.Fprintf(os.Stderr, "NEGATIVE CONTROL GROUP IN NO TIER: %s runs on neither the pull request nor the nightly; name %q or %q\n", g, whenPullRequest, whenNightly) + bad = true + } + if bad { + os.Exit(1) + } + fmt.Printf("every one of the %d negative controls is in a group or in an explained exclusion, and every group runs on the pull request or nightly\n", len(defs)) + case "matrix": + when := whenPullRequest + if len(os.Args) == 3 { + when = os.Args[2] + } else if len(os.Args) > 3 { + fmt.Fprint(os.Stderr, usage) + os.Exit(2) + } + m, err := loadManifest(root) + if err != nil { + fail(err) + } + out, err := m.matrix(when) + if err != nil { + fail(err) + } + fmt.Println(string(out)) + case "targets": + if len(os.Args) != 3 { + fmt.Fprint(os.Stderr, usage) + os.Exit(2) + } + m, err := loadManifest(root) + if err != nil { + fail(err) + } + targets, err := m.targetsOf(os.Args[2]) + if err != nil { + fail(err) + } + fmt.Println(targets) + default: + fmt.Fprint(os.Stderr, usage) + os.Exit(2) + } +} + +// repoRoot walks up from the working directory to the tree that holds the +// Makefile, so the tool runs the same from the root and from its own package. +func repoRoot() (string, error) { + dir, err := os.Getwd() + if err != nil { + return "", err + } + for { + if _, err := os.Stat(filepath.Join(dir, "Makefile")); err == nil { + return dir, nil + } + parent := filepath.Dir(dir) + if parent == dir { + return "", fmt.Errorf("no Makefile in any parent of the working directory") + } + dir = parent + } +} + +func fail(err error) { + fmt.Fprintln(os.Stderr, "negativecontrols:", err) + os.Exit(1) +} diff --git a/tools/negativecontrols/makefile.go b/tools/negativecontrols/makefile.go new file mode 100644 index 000000000..6c95534f0 --- /dev/null +++ b/tools/negativecontrols/makefile.go @@ -0,0 +1,239 @@ +// Package main enumerates the repository's negative-control make targets and +// holds them against the manifest the pull-request CI leg runs from. +// +// A negative control proves a gate is watching by breaking what the gate +// watches and requiring the gate to go red. A control whose sabotage pattern +// drifts off its target line patches nothing, refuses, and says so. That +// refusal only helps where somebody reads it, so the leg runs every control on +// every pull request, and this package is the part that makes "every" mean +// what it says: the target list is read out of the Makefile, not typed into a +// workflow, so a control added tomorrow is either in a job or in an exclusion +// with a reason. +package main + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// marker is the substring every negative-control target name carries. The rule +// is a substring rather than a suffix on purpose: the conformance family spells +// its controls `conformance-negative-control-`, and a suffix rule would +// let a whole family sit outside the leg without anybody choosing that. +const marker = "negative-control" + +// makefileSet is the Makefile plus every file it includes, in include order. +// The list comes out of the Makefile's own `include` lines rather than a glob +// typed here, so a new include is picked up by reading it. +func makefileSet(root string) ([]string, error) { + top := filepath.Join(root, "Makefile") + body, err := os.ReadFile(top) + if err != nil { + return nil, err + } + files := []string{top} + seen := map[string]bool{top: true} + for line := range strings.SplitSeq(string(body), "\n") { + trimmed := strings.TrimSpace(line) + rest, ok := strings.CutPrefix(trimmed, "include ") + if !ok { + rest, ok = strings.CutPrefix(trimmed, "-include ") + } + if !ok { + continue + } + // `include $(wildcard make/*.mk)` and a plain path both reduce to a + // glob once the wildcard call is unwrapped. + if inner, ok := strings.CutPrefix(strings.TrimSpace(rest), "$(wildcard"); ok { + rest = strings.TrimSuffix(strings.TrimSpace(inner), ")") + } + for field := range strings.FieldsSeq(rest) { + pattern := strings.TrimSpace(field) + if pattern == "" || strings.Contains(pattern, "$") { + return nil, fmt.Errorf("include line %q carries a variable this reader does not expand", trimmed) + } + matches, err := filepath.Glob(filepath.Join(root, pattern)) + if err != nil { + return nil, err + } + sort.Strings(matches) + for _, match := range matches { + if !seen[match] { + seen[match] = true + files = append(files, match) + } + } + } + } + return files, nil +} + +// definition is one negative-control target and the file that declares it. +type definition struct { + Target string + File string +} + +// enumerate reads every file in the set and returns each explicit target whose +// name carries the marker, sorted by name. +func enumerate(root string) ([]definition, error) { + files, err := makefileSet(root) + if err != nil { + return nil, err + } + found := map[string]string{} + for _, file := range files { + body, err := os.ReadFile(file) + if err != nil { + return nil, err + } + rel, err := filepath.Rel(root, file) + if err != nil { + rel = file + } + targets, err := targetsIn(string(body)) + if err != nil { + return nil, fmt.Errorf("%s: %w", rel, err) + } + for _, target := range targets { + if _, ok := found[target]; !ok { + found[target] = rel + } + } + } + out := make([]definition, 0, len(found)) + for target, file := range found { + out = append(out, definition{Target: target, File: file}) + } + sort.Slice(out, func(i, j int) bool { return out[i].Target < out[j].Target }) + return out, nil +} + +// targetsIn pulls the marked target names out of one makefile's text. It walks +// logical lines: a recipe line starts with a tab, a `define` block is skipped +// whole, and a trailing backslash continues the line. What is left is a rule +// head when a colon stands outside a variable reference with no `=` before it. +func targetsIn(body string) ([]string, error) { + var targets []string + var pending string + continuing := false + inDefine := false + for raw := range strings.SplitSeq(body, "\n") { + if continuing { + pending += " " + strings.TrimSpace(strings.TrimSuffix(raw, "\\")) + if strings.HasSuffix(raw, "\\") { + continue + } + continuing = false + names, err := headTargets(pending) + if err != nil { + return nil, err + } + targets = append(targets, names...) + pending = "" + continue + } + trimmed := strings.TrimSpace(raw) + if inDefine { + if strings.HasPrefix(trimmed, "endef") { + inDefine = false + } + continue + } + if strings.HasPrefix(trimmed, "define ") { + inDefine = true + continue + } + // A recipe line starts with a tab; a prerequisite continuation that + // also starts with a tab is consumed by the branch above. + if strings.HasPrefix(raw, "\t") || trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if cut, ok := strings.CutSuffix(raw, "\\"); ok { + pending = strings.TrimSpace(cut) + continuing = true + continue + } + names, err := headTargets(trimmed) + if err != nil { + return nil, err + } + targets = append(targets, names...) + } + return targets, nil +} + +// headTargets returns the marked names on the left of a rule's colon, or +// nothing when the line is an assignment, a directive, or has no rule colon. +// +// `.PHONY` is the one line whose RIGHT side names targets, and it is read as +// well as the left. Every control in this tree carries both a `.PHONY` line and +// a rule head, so the two readings agree today; reading both is what keeps a +// control findable if one of them is ever spelled through a variable. +// +// A marked head spelled through a variable (`$(GENERATED)-negative-control`) or +// as a pattern rule (`pattern-%-negative-control`) is REFUSED rather than +// dropped. This reader does not expand variables and does not know a pattern's +// instances, so it cannot name the controls such a head stands for; dropping it +// would leave those controls in no group and in no exclusion, with every test +// here green, which is the one outcome this package exists to prevent. +func headTargets(line string) ([]string, error) { + colon := ruleColon(line) + if colon < 0 { + return nil, nil + } + head := line[:colon] + if strings.ContainsAny(head, "=") { + return nil, nil + } + names := strings.Fields(head) + if len(names) == 1 && names[0] == ".PHONY" { + names = strings.Fields(strings.TrimLeft(line[colon:], ":")) + } + var out []string + for _, name := range names { + if !strings.Contains(name, marker) { + continue + } + if strings.ContainsAny(name, "$%") { + return nil, fmt.Errorf("the rule head %q names a %s through a variable or a pattern, and this reader expands neither, so it cannot say which controls the head stands for: spell them out, or take the marker out of the name", name, marker) + } + out = append(out, name) + } + return out, nil +} + +// ruleColon finds the colon that separates a rule's targets from its +// prerequisites: the first colon outside `$(...)`, ignoring `:=` and `::=`. +func ruleColon(line string) int { + depth := 0 + for i := 0; i < len(line); i++ { + switch line[i] { + case '$': + if i+1 < len(line) && (line[i+1] == '(' || line[i+1] == '{') { + depth++ + i++ + } + case ')', '}': + if depth > 0 { + depth-- + } + case ':': + if depth > 0 { + continue + } + rest := line[i:] + if strings.HasPrefix(rest, "::=") || strings.HasPrefix(rest, ":=") { + return -1 + } + if strings.HasPrefix(rest, "::") { + return i + } + return i + } + } + return -1 +} diff --git a/tools/negativecontrols/manifest.go b/tools/negativecontrols/manifest.go new file mode 100644 index 000000000..b6500299e --- /dev/null +++ b/tools/negativecontrols/manifest.go @@ -0,0 +1,221 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// manifestPath is the checked-in plan the CI leg runs from, relative to the +// repository root. +const manifestPath = "make/negative-controls.json" + +// Manifest is the plan: every enumerated control is in exactly one group or in +// the exclusion list, and nowhere else. +type Manifest struct { + Groups []Group `json:"groups"` + Skipped []Exclusion `json:"excluded"` +} + +// The two tiers a group runs in. The owner's rule for CI that runs per commit +// is one to two minutes, so a group rides the pull request exactly when it +// fits that budget, and a control that cannot fit it alone runs nightly +// instead. Every group names its tier and a group naming neither is refused: a +// typo may not quietly take a control off both workflows. +const ( + whenPullRequest = "pull-request" + whenNightly = "nightly" +) + +// Group is one matrix job: a set of controls that share a toolchain and run in +// one make invocation. +// +// The toolchain fields carry the version a row's setup step installs, and they +// are the same fields the conformance matrix uses, so the workflow conditions +// each step on a field rather than on a language name. A row that names none of +// them is a BASE row: Go, the host C and C++ compilers, and the sibling +// runtimes, which every job clones. +type Group struct { + // Name is the matrix row's name, the job's display name, and the word the + // leg hands back to `negativecontrols targets`. + Name string `json:"name"` + // When is the tier this group runs in: pull-request or nightly. + When string `json:"when"` + // Why says what this row's controls have in common, for a reader of the + // plan who is not reading the workflow. A nightly row's why line carries + // the measurement that put it there. + Why string `json:"why,omitempty"` + // Targets are the make targets this row runs, in one invocation. + Targets []string `json:"targets"` + + Rust string `json:"rust,omitempty"` + Dotnet string `json:"dotnet,omitempty"` + Node string `json:"node,omitempty"` + Dart string `json:"dart,omitempty"` + Java string `json:"java,omitempty"` + OTP string `json:"otp,omitempty"` + Elixir string `json:"elixir,omitempty"` +} + +// Exclusion is a control the leg does not run, and why. An exclusion is a +// choice somebody wrote down; the test refuses an empty reason, so a control +// cannot leave the leg silently. +type Exclusion struct { + Target string `json:"target"` + Reason string `json:"reason"` +} + +// versionedToolchains are the fields a group and a conformance registry entry +// (test/conformance//ci.json) spell the same way, with the same version +// string, because a group's setup step and a conformance leg's setup step +// install the same toolchain. `dotnet` is not among them on purpose: the .NET +// pin lives in .github/dotnet-version and both files carry a marker instead of +// the version, so there is no version here to hold against anything. +var versionedToolchains = []string{"rust", "node", "dart", "java", "otp", "elixir"} + +// toolchain returns the version this group installs for one of the fields in +// versionedToolchains, or the empty string when the group names none. +func (g Group) toolchain(field string) string { + switch field { + case "rust": + return g.Rust + case "node": + return g.Node + case "dart": + return g.Dart + case "java": + return g.Java + case "otp": + return g.OTP + case "elixir": + return g.Elixir + } + return "" +} + +// covered returns every target the manifest accounts for, and reports the +// first target that appears twice. +func (m Manifest) covered() (map[string]string, error) { + seen := map[string]string{} + for _, g := range m.Groups { + for _, t := range g.Targets { + if where, ok := seen[t]; ok { + return nil, fmt.Errorf("%s appears in both %s and %s", t, where, g.Name) + } + seen[t] = g.Name + } + } + for _, e := range m.Skipped { + if where, ok := seen[e.Target]; ok { + return nil, fmt.Errorf("%s appears in both %s and the exclusion list", e.Target, where) + } + seen[e.Target] = "excluded" + } + return seen, nil +} + +// loadManifest reads the plan from the repository root. +func loadManifest(root string) (Manifest, error) { + var m Manifest + body, err := os.ReadFile(filepath.Join(root, manifestPath)) + if err != nil { + return m, err + } + if err := json.Unmarshal(body, &m); err != nil { + return m, fmt.Errorf("%s: %w", manifestPath, err) + } + return m, nil +} + +// reconcile holds the manifest against the makefiles. It returns the controls +// the makefiles define and the manifest does not account for, and the controls +// the manifest names and no makefile defines. +func reconcile(defs []definition, m Manifest) (missing, stale []string, err error) { + covered, err := m.covered() + if err != nil { + return nil, nil, err + } + defined := map[string]bool{} + for _, d := range defs { + defined[d.Target] = true + if _, ok := covered[d.Target]; !ok { + missing = append(missing, d.Target) + } + } + for target := range covered { + if !defined[target] { + stale = append(stale, target) + } + } + sort.Strings(missing) + sort.Strings(stale) + return missing, stale, nil +} + +// tiers reports the groups whose `when` is neither tier, by name. A group that +// names no tier runs on neither workflow, which is a control running nowhere +// dressed as a plan. +func (m Manifest) tiers() []string { + var bad []string + for _, g := range m.Groups { + if g.When != whenPullRequest && g.When != whenNightly { + bad = append(bad, fmt.Sprintf("%s (when %q)", g.Name, g.When)) + } + } + sort.Strings(bad) + return bad +} + +// matrix renders one tier's groups as the GitHub Actions matrix that tier's +// workflow expands, so the set a workflow runs is the set this file names and +// nothing else. A row carries its name and its toolchain fields; the targets +// themselves come back through `targets ` inside the job, which keeps a +// hundred-target line out of the matrix value. +func (m Manifest) matrix(when string) ([]byte, error) { + type row struct { + Name string `json:"name"` + Rust string `json:"rust"` + Dotnet string `json:"dotnet"` + Node string `json:"node"` + Dart string `json:"dart"` + Java string `json:"java"` + OTP string `json:"otp"` + Elixir string `json:"elixir"` + } + if when != whenPullRequest && when != whenNightly { + return nil, fmt.Errorf("%q is not a tier: the tiers are %s and %s", when, whenPullRequest, whenNightly) + } + rows := make([]row, 0, len(m.Groups)) + for _, g := range m.Groups { + if g.When != when { + continue + } + rows = append(rows, row{ + Name: g.Name, + Rust: g.Rust, + Dotnet: g.Dotnet, + Node: g.Node, + Dart: g.Dart, + Java: g.Java, + OTP: g.OTP, + Elixir: g.Elixir, + }) + } + return json.Marshal(struct { + Include []row `json:"include"` + }{rows}) +} + +// targetsOf returns one group's make targets as the single line the leg passes +// to make. +func (m Manifest) targetsOf(name string) (string, error) { + for _, g := range m.Groups { + if g.Name == name { + return strings.Join(g.Targets, " "), nil + } + } + return "", fmt.Errorf("no group named %q in %s", name, manifestPath) +} diff --git a/tools/negativecontrols/negativecontrols_test.go b/tools/negativecontrols/negativecontrols_test.go new file mode 100644 index 000000000..c11ffd00e --- /dev/null +++ b/tools/negativecontrols/negativecontrols_test.go @@ -0,0 +1,552 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "testing" +) + +// TestEveryDefinedControlIsInThePlan is the rule the pull-request leg rests on. +// The leg's matrix comes out of make/negative-controls.json, and this test holds +// that file against the Makefile and its includes: a control the makefiles define +// and the plan does not carry fails here, and so does a control the plan names +// and no makefile defines. Adding a negative control therefore costs one line in +// the plan, and forgetting that line is a red test rather than a control that +// runs nowhere. +func TestEveryDefinedControlIsInThePlan(t *testing.T) { + root := testRoot(t) + defs, err := enumerate(root) + if err != nil { + t.Fatal(err) + } + if len(defs) == 0 { + t.Fatal("no negative-control targets found: this test would pass over an empty set") + } + m, err := loadManifest(root) + if err != nil { + t.Fatal(err) + } + missing, stale, err := reconcile(defs, m) + if err != nil { + t.Fatal(err) + } + for _, target := range missing { + t.Errorf("%s is defined and the pull-request leg does not run it: add it to a group in %s, or to the exclusion list with a reason", target, manifestPath) + } + for _, target := range stale { + t.Errorf("%s is named in %s and no makefile defines it: drop the line", target, manifestPath) + } + t.Logf("%d negative controls, all of them in a group or in an explained exclusion", len(defs)) +} + +// TestEveryExclusionCarriesAReason keeps the exclusion list from becoming a +// silent skip. A control leaves the leg only where somebody wrote down why. +func TestEveryExclusionCarriesAReason(t *testing.T) { + m, err := loadManifest(testRoot(t)) + if err != nil { + t.Fatal(err) + } + for _, e := range m.Skipped { + if strings.TrimSpace(e.Reason) == "" { + t.Errorf("%s is excluded from the leg with no reason", e.Target) + } + } +} + +// TestGroupsAreNamedAndPopulated catches the two shapes of a plan that would +// expand into a matrix nothing runs: an unnamed row, and an empty one. +func TestGroupsAreNamedAndPopulated(t *testing.T) { + m, err := loadManifest(testRoot(t)) + if err != nil { + t.Fatal(err) + } + if len(m.Groups) == 0 { + t.Fatal("the plan carries no groups, so the leg's matrix is empty") + } + names := map[string]bool{} + for _, g := range m.Groups { + if strings.TrimSpace(g.Name) == "" { + t.Error("a group has no name, so its matrix row has no job name") + } + if names[g.Name] { + t.Errorf("two groups are named %q", g.Name) + } + names[g.Name] = true + if len(g.Targets) == 0 { + t.Errorf("group %q runs no targets", g.Name) + } + } +} + +// TestEveryGroupRunsInATier is the owner's rule made mechanical. CI that runs +// per commit finishes inside one to two minutes, so a group either fits that +// budget and rides the pull request, or it does not and runs nightly. A group +// that names neither tier appears in no workflow's matrix, which is a control +// running nowhere with a plan entry that looks like coverage. +func TestEveryGroupRunsInATier(t *testing.T) { + m, err := loadManifest(testRoot(t)) + if err != nil { + t.Fatal(err) + } + for _, bad := range m.tiers() { + t.Errorf("group %s runs on neither the pull request nor the nightly: name %q or %q", bad, whenPullRequest, whenNightly) + } + for _, when := range []string{whenPullRequest, whenNightly} { + out, err := m.matrix(when) + if err != nil { + t.Fatalf("the %s matrix does not render: %v", when, err) + } + if strings.Contains(string(out), `"include":[]`) || strings.Contains(string(out), `"include":null`) { + t.Errorf("the %s matrix is empty, so its workflow expands to no job", when) + } + } +} + +// TestTheLegRunsTheManifestAndNotATypedList closes the loop between this +// package and the workflows. The enumeration is only worth its cost while a +// leg's target list IS the manifest: a job that typed its own list would pass +// every test above and still miss a control. So both workflows are read here, +// and each must reach its matrix and its targets through this tool: ci.yml for +// the pull-request tier, certify.yml for the nightly one, which is where this +// repository's schedule lives. +func TestTheLegRunsTheManifestAndNotATypedList(t *testing.T) { + root := testRoot(t) + for workflow, wants := range map[string][]string{ + "ci.yml": { + "go run ./tools/negativecontrols check", + "go run ./tools/negativecontrols matrix", + "go run ./tools/negativecontrols targets", + }, + "certify.yml": { + "go run ./tools/negativecontrols matrix nightly", + "go run ./tools/negativecontrols targets", + }, + } { + body, err := os.ReadFile(filepath.Join(root, ".github", "workflows", workflow)) + if err != nil { + t.Fatal(err) + } + for _, want := range wants { + if !strings.Contains(string(body), want) { + t.Errorf(".github/workflows/%s does not run %q, so that tier's target list is not the one this package enumerates", workflow, want) + } + } + } + m, err := loadManifest(root) + if err != nil { + t.Fatal(err) + } + // A group name is a matrix value, so it may not carry shell metacharacters + // or whitespace: the leg passes it to the tool as one word. + for _, g := range m.Groups { + if strings.ContainsAny(g.Name, " \t\"'$`;&|<>()") { + t.Errorf("group name %q is not a single shell-safe word", g.Name) + } + } +} + +// TestTargetsInReadsTheRuleHeadsAMakefileWrites pins the parser against the +// spellings this tree actually uses, each of which a simpler reader gets wrong: +// several targets on one head, a head continued over a backslash, a recipe line +// that names a target, a `define` block, and `:=` assignment. +func TestTargetsInReadsTheRuleHeadsAMakefileWrites(t *testing.T) { + const body = ` +# a comment: fake-negative-control: not a rule +NC_FLAGS := -Ifoo:bar-negative-control +.PHONY: phony-negative-control alpha-negative-control + +alpha-negative-control beta-negative-control: prereq + @echo not-a-negative-control-target + +gamma-negative-control \ + delta-negative-control: prereq + @echo two + +define A_MACRO +inside-negative-control: nothing +endef + +double-negative-control:: prereq + @echo double + +VAR_WITH_COLON = a:b-negative-control +` + targets, err := targetsIn(body) + if err != nil { + t.Fatal(err) + } + got := map[string]bool{} + for _, target := range targets { + got[target] = true + } + for _, want := range []string{ + "phony-negative-control", "alpha-negative-control", "beta-negative-control", + "gamma-negative-control", "delta-negative-control", "double-negative-control", + } { + if !got[want] { + t.Errorf("targetsIn missed %s", want) + } + } + for _, unwanted := range []string{ + "not-a-negative-control-target", "inside-negative-control", + "a:b-negative-control", "-Ifoo:bar-negative-control", + } { + if got[unwanted] { + t.Errorf("targetsIn invented %s", unwanted) + } + } +} + +// TestAMarkedHeadThisReaderCannotNameIsRefused is the other half of the parser +// contract. A control whose head is spelled through a variable or as a pattern +// rule has a name this reader cannot resolve, and a reader that drops such a +// head leaves those controls in no group, in no exclusion, and in no job, with +// every test in this file green. So the head is refused by name instead, which +// is the one outcome an author can act on. +func TestAMarkedHeadThisReaderCannotNameIsRefused(t *testing.T) { + for _, body := range []string{ + "$(GENERATED)-negative-control: prereq\n\t@echo generated\n", + "pattern-%-negative-control: prereq\n\t@echo pattern\n", + ".PHONY: $(LANGS:%=packet-%-negative-control)\n", + "head-negative-control \\\n\t$(OTHER)-negative-control: prereq\n\t@echo continued\n", + } { + targets, err := targetsIn(body) + if err == nil { + t.Errorf("targetsIn read %q and returned %q instead of refusing a head it cannot name", body, targets) + continue + } + if !strings.Contains(err.Error(), marker) { + t.Errorf("the refusal for %q does not name the head: %v", body, err) + } + } +} + +// TestEnumerateReadsEveryIncludedFile proves the reader follows the Makefile's +// own include lines rather than a glob typed into this package: every file the +// tree includes contributes, and the per-language includes are where most of +// the toolchain-bound controls live. +func TestEnumerateReadsEveryIncludedFile(t *testing.T) { + root := testRoot(t) + files, err := makefileSet(root) + if err != nil { + t.Fatal(err) + } + seen := map[string]bool{} + for _, f := range files { + rel, err := filepath.Rel(root, f) + if err != nil { + t.Fatal(err) + } + seen[rel] = true + } + for _, want := range []string{"Makefile", "make/js.mk", "make/checks/packet-arm-defaults.mk"} { + if !seen[want] { + t.Errorf("the include set does not carry %s", want) + } + } +} + +// legs are the two jobs that expand this package's plan: one per tier, each in +// the workflow that tier runs on. +var legs = []struct { + workflow string + job string + matrixJob string + command string +}{ + {"ci.yml", "negative-controls", "negative-controls-matrix", "go run ./tools/negativecontrols matrix"}, + {"certify.yml", "negative-controls-nightly", "negative-controls-nightly-matrix", "go run ./tools/negativecontrols matrix nightly"}, +} + +// TestEachLegExpandsTheToolsMatrix reads the workflows as YAML rather than as +// text. The test above it asks whether the file CONTAINS the tool's commands, +// which a leg that kept the old expression in a comment and typed an include +// list beside it still satisfies. This one asks what the leg actually expands: +// its `strategy.matrix` has to BE the matrix job's output, and it has to +// `needs` that job, or the leg runs whatever somebody remembered rather than +// the plan. +func TestEachLegExpandsTheToolsMatrix(t *testing.T) { + root := testRoot(t) + for _, leg := range legs { + body, err := os.ReadFile(filepath.Join(root, ".github", "workflows", leg.workflow)) + if err != nil { + t.Fatal(err) + } + doc, err := parseWorkflow(string(body)) + if err != nil { + t.Fatalf(".github/workflows/%s does not parse: %v", leg.workflow, err) + } + + matrix, err := mappingAt(doc, "jobs", leg.job, "strategy", "matrix") + if err != nil { + t.Errorf(".github/workflows/%s: %v", leg.workflow, err) + continue + } + want := fmt.Sprintf("${{ fromJSON(needs.%s.outputs.matrix) }}", leg.matrixJob) + if matrix != want { + t.Errorf(".github/workflows/%s: the %s job expands %#v as its matrix, want %q: a matrix written any other way is a target list this package did not enumerate", leg.workflow, leg.job, matrix, want) + } + + needsValue, err := mappingAt(doc, "jobs", leg.job, "needs") + if err != nil { + t.Errorf(".github/workflows/%s: the %s job names no `needs`, so its matrix expression resolves to nothing: %v", leg.workflow, leg.job, err) + continue + } + needs, err := stringsOf(needsValue) + if err != nil { + t.Errorf(".github/workflows/%s: the %s job's `needs` is not a job name or a list of them: %v", leg.workflow, leg.job, err) + continue + } + if !slices.Contains(needs, leg.matrixJob) { + t.Errorf(".github/workflows/%s: the %s job needs %q and not %s", leg.workflow, leg.job, needs, leg.matrixJob) + } + + // And the other end of the same wire: the matrix job's output is a + // step's, and that step runs this tool. + output, err := mappingAt(doc, "jobs", leg.matrixJob, "outputs", "matrix") + if err != nil { + t.Errorf(".github/workflows/%s: %v", leg.workflow, err) + continue + } + id, ok := stepIDOf(output) + if !ok { + t.Errorf(".github/workflows/%s: the %s job's matrix output is %#v, which names no step", leg.workflow, leg.matrixJob, output) + continue + } + steps, err := mappingAt(doc, "jobs", leg.matrixJob, "steps") + if err != nil { + t.Errorf(".github/workflows/%s: %v", leg.workflow, err) + continue + } + run, ok := runOfStep(steps, id) + if !ok { + t.Errorf(".github/workflows/%s: the %s job has no step with id %q", leg.workflow, leg.matrixJob, id) + continue + } + if !strings.Contains(run, leg.command) { + t.Errorf(".github/workflows/%s: step %q of %s does not run %q, so the matrix it publishes is not the plan's:\n%s", leg.workflow, id, leg.matrixJob, leg.command, run) + } + } +} + +// stepIDOf reads the step id out of a `${{ steps..outputs.matrix }}` +// expression. +func stepIDOf(value any) (string, bool) { + text, ok := value.(string) + if !ok { + return "", false + } + rest, ok := strings.CutPrefix(strings.TrimSpace(text), "${{ steps.") + if !ok { + return "", false + } + id, _, ok := strings.Cut(rest, ".outputs.matrix }}") + if !ok || id == "" { + return "", false + } + return id, true +} + +// runOfStep finds one step of a job by its id and returns its `run` script. +func runOfStep(steps any, id string) (string, bool) { + items, ok := steps.([]any) + if !ok { + return "", false + } + for _, item := range items { + step, ok := item.(map[string]any) + if !ok { + continue + } + if step["id"] != id { + continue + } + run, ok := step["run"].(string) + return run, ok + } + return "", false +} + +// TestParseWorkflowReadsTheShapesAWorkflowWrites pins the reader against the +// spellings these two files use, each of which a line-at-a-time scan gets +// wrong: a scalar carrying an expression, a `needs` written as a list, a step +// sequence whose items are mappings, an inline comment after a value, and a +// `run:` block whose script lines look like mapping entries and sequence items +// and are neither. +func TestParseWorkflowReadsTheShapesAWorkflowWrites(t *testing.T) { + const body = `name: Example + +# a comment: not: a: mapping +on: + schedule: + - cron: '17 9 * * *' + workflow_dispatch: + +jobs: + plan-job: + runs-on: ubuntu-latest + outputs: + matrix: ${{ steps.plan.outputs.matrix }} + steps: + - uses: actions/checkout@abc123 # v7.0.1 + - id: plan + name: the plan, as a matrix + run: | + matrix=$(go run ./tools/negativecontrols matrix) + # - not: a sequence item + if grep -q 'a: b' out; then echo "::error::no"; fi + + leg: + needs: + - plan-job + - other + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.plan-job.outputs.matrix) }} +` + doc, err := parseWorkflow(body) + if err != nil { + t.Fatal(err) + } + if doc["name"] != "Example" { + t.Errorf("the document's name is %#v", doc["name"]) + } + matrix, err := mappingAt(doc, "jobs", "leg", "strategy", "matrix") + if err != nil { + t.Fatal(err) + } + if want := "${{ fromJSON(needs.plan-job.outputs.matrix) }}"; matrix != want { + t.Errorf("the leg's matrix reads %#v, want %q", matrix, want) + } + needsValue, err := mappingAt(doc, "jobs", "leg", "needs") + if err != nil { + t.Fatal(err) + } + needs, err := stringsOf(needsValue) + if err != nil { + t.Fatal(err) + } + if len(needs) != 2 || needs[0] != "plan-job" || needs[1] != "other" { + t.Errorf("the leg needs %q", needs) + } + output, err := mappingAt(doc, "jobs", "plan-job", "outputs", "matrix") + if err != nil { + t.Fatal(err) + } + if id, ok := stepIDOf(output); !ok || id != "plan" { + t.Errorf("the output names step %q (read %v)", id, ok) + } + steps, err := mappingAt(doc, "jobs", "plan-job", "steps") + if err != nil { + t.Fatal(err) + } + if items, ok := steps.([]any); !ok || len(items) != 2 { + t.Fatalf("the plan job has %#v for steps", steps) + } + run, ok := runOfStep(steps, "plan") + if !ok { + t.Fatal("the plan step has no run script") + } + for _, want := range []string{"go run ./tools/negativecontrols matrix", "# - not: a sequence item", "if grep -q 'a: b' out"} { + if !strings.Contains(run, want) { + t.Errorf("the run script lost %q:\n%s", want, run) + } + } + // A single-quoted cron value keeps its colon-free text, and a `- ` item + // under a nested key is a sequence and not a mapping. + schedule, err := mappingAt(doc, "on", "schedule") + if err != nil { + t.Fatal(err) + } + if items, ok := schedule.([]any); !ok || len(items) != 1 { + t.Fatalf("the schedule reads %#v", schedule) + } + // A line that opens no block and follows nothing is a refusal, not a + // silently dropped line. + if _, err := parseWorkflow("jobs:\n leg:\n runs-on: x\n stray\n"); err == nil { + t.Error("a line at an indentation no block opened parsed clean") + } +} + +// TestToolchainPinsMatchTheConformanceRegistry holds the plan's toolchain +// versions against test/conformance//ci.json, which is where the same +// version is written for the conformance legs. Both files install a toolchain +// for the same generated code, so a bump in one and not the other means a +// negative control runs against a runtime its own conformance leg no longer +// uses, and nothing else in this tree would say so. +func TestToolchainPinsMatchTheConformanceRegistry(t *testing.T) { + root := testRoot(t) + files, err := filepath.Glob(filepath.Join(root, "test", "conformance", "*", "ci.json")) + if err != nil { + t.Fatal(err) + } + if len(files) == 0 { + t.Fatal("no test/conformance/*/ci.json: this test would pass over an empty set") + } + registry := map[string]string{} + source := map[string]string{} + for _, file := range files { + body, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + var entry map[string]string + if err := json.Unmarshal(body, &entry); err != nil { + t.Fatalf("%s: %v", file, err) + } + lang := filepath.Base(filepath.Dir(file)) + for _, field := range versionedToolchains { + version := entry[field] + if version == "" { + continue + } + if held, ok := registry[field]; ok && held != version { + t.Errorf("the registry pins %s at %s in %s and at %s in %s", field, held, source[field], version, lang) + continue + } + registry[field] = version + source[field] = lang + } + } + m, err := loadManifest(root) + if err != nil { + t.Fatal(err) + } + for _, g := range m.Groups { + for _, field := range versionedToolchains { + version := g.toolchain(field) + if version == "" { + continue + } + held, ok := registry[field] + if !ok { + t.Errorf("group %q installs %s %s and no test/conformance/*/ci.json names that toolchain", g.Name, field, version) + continue + } + if held != version { + t.Errorf("group %q installs %s %s and test/conformance/%s/ci.json pins %s: bump both or neither", g.Name, field, version, source[field], held) + } + } + // The .NET pin is the one version neither file carries: it lives in + // .github/dotnet-version, and both workflows read it from there. A + // group that wrote a version here would be a second pin. + if strings.ContainsAny(g.Dotnet, "0123456789") { + t.Errorf("group %q pins dotnet at %q: the SDK version lives in .github/dotnet-version, and this field only says the row needs it", g.Name, g.Dotnet) + } + } +} + +// testRoot is the tree under test: the package runs from its own directory, and +// the makefiles it reads are the repository's. +func testRoot(t *testing.T) string { + t.Helper() + root, err := repoRoot() + if err != nil { + t.Fatal(err) + } + return root +} diff --git a/tools/negativecontrols/workflow.go b/tools/negativecontrols/workflow.go new file mode 100644 index 000000000..81782399e --- /dev/null +++ b/tools/negativecontrols/workflow.go @@ -0,0 +1,276 @@ +package main + +import ( + "fmt" + "strings" +) + +// A GitHub Actions workflow is block YAML, and the questions this package asks +// of one are structural: which job a leg needs, and what its `strategy.matrix` +// is. A substring scan answers neither. It reports that the text appears +// somewhere in the file, so a leg that replaced its matrix expression with a +// hand-typed include list and left the old expression in a comment reads the +// same as a leg that runs the plan. +// +// So the workflow is parsed. This reader covers the subset the workflows in +// this tree are written in: block mappings, block sequences, block scalars +// (`|` and `>`), plain and quoted scalars, and comment lines. It has no +// anchors, no multi-document files and no tag resolution; a flow collection +// (`branches: [ main ]`) stays the scalar text it was written as, because +// nothing here reads one. Every scalar stays a string, quotes and inline +// comments included, for the same reason. A spelling this reader does not +// understand is an error rather than a silent omission. + +// yamlLine is one significant line: its indentation, its text with that +// indentation stripped, whether it is a whole-line comment, and where it came +// from. +type yamlLine struct { + indent int + text string + comment bool + number int +} + +// significantLines drops blank lines and measures the indentation of what is +// left. A comment line is kept and marked: it carries no structure, so every +// walk below steps over it, but a comment inside a block scalar is part of the +// script that scalar holds and a reader that dropped it would hand back a +// script the workflow does not run. A blank line carries neither, so dropping +// it costs the reader nothing. +func significantLines(body string) []yamlLine { + var out []yamlLine + for i, raw := range strings.Split(body, "\n") { + text := strings.TrimLeft(raw, " ") + if text == "" { + continue + } + out = append(out, yamlLine{ + indent: len(raw) - len(text), + text: strings.TrimRight(text, " "), + comment: strings.HasPrefix(text, "#"), + number: i + 1, + }) + } + return out +} + +// skipComments advances past the comment lines standing at the cursor, so a +// walk that reads structure never has to look at one. +func skipComments(lines []yamlLine, at *int) { + for *at < len(lines) && lines[*at].comment { + *at++ + } +} + +// parseWorkflow reads a workflow file into nested values: a mapping is a +// map[string]any, a sequence is a []any, and a scalar is a string. +func parseWorkflow(body string) (map[string]any, error) { + lines := significantLines(body) + at := 0 + skipComments(lines, &at) + if at == len(lines) { + return nil, fmt.Errorf("the workflow holds nothing but comments") + } + value, err := parseNode(lines, &at, lines[at].indent) + if err != nil { + return nil, err + } + skipComments(lines, &at) + if at != len(lines) { + return nil, fmt.Errorf("line %d: %q sits at an indentation no block opened", lines[at].number, lines[at].text) + } + doc, ok := value.(map[string]any) + if !ok { + return nil, fmt.Errorf("the workflow's top level is not a mapping") + } + return doc, nil +} + +// parseNode reads one block at the given indentation: a sequence when the +// first line opens with a dash, a mapping otherwise. +func parseNode(lines []yamlLine, at *int, indent int) (any, error) { + skipComments(lines, at) + if *at >= len(lines) { + return nil, fmt.Errorf("a block was expected and the file ended") + } + if isSequenceItem(lines[*at].text) { + return parseSequence(lines, at, indent) + } + return parseMapping(lines, at, indent) +} + +func isSequenceItem(text string) bool { + return text == "-" || strings.HasPrefix(text, "- ") +} + +// parseMapping reads the entries standing at one indentation. A value on the +// key's own line is a scalar; `|` or `>` opens a block scalar, whose lines are +// every line indented past the key; an empty value opens a nested block, which +// a sequence may share the key's indentation with. +func parseMapping(lines []yamlLine, at *int, indent int) (map[string]any, error) { + out := map[string]any{} + for { + skipComments(lines, at) + if *at >= len(lines) || lines[*at].indent != indent || isSequenceItem(lines[*at].text) { + break + } + line := lines[*at] + key, rest, ok := splitMappingKey(line.text) + if !ok { + return nil, fmt.Errorf("line %d: %q is neither a mapping entry nor a sequence item", line.number, line.text) + } + if _, seen := out[key]; seen { + return nil, fmt.Errorf("line %d: the key %q appears twice in one mapping", line.number, key) + } + *at++ + if strings.HasPrefix(rest, "|") || strings.HasPrefix(rest, ">") { + // A block scalar's lines are its script, comment lines included. + var block []string + for *at < len(lines) && lines[*at].indent > indent { + block = append(block, lines[*at].text) + *at++ + } + out[key] = strings.Join(block, "\n") + continue + } + if rest != "" { + out[key] = rest + continue + } + peek := *at + skipComments(lines, &peek) + switch { + case peek < len(lines) && lines[peek].indent > indent: + *at = peek + child, err := parseNode(lines, at, lines[peek].indent) + if err != nil { + return nil, err + } + out[key] = child + case peek < len(lines) && lines[peek].indent == indent && isSequenceItem(lines[peek].text): + *at = peek + child, err := parseSequence(lines, at, indent) + if err != nil { + return nil, err + } + out[key] = child + default: + out[key] = "" + } + } + return out, nil +} + +// parseSequence reads the items standing at one indentation. An item's first +// line carries its content after the dash, and the item's remaining lines are +// indented two past the dash, so the item is read as a block of its own. +func parseSequence(lines []yamlLine, at *int, indent int) ([]any, error) { + var out []any + for { + skipComments(lines, at) + if *at >= len(lines) || lines[*at].indent != indent || !isSequenceItem(lines[*at].text) { + break + } + line := lines[*at] + inner := strings.TrimSpace(strings.TrimPrefix(line.text, "-")) + *at++ + if inner == "" { + peek := *at + skipComments(lines, &peek) + if peek >= len(lines) || lines[peek].indent <= indent { + out = append(out, "") + continue + } + *at = peek + child, err := parseNode(lines, at, lines[peek].indent) + if err != nil { + return nil, err + } + out = append(out, child) + continue + } + item := []yamlLine{{indent: indent + 2, text: inner, number: line.number}} + for *at < len(lines) && lines[*at].indent > indent { + item = append(item, lines[*at]) + *at++ + } + if _, _, ok := splitMappingKey(inner); !ok { + if len(item) > 1 { + return nil, fmt.Errorf("line %d: a scalar sequence item carries an indented block", line.number) + } + out = append(out, inner) + continue + } + innerAt := 0 + child, err := parseNode(item, &innerAt, indent+2) + if err != nil { + return nil, err + } + out = append(out, child) + } + return out, nil +} + +// splitMappingKey cuts a line at the colon that ends its key: the first colon +// outside quotes that a space or the end of the line follows. A colon inside a +// quoted scalar is part of the value, which is what keeps `run: echo "a: b"` +// one entry. +func splitMappingKey(text string) (key, rest string, ok bool) { + var quote byte + for i := 0; i < len(text); i++ { + c := text[i] + switch { + case quote != 0: + if c == quote { + quote = 0 + } + case c == '\'' || c == '"': + quote = c + case c == ':' && (i+1 == len(text) || text[i+1] == ' '): + key = strings.TrimSpace(text[:i]) + if key == "" { + return "", "", false + } + return key, strings.TrimSpace(text[i+1:]), true + } + } + return "", "", false +} + +// mappingAt walks a path of mapping keys and returns the value at its end, +// naming the step that failed rather than the whole path. +func mappingAt(doc map[string]any, path ...string) (any, error) { + var current any = doc + for i, key := range path { + m, ok := current.(map[string]any) + if !ok { + return nil, fmt.Errorf("%s is not a mapping", strings.Join(path[:i], ".")) + } + next, ok := m[key] + if !ok { + return nil, fmt.Errorf("%s has no %q", strings.Join(append([]string{"the workflow"}, path[:i]...), "."), key) + } + current = next + } + return current, nil +} + +// stringsOf reads a field that YAML lets a workflow write either as one scalar +// or as a sequence of them, which is how `needs:` is spelled both ways. +func stringsOf(value any) ([]string, error) { + switch v := value.(type) { + case string: + return []string{v}, nil + case []any: + out := make([]string, 0, len(v)) + for _, item := range v { + s, ok := item.(string) + if !ok { + return nil, fmt.Errorf("a sequence item is not a scalar") + } + out = append(out, s) + } + return out, nil + } + return nil, fmt.Errorf("the value is neither a scalar nor a sequence of them") +}