diff --git a/.github/actions/setup-rust/CHANGELOG.md b/.github/actions/setup-rust/CHANGELOG.md index 7823d44d3..30d6a3d9a 100644 --- a/.github/actions/setup-rust/CHANGELOG.md +++ b/.github/actions/setup-rust/CHANGELOG.md @@ -2,6 +2,16 @@ ## Unreleased +- Set `disable_annotations: true` on both uses of + `mozilla-actions/sccache-action`. Its post-job step runs + `sccache --show-stats` and fails the job on an error, and after a server + fallback that command restarts the dead server, so a second startup timeout + could fail a job that had built without the cache. The input switches off the + action's whole post report, so its statistics table and notice no longer + appear; report statistics with a step guarded on + `sccache-status == 'started'`. A contract rejects any sccache-action use + without it. + - Raise sccache's server startup timeout to 60 s through an `SCCACHE_CONF` config file, merged into any config the caller already names, and make a server that will not start fail open. The 10 s default intermittently expired diff --git a/.github/actions/setup-rust/README.md b/.github/actions/setup-rust/README.md index 3af1bd5b5..66930ad16 100644 --- a/.github/actions/setup-rust/README.md +++ b/.github/actions/setup-rust/README.md @@ -291,6 +291,15 @@ annotation titled `sccache-fallback`, the line `sccache-status=fallback` as an output (`started` otherwise). The annotation title is a contract that estate-wide detectors match on; it never changes. +The sccache action's own end-of-job statistics table and notice are switched off +(`disable_annotations: true` on both of its uses, held by a contract). Despite +its name the input disables the action's whole post-job report, which runs +`sccache --show-stats` and fails the job if that errors. After a fallback the +server is dead and `--show-stats` restarts it, so a second startup timeout +would turn a lost cache into a red job. Report statistics yourself with a step +guarded on `sccache-status == 'started'` (the output is empty when no server +was started), or with the `sccache-report` action once it is available. + Some exports have to be put back rather than made. The last thing `mozilla-actions/sccache-action` does is write `ACTIONS_CACHE_SERVICE_V2=on` to `GITHUB_ENV`, along with the runner's own `ACTIONS_RESULTS_URL` and diff --git a/.github/actions/setup-rust/action.yml b/.github/actions/setup-rust/action.yml index 79faeec8e..7374b15e7 100644 --- a/.github/actions/setup-rust/action.yml +++ b/.github/actions/setup-rust/action.yml @@ -810,11 +810,20 @@ runs: record ACTIONS_RUNTIME_TOKEN runtime_token secret # x86_64-apple-darwin binaries were dropped after sccache v0.12.0 + # `disable_annotations: true` on EVERY sccache-action use, which a contract + # enforces. Despite its name the input switches off the action's whole + # post-job report, not only annotations. That report runs + # `sccache --show-stats` and fails the job when it errors; after the server + # fallback below the server is dead, `--show-stats` restarts it, and a + # second timeout would turn a lost cache into a red job. Statistics are + # the caller's to report, with a step guarded on + # `sccache-status == 'started'`. - name: Run sccache (x86_64 macOS) if: ${{ inputs.use-sccache == 'true' && github.event_name != 'release' && runner.os == 'macOS' && runner.arch == 'X64' }} uses: mozilla-actions/sccache-action@fc920bf0ec8de6ee65d409111f7ec508035751ba with: version: v0.12.0 + disable_annotations: true - name: Run sccache # Pinned, like its x86_64 macOS sibling. Left unset, the action asks the # GitHub API for the latest release on every job: a floating dependency @@ -829,6 +838,7 @@ runs: uses: mozilla-actions/sccache-action@fc920bf0ec8de6ee65d409111f7ec508035751ba with: version: v0.17.0 + disable_annotations: true - name: Restore the caller's cache service selection # AFTER both sccache-action steps and BEFORE anything starts a server, # deliberately. Undoes what those steps wrote to GITHUB_ENV, so a caller diff --git a/.github/actions/tests/test_sccache_action_post_report.py b/.github/actions/tests/test_sccache_action_post_report.py new file mode 100644 index 000000000..e9182b706 --- /dev/null +++ b/.github/actions/tests/test_sccache_action_post_report.py @@ -0,0 +1,163 @@ +"""Contract: every ``mozilla-actions/sccache-action`` use disables its post report. + +The sccache action registers a post-job step that runs ``sccache --show-stats`` +and fails the job when that errors. After a sccache server fallback the server +is dead and ``--show-stats`` restarts it, so a second startup timeout turns a +lost compiler cache into a red job, which is the failure the fail-open start +exists to prevent. The only switch is the ``disable_annotations`` input, and +despite its name it returns before any statistics call. Statistics are reported +by the caller, with a step guarded on ``sccache-status``. + +The scan covers every composite action manifest and workflow in the +repository, not only ``setup-rust``, so a new use anywhere cannot reintroduce +the post step. A step whose value is anything but a literal true does not +count: ``false`` and an expression a caller could set to false would both leave +the report on. +""" + +from __future__ import annotations + +import typing as typ +from pathlib import Path + +import pytest +import yaml + +GITHUB_ROOT = Path(__file__).resolve().parents[2] +#: Compared after lower-casing: GitHub reads a repository owner and name +#: case-insensitively, so `Mozilla-Actions/sccache-action@` is the same action. +SCCACHE_ACTION_PREFIX = "mozilla-actions/sccache-action@" +INPUT = "disable_annotations" + + +def _children(node: object) -> list[object]: + """Return the values and items directly below a mapping or sequence node.""" + if isinstance(node, dict): + return list(node.values()) + return list(node) if isinstance(node, list) else [] + + +def _step_lists(node: object) -> typ.Iterator[list[object]]: + """Yield every ``steps`` list in a parsed workflow or action manifest.""" + if isinstance(node, dict) and isinstance(node.get("steps"), list): + yield node["steps"] + for child in _children(node): + yield from _step_lists(child) + + +def sccache_action_uses(document: object) -> list[dict[str, object]]: + """Return every step in ``document`` that runs the sccache action.""" + return [ + step + for steps in _step_lists(document) + for step in steps + if isinstance(step, dict) + and isinstance(step.get("uses"), str) + and str(step["uses"]).lower().startswith(SCCACHE_ACTION_PREFIX) + ] + + +def leaves_post_report_on(step: dict[str, object]) -> bool: + """Report whether ``step`` fails to switch the post report off.""" + inputs = step.get("with") + value = inputs.get(INPUT) if isinstance(inputs, dict) else None + return not (value is True or value == "true") + + +def _manifests() -> list[Path]: + """Return every workflow and composite action manifest in the repository.""" + return sorted( + path + for pattern in ("*.yml", "*.yaml") + for path in GITHUB_ROOT.rglob(pattern) + if "node_modules" not in path.parts + ) + + +def _uses_in_repository() -> list[tuple[Path, dict[str, object]]]: + """Return each sccache-action use with the file that declares it.""" + found: list[tuple[Path, dict[str, object]]] = [] + for path in _manifests(): + document = yaml.safe_load(path.read_text(encoding="utf-8")) + found.extend((path, step) for step in sccache_action_uses(document)) + return found + + +def test_the_repository_still_uses_the_sccache_action() -> None: + """A scan over nothing passes whatever it checks, so require some uses. + + ``setup-rust`` runs the action twice, once for the x86_64 macOS pin and + once for the rest. + """ + uses = _uses_in_repository() + assert len(uses) >= 2, ( + f"expected setup-rust's two sccache-action uses, found {len(uses)}; " + "if the action is gone, retire this contract" + ) + + +def test_every_sccache_action_use_disables_the_post_report() -> None: + """No use may leave the post-job ``--show-stats`` step to fail a fallback.""" + offenders = [ + f"{path.relative_to(GITHUB_ROOT)}: {step.get('name', step.get('uses'))}" + for path, step in _uses_in_repository() + if leaves_post_report_on(step) + ] + assert not offenders, ( + f"these sccache-action uses must set {INPUT}: true, otherwise the post " + f"step restarts a dead server and can fail the job: {offenders}" + ) + + +@pytest.mark.parametrize( + ("with_block", "expected"), + [ + ({"version": "v0.17.0"}, True), + ({INPUT: False}, True), + ({INPUT: "false"}, True), + ({INPUT: "${{ inputs.quiet }}"}, True), + (None, True), + ({INPUT: True}, False), + ({INPUT: "true"}, False), + ], +) +def test_only_a_literal_true_counts_as_disabled( + with_block: dict[str, object] | None, *, expected: bool +) -> None: + """False, a missing block and an expression all leave the report on.""" + step: dict[str, object] = {"uses": f"{SCCACHE_ACTION_PREFIX}abc"} + if with_block is not None: + step["with"] = with_block + assert leaves_post_report_on(step) is expected + + +@pytest.mark.parametrize( + "uses", + [ + "Mozilla-Actions/sccache-action@abc", + "MOZILLA-ACTIONS/SCCACHE-ACTION@abc", + ], +) +def test_a_case_variant_of_the_action_name_is_still_scanned(uses: str) -> None: + """GitHub resolves the owner and name case-insensitively, so must the scan.""" + document = {"jobs": {"build": {"steps": [{"uses": uses}]}}} + uses_found = sccache_action_uses(document) + assert len(uses_found) == 1, f"{uses!r} escaped the scan" + assert leaves_post_report_on(uses_found[0]) + + +def test_a_new_use_without_the_input_is_found_in_a_workflow() -> None: + """The scan reads workflow jobs as well as composite action manifests.""" + document = yaml.safe_load( + "jobs:\n" + " build:\n" + " runs-on: ubuntu-latest\n" + " steps:\n" + " - uses: mozilla-actions/sccache-action@abc\n" + " - uses: mozilla-actions/sccache-action@abc\n" + " with:\n" + " disable_annotations: true\n" + ) + uses = sccache_action_uses(document) + assert len(uses) == 2 + assert [leaves_post_report_on(step) for step in uses] == [True, False] diff --git a/docs/developers-guide.md b/docs/developers-guide.md index a703e9922..66910b911 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -362,6 +362,20 @@ not from `RUSTC_WRAPPER`: an inherited wrapper may name this very binary, and stopping the server behind it would discard the statistics of everything compiled so far in the job. +The nested `mozilla-actions/sccache-action` registers a post-job step that runs +`sccache --show-stats` and calls `setFailed` on an error. After a fallback that +command restarts the dead server, so it could fail a job whose build had +already succeeded without the cache. Its only switch is the +`disable_annotations` input, which despite the name returns before any +statistics call, so both uses in `setup-rust` set it to `true`. The cost is +that the action's own table and notice are gone, which is why consumers report +statistics themselves, guarded on `sccache-status == 'started'`. +`.github/actions/tests/test_sccache_action_post_report.py` scans every workflow +and action manifest in the repository and fails for any use that lacks a literal +`true`, including one added somewhere other than `setup-rust`; mutation proof +covers removing it from either use, setting it `false`, adding an unprotected +use elsewhere, and renaming the action out from under the scan. + The manifest tests hold the whole chain: selection, record, sccache steps, restore, wrapper export, start. `GITHUB_ENV` reaches only the next step, so no two of these can be merged. diff --git a/docs/users-guide.md b/docs/users-guide.md index e829ae682..f65443ecc 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -143,6 +143,16 @@ existing callers need do nothing. A caller that itself runs `fallback`, since asking a server that never started would try to start it again. +The sccache action's own end-of-job statistics table and notice are no longer +produced: `setup-rust` sets `disable_annotations: true` on both of its uses of +the action. Despite its name that input disables the whole post-job report, +which runs `sccache --show-stats` and fails the job if it errors. After a +fallback the server is dead and `--show-stats` restarts it, so a second startup +timeout would turn a lost cache into a red job. To keep the table, report it +from a step guarded on `sccache-status == 'started'`, which is empty when no +server was started and `fallback` after a failed start, or use the +`sccache-report` action once it is available. + Some exports have to be put back rather than made, and that is the reason `use-sccache: 'true'` used to be unusable on Ubicloud. The last thing `mozilla-actions/sccache-action` does is write `ACTIONS_CACHE_SERVICE_V2=on` to