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

Add support for python first src layout - take 2 #1185

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

messense
Copy link
Member

Implements #501 (comment)

@messense messense requested a review from konstin October 10, 2022 04:28
@netlify
Copy link

netlify bot commented Oct 10, 2022

Deploy Preview for maturin-guide ready!

Name Link
🔨 Latest commit 1b36411
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/634407af7bc58500082848aa
😎 Deploy Preview https://deploy-preview-1185--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

for parent in fs::canonicalize(&path)?.ancestors().skip(1) {
if !parent.starts_with(&workspace_root) {
// Allow looking outside to the parent directory of Cargo workspace root
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'm a bit worried about this. It's for supporting maturin build --manifest-path /path/to/rust/Cargo.toml when pyproject.toml is located in the parent directory of the rust crate.

But it might have unexpected side effects, I'm not sure.

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 i'd make --manifest-path take either Cargo.toml or pyproject.toml, whichever is in the project root. that would be an unexpected break from the normal cargo semantics, but i think it makes sense overall to point to the most top level configuration file

Copy link
Member Author

Choose a reason for hiding this comment

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

make --manifest-path take either Cargo.toml or pyproject.toml, whichever is in the project root

It's certainly doable but not as intuitive as just Cargo.toml, we can explore it later.

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.

💯

```
my-project
├── Readme.md
├── pyproject.toml
├── src
│   └── my_project
│       ├── __init__.py
│       └── bar.py
└── rust
    ├── Cargo.toml
    └── src
        └── lib.rs
```
@messense messense marked this pull request as ready for review October 10, 2022 12:19
@messense messense merged commit 0370ab7 into PyO3:main Oct 10, 2022
@messense messense deleted the src-layout branch October 10, 2022 12:19
This was referenced Oct 10, 2022
@messense messense linked an issue Oct 10, 2022 that may be closed by this pull request
6 tasks
@messense messense added this to the 0.14.0 milestone Oct 14, 2022
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Nov 20, 2022
https://build.opensuse.org/request/show/1036879
by user mia + dimstar_suse
- Update to v0.14.0:
* Breaking Change: Remove support for specifying python package
  metadata in `Cargo.toml`
  Python package metadata should be specified in the `project`
  section of `pyproject.toml` instead as PEP 621 specifies.
  gh#PyO3/maturin#1200
* Initial support for shipping bin targets as wasm32-wasi binaries
  that are run through wasmtime
  Note that wasmtime currently only support the five most popular
  platforms and that wasi binaries have restrictions when
  interacting with the host.
  Usage is by setting `--target wasm32-wasi`.
  gh#PyO3/maturin#1107
* Add support for python first `src` project layout
  gh#PyO3/maturin#1185
* Add `--src` option to generate src layout for mixed Python/Rust
  projects
  gh#PyO3/maturin#1189
* Add Python metadata support for `license-file
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.

TODO list for 1.0
2 participants