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

Implement maturin upload command #484

Merged
merged 3 commits into from
Apr 9, 2021
Merged

Implement maturin upload command #484

merged 3 commits into from
Apr 9, 2021

Conversation

ravenexp
Copy link
Contributor

@ravenexp ravenexp commented Apr 3, 2021

It behaves like twine upload, but is restricted to the python wheels and source distributions.

The upload command mostly reuses the publish command implementation.

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Thank you for working on finishing this!

src/main.rs Outdated
if files.is_empty() {
println!("⚠ Warning: No files given, exiting.");
return Ok(());
}
let metadata: Vec<Vec<(String, String)>> = files

// FIXME: All uploaded files are expected to share the build context
Copy link
Member

Choose a reason for hiding this comment

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

I think the easiest way to check this is to read all metadata and check they are equal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a sanity check ensuring that all wheels come from the same Python package.

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated
```
Uploads the distribution files to the repository (package index)

It behaves like `twine upload`, but is restricted to the python wheels and source distributions.
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't be so bold to claim that it has the same behaviour as twine. There are a lot of edge cases maturin doesn't (and will never) cover.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reworded it to look more like the original comment.

Readme.md Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
Split out the maturin build context members from `PublishOpt`.
Extract the build context handling code from `upload_ui()`.

The existing functionality is not changed in this commit.
The trimmed publish options structure will be reused for
the new `maturin upload` command.
Uploads a set of wheels and/or source distributions
that belong to a single python package.

Refuse to upload files from different packages in one
command invocation.
@ravenexp
Copy link
Contributor Author

ravenexp commented Apr 9, 2021

Applied suggestions and rebased on main.

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Thanks again!

@konstin
Copy link
Member

konstin commented Apr 9, 2021

Note that currently the source distribution upload fails because the PKG-INFO is not in the root directory, but we can fix that later.

@konstin konstin merged commit 179d6f5 into PyO3:main Apr 9, 2021
@ravenexp
Copy link
Contributor Author

ravenexp commented Apr 9, 2021

Note that currently the source distribution upload fails because the PKG-INFO is not in the root directory, but we can fix that later.

Does it? I don't make source distributions myself, but I've tried to upload test-data/pyo3_mixed-2.1.1.tar.gz and everything looked fine.

@konstin
Copy link
Member

konstin commented Apr 9, 2021

I just tried the following on the main branch:

$ rm -rf test-crates/pyo3-mixed/target/wheels/
$ cargo run -- build -m test-crates/pyo3-mixed/Cargo.toml 
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/maturin build -m test-crates/pyo3-mixed/Cargo.toml`
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.6m at python3.6, CPython 3.7m at python3.7, CPython 3.8 at python3.8, CPython 3.9 at python3.9, CPython 3.10 at python3.10
📦 Built source distribution to /home/konsti/maturin/test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3.tar.gz
   Compiling pyo3 v0.13.2
   Compiling pyo3-mixed v2.1.3 (/home/konsti/maturin/test-crates/pyo3-mixed)
    Finished dev [unoptimized + debuginfo] target(s) in 4.16s
📦 Built wheel for CPython 3.6m to /home/konsti/maturin/test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp36-cp36m-manylinux_2_24_x86_64.whl
   Compiling pyo3 v0.13.2
   Compiling pyo3-mixed v2.1.3 (/home/konsti/maturin/test-crates/pyo3-mixed)
    Finished dev [unoptimized + debuginfo] target(s) in 4.00s
📦 Built wheel for CPython 3.7m to /home/konsti/maturin/test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp37-cp37m-manylinux_2_24_x86_64.whl
   Compiling pyo3 v0.13.2
   Compiling pyo3-mixed v2.1.3 (/home/konsti/maturin/test-crates/pyo3-mixed)
    Finished dev [unoptimized + debuginfo] target(s) in 3.93s
📦 Built wheel for CPython 3.8 to /home/konsti/maturin/test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp38-cp38-manylinux_2_24_x86_64.whl
   Compiling pyo3 v0.13.2
   Compiling pyo3-mixed v2.1.3 (/home/konsti/maturin/test-crates/pyo3-mixed)
    Finished dev [unoptimized + debuginfo] target(s) in 4.02s
📦 Built wheel for CPython 3.9 to /home/konsti/maturin/test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp39-cp39-manylinux_2_24_x86_64.whl
   Compiling pyo3 v0.13.2
   Compiling pyo3-mixed v2.1.3 (/home/konsti/maturin/test-crates/pyo3-mixed)
    Finished dev [unoptimized + debuginfo] target(s) in 3.99s
📦 Built wheel for CPython 3.10 to /home/konsti/maturin/test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp310-cp310-manylinux_2_24_x86_64.whl
(reverse-i-search)`upload ': twine ^Cload --help
$ cargo run -- upload test-crates/pyo3-mixed/target/wheels/*
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
     Running `target/debug/maturin upload test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp310-cp310-manylinux_2_24_x86_64.whl test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp36-cp36m-manylinux_2_24_x86_64.whl test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp37-cp37m-manylinux_2_24_x86_64.whl test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp38-cp38-manylinux_2_24_x86_64.whl test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3-cp39-cp39-manylinux_2_24_x86_64.whl test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3.tar.gz`
💥 maturin failed
  Caused by: Failed to read metadata from source distribution at "test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3.tar.gz"
  Caused by: Source distribution "test-crates/pyo3-mixed/target/wheels/pyo3_mixed-2.1.3.tar.gz" does not contain a PKG-INFO, but it should

@ravenexp
Copy link
Contributor Author

ravenexp commented Apr 9, 2021

Hmm, that means the source distribution tarball that the current maturin version builds for test-crates/pyo3-mixed differs from test-data/pyo3_mixed-2.1.1.tar.gz which is used in the read_metadata_for_source_distribution() unit test.

@konstin
Copy link
Member

konstin commented Apr 9, 2021

I totally missed that! Initially I implemented source distributions with PKG-INFO in the root, then later found out it should be in the main folder. I fixed that in 036fff7 but forgot to update the test.

@ravenexp ravenexp deleted the upload branch April 14, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants