Skip to content

Commit

Permalink
Warn on missing python-source contents
Browse files Browse the repository at this point in the history
Warns when the user explicitly set `python-source` but that folder doesn't contain a python module.

This came up in ruff (astral-sh/ruff#4397, astral-sh/ruff#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.
  • Loading branch information
konstin committed May 13, 2023
1 parent 243b8ec commit 2e9b861
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 4 deletions.
25 changes: 21 additions & 4 deletions src/project_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<Path>,
project_root: &Path,
module_name: &str,
python_root: PathBuf,
python_packages: Vec<String>,
data: Option<PathBuf>,
custom_python_source: bool,
) -> 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_module, rust_module, extension_name) = if parts.len() > 1 {
let mut rust_module = python_root.clone();
rust_module.extend(&parts[0..parts.len() - 1]);
Expand All @@ -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"
);

Expand Down Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions test-crates/wrong-python-source/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions test-crates/wrong-python-source/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "wrong-python-source"
version = "0.1.0"
edition = "2021"

[[bin]]
name = "run_this"

[dependencies]
7 changes: 7 additions & 0 deletions test-crates/wrong-python-source/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[tool.maturin]
bindings = "bin"
python-source = "python"

[build-system]
requires = ["maturin>=0.15,<0.16"]
build-backend = "maturin"
Empty file.
1 change: 1 addition & 0 deletions test-crates/wrong-python-source/src/bin/run_this.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
1 change: 1 addition & 0 deletions test-crates/wrong-python-source/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

28 changes: 28 additions & 0 deletions tests/common/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(())
}
5 changes: 5 additions & 0 deletions tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 2e9b861

Please sign in to comment.