Skip to content

Commit

Permalink
Make maturin develop fail if version info is missing in `pyproject.…
Browse files Browse the repository at this point in the history
…toml` (#2418)

This PR implements the changes discussed in
#2416 (comment) on
top of #2417. Got this done faster than expected this morning, but won't
have any time to incorporate feedback for the next ~ two days. Feel free
to pull this in and butcher it any way you like. :)

Thanks again for your fantastic work on maturin! 🍰

---------

Co-authored-by: messense <[email protected]>
  • Loading branch information
mhils and messense authored Dec 30, 2024
1 parent 0e143c1 commit cd7828f
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 5 deletions.
10 changes: 10 additions & 0 deletions src/develop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,16 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
.editable(true)
.build()?;

// Ensure that version information is present, https://github.com/PyO3/maturin/issues/2416
if build_context
.pyproject_toml
.as_ref()
.is_some_and(|p| !p.warn_invalid_version_info())
{
bail!("Cannot build source distribution without valid version information. \
You need to specify either `project.version` or `project.dynamic = ['version']` in pyproject.toml.");
}

let interpreter =
PythonInterpreter::check_executable(&python, &target, build_context.bridge())?.ok_or_else(
|| anyhow!("Expected `python` to be a python interpreter inside a virtualenv ಠ_ಠ"),
Expand Down
15 changes: 10 additions & 5 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,18 @@ impl Metadata24 {
}

self.name.clone_from(&project.name);
if let Some(version) = &project.version {
if dynamic.contains("version") {
eprintln!("⚠️ Warning: `project.dynamic` must not specify `version` when `project.version` is present in pyproject.toml");

let version_ok = pyproject_toml.warn_invalid_version_info();
if !version_ok {
// This is a hard error for maturin>=2.0, see https://github.com/PyO3/maturin/issues/2416.
let current_major = env!("CARGO_PKG_VERSION_MAJOR").parse::<usize>().unwrap();
if current_major > 1 {
bail!("Invalid version information in pyproject.toml.");
}
}

if let Some(version) = &project.version {
self.version = version.clone();
} else if !dynamic.contains("version") {
eprintln!("⚠️ Warning: `project.version` field is required in pyproject.toml unless it is present in the `project.dynamic` list");
}

if let Some(description) = &project.description {
Expand Down
80 changes: 80 additions & 0 deletions src/pyproject_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,30 @@ impl PyProjectToml {
);
false
}

/// Having a pyproject.toml project table with neither `version` nor `dynamic = ['version']`
/// violates https://packaging.python.org/en/latest/specifications/pyproject-toml/#dynamic.
///
/// Returns true if version information is specified correctly or no project table is present.
pub fn warn_invalid_version_info(&self) -> bool {
let Some(project) = &self.project else {
return true;
};
let has_static_version = project.version.is_some();
let has_dynamic_version = project
.dynamic
.as_ref()
.is_some_and(|d| d.iter().any(|s| s == "version"));
if has_static_version && has_dynamic_version {
eprintln!("⚠️ Warning: `project.dynamic` must not specify `version` when `project.version` is present in pyproject.toml");
return false;
}
if !has_static_version && !has_dynamic_version {
eprintln!("⚠️ Warning: `project.version` field is required in pyproject.toml unless it is present in the `project.dynamic` list");
return false;
}
true
}
}

#[cfg(test)]
Expand Down Expand Up @@ -544,6 +568,62 @@ mod tests {
assert!(!without_constraint.warn_bad_maturin_version());
}

#[test]
fn test_warn_invalid_version_info_conflict() {
let conflict = toml::from_str::<PyProjectToml>(
r#"[build-system]
requires = ["maturin==1.0.0"]
[project]
name = "..."
version = "1.2.3"
dynamic = ['version']
"#,
)
.unwrap();
assert!(!conflict.warn_invalid_version_info());
}

#[test]
fn test_warn_invalid_version_info_missing() {
let missing = toml::from_str::<PyProjectToml>(
r#"[build-system]
requires = ["maturin==1.0.0"]
[project]
name = "..."
"#,
)
.unwrap();
assert!(!missing.warn_invalid_version_info());
}

#[test]
fn test_warn_invalid_version_info_ok() {
let static_ver = toml::from_str::<PyProjectToml>(
r#"[build-system]
requires = ["maturin==1.0.0"]
[project]
name = "..."
version = "1.2.3"
"#,
)
.unwrap();
assert!(static_ver.warn_invalid_version_info());
let dynamic_ver = toml::from_str::<PyProjectToml>(
r#"[build-system]
requires = ["maturin==1.0.0"]
[project]
name = "..."
dynamic = ['version']
"#,
)
.unwrap();
assert!(dynamic_ver.warn_invalid_version_info());
}

#[test]
fn deserialize_include_exclude() {
let single = r#"include = ["single"]"#;
Expand Down

0 comments on commit cd7828f

Please sign in to comment.