From 2e9b8617c33e3f390bd57cf272009d0f44feb821 Mon Sep 17 00:00:00 2001 From: konstin Date: Sat, 13 May 2023 01:16:45 +0200 Subject: [PATCH] Warn on missing python-source contents Warns when the user explicitly set `python-source` but that folder doesn't contain a python module. This came up in ruff (https://github.com/charliermarsh/ruff/pull/4397, https://github.com/charliermarsh/ruff/pull/4399), where the crate name is `ruff_cli` and i didn't realize this takes precedence as module name over the package name. The extra boolean isn't pretty but i wanted to avoid refactoring the entire method just for that. --- src/project_layout.rs | 25 ++++++++++++++--- test-crates/wrong-python-source/Cargo.lock | 7 +++++ test-crates/wrong-python-source/Cargo.toml | 9 ++++++ .../wrong-python-source/pyproject.toml | 7 +++++ .../python/run_this/__init__.py | 0 .../wrong-python-source/src/bin/run_this.rs | 1 + test-crates/wrong-python-source/src/lib.rs | 1 + tests/common/errors.rs | 28 +++++++++++++++++++ tests/run.rs | 5 ++++ 9 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 test-crates/wrong-python-source/Cargo.lock create mode 100644 test-crates/wrong-python-source/Cargo.toml create mode 100644 test-crates/wrong-python-source/pyproject.toml create mode 100644 test-crates/wrong-python-source/python/run_this/__init__.py create mode 100644 test-crates/wrong-python-source/src/bin/run_this.rs create mode 100644 test-crates/wrong-python-source/src/lib.rs diff --git a/src/project_layout.rs b/src/project_layout.rs index 63ee06603..03ae3612a 100644 --- a/src/project_layout.rs +++ b/src/project_layout.rs @@ -178,8 +178,15 @@ impl ProjectResolver { project_root.join(data) } }); - let project_layout = - ProjectLayout::determine(project_root, &module_name, py_root, python_packages, data)?; + let custom_python_source = pyproject.and_then(|x| x.python_source()).is_some(); + let project_layout = ProjectLayout::determine( + project_root, + &module_name, + py_root, + python_packages, + data, + custom_python_source, + )?; Ok(Self { project_layout, cargo_toml_path: manifest_file, @@ -336,15 +343,15 @@ impl ProjectResolver { impl ProjectLayout { /// Checks whether a python module exists besides Cargo.toml with the right name fn determine( - project_root: impl AsRef, + project_root: &Path, module_name: &str, python_root: PathBuf, python_packages: Vec, data: Option, + custom_python_source: bool, ) -> Result { // 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_module, rust_module, extension_name) = if parts.len() > 1 { let mut rust_module = python_root.clone(); rust_module.extend(&parts[0..parts.len() - 1]); @@ -366,6 +373,7 @@ impl ProjectLayout { rust_module = %rust_module.display(), python_module = %python_module.display(), extension_name = %extension_name, + module_name = %module_name, "Project layout resolved" ); @@ -398,6 +406,15 @@ impl ProjectLayout { data, }) } else { + if custom_python_source { + eprintln!( + "⚠️ Warning: You specified the python source as {}, but the python module at \ + {} is missing. No python module will be included.", + python_root.display(), + python_module.display() + ); + } + Ok(ProjectLayout { python_dir: python_root, python_packages, diff --git a/test-crates/wrong-python-source/Cargo.lock b/test-crates/wrong-python-source/Cargo.lock new file mode 100644 index 000000000..b60250be6 --- /dev/null +++ b/test-crates/wrong-python-source/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "wrong-python-source" +version = "0.1.0" diff --git a/test-crates/wrong-python-source/Cargo.toml b/test-crates/wrong-python-source/Cargo.toml new file mode 100644 index 000000000..855bd8413 --- /dev/null +++ b/test-crates/wrong-python-source/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "wrong-python-source" +version = "0.1.0" +edition = "2021" + +[[bin]] +name = "run_this" + +[dependencies] diff --git a/test-crates/wrong-python-source/pyproject.toml b/test-crates/wrong-python-source/pyproject.toml new file mode 100644 index 000000000..6559419d5 --- /dev/null +++ b/test-crates/wrong-python-source/pyproject.toml @@ -0,0 +1,7 @@ +[tool.maturin] +bindings = "bin" +python-source = "python" + +[build-system] +requires = ["maturin>=0.15,<0.16"] +build-backend = "maturin" diff --git a/test-crates/wrong-python-source/python/run_this/__init__.py b/test-crates/wrong-python-source/python/run_this/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test-crates/wrong-python-source/src/bin/run_this.rs b/test-crates/wrong-python-source/src/bin/run_this.rs new file mode 100644 index 000000000..f328e4d9d --- /dev/null +++ b/test-crates/wrong-python-source/src/bin/run_this.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/test-crates/wrong-python-source/src/lib.rs b/test-crates/wrong-python-source/src/lib.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/test-crates/wrong-python-source/src/lib.rs @@ -0,0 +1 @@ + diff --git a/tests/common/errors.rs b/tests/common/errors.rs index d1d095492..4d3b38c99 100644 --- a/tests/common/errors.rs +++ b/tests/common/errors.rs @@ -3,6 +3,9 @@ use anyhow::{bail, Result}; use clap::Parser; use maturin::BuildOptions; use pretty_assertions::assert_eq; +use std::path::Path; +use std::process::Command; +use std::str; pub fn abi3_without_version() -> Result<()> { // The first argument is ignored by clap @@ -129,3 +132,28 @@ pub fn invalid_manylinux_does_not_panic() -> Result<()> { Ok(()) } + +/// The user set `python-source` in pyproject.toml, but there is no python module in there +pub fn warn_on_missing_python_source() -> Result<()> { + let output = Command::new(env!("CARGO_BIN_EXE_maturin")) + .arg("build") + .arg("-m") + .arg( + Path::new("test-crates") + .join("wrong-python-source") + .join("Cargo.toml"), + ) + .output() + .unwrap(); + if !output.status.success() { + bail!( + "Failed to run: {}\n---stdout:\n{}---stderr:\n{}", + output.status, + str::from_utf8(&output.stdout)?, + str::from_utf8(&output.stderr)? + ); + } + + assert!(str::from_utf8(&output.stderr)?.contains("Warning: You specified the python source as")); + Ok(()) +} diff --git a/tests/run.rs b/tests/run.rs index bbdc4ffac..b71456ec9 100644 --- a/tests/run.rs +++ b/tests/run.rs @@ -406,6 +406,11 @@ fn invalid_manylinux_does_not_panic() { handle_result(errors::invalid_manylinux_does_not_panic()) } +#[test] +fn warn_on_missing_python_source() { + handle_result(errors::warn_on_missing_python_source()) +} + #[test] #[cfg_attr(not(target_os = "linux"), ignore)] fn musl() {