add pre-commit checks (#8)

Proposes adding some minimal `pre-commit` checks and a CI job to run them. I think this is a cheap, low-risk way to get a bit more release confidence in changes like #5. ## Notes for Reviewers This includes a very pared-down version of https://github.com/rapidsai/shared-workflows/blob/branch-24.06/.github/workflows/checks.yaml. I'm proposing not depending on `shared-workflows` because this repo doesn't follow the RAPIDS branching model, and because right now the need is so simply (just running `pre-commit run --all-files`). --------- Co-authored-by: Kyle Edwards <[email protected]>
rapidsai · May 15, 2024 · 8f1950b · 8f1950b
1 parent b24d308
commit 8f1950b
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 27 deletions.
diff --git a/.github/workflows/build_and_test.yaml b/.github/workflows/build_and_test.yaml
@@ -27,7 +27,25 @@ permissions:
   statuses: none
 
 jobs:
+  checks:
+    runs-on: ubuntu-latest
+    container:
+      image: rapidsai/ci-conda:latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+      - uses: actions/cache@v4
+        with:
+          path: ~/.cache/pre-commit
+          key: pre-commit-0|${{ hashFiles('.pre-commit-config.yaml') }}
+      - name: Run pre-commit
+        run: |
+          conda install --yes pre-commit
+          pre-commit run --all-files --show-diff-on-failure
   compute-matrices:
+    needs: checks
     runs-on: ubuntu-latest
     outputs:
       BUILD_MATRIX: ${{ steps.compute-matrix.outputs.BUILD_MATRIX }}

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,24 @@
+# Copyright (c) 2024, NVIDIA CORPORATION.
+
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.5.0
+    hooks:
+      - id: trailing-whitespace
+      - id: end-of-file-fixer
+  - repo: https://github.com/PyCQA/isort
+    rev: 5.13.2
+    hooks:
+      - id: isort
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.4.4
+    hooks:
+      - id: ruff
+      - id: ruff-format
+  - repo: https://github.com/rapidsai/pre-commit-hooks
+    rev: v0.0.3
+    hooks:
+      - id: verify-copyright
+
+default_language_version:
+  python: python3
diff --git a/README.md b/README.md
@@ -2,7 +2,7 @@
 
 This repository hosts code for building wheels of [UCX](https://github.com/openucx/ucx/).
 
-## Purpose 
+## Purpose
 RAPIDS publishes multiple libraries that rely on UCX, including [ucxx](https://github.com/rapidsai/ucxx/) and [ucx-py](https://github.com/rapidsai/ucx-py).
 One of the ways that RAPIDS vendors these libraries is in the form of [pip wheels](https://packaging.python.org/en/latest/specifications/binary-distribution-format/).
 For portability, wheels should be as self-contained as possible as per the [manylinux standard](https://peps.python.org/pep-0513/).

diff --git a/python/libucx/libucx/__init__.py b/python/libucx/libucx/__init__.py
@@ -17,10 +17,10 @@
 
 from .load import load_library
 
-
 __version__ = (
-    importlib.resources.files("libucx")
-    .joinpath("VERSION")
-    .read_text()
-    .strip()
+    importlib.resources.files("libucx").joinpath("VERSION").read_text().strip()
 )
+
+__all__ = [
+    "load_library",
+]
diff --git a/python/libucx/libucx/load.py b/python/libucx/libucx/load.py
@@ -16,7 +16,6 @@
 import ctypes
 import os
 
-
 # IMPORTANT: The load order here matters! libucm.so depends on symbols in libucs.so, but
 # it does not express this via a DT_NEEDED entry, presumably because libucs.so also has
 # a dependency on libucm.so and the libraries are attempting to avoid a circular
@@ -33,8 +32,8 @@
     "libucp.so",
 ]
 
-def load_library():
 
+def load_library():
     # Dynamically load libucx.so. Prefer a system library if one is present to
     # avoid clobbering symbols that other packages might expect, but if no
     # other library is present use the one in the wheel.

diff --git a/python/libucx/setup.py b/python/libucx/setup.py
@@ -1,11 +1,12 @@
-from setuptools import setup
-from setuptools.command.build_py import build_py as build_orig
-import subprocess
-from contextlib import contextmanager
+import glob
 import os
+import subprocess
 import tempfile
-import glob
+from contextlib import contextmanager
+
 import packaging.version
+from setuptools import setup
+from setuptools.command.build_py import build_py as build_orig
 
 
 @contextmanager
@@ -24,7 +25,7 @@ def run(self):
 
         with open("VERSION") as f:
             package_version = f.read().strip()
-        
+
         # strip off any other non-UCX version components, like ".post1"
         ucx_semver = packaging.version.parse(package_version).base_version
         ucx_tag = f"v{ucx_semver}"
@@ -33,20 +34,36 @@ def run(self):
 
         with tempfile.TemporaryDirectory() as tmpdir:
             with chdir(tmpdir):
-                subprocess.run(["git", "clone", "-b", f"{ucx_tag}", "https://github.com/openucx/ucx.git", "ucx"])
+                subprocess.run(
+                    [
+                        "git",
+                        "clone",
+                        "-b",
+                        f"{ucx_tag}",
+                        "https://github.com/openucx/ucx.git",
+                        "ucx",
+                    ]
+                )
                 with chdir("ucx"):
                     subprocess.run(["./autogen.sh"])
-                    subprocess.run(["./contrib/configure-release",
-                                    f"--prefix={install_prefix}",
-                                    "--enable-mt",
-                                    "--enable-cma",
-                                    "--enable-numa",
-                                    "--with-gnu-ld",
-                                    "--with-sysroot",
-                                    "--without-verbs",
-                                    "--without-rdmacm",
-                                    "--with-cuda=/usr/local/cuda"])
-                    subprocess.run(["make", "-j"], env={**os.environ, "CPPFLAGS": "-I/usr/local/cuda/include"})
+                    subprocess.run(
+                        [
+                            "./contrib/configure-release",
+                            f"--prefix={install_prefix}",
+                            "--enable-mt",
+                            "--enable-cma",
+                            "--enable-numa",
+                            "--with-gnu-ld",
+                            "--with-sysroot",
+                            "--without-verbs",
+                            "--without-rdmacm",
+                            "--with-cuda=/usr/local/cuda",
+                        ]
+                    )
+                    subprocess.run(
+                        ["make", "-j"],
+                        env={**os.environ, "CPPFLAGS": "-I/usr/local/cuda/include"},
+                    )
                     subprocess.run(["make", "install"])
                     # The config file built into UCX is not relocatable. We need to fix
                     # that so that we can package up UCX and distribute it in a wheel.
@@ -55,7 +72,7 @@ def run(self):
                             "sed",
                             "-i",
                             r"s/^set(prefix.*/set(prefix \"${CMAKE_CURRENT_LIST_DIR}\/..\/..\/..\")/",
-                            f"{install_prefix}/lib/cmake/ucx/ucx-targets.cmake"
+                            f"{install_prefix}/lib/cmake/ucx/ucx-targets.cmake",
                         ]
                     )
                     # The UCX libraries must be able to find each other as dependencies.