fromager: vendor_rust behavior broken in workspaces with example code (eg cryptography) #529
Comments
|
We have a similar issue with meson. We solved it by using a What would configuration options look like? Maybe something to override that default glob pattern (either as a separate glob or a list of globs)? Or does it need more information to describe what to do? @tiran is more familiar with rust, so he might have some ideas, too. Here's our plugin, for reference: import logging
import pathlib
import shutil
from fromager import context, sources, vendor_rust
from packaging.requirements import Requirement
from packaging.version import Version
logger = logging.getLogger(__name__)
def prepare_source(
ctx: context.WorkContext,
req: Requirement,
source_filename: pathlib.Path,
version: Version,
) -> tuple[pathlib.Path, bool]:
source_root_dir, is_new = sources.unpack_source(
ctx=ctx,
req=req,
version=version,
source_filename=source_filename,
)
if is_new:
# Move the 'test cases' directory out of the way to eliminate
# issues with vendoring tests that try to deal with optional
# packages that do not exist anywhere. We move it back after
# vendoring because we want to keep the directory because we
# want the tests to go into the source distribution when we
# repackage it.
test_dir = source_root_dir / "test cases"
dest_dir = source_root_dir.parent / test_dir.name
move_tests = test_dir.exists()
if move_tests:
logger.debug("saving %s", test_dir)
shutil.move(test_dir, dest_dir)
try:
vendor_rust.vendor_rust(req, source_root_dir)
finally:
if move_tests:
logger.debug("restoring %s", test_dir)
shutil.move(dest_dir, test_dir)
return source_root_dir, is_new |
|
Off the top of my head:
Taking a step back here - what is the goal for doing the vendoring here, is it purely due to running with network isolation to allow for offline build - if so if network isolation is turned off perhaps a similar or connected setting/env var to disable vendoring. For reference just invoking |
|
I agree with @pnasrat analysis. My Rust vendor code is naive and brute force. It got the job done and I haven't spent any additional time on it. Let's first try to make the code more clever before we resort to a new option. Until cryptography 44.x, there was no top-level
We need to deal with all cases. |
|
Yes, the vendoring had to do with isolated builds. So, I agree that we could probably skip that step if isolation is disabled. Maybe just log a warning and skip it, without adding a separate option? I also like some of the ideas proposed for making the logic smarter. I imagine we could take that, too far, though. Is there a minimal change that would fix this issue, without trying to solve for all possible cases? Then we could tackle other cases individually. |
tiran
added a commit
to tiran/fromager
that referenced
this issue
Jan 10, 2025
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 <cheimes@redhat.com>
pnasrat
added a commit
to pnasrat/fromager
that referenced
this issue
Jan 10, 2025
See: python-wheel-build#529 While improving the vendor logic for more complex source cases this fixes building cryptography if network isolation turned off Signed-off-by: Pris Nasrat <pris.nasrat@chainguard.dev>
tiran
added a commit
to tiran/fromager
that referenced
this issue
Jan 22, 2025
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 <cheimes@redhat.com>
tiran
added a commit
to tiran/fromager
that referenced
this issue
Jan 23, 2025
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 <cheimes@redhat.com>
tiran
added a commit
to tiran/fromager
that referenced
this issue
Jan 23, 2025
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 <cheimes@redhat.com>
tiran
added a commit
to tiran/fromager
that referenced
this issue
Jan 23, 2025
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 <cheimes@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
vendoring breaks with error for docs file
cryptography-44.0.0/cryptography-44.0.0/docs/development/custom-vectors/aes-192-gcm-siv/verify-aes192gcmsiv/Cargo.tomlI tried to add a patch that adds exclude to the workspace members but it looks as if the vendor rust behavior of fromager is not aware of workspaces
Analysis
Looking at the source fromager assumes any
Cargo.tomlin the source tree is included and does not honor workspace configuration as it just uses a glob. This seems brittle to projects using workspaces with example code in docs as cryptography havefromager/src/fromager/vendor_rust.py
Line 126 in acc45d3
Steps to reproduce
Added a branch with the patch and an e2e test demonstrating here
https://github.com/pnasrat/fromager/tree/vendor-cryptography-fails
@dhellmann wondering what the right behavior to do here, I wonder if rust vendoring should be configurable at the top level of fromager maybe via an env variable as different rebuilders may want different behavior from fromager here.
Logs
fromager log
The text was updated successfully, but these errors were encountered: