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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

* Initial support for shipping bin targets as wasm32-wasi binaries that are run through wasmtime in [#1107](https://github.com/PyO3/maturin/pull/1107). 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`.
* Initial support for shipping bin targets as wasm32-wasi binaries that are run through wasmtime in [#1107](https://github.com/PyO3/maturin/pull/1107).
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`.
* Add support for python first `src` project layout in [#1185](https://github.com/PyO3/maturin/pull/1185)

## [0.13.6] - 2022-10-08

Expand Down
26 changes: 11 additions & 15 deletions src/module_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ pub trait ModuleWriter {

/// Copies the source file to the target path relative to the module base path
fn add_file(&mut self, target: impl AsRef<Path>, source: impl AsRef<Path>) -> Result<()> {
let target = target.as_ref();
let source = source.as_ref();
debug!("Adding {} from {}", target.display(), source.display());

self.add_file_with_permissions(target, source, 0o644)
}

Expand All @@ -62,12 +58,16 @@ pub trait ModuleWriter {
source: impl AsRef<Path>,
permissions: u32,
) -> Result<()> {
let read_failed_context = format!("Failed to read {}", source.as_ref().display());
let mut file = File::open(source.as_ref()).context(read_failed_context.clone())?;
let target = target.as_ref();
let source = source.as_ref();
debug!("Adding {} from {}", target.display(), source.display());

let read_failed_context = format!("Failed to read {}", source.display());
let mut file = File::open(source).context(read_failed_context.clone())?;
let mut buffer = Vec::new();
file.read_to_end(&mut buffer).context(read_failed_context)?;
self.add_bytes_with_permissions(target.as_ref(), &buffer, permissions)
.context(format!("Failed to write to {}", target.as_ref().display()))?;
self.add_bytes_with_permissions(target, &buffer, permissions)
.context(format!("Failed to write to {}", target.display()))?;
Ok(())
}
}
Expand Down Expand Up @@ -640,11 +640,6 @@ pub fn write_bindings_module(
};

if let Some(python_module) = &project_layout.python_module {
if !editable {
write_python_part(writer, python_module)
.context("Failed to add the python module to the package")?;
}

if editable {
let target = project_layout.rust_module.join(&so_filename);
// Remove existing so file to avoid triggering SIGSEV in running process
Expand All @@ -658,9 +653,10 @@ pub fn write_bindings_module(
artifact.display(),
target.display()
))?;
}
} else {
write_python_part(writer, python_module)
.context("Failed to add the python module to the package")?;

if !editable {
let relative = project_layout
.rust_module
.strip_prefix(python_module.parent().unwrap())?;
Expand Down
48 changes: 46 additions & 2 deletions src/project_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ impl ProjectResolver {
manifest_file.display()
);
}

// Set Cargo manifest path
cargo_options.manifest_path = Some(manifest_file.clone());

let cargo_toml = CargoToml::from_path(&manifest_file)?;
cargo_toml.warn_deprecated_python_metadata();

Expand Down Expand Up @@ -123,7 +127,18 @@ impl ProjectResolver {
Some(py_src) => py_src.to_path_buf(),
None => match extra_metadata.python_source.as_ref() {
Some(py_src) => manifest_dir.join(py_src),
None => project_root.to_path_buf(),
None => match pyproject.and_then(|x| x.project_name()) {
Some(project_name) => {
// Detect src layout
let import_name = project_name.replace('-', "_");
if project_root.join("src").join(import_name).is_dir() {
project_root.join("src")
} else {
project_root.to_path_buf()
}
}
None => project_root.to_path_buf(),
},
},
};
let data = match pyproject.and_then(|x| x.data()) {
Expand Down Expand Up @@ -166,8 +181,10 @@ impl ProjectResolver {
// use command line argument if specified
if let Some(path) = cargo_manifest_path {
let workspace_root = Self::resolve_cargo_metadata(&path, cargo_options)?.workspace_root;
let workspace_parent = workspace_root.parent().unwrap_or(&workspace_root);
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.

if !dunce::simplified(parent).starts_with(&workspace_parent) {
break;
}
let pyproject_file = parent.join(PYPROJECT_TOML);
Expand Down Expand Up @@ -196,6 +213,33 @@ impl ProjectResolver {
bail!("Cargo.toml can not be placed outside of the directory containing pyproject.toml");
}
return Ok((path.to_path_buf(), pyproject_file));
} else {
// Detect src layout:
//
// my-project
// ├── Readme.md
// ├── pyproject.toml
// ├── src
// │ └── my_project
// │ ├── __init__.py
// │ └── bar.py
// └── rust
// ├── Cargo.toml
// └── src
// └── lib.rs
let path = current_dir.join("rust").join("Cargo.toml");
if path.exists() {
if pyproject.python_source().is_some() {
// python source directory is specified in pyproject.toml
return Ok((path, pyproject_file));
} else if let Some(project_name) = pyproject.project_name() {
// Check if python source directory in `src/<project_name>`
let import_name = project_name.replace('-', "_");
if current_dir.join("src").join(import_name).is_dir() {
return Ok((path, pyproject_file));
}
}
}
}
}
// check Cargo.toml in current directory
Expand Down
5 changes: 5 additions & 0 deletions src/pyproject_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ impl PyProjectToml {
Ok(pyproject)
}

/// Returns the value of `[project.name]` in pyproject.toml
pub fn project_name(&self) -> Option<&str> {
self.project.as_ref().map(|project| project.name.as_str())
}

/// Returns the values of `[tool.maturin]` in pyproject.toml
#[inline]
pub fn maturin(&self) -> Option<&ToolMaturin> {
Expand Down
14 changes: 14 additions & 0 deletions test-crates/pyo3-mixed-src/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# pyo3-mixed src layout

A package for testing maturin with a src layout mixed pyo3/python project.

## Usage

```bash
pip install .
```

```python
import pyo3_mixed_src
assert pyo3_mixed_src.get_42() == 42
```
7 changes: 7 additions & 0 deletions test-crates/pyo3-mixed-src/check_installed/check_installed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env python3

import pyo3_mixed_src as pyo3_mixed

assert pyo3_mixed.get_42() == 42

print("SUCCESS")
14 changes: 14 additions & 0 deletions test-crates/pyo3-mixed-src/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[build-system]
requires = ["maturin>=0.13,<0.14"]
build-backend = "maturin"

[project]
name = "pyo3-mixed-src"
classifiers = [
"Programming Language :: Python",
"Programming Language :: Rust"
]
requires-python = ">=3.7"

[project.scripts]
get_42 = "pyo3_mixed_src:get_42"
Loading