Add check-pipeline-name-only-steps hook

.pre-commit-hooks.yaml

@@ -29,3 +29,13 @@
     - pre-commit
     - manual
   files: ^restored-packages\.txt$
+- id: check-pipeline-name-only-steps
+  name: check pipeline name-only steps
+  description: check that pipeline steps don't have only a name without uses or other details
+  entry: check-pipeline-name-only-steps
+  language: python
+  stages:
+    - pre-commit
+    - manual
+  types:
+    - yaml

example.pre-commit-config.yaml

@@ -18,6 +18,8 @@ repos:
           (?x)^(
               [^/]+\.ya?ml                      # matches .yaml or .yml files at the top level only
           )$
+      - id: check-pipeline-name-only-steps
+        files: '^[^.][^/]*\.yaml$' # matches non-hidden .yaml files at the top level only
   - repo: https://github.com/chainguard-dev/yam
     rev: 768695300c5f663012a77911eb4920c12e5ed2e5 # frozen: v0.2.26
     hooks:

pre_commit_hooks/check_pipeline_name_only_steps.py

@@ -0,0 +1,88 @@
+from __future__ import annotations
+
+import argparse
+import sys
+from collections.abc import Iterator
+from collections.abc import Sequence
+from typing import Any
+
+import ruamel.yaml
+
+yaml = ruamel.yaml.YAML(typ="safe")
+
+
+def iter_pipelines(melange_cfg: dict[str, Any]) -> Iterator[tuple[str, list[Any]]]:
+    """Yield (label, steps) for every pipeline in a melange config.
+
+    Covers the main build and test pipelines and each subpackage's build and
+    test pipelines. A ``pipeline:`` key with no value (YAML null) yields an
+    empty list, matching how melange treats it.
+    """
+    scopes: list[tuple[str, dict[str, Any]]] = [("main", melange_cfg)]
+    for i, subpkg in enumerate(melange_cfg.get("subpackages") or []):
+        if isinstance(subpkg, dict):
+            scopes.append((f"subpackage '{subpkg.get('name', i)}'", subpkg))
+    for label, scope in scopes:
+        yield f"{label} pipeline", scope.get("pipeline") or []
+        test = scope.get("test") or {}
+        if isinstance(test, dict):
+            yield f"{label} test pipeline", test.get("pipeline") or []
+
+
+def iter_steps(steps: list[Any]) -> Iterator[dict[str, Any]]:
+    """Yield every dict step in *steps*, descending into nested ``pipeline:`` lists."""
+    for step in steps:
+        if not isinstance(step, dict):
+            continue
+        yield step
+        yield from iter_steps(step.get("pipeline") or [])
+
+
+def check_pipeline_steps(melange_cfg: dict[str, Any]) -> list[str]:
+    """Return a message for every step that consists of nothing but a ``name``.
+
+    melange runs such a step as a no-op, so it is either a typo for ``uses:``
+    or a heading that was meant to sit on the step that follows it.
+    """
+    issues = []
+    for label, steps in iter_pipelines(melange_cfg):
+        for step in iter_steps(steps):
+            if set(step) == {"name"}:
+                issues.append(
+                    f"{label} step '{step['name']}' has only a name and does "
+                    "nothing; use 'uses:' for a pipeline, or fold the name into "
+                    "the next step",
+                )
+    return issues
+
+
+def main(argv: Sequence[str] | None = None) -> int:
+    parser = argparse.ArgumentParser(
+        description="Check that no melange pipeline step consists of only a name",
+    )
+    parser.add_argument("filenames", nargs="*", help="Filenames to check")
+    args = parser.parse_args(argv)
+
+    retval = 0
+
+    for filename in args.filenames:
+        try:
+            with open(filename) as f:
+                melange_cfg = yaml.load(f)
+        except Exception as e:
+            print(f"Error loading {filename}: {e}")
+            retval = 1
+            continue
+
+        if not isinstance(melange_cfg, dict):
+            continue
+
+        for issue in check_pipeline_steps(melange_cfg):
+            print(f"{filename}: {issue}")
+            retval = 1
+
+    return retval
+
+
+if __name__ == "__main__":
+    sys.exit(main())

setup.cfg

@@ -22,6 +22,7 @@ python_requires = >=3.9
 [options.entry_points]
 console_scripts =
     shellcheck-run-steps = pre_commit_hooks.shellcheck_run_steps:main
