Skip to content
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

Fix vendor Rust: limit to manifests from backend #532

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

tiran
Copy link
Collaborator

@tiran tiran commented 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: #529

@tiran
Copy link
Collaborator Author

tiran commented Jan 10, 2025

I need to verify the changes with cryptography 42, 43, and 44.

@pnasrat
Copy link
Contributor

pnasrat commented Jan 10, 2025

Thanks @tiran - I also added #533 to control vendoring via network isolation (which means if future rust vendoring issues come up there is a way that fromager users can disable vendoring before fixes)

@pnasrat
Copy link
Contributor

pnasrat commented Jan 16, 2025

Looks like a ci failure

  0%|                                                  | 0/1 [00:00<?, ?pkg/s]maturin: new toplevel dependency maturin==1.6.0 resolves to 1.6.0
maturin: checking if wheel was already uploaded to http://localhost:38821/simple/
maturin: did not find wheel for 1.6.0 in http://localhost:38821/simple/
saved /home/runner/work/fromager/fromager/e2e-output/sdists-repo/downloads/maturin-1.6.0.tar.gz
maturin: preparing source for maturin==1.6.0 from /home/runner/work/fromager/fromager/e2e-output/sdists-repo/downloads/maturin-1.6.0.tar.gz

  0%|                                                  | 0/1 [00:00<?, ?pkg/s]
ERROR: 'Key "setuptools-rust" does not exist.'

@tiran tiran force-pushed the better-vendor-rust branch from a0c23b1 to eec7e9c Compare January 22, 2025 11:46
@tiran
Copy link
Collaborator Author

tiran commented Jan 22, 2025

I have successfully built cryptography==44.0.0 with network isolation + vendored Rust packages with this PR.

@tiran tiran marked this pull request as ready for review January 22, 2025 11:47
Copy link
Member

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK to me, with a couple of small nits.

@pnasrat , have you had a chance to try it out to see if it resolves your issue, too?

@tiran tiran force-pushed the better-vendor-rust branch 3 times, most recently from 90a8b75 to 53daee5 Compare January 23, 2025 11:20
@mergify mergify bot added the ci label 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 <[email protected]>
@tiran tiran force-pushed the better-vendor-rust branch from 53daee5 to 28c0d57 Compare January 23, 2025 11:36
@pnasrat
Copy link
Contributor

pnasrat commented Feb 5, 2025

it'd be great to get this merged - its been working for me on the PR checked out

@dhellmann
Copy link
Member

it'd be great to get this merged - its been working for me on the PR checked out

Excellent, I was just waiting to hear back from you and then it fell off of the top of my todo list.

@dhellmann
Copy link
Member

@Mergifyio refresh

Copy link
Contributor

mergify bot commented Feb 5, 2025

refresh

✅ Pull request refreshed

@dhellmann dhellmann merged commit fb341a3 into python-wheel-build:main Feb 5, 2025
74 checks passed
@dhellmann
Copy link
Member

Released in 0.38.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fromager: vendor_rust behavior broken in workspaces with example code (eg cryptography)
3 participants