From dccdf0d914056a738d618700473a765d7f2dc036 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Wed, 15 May 2024 14:20:56 -0400 Subject: [PATCH] Automatically figure out where to write GIT_COMMIT (#30) In most cases, we will want to put GIT_COMMIT in a directory with the same name as the project (with hyphens replaced by underscores.) Put GIT_COMMIT there by default. Also rename _edit_git_commit() to _write_git_commit(), and don't create a backup file with the old GIT_COMMIT. --- README.md | 16 ++++----- rapids_build_backend/config.py | 24 +++++++------ rapids_build_backend/impls.py | 47 ++++++++++++------------- tests/test_config.py | 5 +-- tests/test_impls.py | 64 ++++++++++++++++++++-------------- tests/test_packages.py | 4 +-- 6 files changed, 86 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index 1b9bdbe..54f3af9 100644 --- a/README.md +++ b/README.md @@ -15,14 +15,14 @@ In cases where more dynamic customization is sensible, suitable environment vari Any option without a default is required. -| Option | Definition | Type | Default | Supports dynamic modification | -|-----------------------|--------------------------------------------------------------------------------------------------|----------------|---------------------|-------------------------------| -| `build-backend` | The wrapped build backend (e.g. `setuptools.build_meta`) | string | | N | -| `commit-file` | The file in which to write the git commit hash | string | "" (No file) | N | -| `dependencies-file` | The path to the `dependencies.yaml` file to use | string | "dependencies.yaml" | Y | -| `disable-cuda` | If true, CUDA version in build environment is ignored when setting package name and dependencies | bool | false | Y | -| `matrix-entry` | A `;`-separated list of `=`-delimited key/value pairs | string | "" | Y | -| `requires` | List of build requirements (in addition to `build-system.requires`) | list[str] | [] | N | +| Option | Definition | Type | Default | Supports dynamic modification | +|-----------------------|--------------------------------------------------------------------------------------------------|----------------|-------------------------------|-------------------------------| +| `build-backend` | The wrapped build backend (e.g. `setuptools.build_meta`) | string | | N | +| `commit-files` | List of files in which to write the git commit hash | list[str] | ["/GIT_COMMIT"] | N | +| `dependencies-file` | The path to the `dependencies.yaml` file to use | string | "dependencies.yaml" | Y | +| `disable-cuda` | If true, CUDA version in build environment is ignored when setting package name and dependencies | bool | false | Y | +| `matrix-entry` | A `;`-separated list of `=`-delimited key/value pairs | string | "" | Y | +| `requires` | List of build requirements (in addition to `build-system.requires`) | list[str] | [] | N | ## Outstanding questions diff --git a/rapids_build_backend/config.py b/rapids_build_backend/config.py index 14dee20..5f76968 100644 --- a/rapids_build_backend/config.py +++ b/rapids_build_backend/config.py @@ -16,23 +16,23 @@ config_val_callable = Callable[[], Union[config_val_type, mutable_config_val_type]] config_options_type = dict[ - str, tuple[Union[config_val_type, config_val_callable], bool] + str, tuple[Union[config_val_type, config_val_callable], bool, bool] ] class Config: """Manage the build configuration for the project.""" - # Mapping from config option to default value (None indicates that option is - # required) and whether it may be overridden by an environment variable or a config + # Mapping from config option to default value, whether it's required, and + # whether it may be overridden by an environment variable or a config # setting. config_options: "config_options_type" = { - "build-backend": (None, False), - "commit-file": ("", False), - "dependencies-file": ("dependencies.yaml", True), - "disable-cuda": (False, True), - "matrix-entry": ("", True), - "requires": (lambda: [], False), + "build-backend": (None, True, False), + "commit-files": (None, False, False), + "dependencies-file": ("dependencies.yaml", False, True), + "disable-cuda": (False, False, True), + "matrix-entry": ("", False, True), + "requires": (lambda: [], False, False), } def __init__(self, dirname=".", config_settings=None): @@ -46,7 +46,9 @@ def __init__(self, dirname=".", config_settings=None): def __getattr__(self, name): config_name = name.replace("_", "-") if config_name in Config.config_options: - default_value, allows_override = Config.config_options[config_name] + default_value, required, allows_override = Config.config_options[ + config_name + ] if callable(default_value): default_value = default_value() @@ -74,7 +76,7 @@ def __getattr__(self, name): try: return self.config[config_name] except KeyError: - if default_value is not None: + if not required: return default_value raise AttributeError(f"Config is missing required attribute '{name}'") diff --git a/rapids_build_backend/impls.py b/rapids_build_backend/impls.py index 18a100c..ae60e40 100644 --- a/rapids_build_backend/impls.py +++ b/rapids_build_backend/impls.py @@ -100,36 +100,27 @@ def _get_git_commit() -> Union[str, None]: @contextmanager -def _edit_git_commit(config): +def _write_git_commits(config, project_name: str): """ - Temporarily modify the git commit of the package being built. + Temporarily write the git commit files for the package being built. If the + `commit-files` config option is not specified, write to `/GIT_COMMIT`. This is useful for projects that want to embed the current git commit in the package at build time. """ - commit_file = config.commit_file - commit = _get_git_commit() - - if commit_file != "" and commit is not None: - bkp_commit_file = os.path.join( - os.path.dirname(commit_file), - f".{os.path.basename(commit_file)}.rapids-build-backend.bak", - ) - try: - try: - shutil.move(commit_file, bkp_commit_file) - except FileNotFoundError: - bkp_commit_file = None + commit_files = config.commit_files + if commit_files is None: + commit_files = [os.path.join(project_name.replace("-", "_"), "GIT_COMMIT")] + commit = _get_git_commit() if commit_files else None + if commit is not None: + for commit_file in commit_files: with open(commit_file, "w") as f: f.write(f"{commit}\n") - + try: yield finally: - # Restore by moving rather than writing to avoid any formatting changes. - if bkp_commit_file: - shutil.move(bkp_commit_file, commit_file) - else: + for commit_file in commit_files: os.unlink(commit_file) else: yield @@ -211,7 +202,9 @@ def _edit_pyproject(config): # (the actual build backend, which conditionally imports these functions). def get_requires_for_build_wheel(config_settings): config = Config(config_settings=config_settings) - with _edit_pyproject(config), _edit_git_commit(config): + pyproject = utils._get_pyproject() + project_name = pyproject["project"]["name"] + with _edit_pyproject(config), _write_git_commits(config, project_name): # Reload the config for a possibly updated tool.rapids-build-backend.requires reloaded_config = Config(config_settings=config_settings) requires = list(reloaded_config.requires) @@ -227,7 +220,9 @@ def get_requires_for_build_wheel(config_settings): def get_requires_for_build_sdist(config_settings): config = Config(config_settings=config_settings) - with _edit_pyproject(config), _edit_git_commit(config): + pyproject = utils._get_pyproject() + project_name = pyproject["project"]["name"] + with _edit_pyproject(config), _write_git_commits(config, project_name): # Reload the config for a possibly updated tool.rapids-build-backend.requires reloaded_config = Config(config_settings=config_settings) requires = list(reloaded_config.requires) @@ -259,7 +254,9 @@ def get_requires_for_build_editable(config_settings): def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): config = Config(config_settings=config_settings) - with _edit_pyproject(config), _edit_git_commit(config): + pyproject = utils._get_pyproject() + project_name = pyproject["project"]["name"] + with _edit_pyproject(config), _write_git_commits(config, project_name): return _get_backend(config.build_backend).build_wheel( wheel_directory, config_settings, metadata_directory ) @@ -267,7 +264,9 @@ def build_wheel(wheel_directory, config_settings=None, metadata_directory=None): def build_sdist(sdist_directory, config_settings=None): config = Config(config_settings=config_settings) - with _edit_pyproject(config), _edit_git_commit(config): + pyproject = utils._get_pyproject() + project_name = pyproject["project"]["name"] + with _edit_pyproject(config), _write_git_commits(config, project_name): return _get_backend(config.build_backend).build_sdist( sdist_directory, config_settings ) diff --git a/tests/test_config.py b/tests/test_config.py index 3dae383..34c59bf 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -25,8 +25,9 @@ def setup_config_project(tmp_path, flag, config_value): @pytest.mark.parametrize( "flag, config_value, expected", [ - ("commit-file", '"pkg/_version.py"', "pkg/_version.py"), - ("commit-file", None, ""), + ("commit-files", '["pkg/_version.py"]', ["pkg/_version.py"]), + ("commit-files", "[]", []), + ("commit-files", None, None), ("disable-cuda", "true", True), ("disable-cuda", "false", False), ("disable-cuda", None, False), diff --git a/tests/test_impls.py b/tests/test_impls.py index f7e724b..0b8f4cb 100644 --- a/tests/test_impls.py +++ b/tests/test_impls.py @@ -1,7 +1,6 @@ # Copyright (c) 2024, NVIDIA CORPORATION. import os.path -import tempfile from contextlib import contextmanager from textwrap import dedent from unittest.mock import Mock, patch @@ -9,9 +8,9 @@ import pytest from rapids_build_backend.impls import ( - _edit_git_commit, _edit_pyproject, _get_cuda_suffix, + _write_git_commits, ) @@ -26,39 +25,50 @@ def set_cwd(cwd): @pytest.mark.parametrize( - "initial_contents", + ("project_name", "directories", "commit_files_config", "expected_commit_files"), [ - "def456\n", - "", - None, + ("test-project", ["test_project"], None, ["test_project/GIT_COMMIT"]), + ( + "test-project", + ["_test_project"], + ["_test_project/GIT_COMMIT"], + ["_test_project/GIT_COMMIT"], + ), + ( + "test-project", + ["_test_project_1", "_test_project_2"], + ["_test_project_1/GIT_COMMIT", "_test_project_2/GIT_COMMIT"], + ["_test_project_1/GIT_COMMIT", "_test_project_2/GIT_COMMIT"], + ), + ( + "test-project", + [], + [], + [], + ), ], ) @patch("rapids_build_backend.impls._get_git_commit", Mock(return_value="abc123")) -def test_edit_git_commit(initial_contents): - def check_initial_contents(filename): - if initial_contents is not None: - with open(filename) as f: - assert f.read() == initial_contents - else: - assert not os.path.exists(filename) - - with tempfile.TemporaryDirectory() as d: - commit_file = os.path.join(d, "commit-file") - bkp_commit_file = os.path.join(d, ".commit-file.rapids-build-backend.bak") - if initial_contents is not None: - with open(commit_file, "w") as f: - f.write(initial_contents) +def test_write_git_commits( + tmp_path, project_name, directories, commit_files_config, expected_commit_files +): + with set_cwd(tmp_path): + for directory in directories: + os.mkdir(directory) config = Mock( - commit_file=commit_file, + commit_files=commit_files_config, ) - with _edit_git_commit(config): - with open(commit_file) as f: - assert f.read() == "abc123\n" - check_initial_contents(bkp_commit_file) + with _write_git_commits(config, project_name): + for expected_commit_file in expected_commit_files: + with open(expected_commit_file) as f: + assert f.read() == "abc123\n" + if not directories: + assert list(os.walk(".")) == [(".", [], [])] - assert not os.path.exists(bkp_commit_file) - check_initial_contents(commit_file) + for directory in directories: + os.rmdir(directory) + assert list(os.walk(".")) == [(".", [], [])] @pytest.mark.parametrize( diff --git a/tests/test_packages.py b/tests/test_packages.py index 590de83..659582f 100644 --- a/tests/test_packages.py +++ b/tests/test_packages.py @@ -67,7 +67,7 @@ def test_simple_setuptools(tmp_path, env, nvcc_version): } package_dir = tmp_path / "pkg" - os.makedirs(package_dir) + os.makedirs(package_dir / "simple_setuptools") generate_from_template(package_dir, "dependencies.yaml", template_args) generate_from_template(package_dir, "pyproject.toml", template_args) @@ -99,7 +99,7 @@ def test_simple_scikit_build_core(tmp_path, env, nvcc_version): } package_dir = tmp_path / "pkg" - os.makedirs(package_dir) + os.makedirs(package_dir / "simple_scikit_build_core") generate_from_template(package_dir, "dependencies.yaml", template_args) generate_from_template(package_dir, "pyproject.toml", template_args)