From 4077e1ef045372c38c3629986dae9f5a5d9dd240 Mon Sep 17 00:00:00 2001 From: Anton Krivoborodov Date: Fri, 25 Sep 2026 13:26:28 +0000 Subject: [PATCH 1/5] ci: add label-gated staged CI for integration pull requests A "come together" pull request that bumps every module hash in known_good.json at once usually fails for a reason visible long before the hour-long image builds finish: a module no longer resolves, a target was renamed, a documentation mount broke. Today every push to such a PR spends a full pipeline to find that out. Adding the 'staged-ci' label now skips the expensive jobs and leaves the cheap, known_good.json-derived validation running: runs - known_good_correct, bzlmod-lock, format, copyright runs - test_and_docs.yml documentation preflight skipped - test_and_docs.yml unit tests, coverage, feature integration tests skipped - build_and_test_{linux,qnx,autosd,ebclfsa}.yml skipped - codeql-multiple-repo-scan.yml Two details make this safe rather than a foot-gun: 1. staged_ci_guard.yml fails while the label is set. GitHub reports a job skipped via `if:` as *skipped*, and branch protection treats a skipped required check as satisfied - so without this guard a staged PR would be mergeable although none of the heavy checks ever ran. The guard has to be configured as a required status check for that protection to exist. 2. Every gated workflow now also listens for the `labeled` and `unlabeled` pull-request events. Without them, adding or removing the label would not trigger a new run and the gate would appear to do nothing. On push, merge_group and release events the label list is empty, so the condition evaluates to true and all jobs run unchanged. Staging is a manual override for a supervised integration pull request, not a way to merge unvalidated changes - the full pipeline still has to pass before the merge. --- .github/workflows/build_and_test_autosd.yml | 4 +- .github/workflows/build_and_test_ebclfsa.yml | 4 +- .github/workflows/build_and_test_linux.yml | 3 + .github/workflows/build_and_test_qnx.yml | 4 +- .../workflows/codeql-multiple-repo-scan.yml | 4 +- .github/workflows/staged_ci_guard.yml | 62 +++++++++++++++++ .github/workflows/test_and_docs.yml | 9 ++- docs/integration_process/reference.rst | 68 +++++++++++++++++++ 8 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/staged_ci_guard.yml diff --git a/.github/workflows/build_and_test_autosd.yml b/.github/workflows/build_and_test_autosd.yml index 8e060fea428..d5de0adc4d5 100644 --- a/.github/workflows/build_and_test_autosd.yml +++ b/.github/workflows/build_and_test_autosd.yml @@ -13,7 +13,7 @@ name: AutoSD - Build & Integration Test on: pull_request: - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled, unlabeled] merge_group: types: [checks_requested] push: @@ -25,6 +25,8 @@ concurrency: cancel-in-progress: ${{ github.ref_name != 'main' && !startsWith(github.ref_name, 'release/') }} jobs: build: + # Skipped while the 'staged-ci' label is set; see staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} strategy: matrix: platform: diff --git a/.github/workflows/build_and_test_ebclfsa.yml b/.github/workflows/build_and_test_ebclfsa.yml index 6451f99da84..44f1dd1d9b6 100644 --- a/.github/workflows/build_and_test_ebclfsa.yml +++ b/.github/workflows/build_and_test_ebclfsa.yml @@ -13,7 +13,7 @@ name: EB corbos Linux for Safety Applications - Build & Integration Test on: pull_request: - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled, unlabeled] merge_group: types: [checks_requested] push: @@ -26,6 +26,8 @@ concurrency: jobs: build: name: aarch64 + # Skipped while the 'staged-ci' label is set; see staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} runs-on: ubuntu-latest container: image: ghcr.io/eclipse-score/devcontainer:v1.11.0 diff --git a/.github/workflows/build_and_test_linux.yml b/.github/workflows/build_and_test_linux.yml index 44eeed1f9ab..fb3a56ee419 100644 --- a/.github/workflows/build_and_test_linux.yml +++ b/.github/workflows/build_and_test_linux.yml @@ -14,6 +14,7 @@ name: Linux - Build & Integration Test on: workflow_dispatch: pull_request: + types: [opened, reopened, synchronize, labeled, unlabeled] release: types: [created] push: @@ -25,6 +26,8 @@ concurrency: cancel-in-progress: ${{ github.ref_name != 'main' && !startsWith(github.ref_name, 'release/') }} jobs: x86_64: + # Skipped while the 'staged-ci' label is set; see staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} runs-on: ubuntu-latest permissions: contents: write # required to upload release assets diff --git a/.github/workflows/build_and_test_qnx.yml b/.github/workflows/build_and_test_qnx.yml index 54e8dbd5039..9851ddf0fb1 100644 --- a/.github/workflows/build_and_test_qnx.yml +++ b/.github/workflows/build_and_test_qnx.yml @@ -13,7 +13,7 @@ name: QNX8 - Build & Integration Test on: pull_request_target: - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled, unlabeled] merge_group: types: [checks_requested] push: @@ -26,6 +26,8 @@ concurrency: jobs: qnx-build-x86_64: name: x86_64 + # Skipped while the 'staged-ci' label is set; see staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} runs-on: ${{ vars.REPO_RUNNER_LABELS && fromJSON(vars.REPO_RUNNER_LABELS) || 'ubuntu-latest' }} environment: ${{ inputs.environment-name }} permissions: diff --git a/.github/workflows/codeql-multiple-repo-scan.yml b/.github/workflows/codeql-multiple-repo-scan.yml index b84d10d80cf..c0317eb4057 100644 --- a/.github/workflows/codeql-multiple-repo-scan.yml +++ b/.github/workflows/codeql-multiple-repo-scan.yml @@ -13,7 +13,7 @@ name: "CodeQL - Multi-Repo Source Scan" on: pull_request: - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled, unlabeled] merge_group: types: [checks_requested] push: @@ -31,6 +31,8 @@ concurrency: jobs: analyze-repos: name: Analyze Multiple Repositories + # Skipped while the 'staged-ci' label is set; see staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} runs-on: ubuntu-latest timeout-minutes: 120 # Prevent indefinite hanging permissions: diff --git a/.github/workflows/staged_ci_guard.yml b/.github/workflows/staged_ci_guard.yml new file mode 100644 index 00000000000..f4eb5c1f961 --- /dev/null +++ b/.github/workflows/staged_ci_guard.yml @@ -0,0 +1,62 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +# +# Staged CI Guard +# +# Companion to the 'staged-ci' label. While that label is present on a pull +# request, the expensive platform and quality jobs skip themselves so a +# "come together" integration PR can be validated cheaply first (module +# resolution + documentation) before an hour of image builds is spent. +# +# GitHub reports a job skipped via `if:` as *skipped*, and branch protection +# treats a skipped required check as satisfied. Without this guard a staged PR +# would therefore be mergeable even though none of the heavy checks ever ran. +# This job always runs and fails while the label is set, so the label must be +# removed - and the full pipeline must pass - before a merge is possible. +# +# Make this check REQUIRED in the repository's branch-protection settings, +# otherwise it does not protect anything. +name: Staged CI Guard +on: + pull_request: + types: [opened, reopened, synchronize, labeled, unlabeled] + merge_group: + types: [checks_requested] +permissions: + contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: ${{ github.ref_name != 'main' && !startsWith(github.ref_name, 'release/') }} +jobs: + staged-ci-guard: + name: Staged CI Guard + runs-on: ubuntu-latest + steps: + - name: Block merge while CI is staged + if: contains(github.event.pull_request.labels.*.name, 'staged-ci') + run: | + { + echo "### :no_entry: CI is staged" + echo + echo "The \`staged-ci\` label is set on this pull request, so the" + echo "platform builds, unit tests, coverage, feature integration" + echo "tests and CodeQL scan were **skipped**." + echo + echo "Remove the \`staged-ci\` label to run the full pipeline." + echo "This check blocks the merge until then." + } >> "$GITHUB_STEP_SUMMARY" + echo "::error::The 'staged-ci' label is set - the full CI pipeline has not run. Remove the label before merging." + exit 1 + - name: Confirm full pipeline is enabled + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} + run: echo "No 'staged-ci' label - the full CI pipeline runs for this change." diff --git a/.github/workflows/test_and_docs.yml b/.github/workflows/test_and_docs.yml index 0e8a9e49c40..fb1474579ed 100644 --- a/.github/workflows/test_and_docs.yml +++ b/.github/workflows/test_and_docs.yml @@ -18,7 +18,7 @@ permissions: id-token: write on: pull_request_target: # Allow forks to access secrets - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled, unlabeled] push: branches: - main @@ -81,6 +81,11 @@ jobs: --github_repo=${{ github.event.repository.name }} rm -rf _build/ - name: Execute Unit Tests with Coverage Analysis + # Skipped while the 'staged-ci' label is set: the documentation preflight + # above is derived from known_good.json and answers whether the module set + # resolves at all, which is what an integration PR needs first. See + # staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} # Only tag runs are authoritative and re-execute every test from scratch, because they # attach the test-report ZIP and the SBOM as official release assets. The condition is # deliberately the same one those upload steps use, so the two can never drift apart. @@ -92,6 +97,8 @@ jobs: python ./scripts/quality_runners.py \ ${{ github.ref_type != 'tag' && '--trust-cache' || '' }} - name: Execute Feature Integration Tests + # Skipped while the 'staged-ci' label is set; see staged_ci_guard.yml. + if: ${{ !contains(github.event.pull_request.labels.*.name, 'staged-ci') }} run: | bazel test --config=linux-x86_64 //feature_integration_tests/test_cases:fit - name: Install uv (required for SBOM license scanning) diff --git a/docs/integration_process/reference.rst b/docs/integration_process/reference.rst index 75a31518c1d..a0e8aa441a7 100644 --- a/docs/integration_process/reference.rst +++ b/docs/integration_process/reference.rst @@ -78,6 +78,11 @@ declared in the workflows. - Code-quality checks, builds the docs and reports, publishes to Pages. - **yes** - wire docs in Step 2, reports in Step 7 + * - `staged_ci_guard.yml `_ + - Fails while the ``staged-ci`` label is set, so a pull request with + skipped heavy checks cannot be merged (see :ref:`staged_ci`). + - **yes** + - none * - `check_release_approvals.yml `_ - Enforces required approvals on PRs targeting ``releases/*`` branches. - release branches only @@ -102,6 +107,69 @@ declared in the workflows. - add runtime targets to `ci/showcase_targets_run.txt `_ +.. _staged_ci: + +Staged CI for integration pull requests +--------------------------------------- + +A "come together" pull request that bumps every module hash in +``known_good.json`` at once often fails for a reason that is visible long before +the hour-long image builds finish — a module no longer resolves, a target was +renamed, a documentation mount broke. Waiting for the full pipeline on every +push wastes both runner time and the integrator's time. + +Adding the ``staged-ci`` label to such a pull request skips the expensive jobs +and leaves only the cheap, ``known_good.json``-derived validation running: + +.. list-table:: + :header-rows: 1 + :widths: 46 27 27 + + * - Check + - with ``staged-ci`` + - without the label + * - ``known_good_correct``, ``bzlmod-lock``, ``format``, ``copyright`` + - runs + - runs + * - ``test_and_docs.yml`` — documentation preflight + - runs + - runs + * - ``test_and_docs.yml`` — unit tests, coverage, feature integration tests + - skipped + - runs + * - ``build_and_test_{linux,qnx,autosd,ebclfsa}.yml`` + - skipped + - runs + * - ``codeql-multiple-repo-scan.yml`` + - skipped + - runs + * - ``staged_ci_guard.yml`` + - **fails** (blocks merge) + - passes + +Workflow +~~~~~~~~ + +#. Add the ``staged-ci`` label when opening the integration pull request. +#. Iterate until the documentation preflight and the generator checks are green. +#. Remove the label. The full pipeline runs, and ``Staged CI Guard`` turns green. +#. Merge once everything passes. + +The label only takes effect on the next workflow run, which is why every gated +workflow listens for the ``labeled`` and ``unlabeled`` pull-request events. + +.. note:: + + ``Staged CI Guard`` must be configured as a **required** status check in the + repository's branch-protection settings. GitHub reports a job skipped via an + ``if:`` condition as *skipped* and treats a skipped required check as + satisfied — without the guard, a staged pull request would be mergeable even + though none of the heavy checks ever ran. + + Staging is a manual override for a supervised integration pull request, not a + way to merge unvalidated changes. The full pipeline still has to pass before + the merge. + .. _reports: Reports From 944d4616da83aee802608a0b0d800791797e9452 Mon Sep 17 00:00:00 2001 From: Anton Krivoborodov Date: Fri, 25 Sep 2026 14:07:49 +0000 Subject: [PATCH 2/5] feat(known_good): allow temporarily disabling a module Staging CI buys time to look at an integration failure; it does not remove the failure. When a single module blocks the whole cycle, it can now be taken out of the integration without losing its pinned state: "enabled": false, "disabled_reason": "blocked by eclipse-score/example#123" Commenting the entry out is not an option - known_good.json is parsed with a strict json.loads and JSON has no comments - and deleting it discards the hash, the bazel_patches and the metadata, which is exactly the state needed to put the module back. The flag keeps all of it, so re-enabling is a two-line revert. The reason is mandatory so a disabled module cannot decay into an unnoticed permanent state, and the generator prints every disabled module on each run. A disabled module contributes no bazel_dep/git_override entry, no coverage target, no docs mount, no unit-test run and no SBOM entry. The flag only governs the generated artefacts, so the two references that would otherwise fail far away from known_good.json now abort the regeneration naming the offending lines: - a --@module//... flag left in .bazelrc, which Bazel resolves on every invocation including 'query', making the whole workspace unusable; - another module's extra_test_config/exclude_test_targets, or a leftover sbom.tracked_modules entry, pointing into the disabled module. Both were verified against the real known_good.json: disabling score_config_management reports the .bazelrc flag, disabling score_logging reports the SBOM entry and score_persistency's test config. Regeneration with nothing disabled is byte-identical to the committed files. Hand-written references (images/*/BUILD, showcases, the ITF scenarios) are not detected and still have to be removed manually; the documentation says so explicitly rather than implying the flag is sufficient for every module. --- docs/integration_process/reference.rst | 69 +++++++ scripts/known_good/models/known_good.py | 69 +++++++ scripts/known_good/models/module.py | 32 +++ .../known_good/tests/test_module_enabled.py | 189 ++++++++++++++++++ .../update_module_from_known_good.py | 59 +++++- scripts/quality_runners.py | 10 +- 6 files changed, 425 insertions(+), 3 deletions(-) create mode 100644 scripts/known_good/tests/test_module_enabled.py diff --git a/docs/integration_process/reference.rst b/docs/integration_process/reference.rst index a0e8aa441a7..a5a2bfcdc5f 100644 --- a/docs/integration_process/reference.rst +++ b/docs/integration_process/reference.rst @@ -170,6 +170,75 @@ workflow listens for the ``labeled`` and ``unlabeled`` pull-request events. way to merge unvalidated changes. The full pipeline still has to pass before the merge. +.. _disabling_a_module: + +Temporarily disabling a module +------------------------------ + +Staging CI buys time to look at a failure; it does not remove the failure. When +a single module blocks the whole integration — an upstream change that will not +land before the next cycle, a broken dependency, a test that cannot pass yet — +that module can be taken out of the integration without losing its pinned state. + +Add ``"enabled": false`` together with a mandatory ``"disabled_reason"`` to its +entry in ``known_good.json``: + +.. code-block:: json + + "score_example": { + "repo": "https://github.com/eclipse-score/example.git", + "hash": "0123456789abcdef0123456789abcdef01234567", + "enabled": false, + "disabled_reason": "blocked by eclipse-score/example#123, re-enable after it lands", + "metadata": { } + } + +The hash, the ``bazel_patches`` list and the metadata stay in the file, so +re-enabling the module is a two-line revert rather than a reconstruction from +the git history. Deleting the entry instead would discard exactly the state the +next integrator needs. + +After the edit, regenerate the Bazel fragments as usual (Step 1) and commit them +together with the ``known_good.json`` change: + +.. code-block:: bash + + python3 scripts/known_good/update_module_from_known_good.py + +A disabled module then contributes no ``bazel_dep``/``git_override`` entry, no +coverage target, no documentation mount, no unit-test run and no SBOM entry. +The generator prints every disabled module and its reason on each run, so the +state cannot decay into an unnoticed permanent one. + +What the flag does not do +~~~~~~~~~~~~~~~~~~~~~~~~~ + +The flag governs the **generated** artefacts only. Anything that names the +module by hand keeps naming a module that no longer exists, and Bazel reports +that far away from ``known_good.json``. Two such references are checked +explicitly and abort the regeneration with the offending lines: + +.. list-table:: + :header-rows: 1 + :widths: 40 60 + + * - Reference + - Why it must go with the module + * - ``--@module//...`` flags in ``.bazelrc`` + - Bazel resolves them on *every* invocation, including ``bazel query``, so + the whole workspace becomes unusable. + * - ``extra_test_config`` / ``exclude_test_targets`` of another module, and + ``sbom.tracked_modules`` + - The entry would point into a module the build no longer contains. + +Other references are **not** detected automatically and have to be removed by +hand — hand-written ``BUILD`` files such as ``images/*/BUILD``, +``showcases/standalone/BUILD`` and the feature-integration test scenarios, plus +any test in ``feature_integration_tests/itf/`` that exercises the module. In +practice the flag is therefore sufficient for a module that only contributes +documentation, coverage and unit tests, and is only the first step for a module +that is wired into the images, the showcases or the integration tests. + .. _reports: Reports diff --git a/scripts/known_good/models/known_good.py b/scripts/known_good/models/known_good.py index 223f065c4ce..a57c21e9ab6 100644 --- a/scripts/known_good/models/known_good.py +++ b/scripts/known_good/models/known_good.py @@ -23,6 +23,54 @@ from .module import Module +def _validate_disabled_modules( + parsed_modules: Dict[str, Dict[str, Module]], + sbom_tracked_modules: list[str], +) -> None: + """Reject a disabled module that the rest of the configuration still depends on. + + Disabling a module only removes it from the *generated* artifacts. Anything that + names it elsewhere keeps naming a module that no longer exists, and Bazel reports + that far from here - a ``--@score_x//...`` flag for an absent module fails *every* + invocation, including ``bazel query``. Failing here instead names the exact + entries that have to go with it. + """ + disabled = { + name: module + for group_modules in parsed_modules.values() + for name, module in group_modules.items() + if not module.enabled + } + if not disabled: + return + + problems: list[str] = [] + + still_tracked = sorted(set(sbom_tracked_modules) & set(disabled)) + if still_tracked: + problems.append("listed in sbom.tracked_modules: " + ", ".join(still_tracked) + " (remove them from that list)") + + # metadata labels of *enabled* modules that point into a disabled module, e.g. + # score_persistency's extra_test_config referencing "@score_logging//...". + for group_modules in parsed_modules.values(): + for module in group_modules.values(): + if not module.enabled: + continue + labels = list(module.metadata.extra_test_config) + list(module.metadata.exclude_test_targets) + for label in labels: + for name in disabled: + if label.startswith(f"@{name}//"): + problems.append( + f"module '{module.name}' still references disabled module '{name}' " + f"via metadata entry '{label}'" + ) + + if problems: + raise ValueError( + "Invalid known_good.json (disabled modules are still referenced):\n - " + "\n - ".join(problems) + ) + + @dataclass class KnownGood: """Known good configuration with modules and metadata. @@ -72,12 +120,33 @@ def from_dict(cls, data: Dict[str, Any]) -> KnownGood: + ", ".join(invalid_modules) ) + _validate_disabled_modules(parsed_modules, sbom_tracked_modules) + return cls( modules=parsed_modules, sbom_tracked_modules=sbom_tracked_modules, timestamp=timestamp, ) + def enabled_modules(self, group: str) -> Dict[str, Module]: + """The modules of ``group`` that take part in the integration. + + Every consumer of the module list wants this rather than ``modules[group]``: + a module with ``"enabled": false`` must not reach the generated Bazel + fragments, the coverage targets, the docs mounts or the test runner. + """ + return {name: module for name, module in self.modules.get(group, {}).items() if module.enabled} + + @property + def disabled_modules(self) -> Dict[str, Module]: + """Every disabled module across all groups, keyed by name.""" + return { + name: module + for group_modules in self.modules.values() + for name, module in group_modules.items() + if not module.enabled + } + def to_dict(self) -> Dict[str, Any]: """Convert KnownGood instance to dictionary for JSON output. diff --git a/scripts/known_good/models/module.py b/scripts/known_good/models/module.py index 65bb27191d1..89e30b155f4 100644 --- a/scripts/known_good/models/module.py +++ b/scripts/known_good/models/module.py @@ -155,6 +155,8 @@ class Module: branch: str = "main" pin_version: bool = False docs: Docs = field(default_factory=Docs) + enabled: bool = True + disabled_reason: str | None = None @classmethod def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: @@ -181,6 +183,15 @@ def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: - docs (bool | dict, optional): Documentation mount for the combined docs site. Mounted by default; use false for a module that exposes no //:docs_bundle. See Docs. + - enabled (bool, optional): Whether the module takes part in the + integration at all (default: true). Set to false to + temporarily drop a module that blocks the + integration; every generated artifact then omits + it. Requires 'disabled_reason'. + - disabled_reason (str, optional): Why the module is disabled and what + has to happen to re-enable it. Mandatory whenever + 'enabled' is false, so a disabled module can never + become a silent permanent state. Returns: Module instance @@ -220,6 +231,22 @@ def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: except ValueError as e: raise ValueError(f"Module '{name}': {e}") from None + enabled = module_data.get("enabled", True) + if not isinstance(enabled, bool): + raise ValueError(f"Module '{name}': invalid 'enabled' value {enabled!r} (expected true or false)") + + disabled_reason = module_data.get("disabled_reason") + if not enabled and not disabled_reason: + raise ValueError( + f"Module '{name}' is disabled but has no 'disabled_reason'. State why it is disabled " + "and what has to happen to re-enable it, so the next integrator does not have to " + "guess whether this is temporary or permanent." + ) + if enabled and disabled_reason: + raise ValueError( + f"Module '{name}' has a 'disabled_reason' but is enabled. Remove the reason, or set \"enabled\": false." + ) + return cls( name=name, hash=commit_hash, @@ -230,6 +257,8 @@ def from_dict(cls, name: str, module_data: Dict[str, Any]) -> Module: branch=branch, pin_version=pin_version, docs=docs, + enabled=enabled, + disabled_reason=disabled_reason, ) @classmethod @@ -292,6 +321,9 @@ def to_dict(self) -> Dict[str, Any]: result["branch"] = self.branch if self.pin_version: result["pin_version"] = True + if not self.enabled: + result["enabled"] = False + result["disabled_reason"] = self.disabled_reason docs = self.docs.to_value() if docs is not None: result["docs"] = docs diff --git a/scripts/known_good/tests/test_module_enabled.py b/scripts/known_good/tests/test_module_enabled.py new file mode 100644 index 00000000000..2d30d681931 --- /dev/null +++ b/scripts/known_good/tests/test_module_enabled.py @@ -0,0 +1,189 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""Unit tests for temporarily disabling a module via known_good.json. + +Self-contained: builds KnownGood objects in memory. Needs no Bazel, git or network. +""" + +import sys +from pathlib import Path + +import pytest + +# Make scripts/ importable so known_good.* package resolves when run via plain pytest. +_SCRIPTS_DIR = Path(__file__).resolve().parents[2] +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +from known_good.models.known_good import KnownGood # noqa: E402 +from known_good.models.module import Module # noqa: E402 +from known_good.update_module_from_known_good import ( # noqa: E402 + check_bazelrc_for_disabled_modules, + generate_docs_bundles_content, +) + +_HASH = "0" * 40 + + +def _raw(name: str, **overrides) -> dict: + module = {"repo": f"https://github.com/eclipse-score/{name}.git", "hash": _HASH} + module.update(overrides) + return module + + +def _raw_known_good(target_sw: dict, sbom: list[str] | None = None) -> dict: + return { + "modules": {"target_sw": target_sw}, + "sbom": {"tracked_modules": sbom or []}, + "timestamp": "2026-01-01T00:00:00Z", + } + + +def _module(name: str, **kwargs) -> Module: + return Module(name=name, hash=_HASH, repo=f"https://github.com/eclipse-score/{name}.git", **kwargs) + + +def test_modules_are_enabled_by_default(): + """Saying nothing about 'enabled' keeps a module in the integration: the common case.""" + module = Module.from_dict("score_logging", _raw("score_logging")) + + assert module.enabled is True + assert module.disabled_reason is None + # The key stays out of the file entirely, so enabling a module leaves no residue. + assert "enabled" not in module.to_dict() + + +def test_disabling_requires_a_reason(): + """A disabled module without a reason becomes a permanent state nobody dares to revert.""" + with pytest.raises(ValueError, match="no 'disabled_reason'"): + Module.from_dict("score_logging", _raw("score_logging", enabled=False)) + + +def test_reason_without_disabling_is_rejected(): + """Catches the half-done edit that leaves the module silently running.""" + with pytest.raises(ValueError, match="but is enabled"): + Module.from_dict("score_logging", _raw("score_logging", disabled_reason="broken")) + + +def test_enabled_must_be_a_boolean(): + with pytest.raises(ValueError, match="invalid 'enabled' value"): + Module.from_dict("score_logging", _raw("score_logging", enabled="false")) + + +def test_disabled_module_round_trips(): + """Disabling is reversible: hash, patches and metadata survive the round trip.""" + raw = _raw( + "score_logging", + enabled=False, + disabled_reason="blocked by eclipse-score/logging#123", + bazel_patches=["//patches/logging:001-fix.patch"], + ) + + result = Module.from_dict("score_logging", raw).to_dict() + + assert result["enabled"] is False + assert result["disabled_reason"] == "blocked by eclipse-score/logging#123" + assert result["hash"] == _HASH + assert result["bazel_patches"] == ["//patches/logging:001-fix.patch"] + + +def test_enabled_modules_filters_the_group(): + known = KnownGood.from_dict( + _raw_known_good( + { + "score_baselibs": _raw("score_baselibs"), + "score_logging": _raw("score_logging", enabled=False, disabled_reason="blocked"), + } + ) + ) + + assert set(known.enabled_modules("target_sw")) == {"score_baselibs"} + assert set(known.disabled_modules) == {"score_logging"} + # The raw mapping still holds both: disabling must not lose the pinned state. + assert set(known.modules["target_sw"]) == {"score_baselibs", "score_logging"} + + +def test_disabled_module_is_not_mounted_in_the_docs(): + known = KnownGood( + modules={ + "target_sw": { + "score_baselibs": _module("score_baselibs"), + "score_logging": _module("score_logging", enabled=False, disabled_reason="blocked"), + } + }, + timestamp="2026-01-01T00:00:00Z", + ) + + content = generate_docs_bundles_content(known) + + assert "score_baselibs" in content + assert "score_logging" not in content + + +def test_disabled_module_may_not_stay_in_the_sbom(): + """The SBOM would otherwise claim a module the build no longer contains.""" + with pytest.raises(ValueError, match="sbom.tracked_modules"): + KnownGood.from_dict( + _raw_known_good( + {"score_logging": _raw("score_logging", enabled=False, disabled_reason="blocked")}, + sbom=["score_logging"], + ) + ) + + +def test_metadata_reference_to_a_disabled_module_is_rejected(): + """An enabled module's test config pointing into a disabled one would break at Bazel time.""" + with pytest.raises(ValueError, match="score_persistency.*score_logging"): + KnownGood.from_dict( + _raw_known_good( + { + "score_logging": _raw("score_logging", enabled=False, disabled_reason="blocked"), + "score_persistency": _raw( + "score_persistency", + metadata={"extra_test_config": ["@score_logging//score/mw/log:config"]}, + ), + } + ) + ) + + +def test_bazelrc_flag_for_a_disabled_module_is_rejected(tmp_path): + """The worst failure mode: such a flag breaks every bazel invocation, even 'query'.""" + known = KnownGood( + modules={"target_sw": {"score_logging": _module("score_logging", enabled=False, disabled_reason="blocked")}}, + timestamp="2026-01-01T00:00:00Z", + ) + bazelrc = tmp_path / ".bazelrc" + bazelrc.write_text("build --@score_logging//score/datarouter:enabled=true\n", encoding="utf-8") + + with pytest.raises(SystemExit, match="still referenced by .bazelrc"): + check_bazelrc_for_disabled_modules(known, bazelrc) + + +def test_bazelrc_check_ignores_comments_and_enabled_modules(tmp_path): + known = KnownGood( + modules={ + "target_sw": { + "score_baselibs": _module("score_baselibs"), + "score_logging": _module("score_logging", enabled=False, disabled_reason="blocked"), + } + }, + timestamp="2026-01-01T00:00:00Z", + ) + bazelrc = tmp_path / ".bazelrc" + bazelrc.write_text( + "# build --@score_logging//score/datarouter:enabled=true\nbuild --@score_baselibs//score/foo:enabled=true\n", + encoding="utf-8", + ) + + check_bazelrc_for_disabled_modules(known, bazelrc) diff --git a/scripts/known_good/update_module_from_known_good.py b/scripts/known_good/update_module_from_known_good.py index 8f8b22241ac..5839c271cb9 100755 --- a/scripts/known_good/update_module_from_known_good.py +++ b/scripts/known_good/update_module_from_known_good.py @@ -254,6 +254,9 @@ def generate_docs_bundles_content(known_good: KnownGood, timestamp: Optional[str section = DOCS_SECTION_BY_GROUP.get(group_name) for module in group_modules.values(): + if not module.enabled: + continue + if not module.docs.enabled: continue @@ -281,7 +284,7 @@ def generate_docs_bundles_content(known_good: KnownGood, timestamp: Optional[str entries.append("\n".join(lines) + "\n") if not entries: - raise SystemExit("No modules to mount: every module in known_good.json has 'docs': false") + raise SystemExit("No modules to mount: every module in known_good.json is disabled or has 'docs': false") header = LICENSE_HEADER if timestamp: @@ -322,6 +325,53 @@ def generate_file_content( return header + "\n".join(blocks) +def check_bazelrc_for_disabled_modules(known_good: KnownGood, bazelrc_path: Path) -> None: + """Abort when .bazelrc still carries a flag belonging to a disabled module. + + This is the failure mode that makes disabling a module look broken rather than + configured. A ``--@score_logging//...:flag=value`` line for a module Bazel no + longer knows about fails *every* invocation - ``build``, ``test``, even + ``query`` - with an opaque "no repository visible" error far away from + known_good.json. Named here, the fix is obvious: delete or move those lines. + + Only the disabled modules are checked; verifying that every *enabled* module's + flags exist is a different job, already done by rc_label_consistency.py. + """ + disabled = known_good.disabled_modules + if not disabled or not bazelrc_path.is_file(): + return + + offenders: List[str] = [] + for lineno, line in enumerate(bazelrc_path.read_text(encoding="utf-8").splitlines(), start=1): + if line.lstrip().startswith("#"): + continue + for name in disabled: + if f"@{name}//" in line: + offenders.append(f"{bazelrc_path.name}:{lineno}: {line.strip()}") + + if offenders: + raise SystemExit( + "ERROR: disabled modules are still referenced by .bazelrc.\n" + "Bazel resolves these flags on every invocation, so the workspace would be\n" + "unusable. Remove or comment out these lines together with the module:\n " + "\n ".join(offenders) + ) + + +def report_disabled_modules(known_good: KnownGood) -> None: + """Print the disabled modules and why, so a regeneration never hides them.""" + disabled = known_good.disabled_modules + if not disabled: + return + + print(f"\nDISABLED: {len(disabled)} module(s) excluded from the integration:") + for name, module in sorted(disabled.items()): + print(f" - {name}: {module.disabled_reason}") + print( + "These modules contribute no Bazel override, coverage target, docs mount,\n" + "unit tests or SBOM entry. Re-enable them by removing 'enabled': false.\n" + ) + + def main() -> None: parser = argparse.ArgumentParser( description="Generate score_modules.MODULE.bazel file(s) from known_good.json", @@ -427,8 +477,13 @@ def main() -> None: generated_files = [] total_module_count = 0 + check_bazelrc_for_disabled_modules(known_good, Path(known_path).parent / ".bazelrc") + report_disabled_modules(known_good) + for group_name, group_modules in known_good.modules.items(): - modules = list(group_modules.values()) + # A disabled module must not reach any generated artifact: no bazel_dep/override, no + # coverage target, no docs mount. Filtering here covers every generator below at once. + modules = [module for module in group_modules.values() if module.enabled] if not modules: logging.warning(f"Skipping empty group: {group_name}") diff --git a/scripts/quality_runners.py b/scripts/quality_runners.py index 2cdddde4965..3a757fd8e37 100644 --- a/scripts/quality_runners.py +++ b/scripts/quality_runners.py @@ -329,8 +329,16 @@ def main() -> bool: if args.modules_to_test: print_centered(f"QR: User requested tests only for specified modules: {', '.join(args.modules_to_test)}") + # Without this, asking for a disabled module yields a silent green run, which reads + # exactly like "the tests passed". + requested_but_disabled = sorted(set(args.modules_to_test) & set(known.disabled_modules)) + if requested_but_disabled: + raise SystemExit( + "QR: refusing to run: these modules are disabled in known_good.json and have no " + f"tests to run: {', '.join(requested_but_disabled)}" + ) - for module in known.modules["target_sw"].values(): + for module in known.enabled_modules("target_sw").values(): if args.modules_to_test and module.name not in args.modules_to_test: print_centered(f"QR: Skipping module {module.name}") continue From bbc588770ca21ea2c184295986c6418b2bf55d44 Mon Sep 17 00:00:00 2001 From: Anton Krivoborodov Date: Fri, 25 Sep 2026 15:24:04 +0000 Subject: [PATCH 3/5] feat(known_good): make disabling a module safe The enabled flag from the previous commit removed a module from the generated Bazel fragments, but three things still pointed at a module that was gone. All three are now handled. .bazelrc flags are the worst case: Bazel resolves --@score_module//... on every invocation, so a leftover flag breaks bazel query and bazel mod too - the very commands needed to diagnose it. The previous commit only detected this and aborted, leaving the integrator to edit .bazelrc by hand. The 12 module-specific flags now live in bazel_common/bazelrc/.bazelrc, grouped by the repository the flag names rather than by the config it belongs to, which is what makes removal a pure deletion. update_module_from_known_good.py generates bazel_common/module_flags.bazelrc with one import per enabled fragment; the root .bazelrc imports that single file. A disabled module is simply absent from the list and comes back unchanged when re-enabled. sbom.tracked_modules was a hard error for no good reason - the SBOM should not be able to veto a deactivation. It is filtered automatically now, and the generator reports which modules it dropped. Wiring this up revealed that generate_sbom_module_content() had never been called from main(); score_sbom.MODULE.bazel was hand-maintained and carried a timestamp from 2026-08-21. Regenerating it changed only the timestamp, confirming the generator had been correct all along. The third case cannot be fixed, only detected. Dropping our bazel_dep removes this repository's request for a module, not the module: if another module declares its own dependency, Bazel falls back to the registry and resolves a released version instead of the pinned commit. The build succeeds and says nothing. check_disabled_modules.py resolves the real graph and fails with the list of modules pulling a disabled one back in. It returns before invoking Bazel when nothing is disabled, so it costs nothing on an ordinary pull request. Only score_config_management, score_kyron and score_time are leaves today. Verified with bazel build --announce_rc that all 12 flags still apply under --config=_common and --config=ferrocene-coverage-per, and end-to-end by disabling score_time in the real workspace: 8 -> 7 git_overrides, SBOM 7 -> 6, and a real bazel mod graph confirming the module was gone. 161 tests pass (was 156). --- .bazelrc | 23 +-- .github/workflows/known_good_correct.yml | 7 + bazel_common/bazelrc/score_baselibs.bazelrc | 23 +++ .../bazelrc/score_communication.bazelrc | 16 ++ .../bazelrc/score_config_management.bazelrc | 16 ++ bazel_common/bazelrc/score_logging.bazelrc | 25 +++ bazel_common/module_flags.bazelrc | 15 ++ docs/integration_process/reference.rst | 85 +++++++- scripts/known_good/check_disabled_modules.py | 190 ++++++++++++++++++ scripts/known_good/models/known_good.py | 33 +-- .../known_good/tests/test_module_enabled.py | 118 +++++++++-- .../update_module_from_known_good.py | 150 ++++++++++++-- 12 files changed, 627 insertions(+), 74 deletions(-) create mode 100644 bazel_common/bazelrc/score_baselibs.bazelrc create mode 100644 bazel_common/bazelrc/score_communication.bazelrc create mode 100644 bazel_common/bazelrc/score_config_management.bazelrc create mode 100644 bazel_common/bazelrc/score_logging.bazelrc create mode 100644 bazel_common/module_flags.bazelrc create mode 100755 scripts/known_good/check_disabled_modules.py diff --git a/.bazelrc b/.bazelrc index 18c8b36e35f..f2939d9f772 100644 --- a/.bazelrc +++ b/.bazelrc @@ -27,22 +27,18 @@ build --tool_java_language_version=17 build --java_runtime_version=remotejdk_17 build --tool_java_runtime_version=remotejdk_17 -build:_common --@score_baselibs//score/json:base_library=nlohmann -build:_common --@score_communication//score/memory/shared/flags:use_typedshmd=False -build:_common --@score_baselibs//score/log_rust:safety_level=qm -build:_common --@score_config_management//score/config_management/config_daemon/code:score_variant=true -build:_common --@score_baselibs//score/analysis/tracing/generic_trace_library:implementation=@score_baselibs//score/analysis/tracing/generic_trace_library/stub_implementation build:_common --cxxopt=-Wno-error=mismatched-new-delete build:_common --host_platform=@score_bazel_platforms//:x86_64-linux-gcc_12.2.0-posix build:_common --extra_toolchains=@score_gcc_x86_64_toolchain//:x86_64-linux-gcc_12.2.0 build:_common --extra_toolchains=//bazel/toolchains:score_ref_int_libclang_toolchain -# Flags needed by datarouter -build:_common --@score_logging//score/datarouter/build_configuration_flags:enable_nonverbose_dlt=False -build:_common --@score_logging//score/datarouter/build_configuration_flags:enable_dynamic_configuration=False -build:_common --@score_logging//score/datarouter/build_configuration_flags:file_transfer=False -build:_common --@score_logging//score/datarouter/build_configuration_flags:use_local_vlan=True -build:_common --@score_logging//score/datarouter/build_configuration_flags:dlt_output_enable=True +# Module-specific flags (--@score_//...) do NOT belong in this file: they can +# only be resolved while that module is part of the integration, so a module disabled +# in known_good.json would leave a flag naming an absent repository behind and break +# every bazel invocation, including 'bazel query'. They live in +# bazel_common/bazelrc/.bazelrc instead; the file imported below is generated +# from known_good.json and pulls in only the enabled modules' fragments. +import %workspace%/bazel_common/module_flags.bazelrc # ─── QNX common flags (shared by all QNX configs) ──────────────────────────── build:_qnx_common --host_platform=@score_bazel_platforms//:x86_64-linux-gcc_12.2.0-posix @@ -99,10 +95,9 @@ test:ferrocene-coverage --run_under=@score_tooling//coverage:llvm_profile_wrappe build:ferrocene-coverage --remote_download_all # Persistency specific flags +# The module-specific --@score_baselibs//... and --@score_logging//... flags of this +# config live in bazel_common/bazelrc/{score_baselibs,score_logging}.bazelrc. build:ferrocene-coverage-per --config=ferrocene-coverage -build:ferrocene-coverage-per --@score_baselibs//score/log_rust:safety_level=qm -build:ferrocene-coverage-per --@score_baselibs//score/json:base_library=nlohmann -build:ferrocene-coverage-per --@score_logging//score/mw/log/flags:KRemote_Logging=False # Output only failed test logs test --test_output=errors diff --git a/.github/workflows/known_good_correct.yml b/.github/workflows/known_good_correct.yml index 094db421b30..90b30471cbb 100644 --- a/.github/workflows/known_good_correct.yml +++ b/.github/workflows/known_good_correct.yml @@ -35,3 +35,10 @@ jobs: git diff exit 1 fi + # Dropping a module's bazel_dep does not remove it from the build: any module that + # declares its own dependency on it makes Bazel fall back to the registry, silently + # swapping the pinned commit for a released version. This resolves the real graph and + # fails if that happened. It exits before touching Bazel when nothing is disabled, so + # it costs nothing on an ordinary pull request. + - name: Verify disabled modules are really gone + run: scripts/known_good/check_disabled_modules.py --known known_good.json diff --git a/bazel_common/bazelrc/score_baselibs.bazelrc b/bazel_common/bazelrc/score_baselibs.bazelrc new file mode 100644 index 00000000000..2b5818ab0cd --- /dev/null +++ b/bazel_common/bazelrc/score_baselibs.bazelrc @@ -0,0 +1,23 @@ +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# Module-specific Bazel flags for score_baselibs. +# +# These flags name labels inside @score_baselibs, so they can only be resolved while +# that module is part of the integration. Keeping them here rather than in the root +# .bazelrc means they disappear together with the module when it is disabled in +# known_good.json - a --@score_baselibs//... flag for an absent module would otherwise +# break every bazel invocation, including 'bazel query'. +# +# Imported via bazel_common/module_flags.bazelrc, which is generated from +# known_good.json. Do not import this file directly. + +build:_common --@score_baselibs//score/json:base_library=nlohmann +build:_common --@score_baselibs//score/log_rust:safety_level=qm +build:_common --@score_baselibs//score/analysis/tracing/generic_trace_library:implementation=@score_baselibs//score/analysis/tracing/generic_trace_library/stub_implementation + +# Persistency-specific coverage config (see rust_coverage_config of score_persistency +# in known_good.json). The flags belong to score_baselibs, so they live here. +build:ferrocene-coverage-per --@score_baselibs//score/log_rust:safety_level=qm +build:ferrocene-coverage-per --@score_baselibs//score/json:base_library=nlohmann diff --git a/bazel_common/bazelrc/score_communication.bazelrc b/bazel_common/bazelrc/score_communication.bazelrc new file mode 100644 index 00000000000..aa456f4fb01 --- /dev/null +++ b/bazel_common/bazelrc/score_communication.bazelrc @@ -0,0 +1,16 @@ +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# Module-specific Bazel flags for score_communication. +# +# These flags name labels inside @score_communication, so they can only be resolved +# while that module is part of the integration. Keeping them here rather than in the +# root .bazelrc means they disappear together with the module when it is disabled in +# known_good.json - a --@score_communication//... flag for an absent module would +# otherwise break every bazel invocation, including 'bazel query'. +# +# Imported via bazel_common/module_flags.bazelrc, which is generated from +# known_good.json. Do not import this file directly. + +build:_common --@score_communication//score/memory/shared/flags:use_typedshmd=False diff --git a/bazel_common/bazelrc/score_config_management.bazelrc b/bazel_common/bazelrc/score_config_management.bazelrc new file mode 100644 index 00000000000..6696abf09b7 --- /dev/null +++ b/bazel_common/bazelrc/score_config_management.bazelrc @@ -0,0 +1,16 @@ +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# Module-specific Bazel flags for score_config_management. +# +# These flags name labels inside @score_config_management, so they can only be +# resolved while that module is part of the integration. Keeping them here rather than +# in the root .bazelrc means they disappear together with the module when it is +# disabled in known_good.json - a --@score_config_management//... flag for an absent +# module would otherwise break every bazel invocation, including 'bazel query'. +# +# Imported via bazel_common/module_flags.bazelrc, which is generated from +# known_good.json. Do not import this file directly. + +build:_common --@score_config_management//score/config_management/config_daemon/code:score_variant=true diff --git a/bazel_common/bazelrc/score_logging.bazelrc b/bazel_common/bazelrc/score_logging.bazelrc new file mode 100644 index 00000000000..69bad62e263 --- /dev/null +++ b/bazel_common/bazelrc/score_logging.bazelrc @@ -0,0 +1,25 @@ +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# Module-specific Bazel flags for score_logging. +# +# These flags name labels inside @score_logging, so they can only be resolved while +# that module is part of the integration. Keeping them here rather than in the root +# .bazelrc means they disappear together with the module when it is disabled in +# known_good.json - a --@score_logging//... flag for an absent module would otherwise +# break every bazel invocation, including 'bazel query'. +# +# Imported via bazel_common/module_flags.bazelrc, which is generated from +# known_good.json. Do not import this file directly. + +# Flags needed by datarouter +build:_common --@score_logging//score/datarouter/build_configuration_flags:enable_nonverbose_dlt=False +build:_common --@score_logging//score/datarouter/build_configuration_flags:enable_dynamic_configuration=False +build:_common --@score_logging//score/datarouter/build_configuration_flags:file_transfer=False +build:_common --@score_logging//score/datarouter/build_configuration_flags:use_local_vlan=True +build:_common --@score_logging//score/datarouter/build_configuration_flags:dlt_output_enable=True + +# Persistency-specific coverage config (see rust_coverage_config of score_persistency +# in known_good.json). The flag belongs to score_logging, so it lives here. +build:ferrocene-coverage-per --@score_logging//score/mw/log/flags:KRemote_Logging=False diff --git a/bazel_common/module_flags.bazelrc b/bazel_common/module_flags.bazelrc new file mode 100644 index 00000000000..790f53c1107 --- /dev/null +++ b/bazel_common/module_flags.bazelrc @@ -0,0 +1,15 @@ +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# Generated from known_good.json at 2026-08-28T09:19:12Z +# Do not edit manually - use scripts/known_good/update_module_from_known_good.py +# +# Each line imports one enabled module's flag fragment from bazelrc/. +# A module disabled in known_good.json is simply absent here, so its flags are gone +# without its fragment file having to be touched. + +import %workspace%/bazel_common/bazelrc/score_baselibs.bazelrc +import %workspace%/bazel_common/bazelrc/score_communication.bazelrc +import %workspace%/bazel_common/bazelrc/score_config_management.bazelrc +import %workspace%/bazel_common/bazelrc/score_logging.bazelrc diff --git a/docs/integration_process/reference.rst b/docs/integration_process/reference.rst index a5a2bfcdc5f..7b87902d471 100644 --- a/docs/integration_process/reference.rst +++ b/docs/integration_process/reference.rst @@ -210,26 +210,93 @@ coverage target, no documentation mount, no unit-test run and no SBOM entry. The generator prints every disabled module and its reason on each run, so the state cannot decay into an unnoticed permanent one. +.. _module_specific_bazelrc_flags: + +Module-specific ``.bazelrc`` flags +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A ``--@score_module//path:flag=value`` line is the most hostile thing that can +outlive its module. Bazel resolves it on *every* invocation, so a flag naming a +repository that is no longer in the graph breaks the entire workspace — not just +the build, but ``bazel query`` and ``bazel mod`` as well, which are exactly the +commands needed to diagnose the problem. + +Such flags therefore do not live in the root ``.bazelrc``. Each is placed in the +fragment of the module whose **repository it names**: + +.. code-block:: text + + bazel_common/bazelrc/score_baselibs.bazelrc + bazel_common/bazelrc/score_logging.bazelrc + ... + +Grouping by owning repository, rather than by the ``--config`` the flag belongs +to, is what makes deactivation a deletion: every line that becomes unresolvable +when a module leaves sits in that module's file and in no other. A single +fragment may contribute to several configs. + +``update_module_from_known_good.py`` generates +``bazel_common/module_flags.bazelrc``, a plain list of ``import`` lines covering +the enabled modules that have a fragment. The root ``.bazelrc`` imports that one +file from its ``build:_common`` block. A disabled module is simply absent from +the generated list, so its flags are gone without its fragment being touched — +and come back unchanged when the module is re-enabled. + +The generator refuses to proceed if a ``--@score_*//`` flag appears in the root +``.bazelrc``, if a fragment holds a flag belonging to a different module, or if +a fragment exists for a module that is not in ``known_good.json``. + +.. _disabled_module_registry_fallback: + +Why disabling is not always possible +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Removing a module's ``bazel_dep`` removes **this repository's** request for it. +It does not remove the module from the build. If any other module declares its +own dependency — ``score_persistency`` requires ``score_logging``, for instance +— Bazel still needs it, cannot find an override any more, and resolves it from +the registry instead: at a released version, not the commit ``known_good.json`` +pinned. The build succeeds. It just builds code nobody selected, and nothing in +the output says so. + +Disabling is therefore only permitted when no enabled module requires the +module. ``scripts/known_good/check_disabled_modules.py`` enforces this in CI by +resolving the real graph and failing with the list of modules that pull a +disabled one back in. It exits before invoking Bazel when nothing is disabled, +so it is free on an ordinary pull request. + +At the time of writing only ``score_config_management``, ``score_kyron`` and +``score_time`` are leaves that can be disabled on their own. Anything else has +to be disabled together with its consumers, which is usually the point at which +staging the CI (:ref:`staged_ci`) is the better answer. + What the flag does not do ~~~~~~~~~~~~~~~~~~~~~~~~~ The flag governs the **generated** artefacts only. Anything that names the module by hand keeps naming a module that no longer exists, and Bazel reports -that far away from ``known_good.json``. Two such references are checked -explicitly and abort the regeneration with the offending lines: +that far away from ``known_good.json``. These references are handled explicitly: .. list-table:: :header-rows: 1 :widths: 40 60 * - Reference - - Why it must go with the module - * - ``--@module//...`` flags in ``.bazelrc`` - - Bazel resolves them on *every* invocation, including ``bazel query``, so - the whole workspace becomes unusable. - * - ``extra_test_config`` / ``exclude_test_targets`` of another module, and - ``sbom.tracked_modules`` - - The entry would point into a module the build no longer contains. + - How it is handled + * - ``--@module//...`` flags + - Removed automatically, because they live in the module's own + ``.bazelrc`` fragment (see above). A stray flag in the root ``.bazelrc`` + aborts the regeneration. + * - ``sbom.tracked_modules`` + - Filtered automatically. The entry stays in ``known_good.json`` and the + generator reports which modules it dropped from the SBOM. + * - ``extra_test_config`` / ``exclude_test_targets`` of another module + - Aborts the regeneration with the offending entry: it points into a + module the build no longer contains, and only a human can decide whether + to drop the flag or keep the module. + * - A ``bazel_dep`` in another enabled module + - Aborts in CI (see above). There is no way to express this in + ``known_good.json``. Other references are **not** detected automatically and have to be removed by hand — hand-written ``BUILD`` files such as ``images/*/BUILD``, diff --git a/scripts/known_good/check_disabled_modules.py b/scripts/known_good/check_disabled_modules.py new file mode 100755 index 00000000000..74f08d01058 --- /dev/null +++ b/scripts/known_good/check_disabled_modules.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python3 +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +"""Verify that a module disabled in known_good.json is really out of the build. + +Setting ``"enabled": false`` removes a module's ``bazel_dep`` and ``git_override`` from +the generated Bazel fragments. That is *not* the same as removing it from the build: +if any other module declares its own ``bazel_dep`` on it, Bazel keeps it in the graph +and silently resolves it from the registry instead - at a released version rather than +the commit known_good.json used to pin. The result builds, and looks fine, against code +nobody chose. + +This check makes that impossible. It asks Bazel for the resolved module graph and fails +if a disabled module is still in it, naming the modules that pull it back in. + +It is free in the normal case: with nothing disabled it exits before ever invoking +Bazel. + +Usage: + python3 scripts/known_good/check_disabled_modules.py + python3 scripts/known_good/check_disabled_modules.py --mod-graph graph.json +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from pathlib import Path +from typing import Any + +# Importable both as a package module and runnable as a plain script from the repo +# root, which is how CI invokes it. Same fallback as update_module_from_known_good.py. +try: + from known_good.models.known_good import load_known_good +except ImportError: + _HERE = str(Path(__file__).resolve().parent) + if _HERE not in sys.path: + sys.path.insert(0, _HERE) + from models.known_good import load_known_good # noqa: E402 + + +def module_name_of(key: str) -> str: + """'score_logging@_' and 'score_logging@0.2.4' both name score_logging.""" + return key.split("@", 1)[0] + + +def collect_edges(root: dict[str, Any]) -> tuple[set[str], dict[str, set[str]]]: + """Walk the 'bazel mod graph --output=json' tree. + + Returns every module name in the graph, plus a reverse index mapping a module to + the modules that declare a dependency on it. + + Bazel prints a node's children only the first time it appears, so the reverse index + is a lower bound - enough to name culprits, not to prove there are no others. + """ + present: set[str] = set() + consumers: dict[str, set[str]] = {} + seen_keys: set[str] = set() + + def walk(node: dict[str, Any]) -> None: + key = node.get("key", "") + if key in seen_keys: + return + seen_keys.add(key) + name = module_name_of(key) + if not node.get("root"): + present.add(name) + for dep in node.get("dependencies", []) or []: + dep_name = module_name_of(dep.get("key", "")) + consumers.setdefault(dep_name, set()).add("" if node.get("root") else name) + walk(dep) + + walk(root) + return present, consumers + + +def run_mod_graph(workspace: Path, bazel: str) -> dict[str, Any]: + try: + result = subprocess.run( + [bazel, "mod", "graph", "--output=json"], + cwd=workspace, + capture_output=True, + text=True, + check=False, + ) + except FileNotFoundError: + raise SystemExit( + f"ERROR: '{bazel}' not found. Pass --bazel, or --mod-graph with a precomputed\n" + "'bazel mod graph --output=json' file." + ) from None + + if result.returncode != 0: + tail = "\n".join(result.stderr.strip().splitlines()[-15:]) + raise SystemExit(f"ERROR: 'bazel mod graph' failed (exit {result.returncode}):\n{tail}") + + return json.loads(result.stdout) + + +def main(argv: list[str]) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--known", default="known_good.json", help="Path to known_good.json") + parser.add_argument("--bazel", default="bazel", help="Bazel binary to invoke (default: bazel)") + parser.add_argument( + "--mod-graph", + type=Path, + help=( + "Use a precomputed 'bazel mod graph --output=json' file instead of invoking Bazel. " + "It must come from an already-regenerated workspace, otherwise it still contains " + "the entries the disabled module was supposed to lose." + ), + ) + args = parser.parse_args(argv) + + known_path = Path(args.known).resolve() + if not known_path.is_file(): + raise SystemExit(f"ERROR: known_good.json not found at {known_path}") + + try: + known_good = load_known_good(known_path) + except ValueError as e: + raise SystemExit(f"ERROR: {e}") from None + + disabled = known_good.disabled_modules + + if not disabled: + print("No modules are disabled in known_good.json - nothing to check.") + return 0 + + print("Disabled modules to verify:") + for name, module in sorted(disabled.items()): + print(f" - {name}: {module.disabled_reason}") + + if args.mod_graph: + graph = json.loads(args.mod_graph.read_text(encoding="utf-8")) + else: + print("\nResolving the module graph ('bazel mod graph')...") + graph = run_mod_graph(known_path.parent, args.bazel) + + present, consumers = collect_edges(graph) + still_present = sorted(set(disabled) & present) + + if not still_present: + print(f"\nOK: none of the {len(disabled)} disabled module(s) appear in the resolved module graph.") + return 0 + + # A disabled module the ROOT still depends on means the generated Bazel fragments are + # stale, not that another module pulls it in. Saying so avoids sending the reader off + # to hunt for a dependency that is not there. + stale = [name for name in still_present if "" in consumers.get(name, set())] + if stale: + print( + "\nERROR: the generated Bazel fragments still declare disabled modules: " + + ", ".join(stale) + + "\nRun 'python3 scripts/known_good/update_module_from_known_good.py' and retry.", + file=sys.stderr, + ) + return 1 + + print("\nERROR: disabled modules are still part of the resolved module graph.", file=sys.stderr) + print( + "Removing a module from known_good.json only drops this repository's bazel_dep and\n" + "git_override. Another module still requires it, so Bazel resolves it from the\n" + "registry instead - at a released version, not the commit that was pinned here.\n", + file=sys.stderr, + ) + for name in still_present: + pullers = sorted(consumers.get(name, set())) + print(f" {name} is required by: {', '.join(pullers) or '(unknown)'}", file=sys.stderr) + print( + "\nEither re-enable these modules, or disable the modules that depend on them too.\n" + "Run 'bazel mod explain ' to see the full path.", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/known_good/models/known_good.py b/scripts/known_good/models/known_good.py index a57c21e9ab6..8f4ee59a254 100644 --- a/scripts/known_good/models/known_good.py +++ b/scripts/known_good/models/known_good.py @@ -23,17 +23,17 @@ from .module import Module -def _validate_disabled_modules( - parsed_modules: Dict[str, Dict[str, Module]], - sbom_tracked_modules: list[str], -) -> None: +def _validate_disabled_modules(parsed_modules: Dict[str, Dict[str, Module]]) -> None: """Reject a disabled module that the rest of the configuration still depends on. Disabling a module only removes it from the *generated* artifacts. Anything that names it elsewhere keeps naming a module that no longer exists, and Bazel reports - that far from here - a ``--@score_x//...`` flag for an absent module fails *every* - invocation, including ``bazel query``. Failing here instead names the exact - entries that have to go with it. + that far from here, so failing here instead names the exact entries that have to + go with it. + + ``sbom.tracked_modules`` is deliberately *not* rejected here: that list states what + the integration ships, so the generator filters disabled modules out of it rather + than demanding a second edit that would have to be undone on re-enabling. """ disabled = { name: module @@ -46,10 +46,6 @@ def _validate_disabled_modules( problems: list[str] = [] - still_tracked = sorted(set(sbom_tracked_modules) & set(disabled)) - if still_tracked: - problems.append("listed in sbom.tracked_modules: " + ", ".join(still_tracked) + " (remove them from that list)") - # metadata labels of *enabled* modules that point into a disabled module, e.g. # score_persistency's extra_test_config referencing "@score_logging//...". for group_modules in parsed_modules.values(): @@ -120,7 +116,7 @@ def from_dict(cls, data: Dict[str, Any]) -> KnownGood: + ", ".join(invalid_modules) ) - _validate_disabled_modules(parsed_modules, sbom_tracked_modules) + _validate_disabled_modules(parsed_modules) return cls( modules=parsed_modules, @@ -147,6 +143,19 @@ def disabled_modules(self) -> Dict[str, Module]: if not module.enabled } + @property + def enabled_sbom_modules(self) -> list[str]: + """``sbom.tracked_modules`` minus the modules that are currently disabled. + + The SBOM must describe what the integration actually builds, so a disabled + module has to drop out of it. Filtering here rather than demanding the entry + be deleted keeps ``sbom.tracked_modules`` a stable statement of intent: it + does not have to be edited when a module is disabled, nor restored when it + is enabled again. + """ + disabled = set(self.disabled_modules) + return [name for name in self.sbom_tracked_modules if name not in disabled] + def to_dict(self) -> Dict[str, Any]: """Convert KnownGood instance to dictionary for JSON output. diff --git a/scripts/known_good/tests/test_module_enabled.py b/scripts/known_good/tests/test_module_enabled.py index 2d30d681931..755940e8a87 100644 --- a/scripts/known_good/tests/test_module_enabled.py +++ b/scripts/known_good/tests/test_module_enabled.py @@ -25,11 +25,14 @@ if str(_SCRIPTS_DIR) not in sys.path: sys.path.insert(0, str(_SCRIPTS_DIR)) +from known_good.check_disabled_modules import collect_edges # noqa: E402 from known_good.models.known_good import KnownGood # noqa: E402 from known_good.models.module import Module # noqa: E402 from known_good.update_module_from_known_good import ( # noqa: E402 - check_bazelrc_for_disabled_modules, + check_bazelrc_fragments, + check_bazelrc_has_no_module_flags, generate_docs_bundles_content, + generate_module_flags_content, ) _HASH = "0" * 40 @@ -130,15 +133,20 @@ def test_disabled_module_is_not_mounted_in_the_docs(): assert "score_logging" not in content -def test_disabled_module_may_not_stay_in_the_sbom(): - """The SBOM would otherwise claim a module the build no longer contains.""" - with pytest.raises(ValueError, match="sbom.tracked_modules"): - KnownGood.from_dict( - _raw_known_good( - {"score_logging": _raw("score_logging", enabled=False, disabled_reason="blocked")}, - sbom=["score_logging"], - ) +def test_disabled_module_is_dropped_from_the_sbom(): + """The SBOM is filtered, not rejected: an SBOM entry alone must not block disabling.""" + known = KnownGood.from_dict( + _raw_known_good( + { + "score_baselibs": _raw("score_baselibs"), + "score_logging": _raw("score_logging", enabled=False, disabled_reason="blocked"), + }, + sbom=["score_baselibs", "score_logging"], ) + ) + + assert known.sbom_tracked_modules == ["score_baselibs", "score_logging"] + assert known.enabled_sbom_modules == ["score_baselibs"] def test_metadata_reference_to_a_disabled_module_is_rejected(): @@ -157,33 +165,101 @@ def test_metadata_reference_to_a_disabled_module_is_rejected(): ) -def test_bazelrc_flag_for_a_disabled_module_is_rejected(tmp_path): - """The worst failure mode: such a flag breaks every bazel invocation, even 'query'.""" +def test_module_flags_file_imports_only_enabled_fragments(tmp_path): + """The generated import list is the whole deactivation mechanism for .bazelrc flags.""" known = KnownGood( - modules={"target_sw": {"score_logging": _module("score_logging", enabled=False, disabled_reason="blocked")}}, + modules={ + "target_sw": { + "score_baselibs": _module("score_baselibs"), + "score_logging": _module("score_logging", enabled=False, disabled_reason="blocked"), + } + }, timestamp="2026-01-01T00:00:00Z", ) + (tmp_path / "score_baselibs.bazelrc").write_text("build --@score_baselibs//a:b=1\n", encoding="utf-8") + (tmp_path / "score_logging.bazelrc").write_text("build --@score_logging//c:d=2\n", encoding="utf-8") + + content = generate_module_flags_content(known, tmp_path, "2026-01-01T00:00:00Z") + + assert "score_baselibs.bazelrc" in content + assert "score_logging" not in content + + +def test_module_flags_file_skips_modules_without_a_fragment(tmp_path): + """Most modules carry no flags at all; they must not produce a dangling import.""" + known = KnownGood( + modules={"target_sw": {"score_time": _module("score_time")}}, + timestamp="2026-01-01T00:00:00Z", + ) + + content = generate_module_flags_content(known, tmp_path, "2026-01-01T00:00:00Z") + + assert "score_time" not in content + + +def test_module_flag_in_the_root_bazelrc_is_rejected(tmp_path): + """The worst failure mode: such a flag breaks every bazel invocation, even 'query'.""" bazelrc = tmp_path / ".bazelrc" bazelrc.write_text("build --@score_logging//score/datarouter:enabled=true\n", encoding="utf-8") - with pytest.raises(SystemExit, match="still referenced by .bazelrc"): - check_bazelrc_for_disabled_modules(known, bazelrc) + with pytest.raises(SystemExit, match="must not live in .bazelrc"): + check_bazelrc_has_no_module_flags(bazelrc, tmp_path / "bazelrc") -def test_bazelrc_check_ignores_comments_and_enabled_modules(tmp_path): +def test_root_bazelrc_check_ignores_comments(tmp_path): + bazelrc = tmp_path / ".bazelrc" + bazelrc.write_text( + "# build --@score_logging//score/datarouter:enabled=true\nbuild --config=_common\n", + encoding="utf-8", + ) + + check_bazelrc_has_no_module_flags(bazelrc, tmp_path / "bazelrc") + + +def test_fragment_holding_another_modules_flag_is_rejected(tmp_path): + """Grouping by owning repository is what makes removal by deletion correct.""" known = KnownGood( modules={ "target_sw": { "score_baselibs": _module("score_baselibs"), - "score_logging": _module("score_logging", enabled=False, disabled_reason="blocked"), + "score_logging": _module("score_logging"), } }, timestamp="2026-01-01T00:00:00Z", ) - bazelrc = tmp_path / ".bazelrc" - bazelrc.write_text( - "# build --@score_logging//score/datarouter:enabled=true\nbuild --@score_baselibs//score/foo:enabled=true\n", - encoding="utf-8", + (tmp_path / "score_baselibs.bazelrc").write_text("build --@score_logging//c:d=2\n", encoding="utf-8") + + with pytest.raises(SystemExit, match="score_logging"): + check_bazelrc_fragments(known, tmp_path) + + +def test_fragment_without_a_matching_module_is_rejected(tmp_path): + """An orphan fragment is a flag nobody can ever disable again.""" + known = KnownGood( + modules={"target_sw": {"score_baselibs": _module("score_baselibs")}}, + timestamp="2026-01-01T00:00:00Z", ) + (tmp_path / "score_gone.bazelrc").write_text("build --@score_gone//a:b=1\n", encoding="utf-8") + + with pytest.raises(SystemExit, match="score_gone"): + check_bazelrc_fragments(known, tmp_path) + + +def test_mod_graph_walk_finds_who_pulls_a_module_back_in(): + """Dropping our bazel_dep does not remove a module another module still requires.""" + graph = { + "key": "reference_integration@_", + "root": True, + "dependencies": [ + { + "key": "score_persistency@_", + "dependencies": [{"key": "score_logging@0.2.4", "dependencies": []}], + } + ], + } + + present, consumers = collect_edges(graph) - check_bazelrc_for_disabled_modules(known, bazelrc) + assert "score_logging" in present + assert "reference_integration" not in present + assert consumers["score_logging"] == {"score_persistency"} diff --git a/scripts/known_good/update_module_from_known_good.py b/scripts/known_good/update_module_from_known_good.py index 5839c271cb9..56f06561081 100755 --- a/scripts/known_good/update_module_from_known_good.py +++ b/scripts/known_good/update_module_from_known_good.py @@ -222,6 +222,15 @@ def generate_sbom_module_content(tracked_modules: List[str], timestamp: Optional "\n" ) +# The root .bazelrc carries only this short notice, not the full SPDX block; the +# generated fragment importer matches it so the file looks like what it sits next to. +BAZELRC_LICENSE_HEADER = ( + "#\n" + "# See the NOTICE file(s) distributed with this work for additional\n" + "# information regarding copyright ownership.\n" + "#\n" +) + def generated_banner(timestamp: str) -> str: """Return the 'do not edit' banner naming the generator, for a generated file.""" @@ -325,38 +334,105 @@ def generate_file_content( return header + "\n".join(blocks) -def check_bazelrc_for_disabled_modules(known_good: KnownGood, bazelrc_path: Path) -> None: - """Abort when .bazelrc still carries a flag belonging to a disabled module. +BAZELRC_FRAGMENT_DIR = "bazelrc" +MODULE_FLAGS_FILENAME = "module_flags.bazelrc" +_MODULE_FLAG_RE = re.compile(r"--@(score_[A-Za-z0-9_]+)//") + + +def generate_module_flags_content(known_good: KnownGood, fragment_dir: Path, timestamp: Optional[str]) -> str: + """Generate the .bazelrc that imports each enabled module's flag fragment. - This is the failure mode that makes disabling a module look broken rather than - configured. A ``--@score_logging//...:flag=value`` line for a module Bazel no - longer knows about fails *every* invocation - ``build``, ``test``, even - ``query`` - with an opaque "no repository visible" error far away from - known_good.json. Named here, the fix is obvious: delete or move those lines. + Module-specific flags (``--@score_x//...``) cannot live in the root .bazelrc: + Bazel resolves them on *every* invocation, including ``bazel query``, so a module + disabled in known_good.json would leave behind a flag naming a repository that is + no longer visible to the root module and break the whole workspace. Routing them + through a generated import list means they appear and disappear with the module. - Only the disabled modules are checked; verifying that every *enabled* module's - flags exist is a different job, already done by rc_label_consistency.py. + ``import`` is used rather than ``try-import`` on purpose: a fragment listed here + belongs to an enabled module, so a missing file is a real error, not something to + swallow silently. """ - disabled = known_good.disabled_modules - if not disabled or not bazelrc_path.is_file(): + header = BAZELRC_LICENSE_HEADER + header += "# Generated from known_good.json" + (f" at {timestamp}" if timestamp else "") + "\n" + header += ( + "# Do not edit manually - use scripts/known_good/update_module_from_known_good.py\n" + "#\n" + f"# Each line imports one enabled module's flag fragment from {fragment_dir.name}/.\n" + "# A module disabled in known_good.json is simply absent here, so its flags are gone\n" + "# without its fragment file having to be touched.\n\n" + ) + + lines = [] + for group_modules in known_good.modules.values(): + for module in group_modules.values(): + if not module.enabled: + continue + if not (fragment_dir / f"{module.name}.bazelrc").is_file(): + continue + lines.append(f"import %workspace%/{fragment_dir.parent.name}/{fragment_dir.name}/{module.name}.bazelrc") + + if not lines: + return header + "# No enabled module contributes Bazel flags.\n" + return header + "\n".join(sorted(lines)) + "\n" + + +def check_bazelrc_has_no_module_flags(bazelrc_path: Path, fragment_dir: Path) -> None: + """Keep module-specific flags out of the root .bazelrc. + + Without this the convention silently decays: the first ``--@score_x//...`` line + added back to .bazelrc reintroduces exactly the failure the fragments exist to + prevent, and nothing would notice until a module is disabled. + """ + if not bazelrc_path.is_file(): return offenders: List[str] = [] for lineno, line in enumerate(bazelrc_path.read_text(encoding="utf-8").splitlines(), start=1): if line.lstrip().startswith("#"): continue - for name in disabled: - if f"@{name}//" in line: - offenders.append(f"{bazelrc_path.name}:{lineno}: {line.strip()}") + match = _MODULE_FLAG_RE.search(line) + if match: + offenders.append(f"{bazelrc_path.name}:{lineno}: {line.strip()} (belongs in {match.group(1)}.bazelrc)") if offenders: raise SystemExit( - "ERROR: disabled modules are still referenced by .bazelrc.\n" - "Bazel resolves these flags on every invocation, so the workspace would be\n" - "unusable. Remove or comment out these lines together with the module:\n " + "\n ".join(offenders) + f"ERROR: module-specific flags must not live in {bazelrc_path.name}.\n" + "Bazel resolves them on every invocation, so they would break the workspace as\n" + "soon as the module is disabled in known_good.json. Move them to\n" + f"{fragment_dir}/.bazelrc:\n " + "\n ".join(offenders) ) +def check_bazelrc_fragments(known_good: KnownGood, fragment_dir: Path) -> None: + """Report fragments whose module is unknown, and flags sitting in the wrong file. + + Both are silent failure modes otherwise: an orphaned fragment is never imported, + and a flag in the wrong fragment survives disabling the module it belongs to. + """ + if not fragment_dir.is_dir(): + return + + all_modules = {name for group_modules in known_good.modules.values() for name in group_modules} + problems: List[str] = [] + + for fragment in sorted(fragment_dir.glob("*.bazelrc")): + module_name = fragment.stem + if module_name not in all_modules: + problems.append(f"{fragment.name}: no module of that name in known_good.json") + continue + for lineno, line in enumerate(fragment.read_text(encoding="utf-8").splitlines(), start=1): + if line.lstrip().startswith("#"): + continue + for referenced in set(_MODULE_FLAG_RE.findall(line)): + if referenced != module_name: + problems.append( + f"{fragment.name}:{lineno}: flag belongs to '{referenced}', move it to {referenced}.bazelrc" + ) + + if problems: + raise SystemExit(f"ERROR: inconsistent Bazel flag fragments in {fragment_dir}:\n " + "\n ".join(problems)) + + def report_disabled_modules(known_good: KnownGood) -> None: """Print the disabled modules and why, so a regeneration never hides them.""" disabled = known_good.disabled_modules @@ -477,7 +553,10 @@ def main() -> None: generated_files = [] total_module_count = 0 - check_bazelrc_for_disabled_modules(known_good, Path(known_path).parent / ".bazelrc") + fragment_dir = Path(output_dir_modules) / BAZELRC_FRAGMENT_DIR + + check_bazelrc_has_no_module_flags(Path(known_path).parent / ".bazelrc", fragment_dir) + check_bazelrc_fragments(known_good, fragment_dir) report_disabled_modules(known_good) for group_name, group_modules in known_good.modules.items(): @@ -548,6 +627,41 @@ def main() -> None: generated_files.append(output_path_docs_bundles) print(f"Generated {output_path_docs_bundles}") + # Generate the SBOM extension config. Disabled modules drop out of sbom.tracked_modules + # automatically: the SBOM has to describe what is actually built, and requiring a second + # manual edit would mean a stale SBOM the moment someone forgets to undo it. + tracked = known_good.enabled_sbom_modules + dropped = [name for name in known_good.sbom_tracked_modules if name not in tracked] + content_sbom = generate_sbom_module_content(tracked, known_good.timestamp) + output_path_sbom = os.path.join(output_dir_modules, "score_sbom.MODULE.bazel") + + if args.dry_run: + print(f"\nDry run: would write to {output_path_sbom}\n") + print("---- BEGIN GENERATED CONTENT FOR SBOM ----") + print(content_sbom) + print("---- END GENERATED CONTENT FOR SBOM ----") + else: + with open(output_path_sbom, "w", encoding="utf-8") as f: + f.write(content_sbom) + generated_files.append(output_path_sbom) + suffix = f" ({len(dropped)} dropped: {', '.join(dropped)})" if dropped else "" + print(f"Generated {output_path_sbom} with {len(tracked)} tracked module(s){suffix}") + + # Generate the importer for the per-module Bazel flag fragments. + content_module_flags = generate_module_flags_content(known_good, fragment_dir, known_good.timestamp) + output_path_module_flags = os.path.join(output_dir_modules, MODULE_FLAGS_FILENAME) + + if args.dry_run: + print(f"\nDry run: would write to {output_path_module_flags}\n") + print("---- BEGIN GENERATED CONTENT FOR MODULE FLAGS ----") + print(content_module_flags) + print("---- END GENERATED CONTENT FOR MODULE FLAGS ----") + else: + with open(output_path_module_flags, "w", encoding="utf-8") as f: + f.write(content_module_flags) + generated_files.append(output_path_module_flags) + print(f"Generated {output_path_module_flags}") + if not args.dry_run and generated_files: print(f"\nSuccessfully generated {len(generated_files)} file(s) with {total_module_count} total modules") From 533ecf349c17612e1cff30c29c7f19caec271ea3 Mon Sep 17 00:00:00 2001 From: Anton Krivoborodov Date: Sat, 26 Sep 2026 09:30:03 +0000 Subject: [PATCH 4/5] fix(known_good): commit the regenerated SBOM fragment The previous commit wired generate_sbom_module_content() into main() but shipped the stale hand-maintained file: an end-to-end test restored the workspace with git checkout, which reverted the regenerated fragment, and it was staged without regenerating again. score_sbom.MODULE.bazel therefore still carried the 2026-08-21 timestamp while the generator produces the one from known_good.json. Caught by the Known Good Matches with Bazel job, which is exactly what it is for. --- bazel_common/score_sbom.MODULE.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel_common/score_sbom.MODULE.bazel b/bazel_common/score_sbom.MODULE.bazel index 8af3b08e353..b8824abf4e4 100644 --- a/bazel_common/score_sbom.MODULE.bazel +++ b/bazel_common/score_sbom.MODULE.bazel @@ -10,7 +10,7 @@ # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -# Generated from known_good.json at 2026-08-21T00:00:00+00:00Z +# Generated from known_good.json at 2026-08-28T09:19:12Z # Do not edit manually - use scripts/known_good/update_module_from_known_good.py sbom_ext = use_extension("@score_sbom//:extensions.bzl", "sbom_metadata") From 0d8a63dd71ab79a606c99149c9e4948d2395660e Mon Sep 17 00:00:00 2001 From: Anton Krivoborodov Date: Sat, 26 Sep 2026 09:40:06 +0000 Subject: [PATCH 5/5] fix(ci): install Bazel in the known_good job so the disabled-module check can run --- .github/workflows/known_good_correct.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/known_good_correct.yml b/.github/workflows/known_good_correct.yml index 90b30471cbb..a86f31e5907 100644 --- a/.github/workflows/known_good_correct.yml +++ b/.github/workflows/known_good_correct.yml @@ -35,6 +35,16 @@ jobs: git diff exit 1 fi + # Only needed by the disabled-module check below, which resolves the real module + # graph. Installing bazelisk is cheap; nothing invokes Bazel unless a module is + # actually disabled, so an ordinary pull request pays for the setup step alone. + - name: Setup Bazel + uses: bazel-contrib/setup-bazel@0.18.0 + with: + bazelisk-cache: true + disk-cache: ${{ github.workflow }} + repository-cache: true + cache-save: ${{ github.event_name == 'push' && github.ref_name == 'main' }} # Dropping a module's bazel_dep does not remove it from the build: any module that # declares its own dependency on it makes Bazel fall back to the registry, silently # swapping the pinned commit for a released version. This resolves the real graph and