From 8b99da1a19fb2f52b77605be9570878b9c75b342 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Thu, 24 Sep 2026 15:12:33 -0700 Subject: [PATCH] Bundle uncrustify as prebuilt wheels and select it per ROS distribution apt uncrustify on Ubuntu 22.04 is 0.72.0 while rolling and jazzy pin 0.78.1 through uncrustify_vendor, and the two disagree on real ROS 2 sources, so a PATH lookup cannot give parity with colcon test. - uncrustify_wheel/: scikit-build-core project that builds both 0.78.1 and 0.72.0 from pinned tarballs into one py3-none wheel per platform, statically linking libstdc++, with cibuildwheel config and tests. - Build uncrustify wheels workflow: cibuildwheel matrix for Linux x86_64/aarch64 and macOS x86_64/arm64, publishing to a uncrustify-bin-v release when that tag does not exist yet. - ros-uncrustify: uses the bundled binary by default with --uncrustify-version {0.78.1,0.72.0}; --system-uncrustify keeps the PATH behavior for other platforms. - Docs: design decision, README, DEVELOPING release flow, changelog, plan with the pyproject wiring procedure that waits on the first wheel release. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/uncrustify-wheels.yml | 177 ++++++ .gitignore | 16 +- .pre-commit-config.yaml | 2 + .pre-commit-hooks.yaml | 5 +- CHANGELOG.md | 23 + DEVELOPING.md | 13 + README.md | 29 +- docs/DESIGN.md | 38 +- docs/PLAN.md | 153 +++++ pyproject.toml | 2 + .../checkers/uncrustify/__init__.py | 156 ++++- tests/test_uncrustify.py | 545 +++++++++++++++--- uncrustify_wheel/.gitignore | 4 + uncrustify_wheel/CMakeLists.txt | 81 +++ uncrustify_wheel/README.md | 89 +++ uncrustify_wheel/pyproject.toml | 60 ++ .../src/ros_uncrustify_bin/__init__.py | 54 ++ .../tests/test_ros_uncrustify_bin.py | 64 ++ 18 files changed, 1357 insertions(+), 154 deletions(-) create mode 100644 .github/workflows/uncrustify-wheels.yml create mode 100644 docs/PLAN.md create mode 100644 uncrustify_wheel/.gitignore create mode 100644 uncrustify_wheel/CMakeLists.txt create mode 100644 uncrustify_wheel/README.md create mode 100644 uncrustify_wheel/pyproject.toml create mode 100644 uncrustify_wheel/src/ros_uncrustify_bin/__init__.py create mode 100644 uncrustify_wheel/tests/test_ros_uncrustify_bin.py diff --git a/.github/workflows/uncrustify-wheels.yml b/.github/workflows/uncrustify-wheels.yml new file mode 100644 index 0000000..fd0cd01 --- /dev/null +++ b/.github/workflows/uncrustify-wheels.yml @@ -0,0 +1,177 @@ +--- +# Builds the `ros-uncrustify-bin` wheels from `uncrustify_wheel/` and publishes them +# to a GitHub release tagged `uncrustify-bin-v`. +# +# Each wheel is `py3-none-` and bundles the prebuilt uncrustify binaries that +# `ros-uncrustify` runs, so consumers never build uncrustify and never depend on the +# version their distribution happens to package. +# The hook consumes the release assets: this repo's own `pyproject.toml` depends on these +# wheel URLs directly, with platform markers picking the right one per machine. +# +# Every cibuildwheel setting other than the architecture lives in +# `uncrustify_wheel/pyproject.toml` under `[tool.cibuildwheel]`. +# +# A release happens only on `main`, and only when the tag does not exist yet. +# Bumping `version` in `uncrustify_wheel/pyproject.toml` is how a new release is produced; +# a push that does not bump it rebuilds the wheels and leaves the existing release alone. +name: Build uncrustify wheels + +on: + push: + branches: [main] + paths: + - uncrustify_wheel/** + - .github/workflows/uncrustify-wheels.yml + pull_request: + paths: + - uncrustify_wheel/** + - .github/workflows/uncrustify-wheels.yml + workflow_dispatch: + +# One run per ref at a time, so two pushes to main cannot race on the same release. +# Superseded pull request runs are cancelled; main runs queue instead. +concurrency: + group: uncrustify-wheels-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + build: + name: ${{ matrix.name }} + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: + include: + - name: manylinux x86_64 + runner: ubuntu-latest + arch: x86_64 + artifact: wheels-manylinux-x86_64 + # Native arm64 runner, so the aarch64 wheels build without QEMU. + - name: manylinux aarch64 + runner: ubuntu-24.04-arm + arch: aarch64 + artifact: wheels-manylinux-aarch64 + # macos-13 was retired in December 2025. macos-15-intel is the x86_64 + # replacement and the last Intel macOS image Actions will offer, through + # August 2027. + - name: macOS x86_64 + runner: macos-15-intel + arch: x86_64 + artifact: wheels-macos-x86_64 + - name: macOS arm64 + runner: macos-15 + arch: arm64 + artifact: wheels-macos-arm64 + steps: + - uses: actions/checkout@v6 + # The cibuildwheel action brings its own Python, so no setup-python step is needed. + - uses: pypa/cibuildwheel@v4.2.1 + with: + package-dir: uncrustify_wheel + env: + CIBW_ARCHS: ${{ matrix.arch }} + - uses: actions/upload-artifact@v7 + with: + name: ${{ matrix.artifact }} + path: wheelhouse/*.whl + if-no-files-found: error + + release: + name: Publish release + runs-on: ubuntu-latest + needs: build + # Pull requests build only. A manual dispatch releases from main and nowhere else. + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' + permissions: + contents: write + steps: + - uses: actions/checkout@v6 + - uses: actions/download-artifact@v8 + with: + pattern: wheels-* + path: dist + merge-multiple: true + + - name: Read version and check the tag + id: version + run: | + PYPROJECT=uncrustify_wheel/pyproject.toml + # -m1 takes the [project] version, not a 'version' key in a later table. + VERSION=$(grep -m1 '^version' "$PYPROJECT" | sed 's/version = "\(.*\)"/\1/') + case "$VERSION" in + [0-9]*.[0-9]*) ;; + *) echo "No version read from $PYPROJECT, got '$VERSION'"; exit 1 ;; + esac + TAG="uncrustify-bin-v$VERSION" + echo "tag=$TAG" >> "$GITHUB_OUTPUT" + if git ls-remote --exit-code --tags origin "refs/tags/$TAG" > /dev/null; then + echo "Tag $TAG already exists, leaving its release untouched." + echo "Bump 'version' in uncrustify_wheel/pyproject.toml to publish new wheels." + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "exists=false" >> "$GITHUB_OUTPUT" + fi + + - name: Generate SHA256SUMS + if: steps.version.outputs.exists == 'false' + run: | + cd dist + sha256sum -- *.whl > SHA256SUMS + cat SHA256SUMS + + # Notes list the uncrustify versions actually found inside the wheels, so they + # cannot drift from what was built. + - name: Publish the release + if: steps.version.outputs.exists == 'false' + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.version.outputs.tag }} + run: | + BUNDLED=$(python3 - <<'PY' + import glob + import posixpath + import zipfile + + versions = set() + for wheel in glob.glob('dist/*.whl'): + with zipfile.ZipFile(wheel) as archive: + for name in archive.namelist(): + head, tail = posixpath.split(name) + if posixpath.basename(head) == 'bin' and tail.startswith('uncrustify-'): + versions.add(tail[len('uncrustify-'):]) + print(', '.join(sorted(versions, reverse=True))) + PY + ) + test -n "$BUNDLED" || { echo "No bundled uncrustify binaries found in the wheels"; exit 1; } + { + echo "Prebuilt uncrustify binaries for \`ros-uncrustify\`, bundled versions: $BUNDLED." + echo + echo "Wheels:" + echo + for wheel in dist/*.whl; do + echo "- \`$(basename "$wheel")\`" + done + echo + echo "\`SHA256SUMS\` covers every wheel in this release." + } > notes.md + cat notes.md + gh release create "$TAG" --title "$TAG" --notes-file notes.md dist/*.whl dist/SHA256SUMS + + # Task 4.4 pastes these into pyproject.toml as direct-URL dependencies with + # platform markers. + - name: Print the asset download URLs + if: steps.version.outputs.exists == 'false' + env: + TAG: ${{ steps.version.outputs.tag }} + run: |- + BASE="${{ github.server_url }}/${{ github.repository }}/releases/download/$TAG" + { + echo "### Wheel download URLs" + echo + echo '```' + for wheel in dist/*.whl; do + echo "$BASE/$(basename "$wheel")" + done + echo "$BASE/SHA256SUMS" + echo '```' + } | tee -a "$GITHUB_STEP_SUMMARY" diff --git a/.gitignore b/.gitignore index 0704a73..97e40a9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,16 @@ -/*.egg-info/ +# Python __pycache__/ -/.pytest_tmp*/ +*.egg-info/ /.venv/ +/.pytest_tmp*/ +.pytest_cache/ +.mypy_cache/ + +# Wheel builds under uncrustify_wheel/ (local `uv build`, cibuildwheel, scikit-build-core) +uncrustify_wheel/dist/ +uncrustify_wheel/build/ +uncrustify_wheel/_skbuild/ +uncrustify_wheel/wheelhouse/ +uncrustify_wheel/.venv/ +*.whl +*.tar.gz diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5424243..08915d1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,6 +14,8 @@ repos: - id: ros-lint-cmake - id: ros-pep257 - id: ros-uncrustify + # Until the ros-uncrustify-bin wheel is wired into pyproject.toml (plan task 4.4). + args: [--system-uncrustify] - id: ros-xmllint - id: ros-clang-format - id: ros-mypy diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 248af0a..c23d622 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -72,8 +72,9 @@ language: python description: > uncrustify with the ament_code_style configuration. Reformats in place and fails - so you re-stage. Requires uncrustify on PATH. Accepts args: [--linelength, "100"] - and [--language, C++]. + so you re-stage. The binary ships with the hook for Linux x86_64/aarch64 and + macOS x86_64/arm64. Accepts args: [--uncrustify-version, "0.72.0"] for humble, + [--system-uncrustify], [--linelength, "100"], and [--language, C++]. entry: ros_code_standard uncrustify types_or: [c, c++] diff --git a/CHANGELOG.md b/CHANGELOG.md index 46daccf..6976035 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,29 @@ All notable changes to this project are documented in this file. Releases follow semantic versioning as described in [DEVELOPING.md](./DEVELOPING.md). +## Unreleased + +### Changed + +- `ros-uncrustify` runs a bundled uncrustify binary instead of whatever is on `PATH`. + The `ros-uncrustify-bin` wheel, built from `uncrustify_wheel/` by the `Build uncrustify wheels` workflow and published to this repo's releases, carries both 0.78.1 (rolling, jazzy) and 0.72.0 (humble) for Linux x86_64 and aarch64 and macOS x86_64 and arm64. + `--uncrustify-version` selects one, default 0.78.1. + `--system-uncrustify` restores the `PATH` lookup for other platforms. + Motivation: Ubuntu 22.04's apt uncrustify is 0.72.0, and with the 0_72 config it reformats files that rolling's pinned 0.78.1 accepts, so the distro binary cannot give parity. + +### Pending + +- The root `pyproject.toml` does not yet depend on the wheel. That wiring waits for the first `uncrustify-bin-v0.1.0` release to exist (plan task 4.4). + +## 0.1.1 + +### Fixed + +- `ros-cpplint` and `ros-lint-cmake` run `ament_cpplint` and `ament_lint_cmake` themselves instead of PyPI cpplint 2.0 and cmakelint 1.4.3 with ament's flags. + Both ament packages vendor modified forks, and cpplint 2.0 reported `build/c++17`, `whitespace/newline`, and wider include-what-you-use findings on `ros2/rcpputils`, which is green under ament_lint on rolling. + Findings now match `colcon test` exactly. +- README migration section maps `ament_(EXCLUDE ...)`, `LANGUAGE`, `FILTERS`, and `MAX_LINE_LENGTH` onto hook fields, with rcpputils as the worked example. + ## 0.1.0 Initial release: the `ament_lint` linters as pre-commit hooks, one hook per `ament_cmake_` package. diff --git a/DEVELOPING.md b/DEVELOPING.md index 2812ac5..ccb2886 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -64,3 +64,16 @@ Then add a `## ` section to `CHANGELOG.md` describing the release. CI fails PRs where the README pin or the changelog section is missing for the version in `pyproject.toml`. On merge to main, CI pushes the tag and publishes a GitHub release whose notes are that changelog section. + +### uncrustify binary wheels + +`uncrustify_wheel/` builds the `ros-uncrustify-bin` wheels that carry the uncrustify binaries `ros-uncrustify` runs. +The `Build uncrustify wheels` workflow builds all four platforms on every push to `main` that touches `uncrustify_wheel/`, and publishes them to a release tagged `uncrustify-bin-v` only when that tag does not exist yet. +Bumping `version` in `uncrustify_wheel/pyproject.toml` is the only way to cut a new one, and the direct-URL dependencies in the root `pyproject.toml` have to be repointed at the new tag in the same change. +Take the exact wheel filenames from the workflow's "Wheel download URLs" step summary, since auditwheel and delocate decide the final platform tags. + +To lint the workflow files locally with the shell and Python rules enabled: + +```shell +uvx --from actionlint-py --with shellcheck-py --with pyflakes actionlint .github/workflows/*.yml +``` diff --git a/README.md b/README.md index ad8b6ec..58bdb4b 100644 --- a/README.md +++ b/README.md @@ -27,8 +27,8 @@ Set up pre-commit hooks in the repository: pre-commit install ``` -`ros-uncrustify` also needs `uncrustify` on `PATH`. -See its [hook reference](#ros-uncrustify) entry. +Nothing else is needed on Linux x86_64 and aarch64 or macOS x86_64 and arm64. +The uncrustify binary ships with the hook. ## Configuration @@ -144,8 +144,6 @@ jobs: - uses: actions/setup-python@v5 with: python-version: '3.10' - # Only needed for ros-uncrustify. - - run: sudo apt-get install -y --no-install-recommends uncrustify - uses: pre-commit/action@v3.0.1 ``` @@ -310,8 +308,14 @@ Unlike bare `pydocstyle`, files whose names begin with `test_` are checked. ### `ros-uncrustify` Runs `uncrustify` with the `ament_code_style` configuration, the same one `ament_uncrustify` ships. -`uncrustify --version` selects the config: 0.78.1 and newer get `ament_code_style_0_78.cfg`, anything older gets `ament_code_style_0_72.cfg`. -Ubuntu 24.04 ships 0.78.1 and Ubuntu 22.04 ships 0.72.0, so both are supported. + +The uncrustify version decides the output, not just the configuration, so the hook ships its own binary rather than using whatever is installed. +`--uncrustify-version` selects it, and the matching ament config follows: 0.78.1 uses `ament_code_style_0_78.cfg`, 0.72.0 uses `ament_code_style_0_72.cfg`. + +| ROS 2 distribution | `--uncrustify-version` | +|---|---| +| rolling, jazzy | `0.78.1` (the default) | +| humble | `0.72.0` | Files are split into a `-l C` group (`.c`, `.cc`, `.h`, `.hh`) and a `-l CPP` group (`.cpp`, `.cxx`, `.hpp`, `.hxx`), matching `ament_uncrustify`, and each group is checked separately. A C++ header named `.h` is therefore parsed as C, as in ament. @@ -322,20 +326,21 @@ This differs from `ament_uncrustify`, which prints the diff and leaves the files Nothing else is written to your repository. > [!NOTE] -> Requires `uncrustify` on `PATH`. -> There is no PyPI package and no upstream Linux binary, so it cannot be installed into the hook virtualenv. -> Ubuntu and Debian: `sudo apt install uncrustify`. -> macOS: `brew install uncrustify`. -> ROS 2 builds it through [`uncrustify_vendor`](https://github.com/ros2/uncrustify_vendor), so sourcing a ROS 2 installation also puts a suitable version on `PATH`. +> The uncrustify binary ships with the hook, as the `ros-uncrustify-bin` wheel, for Linux x86_64, Linux aarch64, macOS x86_64, and macOS arm64. +> Nothing needs to be installed on those platforms. +> Anywhere else, install uncrustify yourself and add `--system-uncrustify`, which uses the binary on `PATH` and picks the ament config from the version it reports. +> A distribution package may not match your ROS 2 distribution's pin: Ubuntu 22.04 packages 0.72.0, and it reformats files that rolling's 0.78.1 accepts. **Optional:** +- `--uncrustify-version {0.78.1,0.72.0}` -- Which bundled uncrustify to run (default `0.78.1`). Ignored with `--system-uncrustify` +- `--system-uncrustify` -- Use the `uncrustify` on `PATH` instead of the bundled one, with the ament config chosen from the version it reports - `--linelength N` -- Override the config's `code_width` of 100 - `--language {C,C++,CPP}` -- Force uncrustify's `-l` instead of choosing it per file extension ```yaml - id: ros-uncrustify - args: [--language, C++] + args: [--uncrustify-version, '0.72.0', --language, C++] ``` --- diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 84ec07b..01b11ed 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -134,8 +134,7 @@ pre-commit's exit code is the test result. ### Shared virtualenv As in polymath_code_standard, every hook declares the same `language: python` and no `additional_dependencies`, so pre-commit builds one virtualenv for the whole repo regardless of which hooks a consumer picks. -All tools are installed up front. -The one exception is uncrustify, discussed below. +All tools are installed up front, including the prebuilt uncrustify wheel for the current platform. ## Challenges @@ -146,26 +145,29 @@ These are the linters that need more than "install from PyPI and pass a config". uncrustify is the primary ROS 2 C++ formatter, and it is the hardest to ship. There is no PyPI package. The upstream GitHub release only publishes source archives and Windows zips. -ROS builds it from source through `uncrustify_vendor`. +ROS builds it from source through `uncrustify_vendor`, and the version it pins is part of the standard: rolling and jazzy pin 0.78.1, humble pins 0.72.0, and the two produce different output on real code. +Ubuntu 22.04's apt package is 0.72.0, so on a 22.04 machine the distro binary fails rcpputils files that rolling's CI accepts. -Phase 1 (this iteration) mirrors how `polymath-go` handles the Go toolchain: -`ros-uncrustify` looks for `uncrustify` on `PATH`, fails with an install hint if absent, and picks `ament_code_style_0_78.cfg` or `ament_code_style_0_72.cfg` from `uncrustify --version` exactly as `ament_uncrustify` does. -Ubuntu 24.04 ships 0.78.1 and Ubuntu 22.04 ships 0.72.0, so both configs matter. +The first implementation took `uncrustify` from `PATH` and picked the ament config from `uncrustify --version`, exactly as `ament_uncrustify` does. +That reproduces ament's behavior, and also ament's failure: it matches whatever the machine has, not what the distribution pins. -Phase 2 options, for follow-up: +Options considered: -1. **Host prebuilt binaries** in this repo's GitHub releases and download them on first use with pinned checksums, the `golangci-lint` pattern. - We control the build, users get a consistent 0.78.1 everywhere. - Cost: a release job that cross-compiles for linux/amd64, linux/arm64, and macOS. +1. **Host prebuilt binaries** in this repo's GitHub releases and download them on first use with pinned checksums, the `golangci-lint` pattern from polymath_code_standard. 2. **Build from source into the hook virtualenv** on first use. - Needs cmake, a C++ compiler, and a Python 3 interpreter, and no third-party libraries. - Measured on 0.78.1 during implementation: 43 CPU-seconds, about 12 s on four cores, for a 1.4 MiB stripped binary that links only libstdc++ and libc. - Cheap enough to be a real fallback where no prebuilt binary exists. -3. **Publish a wheel** the way `clang-format`, `clang-tidy`, and `cppcheck` wheels are built with scikit-build-core. - Cleanest for consumers. - Most upfront work and a PyPI project to maintain. - -Recommendation: option 1, because the download-and-verify code already exists in polymath_code_standard. + Measured on 0.78.1: 43 CPU-seconds, about 12 s on four cores, for a 1.4 MiB stripped binary that links only libstdc++ and libc, needing cmake, a C++ compiler, and Python. + Rejected: consumers should not need a C++ toolchain to run a linter, and a first-commit build step is a poor experience. +3. **Publish wheels** containing the binaries, the way the `clang-format`, `clang-tidy`, and `cppcheck` wheels are built with scikit-build-core. + +Decision: option 3, hosted on this repo rather than PyPI. +A wheel project under `uncrustify_wheel/` bundles both pinned versions in one `py3-none-` wheel per platform (manylinux x86_64 and aarch64, macOS x86_64 and arm64). +A GitHub Action builds it with cibuildwheel whenever that directory changes and publishes the wheels to a release tagged `uncrustify-bin-v`. +The hook depends on those wheels by direct URL with `sys_platform` and `platform_machine` markers, so pip installs the right one into the shared virtualenv and pre-commit needs no extra configuration. +`--uncrustify-version` selects 0.78.1 (default, rolling and jazzy) or 0.72.0 (humble), and `--system-uncrustify` opts out to the `PATH` binary for unsupported platforms. + +Consequences: a Windows consumer has no bundled binary and must use `--system-uncrustify`. +Bumping uncrustify means editing two pins and the project version under `uncrustify_wheel/`, pushing, and then updating the dependency URLs in the hook's `pyproject.toml` once the release exists. +Until the first release is published the hook's dependency lines cannot be resolved, so wiring them is the last step of the rollout. ### clang-tidy: needs a compilation database diff --git a/docs/PLAN.md b/docs/PLAN.md new file mode 100644 index 0000000..aec2089 --- /dev/null +++ b/docs/PLAN.md @@ -0,0 +1,153 @@ +# Implementation plan + +Companion to [DESIGN.md](./DESIGN.md). +This file is the living status board. +Update the status column as tasks are planned, dispatched, and completed. + +Status values: `todo`, `dispatched`, `review`, `done`, `deferred`. + +## Phase 0: scaffold + +Done by hand before any parallel work so every task builds on the same base. + +| # | Task | Status | +|---|---|---| +| 0.1 | Copy `checker.py`, `runner.py`, package skeleton from polymath_code_standard, rename to `ros_code_standard` | done | +| 0.2 | `pyproject.toml` with every tool dependency pinned, package data declared for each config subpackage | done | +| 0.3 | `.pre-commit-hooks.yaml` with all thirteen hook ids, `.pre-commit-config.yaml` dev config | done | +| 0.4 | Repo hygiene: LICENSE, `.gitignore`, `.python-version`, `.envrc`, `Justfile`, CI workflow, DEVELOPING.md, CHANGELOG.md, README skeleton | done | +| 0.5 | `test_files/` minimal ROS 2 package layout to be filled in by each task | done | + +## Phase 1: checkers + +Each task owns the files listed and nothing else, so they run in parallel without conflicts. +Every task adds a `tests/test_.py` and any `test_files/` it needs, and reports the README hook-reference text for its hooks. + +| # | Task | Owns | Status | +|---|---|---|---| +| 1.1 | Python linters: `ros-flake8`, `ros-pep257`, `ros-pycodestyle`, `ros-pyflakes`, `ros-mypy` | `checkers/flake8/`, `checkers/pep257.py`, `checkers/pycodestyle/`, `checkers/pyflakes.py`, `checkers/mypy/`, `test_files/python/`, `tests/test_python_linters.py` | done | +| 1.2 | C++ static checks: `ros-cpplint` (header guard patch, root grouping), `ros-cppcheck`, `ros-clang-format` | `checkers/cpplint/`, `checkers/cppcheck.py`, `checkers/clang_format/`, `test_files/cpp_pkg/`, `tests/test_cpplint.py`, `tests/test_cppcheck.py`, `tests/test_clang_format.py` | done | +| 1.3 | `ros-uncrustify`: PATH lookup, version to config selection, fix-then-fail | `checkers/uncrustify/`, `tests/test_uncrustify.py` | done | +| 1.4 | Package-level checks: `ros-copyright` (ament_copyright passthrough), `ros-lint-cmake`, `ros-xmllint` | `checkers/copyright.py`, `checkers/lint_cmake.py`, `checkers/xmllint/`, `test_files/cpp_pkg/package.xml`, `test_files/cpp_pkg/CMakeLists.txt`, `tests/test_copyright.py`, `tests/test_lint_cmake.py`, `tests/test_xmllint.py` | done | + +## Phase 2: integration + +| # | Task | Status | +|---|---|---| +| 2.1 | Review each Phase 1 result for clarity, style, correctness; re-dispatch fixes | done | +| 2.2 | `uv lock`, `uv run pytest`, `pre-commit run --all-files` against this repo green | done | +| 2.3 | README hook reference assembled from task reports, `ament_lint_common` equivalence block, migration section | done | +| 2.4 | CHANGELOG `0.1.0` entry | done | + +## Phase 2b: parity fixes + +Triggered by running the hooks on `ros2/rcpputils`, which is green under `ament_lint_common` on rolling. + +| # | Task | Owns | Status | +|---|---|---|---| +| 2.5 | Swap PyPI `cpplint` and `cmakelint` for `ament_cpplint` and `ament_lint_cmake` from the ament_lint git repo | `pyproject.toml`, `uv.lock` | done | +| 2.6 | Rewrite `ros-cpplint` as an `ament_cpplint` wrapper | `checkers/cpplint/`, `tests/test_cpplint.py` | done | +| 2.7 | Rewrite `ros-lint-cmake` as an `ament_lint_cmake` wrapper | `checkers/lint_cmake.py`, `tests/test_lint_cmake.py` | done | +| 2.8 | DESIGN.md parity principle, README migration mapping for `EXCLUDE` and `LANGUAGE`, CHANGELOG | docs | done | +| 2.9 | Re-verify: `uv run pytest`, `pre-commit run --all-files` on this repo, hooks over rcpputils clean except the vendored header | | done | + +## Phase 4: prebuilt uncrustify wheels + +Decision (2026-09-24): apt uncrustify does not match the ROS distribution's pin. +Ubuntu 22.04 ships 0.72.0, rolling and jazzy build 0.78.1 through `uncrustify_vendor`, and the two produce different output on rcpputils. +Building from source on the consumer's machine was rejected. +Instead, a wheel project in this repo bundles prebuilt uncrustify binaries, a GitHub Action builds it for each platform whenever the project changes and publishes the wheels to a GitHub release, and the hook depends on those wheels by direct URL with platform markers. + +Contract shared by the three tasks: + +- Project directory `uncrustify_wheel/`, distribution `ros-uncrustify-bin`, import package `ros_uncrustify_bin`, version in `uncrustify_wheel/pyproject.toml`, starting at `0.1.0`. +- Each wheel is tagged `py3-none-` and holds both `ros_uncrustify_bin/bin/uncrustify-0.78.1` and `ros_uncrustify_bin/bin/uncrustify-0.72.0`. + Source pins: `uncrustify-0.78.1.tar.gz` sha256 `ecaf4c0adca14c36dfffa30bc28e69865115ecd602c90eb16a8cddccb41caad2`, `uncrustify-0.72.0.tar.gz` sha256 `d6fff70bc7823fac4c77013055333b79a4839909094e8eee8a14ee8f1777374e`, both from `https://github.com/uncrustify/uncrustify/archive/refs/tags/`. +- Platforms: manylinux x86_64, manylinux aarch64, macOS x86_64, macOS arm64. Windows is out of scope. +- Python API: `VERSIONS = ('0.78.1', '0.72.0')`, `DEFAULT_VERSION = '0.78.1'`, `binary(version=DEFAULT_VERSION) -> pathlib.Path`, raising `ValueError` for an unknown version. +- Release tag `uncrustify-bin-v` with the wheels and a `SHA256SUMS` file as assets. +- Hook: `--uncrustify-version {0.78.1,0.72.0}` (default 0.78.1) selects the bundled binary and the matching ament config; `--system-uncrustify` opts out to the PATH binary with version detection, the previous behavior. + +| # | Task | Owns | Status | +|---|---|---|---| +| 4.1 | Wheel project: scikit-build-core, CMake ExternalProject for both versions with pinned hashes, static linking, cibuildwheel config, local build and manylinux verification | `uncrustify_wheel/**` | done | +| 4.2 | Release workflow: cibuildwheel matrix on push to main touching `uncrustify_wheel/**`, GitHub release upload | `.github/workflows/uncrustify-wheels.yml` | done | +| 4.3 | Hook rewrite: bundled binary by default, `--uncrustify-version`, `--system-uncrustify` | `checkers/uncrustify/`, `tests/test_uncrustify.py` | done | +| 4.4 | Wire `pyproject.toml` dependencies to the release URLs with platform markers, `uv lock`, drop apt uncrustify from CI | `pyproject.toml`, `uv.lock`, `.github/workflows/test.yml` | blocked until the first release exists | +| 4.5 | Docs: DESIGN uncrustify section, README prerequisites and `ros-uncrustify`, DEVELOPING release flow, CHANGELOG | docs | done | + +### Task 4.4 procedure + +The organization is `ros-tooling`, from the README URL commit. +After the first push to `main` runs `Build uncrustify wheels` and the `uncrustify-bin-v0.1.0` release exists: + +1. Copy the exact wheel filenames from the workflow's "Wheel download URLs" step summary. + Expected, from the local cibuildwheel run (auditwheel puts the legacy alias first): + `ros_uncrustify_bin-0.1.0-py3-none-manylinux2014_x86_64.manylinux_2_17_x86_64.whl`, + `ros_uncrustify_bin-0.1.0-py3-none-manylinux2014_aarch64.manylinux_2_17_aarch64.whl`, + `ros_uncrustify_bin-0.1.0-py3-none-macosx_11_0_x86_64.whl`, + `ros_uncrustify_bin-0.1.0-py3-none-macosx_11_0_arm64.whl`. +2. Add to `[project] dependencies` in the root `pyproject.toml`, one line per platform, with the base + `https://github.com/ros-tooling/ros_code_standard/releases/download/uncrustify-bin-v0.1.0/`: + + ```toml + "ros-uncrustify-bin @ ; sys_platform == 'linux' and platform_machine == 'x86_64'", + "ros-uncrustify-bin @ ; sys_platform == 'linux' and platform_machine == 'aarch64'", + "ros-uncrustify-bin @ ; sys_platform == 'darwin' and platform_machine == 'x86_64'", + "ros-uncrustify-bin @ ; sys_platform == 'darwin' and platform_machine == 'arm64'", + ``` + +3. `uv lock`, `uv sync`, then `uv run pytest -q` should report the uncrustify `external` tests running through the wheel (54 in `tests/test_uncrustify.py`, 0 skipped). +4. Remove `args: [--system-uncrustify]` from `.pre-commit-config.yaml` and the two `apt-get install uncrustify` steps from `.github/workflows/test.yml`. +5. Bump the package with `just bump minor`, move the Unreleased changelog section under the new version, commit. + +## Phase 3: deferred + +| # | Task | Status | +|---|---|---| +| 3.1 | `ros-clang-tidy` with `--build-dir` argument | done | +| 3.2 | uncrustify prebuilt binary download with pinned checksums (design option 1) | superseded by Phase 4 | +| 3.3 | Publish to GitHub, first tagged release | deferred | + +## Open questions + +Decisions the implementation made that deserve a second look, collected from the task reports. + +1. `ros-flake8` and `ros-pycodestyle` disagree by design: `ament_flake8.ini` uses `extend-ignore` so flake8 keeps its default ignore list, while `ament_pycodestyle.ini` clears it. + W503, W504, E226 and E704 pass flake8 and fail pycodestyle. + This is ament's real behavior; changing `extend-ignore` to `ignore` would align them and diverge from ROS 2. +2. `ros-mypy` always passes `--explicit-package-bases`, otherwise two staged packages with a `setup.py` each abort mypy with a duplicate module error. + It slightly weakens import resolution for packages rooted in a subdirectory. +3. `ros-copyright` appends `LICENSE` and `CONTRIBUTING.md` only when they exist. + ament in directory mode fails when they are missing. + The hook also does not run on commits that stage no source files, so `always_run: true` may be wanted. +4. `ros-clang-tidy` fails on any diagnostic because clang-tidy exits 0 for warnings, matching ament's report-based behavior rather than the exit code. + No default `--header-filter`, so headers are unchecked unless asked. + The wheel's clang-tidy picks the newest GCC on the machine for libstdc++ and can need `--extra-arg=--gcc-install-dir=...`. +5. `ros-uncrustify`'s `--check` gate is verified on both 0.78.1 and 0.72.0. + A `.h` file is parsed as C, as in ament. + `--uncrustify-version` is per hook; if other hooks ever grow distribution-dependent behavior, a shared `--ros-distro` argument would be the better shape, and changing it later is a breaking argument change. +6. `ros-uncrustify` and `ros-clang-format` both pass on the fixtures today, but their configs conflict on short function bodies and constructor initializer lists. + The README says to pick one. +7. List-valued hook arguments are comma separated (`--filters=-a,-b`, `--libraries a,b`, `--add-ignore D1,D2`) rather than ament's space separated form, because pre-commit appends file names after the args. +8. The `clang-tidy` wheel is about 140 MB and is installed into every consumer's hook virtualenv, even for consumers that never enable `ros-clang-tidy`. + Moving it to `additional_dependencies` on that one hook would cost the shared virtualenv. +9. cppcheck 2.17 runs where ament skipped 2.x. + Consumers migrating should expect new cppcheck findings. + cpplint and lint_cmake are ament's own and match exactly. +10. `ros-lint-cmake` reads `~/.cmakelintrc` because ament does. + A hermetic standard would pass `--config=None`; deferred to the improve phase. + +## Dispatch log + +Newest first. + +- 2026-09-24: Phase 4 tasks 4.1, 4.2, 4.3 complete. Local manylinux2014 wheel built and verified (1.8 MiB, glibc 2.14 floor, no libstdc++ dependency); the bundled 0.78.1 passes all 46 rcpputils C++ files and 0.72.0 reformats `thread_name.cpp`, reproducing the reported symptom. 188 tests pass with the real wheel installed. Task 4.4 blocked until the first release; procedure recorded above. +- 2026-09-24: apt uncrustify 0.72.0 on Ubuntu 22.04 reformats rcpputils files that 0.78.1 accepts. Phase 4 planned: prebuilt wheels built by GitHub Actions. Tasks 4.1, 4.2, 4.3 dispatched in parallel. +- 2026-09-24: Parity fixes complete. `ros-cpplint` and `ros-lint-cmake` are wrappers over `ament_cpplint` and `ament_lint_cmake`. 169 tests pass. All twelve dev hooks pass on this repo. The eight `ament_lint_common` hooks pass on a clone of ros2/rcpputils (64a437c) with the CMake `EXCLUDE` and `LANGUAGE` options translated to `exclude:` and `--language`, with no files modified. +- 2026-09-24: Parity check on ros2/rcpputils failed under cpplint 2.0.2 (build/c++17, whitespace/newline on `{return;}`, wider IWYU tables). ament's vendored cpplint and cmakelint confirmed to be modified forks. Dependencies swapped to `ament_cpplint` and `ament_lint_cmake` from git; checker rewrites dispatched to the original task owners. +- 2026-09-24: Phase 2 complete. Review found relative imports and a filename-swallowing `nargs='+'` in the C++ task, fixed by re-dispatch. Integrator fixed W503/W504, a D403, a mypy annotation, and a YAML anchor in the hook manifest that merged `types_or` into every hook and emptied `ros-xmllint`. `uv run pytest`: 168 passed. `pre-commit run --all-files` over the repo with uncrustify 0.78.1 on PATH: 12 hooks passed. +- 2026-09-24: Task 3.1 (clang-tidy) completed alongside Phase 1 and marked experimental. +- 2026-09-24: Phase 1 tasks 1.1, 1.2, 1.3, 1.4 and Phase 3 task 3.1 dispatched to Opus subagents in parallel. +- 2026-09-24: Phase 0 scaffold complete. Environment resolves with `uv lock`, all flake8 plugins load, core files pass the ament Python style and ament_copyright. +- 2026-09-24: DESIGN.md and PLAN.md written. diff --git a/pyproject.toml b/pyproject.toml index e4a7d5e..305080f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,8 @@ dev = [ ] [tool.pytest.ini_options] +# uncrustify_wheel/tests needs the built wheel installed; run it from that directory. +testpaths = ["tests"] markers = [ # Needs a tool that is not in the virtualenv, such as uncrustify on PATH. Deselect with -m 'not external'. "external: test requires an external tool on PATH", diff --git a/ros_code_standard/checkers/uncrustify/__init__.py b/ros_code_standard/checkers/uncrustify/__init__.py index 3c84605..ce3ca97 100644 --- a/ros_code_standard/checkers/uncrustify/__init__.py +++ b/ros_code_standard/checkers/uncrustify/__init__.py @@ -15,8 +15,18 @@ """ uncrustify with the ament_code_style configuration. -uncrustify has no PyPI package and no upstream Linux binary, so the hook takes it from PATH -the way the polymath-go hook takes the Go toolchain. +The uncrustify version decides the output, not just the configuration. +Ubuntu 22.04 packages 0.72.0 while rolling and jazzy build 0.78.1 through uncrustify_vendor, +and the two disagree on real ROS 2 sources, so taking whatever is on PATH cannot give parity +with a ROS distribution. +The hook therefore runs a binary from the ros-uncrustify-bin wheel by default, which ships +both 0.78.1 and 0.72.0, and --uncrustify-version picks the one that matches the distribution +being targeted. + +--system-uncrustify opts out and uses the uncrustify on PATH, choosing the ament config from +the version that binary reports. +That is the only mode available on a platform the wheel is not built for. + Unlike ament_uncrustify, which only prints a diff, this hook reformats the files it finds and fails so the developer re-stages them. """ @@ -48,19 +58,33 @@ # uncrustify is not always idempotent, so reformatting repeats until the content settles. MAX_PASSES = 5 +# The versions the ros-uncrustify-bin wheel carries, newest first. +BUNDLED_VERSIONS = ('0.78.1', '0.72.0') +DEFAULT_VERSION = '0.78.1' + UNCRUSTIFY_MISSING = ( 'uncrustify was not found on PATH.\n' "Ubuntu and Debian: 'sudo apt install uncrustify'.\n" "macOS: 'brew install uncrustify'.\n" 'ROS 2 builds uncrustify through the uncrustify_vendor package, so sourcing a ROS 2\n' - 'installation puts a suitable version on PATH as well.' + 'installation puts a suitable version on PATH as well.\n' + 'Dropping --system-uncrustify uses the uncrustify that ships with this hook instead.' +) + +BUNDLED_MISSING = ( + 'The uncrustify that ships with this hook is not available on this platform.\n' + 'The ros-uncrustify-bin wheel is built for Linux x86_64, Linux aarch64, macOS x86_64\n' + 'and macOS arm64 only.\n' + 'On any other platform, install uncrustify yourself and add --system-uncrustify to the\n' + "hook's args." ) REFORMATTED = '(files have been reformatted, please re-stage and recommit)' # uncrustify prints strings such as 'Uncrustify-0.72.0_f' or 'Uncrustify_d-0.78.1', -# depending on how it was built. -_VERSION_RE = re.compile(r'^Uncrustify[^0-9]*([0-9]+\.[0-9]+\.[0-9]+)') +# depending on how it was built. The prefix is optional so that the same parse accepts a +# bare '0.78.1' from --uncrustify-version. +_VERSION_RE = re.compile(r'^(?:Uncrustify[^0-9]*)?([0-9]+\.[0-9]+\.[0-9]+)') class UncrustifyError(Exception): @@ -77,11 +101,11 @@ def parse_version(version_output: str) -> tuple[int, ...] | None: def select_config(version_output: str) -> Path | None: """ - Return the bundled ament config for the version uncrustify reports. + Return the ament config for a version, given either a version string or --version output. 0.78.1 changed enough option behavior that ament ships a second configuration for it. That release and anything newer use the 0.78 config, everything older uses the 0.72 one. - Returns None when the version string cannot be parsed. + Returns None when the version cannot be parsed. """ version = parse_version(version_output) if version is None: @@ -208,6 +232,82 @@ def diff_lines(path: str, before: bytes, after: bytes) -> list[str]: ) +def probe_version(binary: str) -> str | Result: + """Return the version string a binary prints, or a failed Result if it will not run.""" + proc = subprocess.run([binary, '--version'], capture_output=True, text=True) + output = (proc.stdout + proc.stderr).strip() + if proc.returncode != 0: + return Result( + name='uncrustify', + passed=False, + output=f"The invocation of '{binary} --version' failed:\n{output}", + ) + return output + + +def _with_config(binary: str, version: str) -> tuple[str, Path] | Result: + """Pair a binary with the ament config for a version, or fail if the version is unusable.""" + config = select_config(version) + if config is None: + return Result( + name='uncrustify', + passed=False, + output=f"Invalid uncrustify version '{version}'", + ) + return binary, config + + +def resolve_system() -> tuple[str, Path] | Result: + """Return the uncrustify on PATH and the ament config for the version it reports.""" + binary = shutil.which('uncrustify') + if binary is None: + return Result(name='uncrustify', passed=False, output=UNCRUSTIFY_MISSING) + reported = probe_version(binary) + if isinstance(reported, Result): + return reported + return _with_config(binary, reported) + + +def resolve_bundled(version: str) -> tuple[str, Path] | Result: + """ + Return the requested uncrustify from the ros-uncrustify-bin wheel and its ament config. + + The wheel is a platform-marked dependency, so on a platform it is not built for there is + simply no ros_uncrustify_bin to import. + The binary is asked for its version once, so a wheel that shipped the wrong build fails + loudly instead of formatting with an uncrustify the config was not written for. + """ + try: + import ros_uncrustify_bin + except ImportError: + return Result(name='uncrustify', passed=False, output=BUNDLED_MISSING) + + # ValueError for a version the distribution does not build, OSError for a wheel that + # was built for another platform and does not carry the file it advertises. + try: + binary = str(ros_uncrustify_bin.binary(version)) + except (ValueError, OSError) as error: + return Result( + name='uncrustify', + passed=False, + output=f'ros-uncrustify-bin cannot supply uncrustify {version}: {error}', + ) + + reported = probe_version(binary) + if isinstance(reported, Result): + return reported + if parse_version(reported) != parse_version(version): + return Result( + name='uncrustify', + passed=False, + output=( + f'The bundled uncrustify {version} at {binary} reports {reported!r}.\n' + 'The ros-uncrustify-bin wheel does not match what it claims; reinstall it.' + ), + ) + return _with_config(binary, version) + + def format_group(binary: str, config: Path, language: str, files: list[str]) -> Result: """ Check one language group and reformat it in place if uncrustify would change anything. @@ -247,7 +347,20 @@ class UncrustifyGroup(CheckerGroup): name = 'uncrustify' def register_args(self, subparser: argparse.ArgumentParser) -> None: - """Register the ament_uncrustify options that this hook forwards.""" + """Register the binary selection options and the ament_uncrustify options.""" + subparser.add_argument( + '--uncrustify-version', + choices=list(BUNDLED_VERSIONS), + default=DEFAULT_VERSION, + help='Which uncrustify shipped with this hook to run. Ignored with ' + '--system-uncrustify', + ) + subparser.add_argument( + '--system-uncrustify', + action='store_true', + help='Run the uncrustify on PATH instead of the one shipped with this hook, and ' + 'pick the ament config from the version it reports', + ) subparser.add_argument( '--linelength', metavar='N', @@ -262,26 +375,13 @@ def register_args(self, subparser: argparse.ArgumentParser) -> None: super().register_args(subparser) def run(self, args: argparse.Namespace) -> list[Result]: - binary = shutil.which('uncrustify') - if binary is None: - return [Result(name='uncrustify', passed=False, output=UNCRUSTIFY_MISSING)] - - version = subprocess.run([binary, '--version'], capture_output=True, text=True) - version_output = (version.stdout + version.stderr).strip() - if version.returncode != 0: - return [Result( - name='uncrustify', - passed=False, - output=f"The invocation of '{binary} --version' failed:\n{version_output}", - )] - - config = select_config(version_output) - if config is None: - return [Result( - name='uncrustify', - passed=False, - output=f"Invalid uncrustify version '{version_output}'", - )] + if args.system_uncrustify: + resolved = resolve_system() + else: + resolved = resolve_bundled(args.uncrustify_version) + if isinstance(resolved, Result): + return [resolved] + binary, config = resolved groups = group_by_language(args.files, normalize_language(args.language)) if not groups: diff --git a/tests/test_uncrustify.py b/tests/test_uncrustify.py index 978f8d9..6a38f9e 100644 --- a/tests/test_uncrustify.py +++ b/tests/test_uncrustify.py @@ -15,15 +15,25 @@ """ Tests for the uncrustify checker group. -The unit tests cover the parts that decide how uncrustify is invoked: which bundled config -matches the installed version, which files go into the C group and which into the CPP group, -and what happens when the binary is not installed at all. -The tests marked `external` need a real uncrustify on PATH and skip without one. +Most of the group is the bookkeeping that decides how uncrustify is invoked: which binary, +which of the two ament configs, and which files go into the C group and which into the CPP +group. +Those tests run everywhere, driving binary resolution with a shell stub that only answers +--version, so no uncrustify is needed. + +The tests marked `external` run a real uncrustify. They find one through the +ros-uncrustify-bin wheel when it is installed, and otherwise through PATH plus +ROS_UNCRUSTIFY_TEST_BINARIES, a path-separated list of binaries to probe. Each skips when +the version it needs is not available. """ import argparse +import os from pathlib import Path import shutil +import subprocess +import sys +import types import pytest @@ -33,10 +43,6 @@ _PROJECT_ROOT = Path(__file__).parent.parent _CPP_PKG = _PROJECT_ROOT / 'test_files' / 'cpp_pkg' -needs_uncrustify = pytest.mark.skipif( - shutil.which('uncrustify') is None, reason='uncrustify not on PATH' -) - CLEAN_CPP = """\ #include @@ -66,14 +72,132 @@ } """ +# Reduced from src/thread_name.cpp of ros2/rcpputils at 64a437c, which is green under +# ament_lint_common on rolling. uncrustify 0.78.1 leaves the call wrapped as written; +# 0.72.0 moves the first argument onto its own line. This is the divergence that made the +# hook ship its own binary rather than trust whatever apt installed. +RCPPUTILS_CALL_WRAP = """\ +#include + +namespace rcpputils +{ + +int wide_char_to_multi_byte( + unsigned code_page, unsigned long flags, const wchar_t * wide, int wide_length, + char * out, int out_length, const char * fallback, bool * used_fallback); + +int size_of(const wchar_t * description) +{ + int size_needed = wide_char_to_multi_byte(CP_UTF8, 0, description, -1, nullptr, 0, nullptr, + nullptr); + return size_needed; +} + +} // namespace rcpputils +""" + def _args(*files: Path, **overrides) -> argparse.Namespace: - namespace = argparse.Namespace(files=[str(f) for f in files], linelength=None, language=None) + namespace = argparse.Namespace( + files=[str(f) for f in files], + linelength=None, + language=None, + uncrustify_version=uncrustify.DEFAULT_VERSION, + system_uncrustify=False, + ) for key, value in overrides.items(): setattr(namespace, key, value) return namespace +def _fake_wheel(**binaries: str) -> types.SimpleNamespace: + """ + Build a stand-in for ros_uncrustify_bin that hands back the given paths. + + Keys are versions with dots written as underscores, so they can be keyword arguments. + """ + paths = {version.replace('_', '.'): value for version, value in binaries.items()} + + def binary(version: str = uncrustify.DEFAULT_VERSION) -> Path: + if version not in paths: + raise ValueError(f'unknown uncrustify version {version!r}') + return Path(paths[version]) + + return types.SimpleNamespace( + VERSIONS=tuple(paths), + DEFAULT_VERSION=uncrustify.DEFAULT_VERSION, + binary=binary, + ) + + +def _install_wheel(monkeypatch, module: object | None) -> None: + """ + Put a fake ros_uncrustify_bin in sys.modules, or None to make the import fail. + + The import machinery hands back whatever sys.modules holds, and a None entry is the + documented way to make an import raise, so the real wheel is never needed. + """ + monkeypatch.setitem(sys.modules, 'ros_uncrustify_bin', module) + + +def _version_stub(tmp_path: Path, name: str, version_output: str, code: int = 0) -> Path: + """Write an executable that only answers --version, standing in for an uncrustify.""" + path = tmp_path / name + path.write_text(f'#!/bin/sh\necho "{version_output}"\nexit {code}\n') + path.chmod(0o755) + return path + + +def _discover_binaries() -> dict[str, Path]: + """ + Map version to a real uncrustify on this machine, for the tests marked external. + + The wheel is preferred because that is what consumers run. PATH and + ROS_UNCRUSTIFY_TEST_BINARIES fill in for a developer who has not installed it yet. + """ + found: dict[str, Path] = {} + try: + import ros_uncrustify_bin + except ImportError: + pass + else: + for version in ros_uncrustify_bin.VERSIONS: + found[version] = Path(ros_uncrustify_bin.binary(version)) + + candidates = [ + Path(entry) + for entry in os.environ.get('ROS_UNCRUSTIFY_TEST_BINARIES', '').split(os.pathsep) + if entry + ] + on_path = shutil.which('uncrustify') + if on_path is not None: + candidates.append(Path(on_path)) + + for candidate in candidates: + if not candidate.is_file(): + continue + probe = subprocess.run([str(candidate), '--version'], capture_output=True, text=True) + parsed = uncrustify.parse_version(probe.stdout + probe.stderr) + if parsed is not None: + found.setdefault('.'.join(str(part) for part in parsed), candidate) + return found + + +BINARIES = _discover_binaries() + + +def needs(*versions: str): + """Skip unless a real uncrustify of every named version is available.""" + missing = [version for version in versions if version not in BINARIES] + return pytest.mark.skipif(bool(missing), reason=f'no uncrustify {", ".join(missing)}') + + +def _wheel_of(monkeypatch, *versions: str) -> None: + """Install a fake wheel backed by the real binaries discovered on this machine.""" + _install_wheel(monkeypatch, _fake_wheel(**{v.replace('.', '_'): str(BINARIES[v]) + for v in versions})) + + # --- version string to config selection --- @@ -89,6 +213,9 @@ def _args(*files: Path, **overrides) -> argparse.Namespace: # A double digit minor sorts correctly because the parse compares integers, # where ament_uncrustify compares the version bytes. ('Uncrustify-0.100.0', 'ament_code_style_0_78.cfg'), + # The same parse serves --uncrustify-version, which passes a bare version. + ('0.78.1', 'ament_code_style_0_78.cfg'), + ('0.72.0', 'ament_code_style_0_72.cfg'), ], ) def test_select_config_per_version(version_output, expected): @@ -104,6 +231,202 @@ def test_select_config_rejects_an_unparseable_version(version_output): def test_parse_version_returns_a_comparable_triple(): assert uncrustify.parse_version('Uncrustify-0.72.0_f') == (0, 72, 0) + assert uncrustify.parse_version('0.72.0') == (0, 72, 0) + + +def test_every_bundled_version_maps_to_a_config(): + for version in uncrustify.BUNDLED_VERSIONS: + assert uncrustify.select_config(version) is not None + assert uncrustify.DEFAULT_VERSION in uncrustify.BUNDLED_VERSIONS + + +# --- bundled binary resolution --- + + +def test_bundled_default_picks_0_78_1_and_its_config(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify-0.78.1', 'Uncrustify-0.78.1_f') + _install_wheel(monkeypatch, _fake_wheel(**{'0_78_1': str(stub)})) + + binary, config = uncrustify.resolve_bundled(uncrustify.DEFAULT_VERSION) + assert binary == str(stub) + assert config.name == 'ament_code_style_0_78.cfg' + + +def test_bundled_0_72_0_picks_the_0_72_config(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify-0.72.0', 'Uncrustify-0.72.0_f') + _install_wheel(monkeypatch, _fake_wheel(**{'0_72_0': str(stub)})) + + binary, config = uncrustify.resolve_bundled('0.72.0') + assert binary == str(stub) + assert config.name == 'ament_code_style_0_72.cfg' + + +def test_version_guard_rejects_a_binary_of_the_wrong_version(tmp_path, monkeypatch): + """A wheel that shipped the wrong build must fail, not silently format with it.""" + stub = _version_stub(tmp_path, 'uncrustify-0.78.1', 'Uncrustify-0.72.0_f') + _install_wheel(monkeypatch, _fake_wheel(**{'0_78_1': str(stub)})) + + result = uncrustify.resolve_bundled('0.78.1') + assert isinstance(result, uncrustify.Result) + assert not result.passed + assert 'reports' in result.output + assert '0.72.0' in result.output + assert 'reinstall' in result.output + + +def test_bundled_binary_that_will_not_run_is_reported(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify-0.78.1', 'boom', code=3) + _install_wheel(monkeypatch, _fake_wheel(**{'0_78_1': str(stub)})) + + result = uncrustify.resolve_bundled('0.78.1') + assert isinstance(result, uncrustify.Result) + assert '--version' in result.output + assert 'boom' in result.output + + +def test_bundled_unknown_version_reports_the_wheels_error(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify-0.78.1', 'Uncrustify-0.78.1_f') + _install_wheel(monkeypatch, _fake_wheel(**{'0_78_1': str(stub)})) + + result = uncrustify.resolve_bundled('0.60.0') + assert isinstance(result, uncrustify.Result) + assert 'cannot supply uncrustify 0.60.0' in result.output + + +def test_a_wheel_missing_its_binary_is_reported(monkeypatch): + """ros_uncrustify_bin.binary raises FileNotFoundError for a wheel built elsewhere.""" + def binary(version: str = uncrustify.DEFAULT_VERSION) -> Path: + raise FileNotFoundError(f'no uncrustify {version} in this wheel') + + _install_wheel( + monkeypatch, + types.SimpleNamespace( + VERSIONS=uncrustify.BUNDLED_VERSIONS, + DEFAULT_VERSION=uncrustify.DEFAULT_VERSION, + binary=binary, + ), + ) + + result = uncrustify.resolve_bundled('0.78.1') + assert isinstance(result, uncrustify.Result) + assert not result.passed + assert 'cannot supply uncrustify 0.78.1' in result.output + + +def test_missing_wheel_reports_the_platform_message(tmp_path, monkeypatch): + _install_wheel(monkeypatch, None) + monkeypatch.setattr( + uncrustify, 'format_group', lambda *a: pytest.fail('uncrustify must not be invoked') + ) + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + + results = uncrustify.UncrustifyGroup().run(_args(source)) + assert [(r.name, r.passed, r.skipped) for r in results] == [('uncrustify', False, False)] + assert results[0].output == uncrustify.BUNDLED_MISSING + assert 'Linux x86_64' in results[0].output + assert 'macOS arm64' in results[0].output + assert '--system-uncrustify' in results[0].output + + +def test_bundled_mode_is_the_default(tmp_path, monkeypatch): + """Without --system-uncrustify nothing reaches PATH, even when an uncrustify is there.""" + _install_wheel(monkeypatch, None) + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: pytest.fail('PATH must not be used')) + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + + assert uncrustify.UncrustifyGroup().run(_args(source))[0].output == uncrustify.BUNDLED_MISSING + + +# --- system binary resolution --- + + +def test_system_mode_uses_path_and_the_detected_version(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify', 'Uncrustify-0.72.0_f') + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: str(stub)) + _install_wheel(monkeypatch, None) + + binary, config = uncrustify.resolve_system() + assert binary == str(stub) + assert config.name == 'ament_code_style_0_72.cfg' + + +def test_system_mode_ignores_the_requested_version(tmp_path, monkeypatch): + """--uncrustify-version selects a bundled binary, so it has nothing to say here.""" + stub = _version_stub(tmp_path, 'uncrustify', 'Uncrustify-0.72.0_f') + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: str(stub)) + seen = [] + + def _record(binary, config, language, files): + seen.append((binary, config.name)) + return uncrustify.Result(name='uncrustify', passed=True) + + monkeypatch.setattr(uncrustify, 'format_group', _record) + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + + uncrustify.UncrustifyGroup().run( + _args(source, system_uncrustify=True, uncrustify_version='0.78.1') + ) + assert seen == [(str(stub), 'ament_code_style_0_72.cfg')] + + +def test_system_mode_missing_binary_fails_with_an_install_hint(tmp_path, monkeypatch): + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: None) + monkeypatch.setattr( + uncrustify, 'format_group', lambda *a: pytest.fail('uncrustify must not be invoked') + ) + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + + results = uncrustify.UncrustifyGroup().run(_args(source, system_uncrustify=True)) + assert [(r.name, r.passed, r.skipped) for r in results] == [('uncrustify', False, False)] + assert results[0].output == uncrustify.UNCRUSTIFY_MISSING + assert 'apt install uncrustify' in results[0].output + assert 'brew install uncrustify' in results[0].output + assert 'uncrustify_vendor' in results[0].output + + +def test_system_mode_missing_binary_makes_the_runner_exit_nonzero(tmp_path, monkeypatch): + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: None) + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + assert runner.main(['uncrustify', '--system-uncrustify', str(source)]) == 1 + + +def test_system_mode_unparseable_version_fails(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify', 'Frobnicate 1.2') + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: str(stub)) + + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + results = uncrustify.UncrustifyGroup().run(_args(source, system_uncrustify=True)) + assert not results[0].passed + assert 'Invalid uncrustify version' in results[0].output + + +# --- argument parsing --- + + +def test_hook_arguments_have_the_documented_defaults(): + parser = argparse.ArgumentParser() + uncrustify.UncrustifyGroup().register_args(parser) + parsed = parser.parse_args(['a.cpp']) + assert parsed.uncrustify_version == '0.78.1' + assert parsed.system_uncrustify is False + assert parsed.files == ['a.cpp'] + + parsed = parser.parse_args(['--uncrustify-version', '0.72.0', '--system-uncrustify', 'a.cpp']) + assert parsed.uncrustify_version == '0.72.0' + assert parsed.system_uncrustify is True + + +def test_an_unbundled_version_is_rejected_by_argparse(): + parser = argparse.ArgumentParser() + uncrustify.UncrustifyGroup().register_args(parser) + with pytest.raises(SystemExit): + parser.parse_args(['--uncrustify-version', '0.69.0', 'a.cpp']) # --- language grouping --- @@ -147,6 +470,16 @@ def test_normalize_language_maps_the_ament_spelling(): assert uncrustify.normalize_language(None) is None +def test_no_matching_files_skips(tmp_path, monkeypatch): + stub = _version_stub(tmp_path, 'uncrustify-0.78.1', 'Uncrustify-0.78.1_f') + _install_wheel(monkeypatch, _fake_wheel(**{'0_78_1': str(stub)})) + unrelated = tmp_path / 'notes.md' + unrelated.write_text('hello\n') + + results = uncrustify.UncrustifyGroup().run(_args(unrelated)) + assert [(r.passed, r.skipped) for r in results] == [(True, True)] + + # --- line length override --- @@ -157,12 +490,12 @@ def test_config_code_width_of_both_bundled_configs(): def test_linelength_config_is_none_when_it_matches_the_config(): - config = uncrustify.select_config('Uncrustify-0.78.1') + config = uncrustify.select_config('0.78.1') assert uncrustify.linelength_config(config, 100) is None def test_linelength_config_appends_an_override(): - config = uncrustify.select_config('Uncrustify-0.78.1') + config = uncrustify.select_config('0.78.1') override = uncrustify.linelength_config(config, 120) try: assert override is not None @@ -175,7 +508,9 @@ def test_linelength_config_appends_an_override(): def test_linelength_override_is_removed_after_the_run(tmp_path, monkeypatch): - """The temp config must not outlive the hook, even though the check itself fails.""" + """The temp config must not outlive the hook.""" + stub = _version_stub(tmp_path, 'uncrustify-0.78.1', 'Uncrustify-0.78.1_f') + _install_wheel(monkeypatch, _fake_wheel(**{'0_78_1': str(stub)})) source = tmp_path / 'messy.cpp' source.write_text(MESSY_CPP) seen = [] @@ -184,12 +519,6 @@ def _record(binary, config, language, files): seen.append(Path(config)) return uncrustify.Result(name='uncrustify', passed=True) - monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: '/usr/bin/uncrustify') - monkeypatch.setattr( - uncrustify.subprocess, - 'run', - lambda *a, **k: argparse.Namespace(returncode=0, stdout='Uncrustify-0.78.1', stderr=''), - ) monkeypatch.setattr(uncrustify, 'format_group', _record) uncrustify.UncrustifyGroup().run(_args(source, linelength=120)) @@ -197,61 +526,6 @@ def _record(binary, config, language, files): assert not seen[0].exists() -# --- missing binary --- - - -def test_missing_binary_fails_with_an_install_hint(tmp_path, monkeypatch): - monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: None) - monkeypatch.setattr( - uncrustify, 'format_group', lambda *a: pytest.fail('uncrustify must not be invoked') - ) - source = tmp_path / 'greeter.cpp' - source.write_text(CLEAN_CPP) - - results = uncrustify.UncrustifyGroup().run(_args(source)) - assert [(r.name, r.passed, r.skipped) for r in results] == [('uncrustify', False, False)] - assert results[0].output == uncrustify.UNCRUSTIFY_MISSING - assert 'apt install uncrustify' in results[0].output - assert 'brew install uncrustify' in results[0].output - assert 'uncrustify_vendor' in results[0].output - - -def test_missing_binary_makes_the_runner_exit_nonzero(tmp_path, monkeypatch): - monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: None) - source = tmp_path / 'greeter.cpp' - source.write_text(CLEAN_CPP) - assert runner.main(['uncrustify', str(source)]) == 1 - - -def test_unparseable_version_fails(tmp_path, monkeypatch): - monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: '/usr/bin/uncrustify') - monkeypatch.setattr( - uncrustify.subprocess, - 'run', - lambda *a, **k: argparse.Namespace(returncode=0, stdout='Frobnicate 1.2', stderr=''), - ) - source = tmp_path / 'greeter.cpp' - source.write_text(CLEAN_CPP) - - results = uncrustify.UncrustifyGroup().run(_args(source)) - assert not results[0].passed - assert 'Invalid uncrustify version' in results[0].output - - -def test_no_matching_files_skips(tmp_path, monkeypatch): - monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: '/usr/bin/uncrustify') - monkeypatch.setattr( - uncrustify.subprocess, - 'run', - lambda *a, **k: argparse.Namespace(returncode=0, stdout='Uncrustify-0.78.1', stderr=''), - ) - unrelated = tmp_path / 'notes.md' - unrelated.write_text('hello\n') - - results = uncrustify.UncrustifyGroup().run(_args(unrelated)) - assert [(r.passed, r.skipped) for r in results] == [(True, True)] - - # --- diff rendering --- @@ -268,22 +542,25 @@ def test_diff_lines_of_identical_content_is_empty(): assert uncrustify.diff_lines('a.cpp', b'int a = 1;\n', b'int a = 1;\n') == [] -# --- end to end, needs the real binary --- +# --- end to end, needs a real binary --- @pytest.mark.external -@needs_uncrustify -def test_clean_file_passes(tmp_path): +@needs('0.78.1') +def test_clean_file_passes(tmp_path, monkeypatch): + _wheel_of(monkeypatch, '0.78.1') source = tmp_path / 'greeter.cpp' source.write_text(CLEAN_CPP) + results = uncrustify.UncrustifyGroup().run(_args(source)) assert [r.passed for r in results] == [True], results[0].output assert source.read_text() == CLEAN_CPP @pytest.mark.external -@needs_uncrustify -def test_messy_file_is_reformatted_and_reported(tmp_path): +@needs('0.78.1') +def test_messy_file_is_reformatted_and_reported(tmp_path, monkeypatch): + _wheel_of(monkeypatch, '0.78.1') source = tmp_path / 'greeter.cpp' source.write_text(MESSY_CPP) @@ -305,8 +582,9 @@ def test_messy_file_is_reformatted_and_reported(tmp_path): @pytest.mark.external -@needs_uncrustify -def test_runner_exit_codes(tmp_path): +@needs('0.78.1') +def test_runner_exit_codes(tmp_path, monkeypatch): + _wheel_of(monkeypatch, '0.78.1') clean = tmp_path / 'clean.cpp' clean.write_text(CLEAN_CPP) messy = tmp_path / 'messy.cpp' @@ -318,8 +596,9 @@ def test_runner_exit_codes(tmp_path): @pytest.mark.external -@needs_uncrustify -def test_c_and_cpp_groups_are_checked_separately(tmp_path): +@needs('0.78.1') +def test_c_and_cpp_groups_are_checked_separately(tmp_path, monkeypatch): + _wheel_of(monkeypatch, '0.78.1') header = tmp_path / 'legacy.h' header.write_text('int add(int a, int b);\n') source = tmp_path / 'greeter.cpp' @@ -333,9 +612,10 @@ def test_c_and_cpp_groups_are_checked_separately(tmp_path): @pytest.mark.external -@needs_uncrustify -def test_linelength_override_changes_the_verdict(tmp_path): - """A line that fits in 100 columns but not in 40 is only wrapped at the shorter width.""" +@needs('0.78.1') +def test_linelength_override_changes_the_verdict(tmp_path, monkeypatch): + """A line that fits in 100 columns but not in 20 is only wrapped at the shorter width.""" + _wheel_of(monkeypatch, '0.78.1') source = tmp_path / 'wide.cpp' source.write_text( 'int compute(int alpha, int beta, int gamma)\n' @@ -348,16 +628,97 @@ def test_linelength_override_changes_the_verdict(tmp_path): @pytest.mark.external -@needs_uncrustify -@pytest.mark.skipif(not _CPP_PKG.is_dir(), reason='test_files/cpp_pkg not populated yet') -def test_bundled_cpp_package_is_clean(): +@needs('0.78.1') +def test_system_mode_runs_the_path_binary(tmp_path, monkeypatch): + """--system-uncrustify reaches a real binary and formats with the config for its version.""" + monkeypatch.setattr(uncrustify.shutil, 'which', lambda _: str(BINARIES['0.78.1'])) + _install_wheel(monkeypatch, None) + source = tmp_path / 'greeter.cpp' + source.write_text(CLEAN_CPP) + + results = uncrustify.UncrustifyGroup().run(_args(source, system_uncrustify=True)) + assert [r.passed for r in results] == [True], results[0].output + + +@pytest.mark.external +@needs('0.78.1') +def test_bundled_cpp_package_is_clean(monkeypatch): + _wheel_of(monkeypatch, '0.78.1') sources = sorted( - str(path) + path for path in _CPP_PKG.rglob('*') if path.suffix.lstrip('.') in uncrustify.C_EXTENSIONS + uncrustify.CPP_EXTENSIONS ) - if not sources: - pytest.skip('test_files/cpp_pkg has no C or C++ sources yet') - results = uncrustify.UncrustifyGroup().run(_args(*[Path(s) for s in sources])) + assert sources, 'test_files/cpp_pkg has no C or C++ sources' + results = uncrustify.UncrustifyGroup().run(_args(*sources)) failures = [r.output for r in results if not r.passed] assert not failures, '\n'.join(failures) + + +# --- the parity regression that motivated the bundled binary --- + + +@pytest.mark.external +@needs('0.78.1') +def test_rcpputils_call_wrap_passes_under_0_78_1(tmp_path, monkeypatch): + _wheel_of(monkeypatch, '0.78.1') + source = tmp_path / 'thread_name.cpp' + source.write_text(RCPPUTILS_CALL_WRAP) + + results = uncrustify.UncrustifyGroup().run(_args(source)) + assert [r.passed for r in results] == [True], results[0].output + assert source.read_text() == RCPPUTILS_CALL_WRAP + + +@pytest.mark.external +@needs('0.72.0') +def test_rcpputils_call_wrap_is_reformatted_under_0_72_0(tmp_path, monkeypatch): + """The symptom: apt's 0.72.0 rewrites a file that rolling's 0.78.1 accepts.""" + _wheel_of(monkeypatch, '0.72.0') + source = tmp_path / 'thread_name.cpp' + source.write_text(RCPPUTILS_CALL_WRAP) + + results = uncrustify.UncrustifyGroup().run(_args(source, uncrustify_version='0.72.0')) + assert [r.passed for r in results] == [False] + assert 'int size_needed = wide_char_to_multi_byte(\n' in source.read_text() + + +@pytest.mark.external +@needs('0.78.1', '0.72.0') +def test_the_two_bundled_versions_disagree_on_the_same_file(tmp_path, monkeypatch): + """One hook, two verdicts, chosen by --uncrustify-version alone.""" + _wheel_of(monkeypatch, '0.78.1', '0.72.0') + rolling = tmp_path / 'rolling.cpp' + rolling.write_text(RCPPUTILS_CALL_WRAP) + humble = tmp_path / 'humble.cpp' + humble.write_text(RCPPUTILS_CALL_WRAP) + + assert uncrustify.UncrustifyGroup().run(_args(rolling, uncrustify_version='0.78.1'))[0].passed + assert not uncrustify.UncrustifyGroup().run( + _args(humble, uncrustify_version='0.72.0') + )[0].passed + + +@pytest.mark.external +@needs('0.78.1', '0.72.0') +@pytest.mark.skipif( + not os.environ.get('ROS_RCPPUTILS_CLONE'), reason='ROS_RCPPUTILS_CLONE is not set' +) +def test_rcpputils_thread_name_parity(tmp_path, monkeypatch): + """The whole upstream file, when a clone is available to point at.""" + original = Path(os.environ['ROS_RCPPUTILS_CLONE']) / 'src' / 'thread_name.cpp' + if not original.is_file(): + pytest.skip(f'{original} does not exist') + content = original.read_bytes() + + rolling = tmp_path / 'rolling.cpp' + rolling.write_bytes(content) + humble = tmp_path / 'humble.cpp' + humble.write_bytes(content) + + _wheel_of(monkeypatch, '0.78.1', '0.72.0') + assert uncrustify.UncrustifyGroup().run(_args(rolling, uncrustify_version='0.78.1'))[0].passed + assert not uncrustify.UncrustifyGroup().run( + _args(humble, uncrustify_version='0.72.0') + )[0].passed + assert rolling.read_bytes() == content diff --git a/uncrustify_wheel/.gitignore b/uncrustify_wheel/.gitignore new file mode 100644 index 0000000..4e7a394 --- /dev/null +++ b/uncrustify_wheel/.gitignore @@ -0,0 +1,4 @@ +build/ +dist/ +*.egg-info/ +__pycache__/ diff --git a/uncrustify_wheel/CMakeLists.txt b/uncrustify_wheel/CMakeLists.txt new file mode 100644 index 0000000..601a8c8 --- /dev/null +++ b/uncrustify_wheel/CMakeLists.txt @@ -0,0 +1,81 @@ +# Copyright 2026 Polymath Robotics, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +cmake_minimum_required(VERSION 3.22) +project(ros_uncrustify_bin LANGUAGES NONE) + +include(ExternalProject) + +# The two versions the ROS 2 distributions pin: 0.78.1 is what rolling and jazzy build through +# uncrustify_vendor, 0.72.0 is what Ubuntu 22.04 and humble ship. +set(UNCRUSTIFY_VERSIONS "0.78.1" "0.72.0") +set(UNCRUSTIFY_SHA256_0.78.1 + "ecaf4c0adca14c36dfffa30bc28e69865115ecd602c90eb16a8cddccb41caad2") +set(UNCRUSTIFY_SHA256_0.72.0 + "d6fff70bc7823fac4c77013055333b79a4839909094e8eee8a14ee8f1777374e") + +# A wheel binary runs on machines that know nothing about the build image, so the C++ runtime +# travels with it. glibc stays dynamic: manylinux2014 gives it a 2.17 floor, which is older +# than any supported Ubuntu, and macOS has no static libc at all. +# -s drops the symbol table, which a consumer has no use for and which is a quarter of the +# size of the binary. +if(UNIX AND NOT APPLE) + set(UNCRUSTIFY_LINK_FLAGS "-static-libstdc++ -static-libgcc -s") +else() + set(UNCRUSTIFY_LINK_FLAGS "") +endif() + +# Python is only a build-time dependency of uncrustify, which runs it to generate sources and +# documentation. scikit-build-core hands us the interpreter it is building for. +if(DEFINED Python3_EXECUTABLE) + set(UNCRUSTIFY_PYTHON "${Python3_EXECUTABLE}") +elseif(DEFINED Python_EXECUTABLE) + set(UNCRUSTIFY_PYTHON "${Python_EXECUTABLE}") +else() + find_package(Python3 REQUIRED COMPONENTS Interpreter) + set(UNCRUSTIFY_PYTHON "${Python3_EXECUTABLE}") +endif() + +foreach(version IN LISTS UNCRUSTIFY_VERSIONS) + set(target "uncrustify_${version}") + externalproject_add(${target} + URL "https://github.com/uncrustify/uncrustify/archive/refs/tags/uncrustify-${version}.tar.gz" + URL_HASH "SHA256=${UNCRUSTIFY_SHA256_${version}}" + DOWNLOAD_NO_PROGRESS TRUE + DOWNLOAD_EXTRACT_TIMESTAMP FALSE + CMAKE_ARGS + -DCMAKE_BUILD_TYPE=Release + -DCMAKE_EXE_LINKER_FLAGS=${UNCRUSTIFY_LINK_FLAGS} + # ExternalProject passes the generator down but not the tool behind it, and the build + # backend's ninja lives in a temporary environment that is gone by the next build. + -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM} + -DPython3_EXECUTABLE=${UNCRUSTIFY_PYTHON} + # 0.72.0 still uses FindPythonInterp, which reads the old spelling. + -DPYTHON_EXECUTABLE=${UNCRUSTIFY_PYTHON} + -DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES} + -DCMAKE_OSX_DEPLOYMENT_TARGET=${CMAKE_OSX_DEPLOYMENT_TARGET} + -DCMAKE_OSX_SYSROOT=${CMAKE_OSX_SYSROOT} + # uncrustify's own test suite runs the binary, which a cross build cannot do. + -DBUILD_TESTING=OFF + BUILD_BYPRODUCTS "/uncrustify" + INSTALL_COMMAND "" + USES_TERMINAL_DOWNLOAD TRUE + USES_TERMINAL_BUILD TRUE + ) + externalproject_get_property(${target} BINARY_DIR) + # PROGRAMS rather than FILES so the executable bit survives into the wheel. + install(PROGRAMS "${BINARY_DIR}/uncrustify" + DESTINATION "ros_uncrustify_bin/bin" + RENAME "uncrustify-${version}") +endforeach() diff --git a/uncrustify_wheel/README.md b/uncrustify_wheel/README.md new file mode 100644 index 0000000..6494578 --- /dev/null +++ b/uncrustify_wheel/README.md @@ -0,0 +1,89 @@ +# ros-uncrustify-bin + +Prebuilt [uncrustify](https://github.com/uncrustify/uncrustify) binaries, packaged as a Python +wheel so the `ros-uncrustify` hook of +[ros_code_standard](https://github.com/polymathrobotics/ros_code_standard) gets the exact version +a ROS 2 distribution pins instead of whatever the operating system happens to ship. + +uncrustify has no PyPI package and no upstream Linux release binary, and the versions differ in +what they accept: Ubuntu 22.04 ships 0.72.0, while rolling and jazzy build 0.78.1 through +`uncrustify_vendor`, and the two reformat the same ROS 2 sources differently. +Each wheel therefore carries both versions. + +## What is in the wheel + +``` +ros_uncrustify_bin/__init__.py +ros_uncrustify_bin/bin/uncrustify-0.78.1 +ros_uncrustify_bin/bin/uncrustify-0.72.0 +``` + +The binaries are built from the pinned upstream source tarballs with their sha256 checked, in +`Release` mode, with the C++ runtime linked statically on Linux so the binary does not need the +build image's libstdc++. + +## Python API + +```python +import ros_uncrustify_bin + +ros_uncrustify_bin.VERSIONS # ('0.78.1', '0.72.0') +ros_uncrustify_bin.DEFAULT_VERSION # '0.78.1' +ros_uncrustify_bin.binary() # Path to the 0.78.1 executable +ros_uncrustify_bin.binary('0.72.0') # Path to the 0.72.0 executable +``` + +`binary()` raises `ValueError` for a version this distribution does not build and +`FileNotFoundError` when the installed wheel does not carry the requested binary. + +## Platforms + +One wheel per platform, each tagged `py3-none-` because nothing here touches the Python +C API: + +| Platform | Wheel tag | +|---|---| +| Linux x86\_64 | `py3-none-manylinux2014_x86_64.manylinux_2_17_x86_64` | +| Linux aarch64 | `py3-none-manylinux2014_aarch64.manylinux_2_17_aarch64` | +| macOS x86\_64 | `py3-none-macosx_11_0_x86_64` | +| macOS arm64 | `py3-none-macosx_11_0_arm64` | + +The manylinux2014 base gives a glibc 2.17 floor, which covers Ubuntu 22.04 and 24.04 and +everything newer. +Windows and musl are out of scope. + +## Building locally + +From this directory, for the platform you are on: + +```sh +uv build --wheel +``` + +CMake downloads and builds both uncrustify versions, so the first build takes a few minutes. +To reproduce what CI produces, including the manylinux container and `auditwheel`: + +```sh +uvx cibuildwheel --platform linux --archs x86_64 . +``` + +All cibuildwheel configuration lives in `pyproject.toml`, so the GitHub workflow only chooses the +runner and the architecture. + +## Bumping an uncrustify version + +1. Edit the version and its `UNCRUSTIFY_SHA256_` in `CMakeLists.txt`, taking the checksum + from `https://github.com/uncrustify/uncrustify/archive/refs/tags/uncrustify-.tar.gz`. +2. Update `VERSIONS` and, if it moved, `DEFAULT_VERSION` in `src/ros_uncrustify_bin/__init__.py`, + and the version asserted by `test-command` in `pyproject.toml`. +3. Raise `version` in `pyproject.toml` and `__version__` in `src/ros_uncrustify_bin/__init__.py`. +4. Make sure `ros_code_standard/checkers/uncrustify/` has an ament config for the new version and + that the hook's `--uncrustify-version` choices list it. + +## Releases + +Pushing these changes to `main` runs the wheel workflow, which builds every platform and publishes +the wheels and a `SHA256SUMS` file to the GitHub release `uncrustify-bin-v` of the +ros_code_standard repository. +The hook depends on those wheel URLs directly, with platform markers, so consumers never build +uncrustify themselves. diff --git a/uncrustify_wheel/pyproject.toml b/uncrustify_wheel/pyproject.toml new file mode 100644 index 0000000..8a8faea --- /dev/null +++ b/uncrustify_wheel/pyproject.toml @@ -0,0 +1,60 @@ +[build-system] +requires = ["scikit-build-core>=0.10"] +build-backend = "scikit_build_core.build" + +[project] +name = "ros-uncrustify-bin" +version = "0.1.0" +description = "Prebuilt uncrustify binaries for the ros_code_standard pre-commit hooks" +readme = "README.md" +requires-python = ">=3.10" +license = "Apache-2.0" +authors = [{ name = "Polymath Robotics, Inc." }] +keywords = ["uncrustify", "ros", "ament", "pre-commit"] +classifiers = [ + "Development Status :: 4 - Beta", + "Intended Audience :: Developers", + "Operating System :: MacOS", + "Operating System :: POSIX :: Linux", + "Programming Language :: Python :: 3", + "Topic :: Software Development :: Quality Assurance", +] +dependencies = [] + +[tool.scikit-build] +minimum-version = "build-system.requires" +# No build-dir on purpose: a CMake cache holds absolute paths, and cibuildwheel copies the +# project into a container under a different prefix, where a leftover cache aborts the build. + +# The binaries are architecture specific but call no Python C API, so one py3 wheel per +# platform covers every interpreter. +wheel.py-api = "py3" +wheel.platlib = true +wheel.packages = ["src/ros_uncrustify_bin"] +# uncrustify 0.72.0 declares cmake_minimum_required(VERSION 3.2) and uses FindPythonInterp, +# neither of which CMake 4 accepts, so the build pins the 3.x series. +cmake.version = ">=3.22,<4" +sdist.include = ["CMakeLists.txt"] + +[tool.cibuildwheel] +# The wheel holds no Python extension, so a single interpreter produces the py3 wheel that +# every supported interpreter installs. +build = "cp312-*" +# cp312-* already excludes PyPy and every other implementation; musl and Windows are the +# two that still need saying no to. +skip = ["*-musllinux_*", "*-win*"] +build-frontend = "build" +# glibc 2.17, so the wheels run on Ubuntu 22.04 and 24.04 and everything newer. +manylinux-x86_64-image = "manylinux2014" +manylinux-aarch64-image = "manylinux2014" +test-requires = ["pytest"] +# {package} is the directory passed to cibuildwheel, so this works from any working directory. +test-command = [ + "pytest {package}/tests", + "python -c \"import ros_uncrustify_bin as u, subprocess as s; out = s.run([str(u.binary('0.78.1')), '--version'], capture_output=True, text=True).stdout; print(out); assert out.startswith('Uncrustify-0.78.1'), out\"", + "python -c \"import ros_uncrustify_bin as u, subprocess as s; out = s.run([str(u.binary('0.72.0')), '--version'], capture_output=True, text=True).stdout; print(out); assert out.startswith('Uncrustify-0.72.0'), out\"", +] + +[tool.cibuildwheel.macos] +# The oldest deployment target the GitHub macOS runners' toolchain still supports. +environment = { MACOSX_DEPLOYMENT_TARGET = "11.0" } diff --git a/uncrustify_wheel/src/ros_uncrustify_bin/__init__.py b/uncrustify_wheel/src/ros_uncrustify_bin/__init__.py new file mode 100644 index 0000000..9d1bbc8 --- /dev/null +++ b/uncrustify_wheel/src/ros_uncrustify_bin/__init__.py @@ -0,0 +1,54 @@ +# Copyright 2026 Polymath Robotics, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Prebuilt uncrustify binaries for the ros_code_standard hooks. + +uncrustify has no PyPI package and no upstream Linux release binary, so this distribution +carries binaries built from the pinned upstream sources, one per version that a supported ROS 2 +distribution pins. +The wheels are platform specific and the binaries live in ``ros_uncrustify_bin/bin``. +""" + +import importlib.resources +from pathlib import Path + +__version__ = '0.1.0' + +#: Every uncrustify version this distribution builds, newest first. +VERSIONS: tuple[str, ...] = ('0.78.1', '0.72.0') + +#: The version ROS 2 rolling and jazzy build through uncrustify_vendor. +DEFAULT_VERSION: str = '0.78.1' + +_BIN_DIR = importlib.resources.files(__package__) / 'bin' + + +def binary(version: str = DEFAULT_VERSION) -> Path: + """ + Return the path to the bundled uncrustify executable for a version. + + Raises ValueError for a version this distribution does not build, and FileNotFoundError + when the installed wheel does not carry it, which means the wheel was built for another + platform or repaired by hand. + """ + if version not in VERSIONS: + known = ', '.join(VERSIONS) + raise ValueError(f"Unknown uncrustify version '{version}', expected one of: {known}") + path = Path(str(_BIN_DIR / f'uncrustify-{version}')) + if not path.is_file(): + raise FileNotFoundError( + f"The installed ros-uncrustify-bin wheel has no uncrustify {version} at '{path}'" + ) + return path diff --git a/uncrustify_wheel/tests/test_ros_uncrustify_bin.py b/uncrustify_wheel/tests/test_ros_uncrustify_bin.py new file mode 100644 index 0000000..5d9a0da --- /dev/null +++ b/uncrustify_wheel/tests/test_ros_uncrustify_bin.py @@ -0,0 +1,64 @@ +# Copyright 2026 Polymath Robotics, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Tests for the ros_uncrustify_bin wheel. + +They exercise an installed wheel, not a source tree, so a run from the ros_code_standard +repository, where the wheel is not a dependency, skips them. +""" + +import os +import subprocess + +import pytest + +ros_uncrustify_bin = pytest.importorskip( + 'ros_uncrustify_bin', + reason='install a ros-uncrustify-bin wheel to run these tests', +) + + +def report_version(version: str) -> str: + """Return the first line that `uncrustify --version` prints for a bundled binary.""" + binary = ros_uncrustify_bin.binary(version) + assert binary.is_file(), binary + assert os.access(binary, os.X_OK), f'{binary} is not executable' + proc = subprocess.run([str(binary), '--version'], capture_output=True, text=True, check=True) + return proc.stdout.strip() + + +def test_versions() -> None: + """The advertised versions and the default are the ones the hook selects between.""" + assert ros_uncrustify_bin.VERSIONS == ('0.78.1', '0.72.0') + assert ros_uncrustify_bin.DEFAULT_VERSION == '0.78.1' + assert ros_uncrustify_bin.DEFAULT_VERSION in ros_uncrustify_bin.VERSIONS + + +def test_default_binary_is_the_default_version() -> None: + """binary() with no argument is binary(DEFAULT_VERSION).""" + assert ros_uncrustify_bin.binary() == ros_uncrustify_bin.binary('0.78.1') + + +@pytest.mark.parametrize('version', ros_uncrustify_bin.VERSIONS) +def test_binary_reports_its_version(version: str) -> None: + """Each bundled binary runs and prints the version it was built from.""" + # uncrustify appends a build flavor, as in 'Uncrustify-0.78.1_f'. + assert report_version(version).startswith(f'Uncrustify-{version}') + + +def test_unknown_version() -> None: + """A version this distribution does not build is a ValueError, not a missing file.""" + with pytest.raises(ValueError, match='9.9.9'): + ros_uncrustify_bin.binary('9.9.9')