Skip to content

Commit

Permalink
Fix vendor Rust: limit to manifests from backend
Browse files Browse the repository at this point in the history
Fromager was vendoring crates from all `Cargo.toml` files in a project.
This approach is causing issues for projects that have cargo files in
tests and example directories.

The `vendor_rust()` function now only vendors crates from `Cargo.toml`
in the project's root directory and additional cargo files listed in
`tools.maturin` or `tools.setuptools-rust` entries.

Fixes: python-wheel-build#529
Signed-off-by: Christian Heimes <[email protected]>
  • Loading branch information
tiran committed Jan 10, 2025
1 parent acc45d3 commit a0c23b1
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/fromager/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def __init__(
f"Failed to install {req_type} dependency. "
f"Check all {req_type} dependencies:\n{formatted_reqs}"
)
super().__init__(f'\n{"*" * 40}\n{msg}\n{"*" * 40}\n')
super().__init__(f"\n{'*' * 40}\n{msg}\n{'*' * 40}\n")


class BuildEnvironment:
Expand Down
6 changes: 3 additions & 3 deletions src/fromager/finders.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def find_sdist(
f"{req.name}-{dist_version}",
# Sometimes the sdist uses '.' instead of '-' in the
# package name portion.
f'{req.name.replace("-", ".")}-{dist_version}',
f"{req.name.replace('-', '.')}-{dist_version}",
]
)
# Case-insensitive globbing was added to Python 3.12, but we
Expand Down Expand Up @@ -115,7 +115,7 @@ def find_wheel(
f"{req.name}-{dist_version}-{candidate_bases_build_tag}",
# Sometimes the sdist uses '.' instead of '-' in the
# package name portion.
f'{req.name.replace("-", ".")}-{dist_version}-{candidate_bases_build_tag}',
f"{req.name.replace('-', '.')}-{dist_version}-{candidate_bases_build_tag}",
]
)
# Case-insensitive globbing was added to Python 3.12, but we
Expand Down Expand Up @@ -181,7 +181,7 @@ def find_source_dir(
canonical_name = canonicalize_name(req.name)
canonical_based = f"{canonical_name}-{dist_version}"
name_based = f"{req.name}-{dist_version}"
dotted_name = f'{req.name.replace("-", ".")}-{dist_version}'
dotted_name = f"{req.name.replace('-', '.')}-{dist_version}"

candidate_bases = set(
[
Expand Down
43 changes: 31 additions & 12 deletions src/fromager/vendor_rust.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,28 +86,25 @@ def _cargo_config(project_dir: pathlib.Path) -> None:
tomlkit.dump(cfg, f)


def _should_vendor_rust(req: Requirement, project_dir: pathlib.Path) -> bool:
def _detect_rust_build_backend(
req: Requirement, pyproject_toml: dict[str, typing.Any]
) -> str | None:
"""Detect if project has build requirement on Rust
Detects setuptools-rust and maturin.
"""
pyproject_toml = dependencies.get_pyproject_contents(project_dir)
if not pyproject_toml:
logger.debug(f"{req.name}: has no pyproject.toml")
return False

build_backend = dependencies.get_build_backend(pyproject_toml)

for reqstring in build_backend["requires"]:
req = Requirement(reqstring)
if req.name in RUST_BUILD_REQUIRES:
logger.debug(
f"{req.name}: build-system requires {req.name}, vendoring crates"
f"{req.name}: build-system requires '{req.name}', vendoring crates"
)
return True
return req.name

logger.debug(f"{req.name}: no Rust build plugin detected")
return False
return None


def vendor_rust(
Expand All @@ -119,11 +116,33 @@ def vendor_rust(
``setuptools-rust`` or ``maturin``, and has a ``Cargo.toml``, otherwise
``False``.
"""
if not _should_vendor_rust(req, project_dir):
pyproject_toml = dependencies.get_pyproject_contents(project_dir)
if not pyproject_toml:
logger.debug(f"{req.name}: has no pyproject.toml")
return False

# check for Cargo.toml
manifests = list(project_dir.glob("**/Cargo.toml"))
backend = _detect_rust_build_backend(req, pyproject_toml)
if backend is None:
return False

manifests: list[pathlib.Path] = []
# check for Cargo.toml in project root
root_cargo_toml = project_dir / "Cargo.toml"
if root_cargo_toml.is_file():
manifests.append(root_cargo_toml)

# maturin and setuptools-rust can have Cargo.toml in other directories
if backend == "maturin":
tool_maturin: dict[str, typing.Any] = pyproject_toml["tool"]["maturin"]
if "manifest-path" in tool_maturin:
manifests.append(project_dir / tool_maturin["manifest-path"])
elif backend == "setuptools-rust":
ext_modules: list[dict[str, typing.Any]]
ext_modules = pyproject_toml["tool"]["setuptools-rust"]["ext-modules"]
for ext_module in ext_modules:
if "path" in ext_module:
manifests.append(project_dir / ext_module["path"])

if not manifests:
logger.debug(f"{req.name}: has no Cargo.toml files")
return False
Expand Down
4 changes: 2 additions & 2 deletions src/fromager/wheels.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ def build_wheel(
textwrap.dedent(
f"""
[build_ext]
parallel = {extra_environ['MAX_JOBS']}
parallel = {extra_environ["MAX_JOBS"]}
"""
)
)
Expand Down Expand Up @@ -415,5 +415,5 @@ def resolve_prebuilt_wheel(
if wheel_url and resolved_version:
return (wheel_url, resolved_version)
raise ValueError(
f'Could not find a prebuilt wheel for {req} on {" or ".join(wheel_server_urls)}'
f"Could not find a prebuilt wheel for {req} on {' or '.join(wheel_server_urls)}"
)

0 comments on commit a0c23b1

Please sign in to comment.