Migrate from cmake-format to gersemi

[KyleFromNVIDIA]

Description

Issue: rapidsai/build-planning#234
Ops-Bot-Merge-Barrier: true

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.
• The cmake-format.json is up to date with these changes.
• I have added new files under rapids-cmake/
  • I have added include guards (include_guard(GLOBAL))
  • I have added the associated docs/ rst file and update the api.rst

[KyleFromNVIDIA]

This plugin currently relies on undocumented gersemi APIs and is thus tied to a specific gersemi version. We shouldn't rely on downstream rapids-cmake clients using that specific gersemi version. I'll rework this in a follow-up to not have the plugin rely on these APIs.

[jameslamb]

Suggested change

	found_definitions = find_all_custom_command_definitions(paths, pool, warning_sink, True)
	found_definitions = find_all_custom_command_definitions(paths, pool, warning_sink, respect_ignore_files=True)

Sounds fine to me. Until then, could we please make these boolean and int-literal arguments keyword arguments?

The others are fine to leave as positional if you want because their names are informative, but adding respect_ignore_files= here (for example) significantly improves my understanding of what this line of code might be doing.

[jameslamb]

Sorry for the huge review but in my defense it's a big PR 😅

NOTE: I completely ignored changes in rapids-cmake/ and testing/, assuming those are just automatic reformatting from gersemi. I'll defer to @robertmaynard or someone else to comment on the style preferences there.

Overall, I'm supportive of the general idea here, which as I understand it is:

• auto-generate gersemi stubs for all of rapids-cmake, CPM, and rapids-logger (public / non-"detail" APIs only)
• wrap those in a Python package that implements a gersemi extension (https://github.com/dask/distributed/blob/1c130e4e8c451efdec8e5cbdcf2ddd53541724f9/pyproject.toml#L63-L69)
  • RAPIDS-version it (so e.g. gersemi-rapids-cmake==26.04 has stubs for rapids-cmake 26.04)
  • distribute that Python package on the RAPIDS nightly index (and pypi.org, right?)
  • use it in pre-commit hooks across RAPIDS to auto-format CMake code

This gives us a path to managing CMake autoformatting that I think would be a better local development experience than the one we have now with cmake-format, and would eliminate duplication across RAPIDS (rapidsai/pre-commit-hooks#62).

It looks like .gersemirc being respected means projects could choose their own customizations.

My biggest concern is about the cached Python package in pre-commit's cache being out of date with the tip of rapids-cmake. Started a thread about that on the PR... we should get to an agreement on that before this is merged.

[jameslamb]

Suggested change

	select = ["E", "F", "W"]
	select = [
	# pycodestyle Error
	"E",
	# pyflakes
	"F",
	# pycodestyle Warning
	"W"
	]

Let's split these onto 1 per line and annotate, like cudf does: https://github.com/rapidsai/cudf/blob/aa0163acce098287184b19c0d9305583607c9aeb/pyproject.toml#L46

I've found that helpful as these lists grow, especially for ruff rules where people have some familiarity with the upstream project (like pyflakes / flake8).

[jameslamb]

Suggested change

	- setuptools
	- setuptools>=77.7.0

Let's keep setuptools on the same minimum as everything else in RAPIDS (ref: rapidsai/build-planning#152)

[jameslamb]

Suggested change

	rapids-logger "Generating build requirements"
	rapids-dependency-file-generator \
	--output requirements \
	--file-key "py_build_gersemi_rapids_cmake" \
	--matrix "" \
	| tee /tmp/requirements-build.txt
	rapids-logger "Installing build requirements"
	rapids-pip-retry install \
	-v \
	--prefer-binary \
	-r /tmp/requirements-build.txt

Since the wheel is getting build with build isolation (where a dedicated virtualenv is set up using the build dependencies declares in pyproject.toml), I think all of this is unnecessary and should be removed.

[jameslamb]

The convention across the rest of RAPIDS is to put this stuff in a file ci/validate_wheel.sh.

For example: https://github.com/rapidsai/nx-cugraph/blob/2530c42e24d9b92190349e61a2941ebab225ac85/ci/build_wheel_nx-cugraph.sh#L10

Could we do that here, for consistency? I know the indirection seems like it might not be worth it, but I think it is to make all-of-RAPIDS searches and automated updates easier.

[jameslamb]

Given that this technically covers more than just rapids-cmake, would you consider changing the project name and wheel distribution name to rapids-gersemi-plugin or something?

That'd match all the other rapids-* tools (rapids-build-backend, rapids-dependency-file-generator, rapids-metadata, etc.).

I looked on PyPI and don't see any gersemi plugins published there beginning with gersemi-* (https://pypi.org/search/?q=gersemi), so it doesn't seem to me like the gersemi- prefix is helping us with intent that way that pytest plugins often are named pytest-*

[jameslamb]

Ah, I see at https://github.com/BlankSpruce/gersemi?tab=readme-ov-file#extensions that the module name has to follow the naming pattern gersemi_{import_name}.

Welp ok if that's the restriction, then I understand the module names and why we'd want the distribution name to match (gersemi-rapids-cmake).

I still think I'd prefer the distribution name rapids-gersemi-plugin even if the import name has to be gersemi_{import_name} (they are not required to be the same), but I don't feel too strongly. Do whatever you want with this suggestion.

[KyleFromNVIDIA]

We could call it gersemi-rapidsai or something like that

[jameslamb]

Suggested change

	- id: ruff-format
	- id: ruff-format
	args: [--config, "pyproject.toml"]

[jameslamb]

Suggested change

	needs: telemetry-setup
	needs: checks

This suggestion isn't only about consistency... it could also prevent CI being blocked. Note that telemetry-setup here has continue-on-error: true, so that it could fail and not block CI.

I think it being in a needed: entry would mean it being skipped would cause wheel-build-gersemi-rapids-cmake to be skipped, which would fail pr-builder, which would block CI.

telemetry-setup won't be required here if you accept the suggestion to remove rapids-telemetry-record from the wheel-building script. Let's rely on checks as most other RAPIDS wheel-build-* workflows do.

[jameslamb]

This warning should be addressed:

  /tmp/pip-build-env-4nmewhtb/overlay/lib/python3.10/site-packages/setuptools/command/build_py.py:212: _Warning: Package 'gersemi_rapids_cmake_detail.stubs.rapids' is absent from the `packages` configuration.
  !!

          ********************************************************************************
          ############################
          # Package would be ignored #
          ############################
          Python recognizes 'gersemi_rapids_cmake_detail.stubs.rapids' as an importable package[^1],
          but it is absent from setuptools' `packages` configuration.

          This leads to an ambiguous overall configuration. If you want to distribute this
          package, please make sure that 'gersemi_rapids_cmake_detail.stubs.rapids' is explicitly added
          to the `packages` configuration field.

          Alternatively, you can also rely on setuptools' discovery methods
          (for example by using `find_namespace_packages(...)`/`find_namespace:`
          instead of `find_packages(...)`/`find:`).

          You can read more about "package discovery" on setuptools documentation page:

          - https://setuptools.pypa.io/en/latest/userguide/package_discovery.html

          If you don't want 'gersemi_rapids_cmake_detail.stubs.rapids' to be distributed and are
          already explicitly excluding 'gersemi_rapids_cmake_detail.stubs.rapids' via
          `find_namespace_packages(...)/find_namespace` or `find_packages(...)/find`,
          you can try to use `exclude_package_data`, or `include-package-data=False` in
          combination with a more fine grained `package-data` configuration.

          You can read more about "package data files" on setuptools documentation page:

          - https://setuptools.pypa.io/en/latest/userguide/datafiles.html


          [^1]: For Python, any directory (with suitable naming) can be imported,
                even if it does not contain any `.py` files.
                On the other hand, currently there is no concept of package data
                directory, all directories are treated like packages.
          ********************************************************************************

  !!

(build link)

In my experience it's often a bit difficult to get this working in setuptools, but these will help you:

• docs on configuration: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
• example of bundling non-Python files: https://github.com/dask/distributed/blob/1c130e4e8c451efdec8e5cbdcf2ddd53541724f9/pyproject.toml#L63-L69

[jameslamb]

Suggested change

	- gersemi-rapids-cmake
	# should match rapids-cmake version
	- gersemi-rapids-cmake==26.04

This needs to exactly match the rapids-cmake version (and general RAPIDS version), right? At a minimum we should put a comment like that here.

But beyond that... do you have a plan for keeping that in sync? Will it just have to be another thing we add to update-version.sh scripts everywhere?

[jameslamb]

I re-read @vyasr 's comments at rapidsai/pre-commit-hooks#62 (comment) today and it reminded me... we should have a plan to deal with the following scenario:

1. you've run pre-commit run on your clone of cuDF early in 26.04 development
2. some API-breaking changes are merged in rapids-cmake 26.04
3. you run pre-commit run on your clone of cuDF again
4. gersemi checks break, because pre-commit matches the cached gersemi-rapids-cmake==26.04.00, which is inconsistent with the tip of rapids-cmake

I'd consider this blocking only because the answer might be "don't use pre-commit and a Python package".

[KyleFromNVIDIA]

Crap. We might have to take the same approach we took with rapids-metadata... and I've heard of people and/or software packages (don't remember which) getting grumpy about verify-alpha-spec making internet requests.

[KyleFromNVIDIA]

One possibility is, in cases where a breaking change was made to rapids-cmake, we update the gersemi-rapids-cmake dependency to >=26.04.00a123 across the board... but I don't like that solution either.

[jameslamb]

We might have to take the same approach we took with rapids-metadata... and I've heard of people and/or software packages (don't remember which) getting grumpy about verify-alpha-spec making internet requests.

Yeah I'd love to avoid introducing more network requests if we could. Especially unauthenticated requests to anything owned by GitHub, which face lower rate limits than authenticated requests.

On the other hand, the solution we have for formatting CMake code today already relies on fetching something from GitHub at runtime

