From 8f1950bc5955f072950c52781ec2496850a317c5 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Wed, 15 May 2024 18:31:57 -0500 Subject: [PATCH] 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 --- .github/workflows/build_and_test.yaml | 18 +++++++++ .pre-commit-config.yaml | 24 ++++++++++++ README.md | 2 +- python/libucx/libucx/__init__.py | 10 ++--- python/libucx/libucx/load.py | 3 +- python/libucx/setup.py | 55 ++++++++++++++++++--------- 6 files changed, 85 insertions(+), 27 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.github/workflows/build_and_test.yaml b/.github/workflows/build_and_test.yaml index 9db1a26..9097074 100644 --- 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 new file mode 100644 index 0000000..476bc6d --- /dev/null +++ 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 index 1c356ad..a17f8a1 100644 --- 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 index 58c0cea..35b2557 100644 --- 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 index 7264544..86dd287 100644 --- 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 index f59a566..7888842 100644 --- 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.