+    check-pipeline-name-only-steps = pre_commit_hooks.check_pipeline_name_only_steps:main
 
 [bdist_wheel]
 universal = True

test-data/README.md

@@ -0,0 +1,30 @@
+# Test data for pre-commit hooks
+
+Sample melange YAML files for exercising the hooks in this repository by hand.
+
+## Running a hook against a test file
+
+From the root of this repository:
+
+```bash
+pre-commit try-repo . check-pipeline-name-only-steps --files test-data/pipeline-name-only-bad.yaml
+```
+
+`pre-commit try-repo` accepts any path to a checkout of this repository, so the
+same command works from another repo by replacing `.` with that path.
+
+## Files
+
+### pipeline-name-only-bad.yaml
+
+- Hook: `check-pipeline-name-only-steps`
+- Expected: fails with five findings, one each in the main pipeline, the main
+  test pipeline, a subpackage pipeline, a nested `pipeline:` inside a
+  subpackage step, and a subpackage test pipeline.
+
+### pipeline-name-only-good.yaml
+
+- Hook: `check-pipeline-name-only-steps`
+- Expected: passes. Every step has `uses:` or `runs:` alongside any `name:`,
+  and the last subpackage has an empty `pipeline:` key, which melange allows
+  and the hook must not trip over.

test-data/pipeline-name-only-bad.yaml

@@ -0,0 +1,42 @@
+package:
+  name: test-package
+  version: "1.0.0"
+  epoch: 0
+  description: "Test package with incorrectly formatted pipeline steps"
+  copyright:
+    - license: Apache-2.0
+
+environment:
+  contents:
+    packages:
+      - busybox
+
+pipeline:
+  - name: test/go-fips-check  # BAD: This should be 'uses' not 'name'
+  - name: "Configure build"
+    uses: autoconf/configure
+    with:
+      opts: --enable-shared
+  - uses: autoconf/make
+
+test:
+  pipeline:
+    - name: run-tests  # BAD: Only has name, no uses
+    - uses: test/daemon-check-output
+      with:
+        expected_output: |
+          Server started
+
+subpackages:
+  - name: test-subpkg
+    description: "Subpackage with bad pipeline"
+    pipeline:
+      - name: bad-subpkg-step  # BAD: Only has name
+      - uses: split/dev
+      - name: grouped steps
+        pipeline:
+          - name: nested-bad-step  # BAD: name-only inside a nested pipeline
+          - runs: echo hello
+    test:
+      pipeline:
+        - name: subpkg-test-step  # BAD: Only has name in test section

test-data/pipeline-name-only-good.yaml

@@ -0,0 +1,54 @@
+package:
+  name: test-package-good
+  version: "1.0.0"
+  epoch: 0
+  description: "Test package with correctly formatted pipeline steps"
+  copyright:
+    - license: Apache-2.0
+
+environment:
+  contents:
+    packages:
+      - busybox
+      - go-fips-1.22
+
+pipeline:
+  - uses: test/go-fips-check  # GOOD: Uses 'uses' instead of 'name'
+  - name: "Configure build"
+    uses: autoconf/configure  # GOOD: Has both name and uses
+    with:
+      opts: --enable-shared
+  - uses: autoconf/make  # GOOD: Just uses is fine
+  - name: "Run custom script"  # GOOD: Has name and runs
+    runs: |
+      echo "Building package"
+      make install
+
+test:
+  pipeline:
+    - uses: test/go-fips-check  # GOOD: Properly uses 'uses'
+    - name: "Test daemon output"
+      uses: test/daemon-check-output  # GOOD: Has both name and uses
+      with:
+        expected_output: |
+          Server started
+    - uses: test/emptypackage  # GOOD: Just uses
+
+subpackages:
+  - name: test-subpkg-good
+    description: "Subpackage with correct pipeline"
+    pipeline:
+      - uses: split/dev  # GOOD: Uses 'uses'
+      - name: "Move files"
+        runs: |  # GOOD: Has name and runs
+          mkdir -p ${{targets.subpkgdir}}/usr/bin
+          mv usr/bin/tool ${{targets.subpkgdir}}/usr/bin/
+    test:
+      pipeline:
+        - uses: test/emptypackage  # GOOD: Uses 'uses' in test section
+  - name: test-subpkg-meta
+    description: "Meta subpackage: an empty pipeline is allowed and must not trip the hook"
+    pipeline:
+    test:
+      pipeline:
+        - uses: test/emptypackage