FORMAT_FILE_URL="https://raw.githubusercontent.com/rapidsai/rapids-cmake/${RAPIDS_BRANCH}/cmake-format-rapids-cmake.json"
export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-format-rapids-cmake.json
mkdir -p "$(dirname "${RAPIDS_CMAKE_FORMAT_FILE}")"
wget -O ${RAPIDS_CMAKE_FORMAT_FILE} "${FORMAT_FILE_URL}"

(rapidsai/cudf - ci/check_style.sh)

Though that's a single small-ish file, probably smaller than a tarball of all the stubs here would be. And that being in check_style.sh means it's not executed by pre-commit run locally, which leads to that annoying problem of "pre-commit checks pass locally but fail in CI because there are cmake-format errors".

That download might be causing problems too if it was run every time anyone did pre-commit run locally.

in cases where a breaking change was made to rapids-cmake, we update the gersemi-rapids-cmake dependency to >=26.04.00a123 across the board... but I don't like that solution either.

I agree, this would solve the problem but also introduces some new process and maintenance burden that may not be worth it.

---

I can think of some other options, none are that great but maybe they'll spark some ideas.

Option 1: system hook to update pre-commit's cache

I have a weird idea... could we hack in another system hook running before this one that updates pre-commit's cache? It could query the available versions like this (using cudf-cu13 just to have a working example, but this would be for gersemi-rapids-cmake):

pip index versions \
    --index-url https://pypi.anaconda.org/rapidsai-wheels-nightly/simple \
    --pre \
    --json \
    cudf-cu13 \
| jq -r '."latest"'
# 26.4.0a37

And compare that to whatever's installed in pre-commit's cache (I don't know how to do that, but maybe it's possible).

And then try to update the cache (just for gersemi-rapids-cmake) if there's a newer published gersemi-rapids-cmake available?

I don't know how complex it would be to get that script right and to have it work correctly with different RAPIDS versions (e.g. on release/26.02 during burndown, it should get the latest 26.02 version not the absolute latest which would be 26.04.*), the default_language_version in .pre-commit-config.yaml, and everything else that goes into populating that virtualenv.

And this is definitely way out in "unsupported" territory so I don't know how much risk that introduces of this being broken by future changes in pre-commit.

But maybe that'd give us a path to what we want without needing new PRs to all repos?

Option 2: manage the venv outside of pre-commit

Just make this a system hook instead of a python hook and always re-update gersemi-rapids-cmake when it's run.

With a script like this:

#!/bin/bash

VERSION=$(cat ./VERSION)
venv_dir=$(mktemp -d)
python -m virtualenv ${venv_dir}
source ${venv_dir}/bin/activate
python -m pip install \
   --extra-index-url https://pypi.anaconda.org/rapidsai-wheels-nightly/simple/ \
   "gersemi-rapids-cmake>=${VERSION}a0"

gersemi -i, --extensions, rapids_cmake, -- '${@}"

- id: gersemi-format-cmake
  name: gersemi-format-cmake
  entry: ./ci/checks/run-gersemi.sh
  args: [-i, --extensions, rapids_cmake, --]
  files: '(\.cmake$|CMakeLists\.txt$)'
  types_or: [file]
  language: system
  pass_filenames: true

That would still do a bunch of network requests to install Python packages, but those should get cached in our package proxy so shouldn't be a problem in CI. And locally, pip install-ing a few things from pypi.org / the RAPIDS nightly index on every pre-commit run shouldn't be a huge problem.

That script is a naive implementation just to show the idea. It could be updated with things to make it less expensive locally (like checking if the venv already exists and checking if an update of gersemi-rapids-cmake is actually needed).

Would still require a script to be checked into every repo but that's already the case with cmake-format so I think that's acceptable if it gets us the behavior we want.

[jameslamb]

I need to go spend time on other things for a bit, hopefully these bad ideas inspire better ones haha

[KyleFromNVIDIA]

I have another idea. I propose:

Option 3: Add a cache: false option to pre-commit

This is the second time that pre-commit's caching has gotten in our way. I propose adding a cache option to .pre-commit-config.yaml to disable caching the environment for a hook.

I've opened pre-commit/pre-commit#3611 to propose this design change.

[bdice]

My view is that we should start with the simplest possible solution (GitHub network requests) and step back into more complexity from there. Every build of our software already hammers GitHub for rapids-cmake's fetching. I know this can and does bite us from time to time, but it'd be worth trying as a starting point.

If that fails in production, then I would recommend option 2 (custom script / custom management of the gersemi rapids-cmake extension package). I do not like that options 1 and 3 require some hacking around the behavior of pre-commit itself.

[KyleFromNVIDIA]

pre-commit/pre-commit#3611 was a washout.

My other thinking is to do something similar to what the existing scripts are already doing: search locally to see if rapids-cmake is already in a build tree somewhere and use that. I'd be open to specifics on how to carry this out.

[vyasr]

I assume resolving the caching issue is why this work stalled?

[KyleFromNVIDIA]

Yes