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.

Sign up for GitHub

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

Jump to bottom

Update workspace memebers for sdist local dependencies #844

Merged
merged 3 commits into from
Mar 14, 2022
Merged

Update workspace memebers for sdist local dependencies #844

merged 3 commits into from
Mar 14, 2022

Conversation

messense
Copy link
Member

Fixes #838

@netlify
Copy link

netlify bot commented Mar 12, 2022 •
edited
Loading

✔️ Deploy Preview for maturin-guide ready!

🔨 Explore the source changes: 1e62f47

🔍 Inspect the deploy log: https://app.netlify.com/sites/maturin-guide/deploys/622f3bb9ab4d3200095a2bac

😎 Browse the preview: https://deploy-preview-844--maturin-guide.netlify.app

@messense messense mentioned this pull request Mar 12, 2022
Closed
2 tasks
@anthrotype
Copy link

anthrotype commented Mar 14, 2022 •
edited
Loading

@messense I rebuilt maturin off this PR branch but when I do maturin sdist (or build) on the project I am working on (https://github.com/anthrotype/resvg/tree/maturin) I now get the following error:

$ ../maturin/target/release/maturin sdist
🔗 Found bin bindings
📡 Using build options bindings from pyproject.toml
💥 maturin failed
  Caused by: Failed to build source distribution
  Caused by: Failed to run cargo
  Caused by: No such file or directory (os error 2)

@anthrotype
Copy link

anthrotype commented Mar 14, 2022 •
edited
Loading

actually, the same error happens if when I build and run maturin from the latest main branch (HEAD at commit 0c96687), so it's probably not related to #838. Apologies for the noise. Might be good to understand what file is actually raising OSError 2.

I don't get any error when I use the latest stable maturin from PyPI v0.12.10

@messense
Copy link
Member Author

@anthrotype Can you try again, I've reverted #843 which might be the cause of the Failed to run cargo error you mentioned.

@anthrotype
Copy link

thanks, I tried again and it works now! I could build a sdist and then do pip wheel off it without errors about missing workspace manifests.

@messense messense force-pushed the update-sdist-workspace-member branch from a2a8e31 to 84f1991 Compare March 14, 2022 12:55
@messense messense mentioned this pull request Mar 14, 2022
Merged
@messense messense force-pushed the update-sdist-workspace-member branch from 84f1991 to 1e62f47 Compare March 14, 2022 12:57
messense
messense commented Mar 14, 2022
View reviewed changes
src/source_distribution.rs
for member in members.iter() {
if let toml_edit::Value::String(ref s) = member {
let name = s.value();
if known_path_deps.contains_key(name) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to self: workspace members contain paths not just names, so this only handles simple path without subdirectory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll fix this later.

@messense messense merged commit 0d0bf46 into PyO3:main Mar 14, 2022
@messense messense deleted the update-sdist-workspace-member branch March 14, 2022 13:43
This was referenced Mar 14, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pip can't build wheel from sdist when explicitly listing workspaces
2 participants
@messense @anthrotype