-
Notifications
You must be signed in to change notification settings - Fork 108
Add installation of data-files build artifacts into Python package #574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
a12e0c6
13807a7
afc6653
991689e
d0add2d
b7b0c7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| [package] | ||
| name = "generated-files" | ||
| version = "0.1.0" | ||
| edition = "2021" | ||
| build = "build.rs" | ||
|
|
||
| [dependencies] | ||
| pyo3 = "0.27" | ||
|
|
||
| [lib] | ||
| name = "_lib" # private module to be nested into Python package | ||
| crate-type = ["cdylib"] | ||
| path = "rust/lib.rs" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| include build.rs | ||
| include Cargo.toml | ||
| include Cargo.lock | ||
| graft rust | ||
| graft tests | ||
|
|
||
| # Data files generated by Rust build scripts generally do not need to be listed in the manifest | ||
| # bceause they are not generated as part of the `sdist`; the Rust extension is also not bulit in an | ||
| # sdist, so neither are the generated files. You should ensure that all source files needed to | ||
| # generate the build files _are_ included in the manifest, however. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| use std::fs; | ||
| use std::io::{self, Write}; | ||
| use std::path::Path; | ||
|
|
||
| fn main() -> io::Result<()> { | ||
| println!("cargo::rerun-if-changed=build.rs"); | ||
| let out_dir_raw = std::env::var("OUT_DIR").unwrap(); | ||
| let out_dir = Path::new(&out_dir_raw); | ||
| fs::File::create(out_dir.join("my_file.txt"))?.write_all(b"Generated by a build script.\n")?; | ||
|
|
||
| let sub_dir = out_dir.join("dir"); | ||
| fs::create_dir_all(&sub_dir)?; | ||
| fs::File::create(sub_dir.join("a.txt"))?.write_all(b"This is file A.\n")?; | ||
| fs::File::create(sub_dir.join("b.txt"))?.write_all(b"This is file B.\n")?; | ||
| Ok(()) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| from pathlib import Path | ||
|
|
||
| import nox | ||
|
|
||
| SETUPTOOLS_RUST = Path(__file__).parents[2] | ||
|
|
||
|
|
||
| @nox.session() | ||
| def test(session: nox.Session): | ||
| session.install(str(SETUPTOOLS_RUST), "setuptools", "pytest") | ||
| session.install("--no-build-isolation", ".") | ||
| session.run("pytest", "tests", *session.posargs) | ||
|
|
||
|
|
||
| @nox.session() | ||
| def test_inplace(session: nox.Session): | ||
| session.install(str(SETUPTOOLS_RUST), "setuptools", "pytest") | ||
| session.install("--no-build-isolation", "--editable", ".") | ||
| try: | ||
| session.run("pytest", "tests", *session.posargs) | ||
| finally: | ||
| # Clear out any data files that _did_ exist | ||
| session.run( | ||
| "python", | ||
| "-c", | ||
| """ | ||
| import os | ||
| import shutil | ||
| from pathlib import Path | ||
| import generated_files | ||
|
|
||
| try: | ||
| os.remove(Path(generated_files.__file__).parent / "my_file.txt") | ||
| except FileNotFoundError: | ||
| pass | ||
| shutil.rmtree(Path(generated_files.__file__).parent / "_data", ignore_errors=True)""", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| [build-system] | ||
| requires = ["setuptools", "setuptools-rust"] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
||
| [project] | ||
| name = "generated-files" | ||
| version = "1.0" | ||
|
|
||
| [tool.setuptools.packages] | ||
| # Pure Python packages/modules | ||
| find = { where = ["python"] } | ||
|
|
||
| [[tool.setuptools-rust.ext-modules]] | ||
| target = "generated_files._lib" | ||
| [tool.setuptools-rust.ext-modules.generated-files] | ||
| # Keys correspond to files/directories in the Rust extension's build directory `OUT_DIR`. | ||
| # Values are Python packages that the corresponding file or directory should be placed inside. | ||
| "my_file.txt" = "generated_files" | ||
| "dir" = "generated_files._data" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| __all__ = ["library_ok", "data_files_content"] | ||
|
|
||
| from pathlib import Path | ||
| from ._lib import library_ok | ||
|
|
||
|
|
||
| def data_files_content() -> dict[Path, str]: | ||
| us = Path(__file__).parent | ||
| paths = [us / "my_file.txt"] | ||
| paths.extend((us / "_data" / "dir").glob("*.txt")) | ||
| return {path.relative_to(us): path.read_text() for path in paths} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| #[pyo3::pymodule] | ||
| mod _lib { | ||
| use pyo3::prelude::*; | ||
|
|
||
| #[pyfunction] | ||
| fn library_ok() -> bool { | ||
| true | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| from pathlib import Path | ||
| import generated_files | ||
|
|
||
|
|
||
| def test_rust_extension(): | ||
| assert generated_files.library_ok() | ||
|
|
||
|
|
||
| def test_data_file_content(): | ||
| expected = { | ||
| Path("my_file.txt"): "Generated by a build script.\n", | ||
| Path("_data") / "dir" / "a.txt": "This is file A.\n", | ||
| Path("_data") / "dir" / "b.txt": "This is file B.\n", | ||
| } | ||
| assert generated_files.data_files_content() == expected |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| from setuptools import Distribution | ||
| from setuptools.command.build_ext import build_ext as CommandBuildExt | ||
| from setuptools.command.build_ext import get_abi3_suffix | ||
| from setuptools.command.build_py import build_py as setuptools_build_py | ||
| from setuptools.command.install_scripts import install_scripts as CommandInstallScripts | ||
|
|
||
| from ._utils import check_subprocess_output, format_called_process_error, Env | ||
|
|
@@ -124,10 +125,12 @@ def run_for_extension(self, ext: RustExtension) -> None: | |
| assert self.plat_name is not None | ||
| if self.target is _Platform.CARGO_DEFAULT: | ||
| self.target = _override_cargo_default_target(self.plat_name, ext.env) | ||
| dylib_paths = self.build_extension(ext) | ||
| self.install_extension(ext, dylib_paths) | ||
| dylib_paths, artifact_dirs = self.build_extension(ext) | ||
| self.install_extension(ext, dylib_paths, artifact_dirs) | ||
|
|
||
| def build_extension(self, ext: RustExtension) -> List["_BuiltModule"]: | ||
| def build_extension( | ||
| self, ext: RustExtension | ||
| ) -> Tuple[List["_BuiltModule"], List[Path]]: | ||
| env = _prepare_build_environment(ext.env, ext) | ||
|
|
||
| if not os.path.exists(ext.path): | ||
|
|
@@ -201,13 +204,21 @@ def build_extension(self, ext: RustExtension) -> List["_BuiltModule"]: | |
| targets: List[Optional[str]] = [None] | ||
| elif self.target is _Platform.UNIVERSAL2: | ||
| targets = list(_UNIVERSAL2_TARGETS) | ||
| if ext.generated_files: | ||
| raise PlatformError( | ||
| "generated files are not supported for universal2 wheels" | ||
| ) | ||
| else: | ||
| targets = [self.target] | ||
|
|
||
| cargo_messages = "" | ||
| cargo_messages: Dict[str, List[str]] = {} | ||
| for target in targets: | ||
| target_command = command.copy() | ||
| if target is not None: | ||
| if target is None: | ||
| # Normalize the entries in `cargo_messages` to always be in terms of the | ||
| # actual target triple. | ||
| target = get_rust_host(ext.env) | ||
| else: | ||
| target_command += ["--target", target] | ||
| if rustc_args: | ||
| target_command += ["--"] | ||
|
|
@@ -221,12 +232,12 @@ def build_extension(self, ext: RustExtension) -> List["_BuiltModule"]: | |
| # If quiet, capture all output and only show it in the exception | ||
| # If not quiet, forward all cargo output to stderr | ||
| stderr = subprocess.PIPE if quiet else None | ||
| cargo_messages += check_subprocess_output( | ||
| cargo_messages[target] = check_subprocess_output( | ||
| target_command, | ||
| env=env, | ||
| stderr=stderr, | ||
| text=True, | ||
| ) | ||
| ).splitlines() | ||
| except subprocess.CalledProcessError as e: | ||
| # Don't include stdout in the formatted error as it is a huge dump | ||
| # of cargo json lines which aren't helpful for the end user. | ||
|
|
@@ -246,7 +257,7 @@ def build_extension(self, ext: RustExtension) -> List["_BuiltModule"]: | |
| if ext._uses_exec_binding(): | ||
| # Find artifact from cargo messages | ||
| artifacts = _find_cargo_artifacts( | ||
| cargo_messages.splitlines(), | ||
| [line for messages in cargo_messages.values() for line in messages], | ||
| package_id=package_id, | ||
| kinds={"bin"}, | ||
| ) | ||
|
|
@@ -276,7 +287,7 @@ def build_extension(self, ext: RustExtension) -> List["_BuiltModule"]: | |
| else: | ||
| # Find artifact from cargo messages | ||
| artifacts = _find_cargo_artifacts( | ||
| cargo_messages.splitlines(), | ||
| [line for messages in cargo_messages.values() for line in messages], | ||
| package_id=package_id, | ||
| kinds={"cdylib", "dylib"}, | ||
| ) | ||
|
|
@@ -300,10 +311,28 @@ def build_extension(self, ext: RustExtension) -> List["_BuiltModule"]: | |
|
|
||
| # guaranteed to be just one element after checks above | ||
| dylib_paths.append(_BuiltModule(ext.name, artifact_path)) | ||
| return dylib_paths | ||
|
|
||
| if not ext.generated_files: | ||
| return dylib_paths, [] | ||
|
|
||
| out_dirs = [ | ||
| out_dir | ||
| for target, messages in cargo_messages.items() | ||
| if (out_dir := _find_cargo_out_dir(messages, package_id)) is not None | ||
| ] | ||
| if not out_dirs: | ||
| raise ExecError( | ||
| f"extension {ext.name} requests data files, but no corresponding" | ||
| " build-script out directories could be found" | ||
| ) | ||
|
|
||
| return dylib_paths, out_dirs | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would prefer to have That way we can have
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've rearranged this and included the defensive assertion at the end of |
||
|
|
||
| def install_extension( | ||
| self, ext: RustExtension, dylib_paths: List["_BuiltModule"] | ||
| self, | ||
| ext: RustExtension, | ||
| dylib_paths: List["_BuiltModule"], | ||
| build_artifact_dirs: List[Path], | ||
| ) -> None: | ||
| debug_build = self._is_debug_build(ext) | ||
|
|
||
|
|
@@ -400,6 +429,39 @@ def install_extension( | |
| mode |= (mode & 0o444) >> 2 # copy R bits to X | ||
| os.chmod(ext_path, mode) | ||
|
|
||
| if not ext.generated_files: | ||
| return | ||
|
|
||
| # We'll delegate the finding of the package directories to Setuptools, so we | ||
| # can be sure we're handling editable installs and other complex situations | ||
| # correctly. | ||
| build_py = cast(setuptools_build_py, self.get_finalized_command("build_py")) | ||
|
|
||
| def get_package_dir(package: str) -> Path: | ||
| if self.inplace: | ||
| # If `inplace`, we have to ask `build_py` (like `build_ext` would). | ||
| return Path(build_py.get_package_dir(package)) | ||
| # ... If not, `build_ext` knows where to put the package. | ||
| return Path(build_ext.build_lib) / Path(*package.split(".")) | ||
|
|
||
| for source, package in ext.generated_files.items(): | ||
| dest = get_package_dir(package) | ||
| dest.mkdir(mode=0o755, parents=True, exist_ok=True) | ||
| for artifact_dir in build_artifact_dirs: | ||
| source_full = artifact_dir / source | ||
| dest_full = dest / source_full.name | ||
| if source_full.is_file(): | ||
| logger.info( | ||
| "Copying data file from %s to %s", source_full, dest_full | ||
| ) | ||
| shutil.copy2(source_full, dest_full) | ||
| elif source_full.is_dir(): | ||
| logger.info( | ||
| "Copying data directory from %s to %s", source_full, dest_full | ||
| ) | ||
| shutil.copytree(source_full, dest_full, dirs_exist_ok=True) | ||
| # This tacitly makes "no match" a silent non-error. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably prefer no match to be a build failure? IMO this implies the configuration is wrong. What use case is there where silently missing files is desirable?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't remember my reasoning, but honestly it might just have been that I wrote the loop like this, then realised the implication and put it in a comment rather than actually following the thought all the way through to see what should happen. It's even easier to error on missing when there's only one artifact directory, so I did that in b7b0c7a. |
||
|
|
||
| def get_dylib_ext_path(self, ext: RustExtension, target_fname: str) -> str: | ||
| assert self.plat_name is not None | ||
| build_ext = cast(CommandBuildExt, self.get_finalized_command("build_ext")) | ||
|
|
@@ -835,6 +897,20 @@ def _find_cargo_artifacts( | |
| return artifacts | ||
|
|
||
|
|
||
| def _find_cargo_out_dir(cargo_messages: List[str], package_id: str) -> Optional[Path]: | ||
| # Chances are that the line we're looking for will be the third laste line in the | ||
| # messages. The last is the completion report, the penultimate is generally the | ||
|
jakelishman marked this conversation as resolved.
Outdated
|
||
| # build of the final artifact. | ||
| for messsage in reversed(cargo_messages): | ||
| if "build-script-executed" not in messsage or package_id not in messsage: | ||
| continue | ||
| parsed = json.loads(messsage) | ||
| if parsed.get("package_id") == package_id: | ||
| out_dir = parsed.get("out_dir") | ||
| return None if out_dir is None else Path(out_dir) | ||
| return None | ||
|
|
||
|
|
||
| def _replace_cross_target_dir(path: str, ext: RustExtension, *, quiet: bool) -> str: | ||
| """Replaces target director from `cross` docker build with the correct | ||
| local path. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.