Skip to content

Commit

Permalink
Automatically figure out where to write GIT_COMMIT (#30)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
KyleFromNVIDIA authored May 15, 2024
1 parent aec9ee7 commit dccdf0d
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 74 deletions.
16 changes: 8 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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] | ["<project_name>/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
Expand Down
24 changes: 13 additions & 11 deletions rapids_build_backend/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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()

Expand Down Expand Up @@ -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}'")
Expand Down
47 changes: 23 additions & 24 deletions rapids_build_backend/impls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<project_name>/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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -259,15 +254,19 @@ 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
)


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
)
Expand Down
5 changes: 3 additions & 2 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
64 changes: 37 additions & 27 deletions tests/test_impls.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
# Copyright (c) 2024, NVIDIA CORPORATION.

import os.path
import tempfile
from contextlib import contextmanager
from textwrap import dedent
from unittest.mock import Mock, patch

import pytest

from rapids_build_backend.impls import (
_edit_git_commit,
_edit_pyproject,
_get_cuda_suffix,
_write_git_commits,
)


Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions tests/test_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit dccdf0d

Please sign in to comment.