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

Issue with README building sdist #2154

Closed
1 of 2 tasks
qjerome opened this issue Jul 23, 2024 · 8 comments · Fixed by #2184
Closed
1 of 2 tasks

Issue with README building sdist #2154

qjerome opened this issue Jul 23, 2024 · 8 comments · Fixed by #2184
Labels
bug Something isn't working sdist Source distribution

Comments

@qjerome
Copy link

qjerome commented Jul 23, 2024

Bug Description

Since maturin v1.7.0 it seems the default behavior taking a readme for the sdist build has changed and it break my workflow.

Issue: even though I have set the correct path to the readme in pyproject.toml maturin keep taking the README.md at the root of my workspace.

NB: for maturin v1.5.x and v1.6.0 everything works fine

Output:

🔗 Found pyo3 bindings
🐍 Found CPython 3.12 at /home/quentin/Workspace/Rust/poppy/python/.env/bin/python
📡 Using build options features from pyproject.toml
💥 maturin failed
  Caused by: Failed to build source distribution
  Caused by: File poppy_py-0.1.8/README.md was already added from /home/user/Rust/poppy/README.md, can't added it from /home/user/Rust/poppy/python/README.md

Your maturin version (maturin --version)

1.7.0

Your Python version (python -V)

3.12.4

Your pip version (pip -V)

24.1.2

What bindings you're using

pyo3

Does cargo build work?

  • Yes, it works

If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash /)?

  • Yes

Steps to Reproduce

git clone https://github.com/hashlookup/poppy.git
cd poppy
maturin sdist -m python/Cargo.toml -o /tmp/sdist

@qjerome qjerome added the bug Something isn't working label Jul 23, 2024
@messense messense added the sdist Source distribution label Jul 23, 2024
plusvic added a commit to VirusTotal/yara-x that referenced this issue Jul 30, 2024
The latest version (1.7.0) has the following issue: PyO3/maturin#2154
@cauliyang
Copy link

same error here

@konstin
Copy link
Member

konstin commented Aug 5, 2024

I think what we need to do is vendor the readmes and license file of workspace crates inside these crates, as oppose to using the top level workspace location they are pointing too.

@samuelcolvin
Copy link

Same error on jiter - see this build.

@ion-elgreco
Copy link

ion-elgreco commented Aug 15, 2024

We are also running into this in delta-rs since maturin 1.7

https://github.com/delta-io/delta-rs/actions/runs/10395762774/job/28799000654

@konstin
Copy link
Member

konstin commented Aug 21, 2024

@samuelcolvin I tested my fix and it fails with jiter, but it's also a case where cargo warns:

$ cd crates/jiter
$ cargo package --list
warning: readme `../../README.md` appears to be a path outside of the package, but there is already a file named `README.md` in the root of the package. The archived crate will contain the copy in the root of the package. Update the readme to point to the path relative to the root of the package to remove this warning.
[...]

I've added warning forwarding for this in #2186. I can be swayed to also support this case in maturin.

@konstin
Copy link
Member

konstin commented Aug 21, 2024

I've implemented a fix for this #2184 and released it as 1.7.1. Please test and report whether that fixes your case, or whether we need different readme handling.

@samuelcolvin
Copy link

@konstin What's the correct way to defining a readme for a maturin package that is within a workspace?

I've tried numerous combinations of paths and deleting the inner readme, nothing seems to work? I assume I'm being dumb.

@ia0
Copy link

ia0 commented Sep 12, 2024

@konstin I'm hitting this issue (sdist with python readme conflicting with rust readme). I can workaround by excluding the python readme from the tool.maturin table. However, I'd prefer excluding the rust readme, but the path is ../rust/cli/README.md which doesn't seem to work (silently ignored). What was your fix supposed to do? And does it work when the rust crate is not in a child directory?

EDIT: What's even more weird, is that I see no README.md in the final tarball. Maybe it's just ignoring both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sdist Source distribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants