Skip to content

Commit

Permalink
Merge pull request #1043 from messense/refactor-manifest-path
Browse files Browse the repository at this point in the history 
Fix inconsistent `Cargo.toml` and `pyproject.toml` path handling
  • Loading branch information
@messense
messense authored Aug 7, 2022
2 parents 087d364 + 7721cde commit 3cc99ae
Show file tree
Hide file tree
Showing 8 changed files with 292 additions and 224 deletions.
1 change: 1 addition & 0 deletions Changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Add Linux 32-bit powerpc architecture support in [#1026](https://github.com/PyO3/maturin/pull/1026)
* Add Linux sparc64 architecture support in [#1027](https://github.com/PyO3/maturin/pull/1027)
* Add PEP 440 local version identifier support in [#1037](https://github.com/PyO3/maturin/pull/1037)
* Fix inconsistent `Cargo.toml` and `pyproject.toml` path handling in [#1043](https://github.com/PyO3/maturin/pull/1043)

## [0.13.1] - 2022-07-26

Expand Down
97 changes: 6 additions & 91 deletions src/build_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ use crate::compile::warn_missing_py_init;
use crate::module_writer::{
add_data, write_bin, write_bindings_module, write_cffi_module, write_python_part, WheelWriter,
};
use crate::project_layout::ProjectLayout;
use crate::source_distribution::source_distribution;
use crate::{compile, Metadata21, ModuleWriter, PyProjectToml, PythonInterpreter, Target};
use anyhow::{anyhow, bail, Context, Result};
use cargo_metadata::Metadata;
use fs_err as fs;
use lddtree::Library;
use sha2::{Digest, Sha256};
use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use std::fmt::{Display, Formatter};
use std::io;
Expand Down Expand Up @@ -72,93 +72,6 @@ impl Display for BridgeModel {
}
}

/// Whether this project is pure rust or rust mixed with python and whether it has wheel data
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ProjectLayout {
/// Contains the canonicalized (i.e. absolute) path to the python part of the project
/// If none, we have a rust crate compiled into a shared library with only some glue python for cffi
/// If some, we have a python package that is extended by a native rust module.
pub python_module: Option<PathBuf>,
/// Contains the canonicalized (i.e. absolute) path to the rust part of the project
pub rust_module: PathBuf,
/// rust extension name
pub extension_name: String,
/// The location of the wheel data, if any
pub data: Option<PathBuf>,
}

impl ProjectLayout {
/// Checks whether a python module exists besides Cargo.toml with the right name
pub fn determine(
project_root: impl AsRef<Path>,
module_name: &str,
py_src: Option<impl AsRef<Path>>,
data: Option<impl AsRef<Path>>,
) -> Result<ProjectLayout> {
// A dot in the module name means the extension module goes into the module folder specified by the path
let parts: Vec<&str> = module_name.split('.').collect();
let project_root = project_root.as_ref();
let python_root = py_src.map_or(Cow::Borrowed(project_root), |py_src| {
Cow::Owned(project_root.join(py_src))
});
let (python_module, rust_module, extension_name) = if parts.len() > 1 {
let mut rust_module = python_root.to_path_buf();
rust_module.extend(&parts[0..parts.len() - 1]);
(
python_root.join(parts[0]),
rust_module,
parts[parts.len() - 1].to_string(),
)
} else {
(
python_root.join(module_name),
python_root.join(module_name),
module_name.to_string(),
)
};

let data = if let Some(data) = data {
let data = if data.as_ref().is_absolute() {
data.as_ref().to_path_buf()
} else {
project_root.join(data)
};
if !data.is_dir() {
bail!("No such data directory {}", data.display());
}
Some(data)
} else if project_root.join(format!("{}.data", module_name)).is_dir() {
Some(project_root.join(format!("{}.data", module_name)))
} else {
None
};

if python_module.is_dir() {
if !python_module.join("__init__.py").is_file()
&& !python_module.join("__init__.pyi").is_file()
{
bail!("Found a directory with the module name ({}) next to Cargo.toml, which indicates a mixed python/rust project, but the directory didn't contain an __init__.py file.", module_name)
}

println!("🍹 Building a mixed python/rust project");

Ok(ProjectLayout {
python_module: Some(python_module),
rust_module,
extension_name,
data,
})
} else {
Ok(ProjectLayout {
python_module: None,
rust_module: project_root.to_path_buf(),
extension_name,
data,
})
}
}
}

/// Contains all the metadata required to build the crate
#[derive(Clone)]
pub struct BuildContext {
Expand All @@ -168,6 +81,8 @@ pub struct BuildContext {
pub bridge: BridgeModel,
/// Whether this project is pure rust or rust mixed with python
pub project_layout: ProjectLayout,
/// Parsed project.toml if any
pub pyproject_toml: Option<PyProjectToml>,
/// Python Package Metadata 2.1
pub metadata21: Metadata21,
/// The name of the crate
Expand Down Expand Up @@ -260,8 +175,8 @@ impl BuildContext {
.context("Failed to create the target directory for the source distribution")?;

let include_cargo_lock = self.cargo_options.locked || self.cargo_options.frozen;
match PyProjectToml::new(self.manifest_path.parent().unwrap()) {
Ok(pyproject) => {
match self.pyproject_toml.as_ref() {
Some(pyproject) => {
let sdist_path = source_distribution(
&self.out,
&self.metadata21,
Expand All @@ -274,7 +189,7 @@ impl BuildContext {
.context("Failed to build source distribution")?;
Ok(Some((sdist_path, "source".to_string())))
}
Err(_) => Ok(None),
None => Ok(None),
}
}

Expand Down
127 changes: 25 additions & 102 deletions src/build_options.rs
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
use crate::auditwheel::PlatformTag;
use crate::build_context::{BridgeModel, ProjectLayout};
use crate::build_context::BridgeModel;
use crate::cross_compile::{find_sysconfigdata, parse_sysconfigdata};
use crate::project_layout::ProjectResolver;
use crate::pyproject_toml::ToolMaturin;
use crate::python_interpreter::{InterpreterConfig, InterpreterKind, MINIMUM_PYTHON_MINOR};
use crate::BuildContext;
use crate::CargoToml;
use crate::Metadata21;
use crate::PyProjectToml;
use crate::PythonInterpreter;
use crate::Target;
use crate::{BuildContext, Metadata21, PythonInterpreter, Target};
use anyhow::{bail, format_err, Context, Result};
use cargo_metadata::{Metadata, MetadataCommand, Node};
use regex::Regex;
Expand All @@ -17,7 +13,7 @@ use std::collections::{HashMap, HashSet};
use std::env;
use std::io;
use std::ops::{Deref, DerefMut};
use std::path::{Path, PathBuf};
use std::path::PathBuf;

// This is used for BridgeModel::Bindings("pyo3-ffi") and BridgeModel::Bindings("pyo3").
// These should be treated almost identically but must be correctly identified
Expand Down Expand Up @@ -213,45 +209,6 @@ impl DerefMut for BuildOptions {
}

impl BuildOptions {
/// Get cargo manifest file path
fn manifest_path(&self) -> Result<PathBuf> {
// use command line argument if specified
if let Some(path) = &self.manifest_path {
return Ok(path.clone());
}
// check `manifest-path` option in pyproject.toml
let current_dir = env::current_dir()
.context("Failed to detect current directory ಠ_ಠ")?
.canonicalize()?;
if current_dir.join("pyproject.toml").is_file() {
let pyproject =
PyProjectToml::new(&current_dir).context("pyproject.toml is invalid")?;
if let Some(path) = pyproject.manifest_path() {
println!("🔗 Found cargo manifest path in pyproject.toml");
// pyproject.toml must be placed at top directory
let manifest_dir = path
.parent()
.context("missing parent directory")?
.canonicalize()?;
if !manifest_dir.starts_with(&current_dir) {
bail!("Cargo.toml can not be placed outside of the directory containing pyproject.toml");
}
return Ok(path.to_path_buf());
}
}
// check Cargo.toml in current directory
let path = PathBuf::from("Cargo.toml");
if path.exists() {
Ok(path)
} else {
Err(format_err!(
"Can't find {} (in {})",
path.display(),
current_dir.display()
))
}
}

/// Finds the appropriate amount for python versions for each [BridgeModel].
fn find_interpreters(
&self,
Expand Down Expand Up @@ -518,60 +475,17 @@ impl BuildOptions {
strip: bool,
editable: bool,
) -> Result<BuildContext> {
let manifest_file = self.manifest_path()?;
if !manifest_file.is_file() {
bail!(
"{} is not the path to a Cargo.toml",
manifest_file.display()
);
}

let cargo_toml = CargoToml::from_path(&manifest_file)?;
let manifest_dir = manifest_file.parent().unwrap();
let pyproject: Option<PyProjectToml> = if manifest_dir.join("pyproject.toml").is_file() {
let pyproject =
PyProjectToml::new(manifest_dir).context("pyproject.toml is invalid")?;
pyproject.warn_missing_maturin_version();
pyproject.warn_missing_build_backend();
Some(pyproject)
} else {
None
};
let pyproject = pyproject.as_ref();
let ProjectResolver {
project_layout,
cargo_toml_path,
cargo_toml,
pyproject_toml,
module_name,
metadata21,
} = ProjectResolver::resolve(self.manifest_path.clone())?;
let pyproject = pyproject_toml.as_ref();
let tool_maturin = pyproject.and_then(|p| p.maturin());

let metadata21 = Metadata21::from_cargo_toml(&cargo_toml, &manifest_dir)
.context("Failed to parse Cargo.toml into python metadata")?;
let extra_metadata = cargo_toml.remaining_core_metadata();

let crate_name = &cargo_toml.package.name;

// If the package name contains minuses, you must declare a module with
// underscores as lib name
let module_name = cargo_toml
.lib
.as_ref()
.and_then(|lib| lib.name.as_ref())
.unwrap_or(crate_name)
.to_owned();

// Only use extension name from extra metadata if it contains dot
let extension_name = extra_metadata
.name
.as_ref()
.filter(|name| name.contains('.'))
.unwrap_or(&module_name);

let data = pyproject
.and_then(|x| x.data())
.or_else(|| extra_metadata.data.as_ref().map(Path::new));
let project_layout = ProjectLayout::determine(
manifest_dir,
extension_name,
extra_metadata.python_source.as_deref(),
data,
)?;

let mut cargo_options = self.cargo.clone();

let mut args_from_pyproject = if let Some(tool_maturin) = tool_maturin {
Expand All @@ -583,7 +497,7 @@ impl BuildOptions {
let cargo_metadata_extra_args = extract_cargo_metadata_args(&cargo_options)?;

let result = MetadataCommand::new()
.manifest_path(&manifest_file)
.manifest_path(&cargo_toml_path)
.other_options(cargo_metadata_extra_args)
.exec();

Expand Down Expand Up @@ -782,15 +696,17 @@ impl BuildOptions {
.target_dir
.clone()
.unwrap_or_else(|| cargo_metadata.target_directory.clone().into_std_path_buf());
let crate_name = cargo_toml.package.name;

Ok(BuildContext {
target,
bridge,
project_layout,
pyproject_toml,
metadata21,
crate_name: crate_name.to_string(),
crate_name,
module_name,
manifest_path: manifest_file,
manifest_path: cargo_toml_path,
target_dir,
out: wheel_dir,
release,
Expand Down Expand Up @@ -1204,6 +1120,11 @@ impl CargoOptions {
fn merge_with_pyproject_toml(&mut self, tool_maturin: ToolMaturin) -> Vec<&'static str> {
let mut args_from_pyproject = Vec::new();

if self.manifest_path.is_none() && tool_maturin.manifest_path.is_some() {
self.manifest_path = tool_maturin.manifest_path.clone();
args_from_pyproject.push("manifest-path");
}

if self.profile.is_none() && tool_maturin.profile.is_some() {
self.profile = tool_maturin.profile.clone();
args_from_pyproject.push("profile");
Expand Down Expand Up @@ -1422,6 +1343,8 @@ mod test {

#[test]
fn test_get_min_python_minor() {
use crate::CargoToml;

// Nothing specified
let cargo_toml = CargoToml::from_path("test-crates/pyo3-pure/Cargo.toml").unwrap();
let metadata21 =
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ mod develop;
mod metadata;
mod module_writer;
mod new_project;
mod project_layout;
mod pyproject_toml;
mod python_interpreter;
mod source_distribution;
Expand Down
Loading

0 comments on commit 3cc99ae

Please sign in to comment.