diff --git a/maturin.schema.json b/maturin.schema.json index 8cd66e4fe..5b4efe9e4 100644 --- a/maturin.schema.json +++ b/maturin.schema.json @@ -24,9 +24,13 @@ }, "bindings": { "description": "Bindings type", - "type": [ - "string", - "null" + "anyOf": [ + { + "$ref": "#/$defs/Bindings" + }, + { + "type": "null" + } ] }, "compatibility": { @@ -272,6 +276,36 @@ } ] }, + "Bindings": { + "description": "The kind of bindings to use.", + "oneOf": [ + { + "description": "PyO3 bindings", + "type": "string", + "const": "pyo3" + }, + { + "description": "pyo3-ffi (raw FFI) bindings", + "type": "string", + "const": "pyo3-ffi" + }, + { + "description": "CFFI bindings", + "type": "string", + "const": "cffi" + }, + { + "description": "UniFFI bindings", + "type": "string", + "const": "uniffi" + }, + { + "description": "Rust binary", + "type": "string", + "const": "bin" + } + ] + }, "CargoCrateType": { "description": "Supported cargo crate types", "oneOf": [ diff --git a/src/bridge/detection.rs b/src/bridge/detection.rs index 8eae29673..ec83288cc 100644 --- a/src/bridge/detection.rs +++ b/src/bridge/detection.rs @@ -5,7 +5,7 @@ //! whether abi3 is enabled, and whether `generate-import-lib` is active. use super::{ - ABI3T_MINIMUM_PYTHON_MINOR, BridgeModel, PyO3, PyO3Crate, PyO3MetadataRaw, StableAbi, + ABI3T_MINIMUM_PYTHON_MINOR, Bindings, BridgeModel, PyO3, PyO3Crate, PyO3MetadataRaw, StableAbi, StableAbiKind, StableAbiVersion, }; use crate::pyproject_toml::{FeatureConditionEnv, FeatureSpec}; @@ -29,7 +29,7 @@ const PYO3_BINDING_CRATES: [PyO3Crate; 2] = [PyO3Crate::PyO3Ffi, PyO3Crate::PyO3 /// to evaluate them. pub fn find_bridge( cargo_metadata: &Metadata, - bridge: Option<&str>, + bridge: Option, cargo_options: &CargoOptions, ) -> Result { let deps = CrateDependencies::resolve(cargo_metadata, cargo_options)?; @@ -40,7 +40,7 @@ pub fn find_bridge( /// also need [`CrateDependencies`] elsewhere resolve it only once. pub fn find_bridge_with_deps( cargo_metadata: &Metadata, - bridge: Option<&str>, + bridge: Option, deps: &CrateDependencies, ) -> Result { let no_extra_features = HashMap::new(); @@ -66,17 +66,18 @@ pub fn find_bridge_with_deps( .collect(); let bridge = if let Some(bindings) = bridge { - if bindings == "cffi" { - BridgeModel::Cffi - } else if bindings == "uniffi" { - BridgeModel::UniFfi - } else if bindings == "bin" { - let bindings = find_pyo3_bindings(cargo_metadata, deps)?; - BridgeModel::Bin(bindings) - } else { - let bindings = - find_pyo3_bindings(cargo_metadata, deps)?.context("unknown binding type")?; - BridgeModel::PyO3(bindings) + match bindings { + Bindings::Cffi => BridgeModel::Cffi, + Bindings::UniFfi => BridgeModel::UniFfi, + Bindings::Bin => { + let bindings = find_pyo3_bindings(cargo_metadata, deps)?; + BridgeModel::Bin(bindings) + } + Bindings::PyO3 | Bindings::PyO3Ffi => { + let bindings = + find_pyo3_bindings(cargo_metadata, deps)?.context("unknown binding type")?; + BridgeModel::PyO3(bindings) + } } } else { match find_pyo3_bindings(cargo_metadata, deps)? { diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index f6d869c2c..9f42bb1c0 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -8,7 +8,7 @@ pub use detection::{ use std::{fmt, str::FromStr}; use anyhow::Context; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use crate::python_interpreter::{ MAXIMUM_PYPY_MINOR, MAXIMUM_PYTHON_MINOR, MINIMUM_PYPY_MINOR, MINIMUM_PYTHON_MINOR, @@ -61,6 +61,178 @@ impl FromStr for PyO3Crate { } } +/// The `bindings` input spelling accepted on the CLI (`--bindings`) and in +/// `[tool.maturin] bindings`. +/// +/// This is the raw, user-facing choice of binding model, kept deliberately +/// distinct from the resolved [`BridgeModel`]: auto-detection and resolved PyO3 +/// metadata are semantic build states, whereas this enum only names the +/// accepted input spellings. [`find_bridge`] converts it into a [`BridgeModel`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Bindings { + /// pyo3 + PyO3, + /// pyo3-ffi + PyO3Ffi, + /// cffi + Cffi, + /// uniffi + UniFfi, + /// bin + Bin, +} + +impl Bindings { + /// All accepted bindings, in declaration order. + pub const ALL: [Bindings; 5] = [ + Bindings::PyO3, + Bindings::PyO3Ffi, + Bindings::Cffi, + Bindings::UniFfi, + Bindings::Bin, + ]; + + /// The accepted spellings as strings, derived from [`Bindings::as_str`]. + /// + /// This is the single list handed to serde, clap and schemars when they + /// need to enumerate or reject variants. + pub const VARIANTS: [&'static str; 5] = [ + Bindings::PyO3.as_str(), + Bindings::PyO3Ffi.as_str(), + Bindings::Cffi.as_str(), + Bindings::UniFfi.as_str(), + Bindings::Bin.as_str(), + ]; + + /// Returns the canonical spelling of this bindings type. + /// + /// This is the one place each spelling is written; `FromStr`, `Display`, + /// serde, clap and schemars all derive from it. + pub const fn as_str(self) -> &'static str { + match self { + Bindings::PyO3 => "pyo3", + Bindings::PyO3Ffi => "pyo3-ffi", + Bindings::Cffi => "cffi", + Bindings::UniFfi => "uniffi", + Bindings::Bin => "bin", + } + } + + /// A one-line human description of this bindings type, used as the + /// per-variant documentation in the generated JSON schema. + pub const fn description(self) -> &'static str { + match self { + Bindings::PyO3 => "PyO3 bindings", + Bindings::PyO3Ffi => "pyo3-ffi (raw FFI) bindings", + Bindings::Cffi => "CFFI bindings", + Bindings::UniFfi => "UniFFI bindings", + Bindings::Bin => "Rust binary", + } + } +} + +impl fmt::Display for Bindings { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl FromStr for Bindings { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + Bindings::ALL + .into_iter() + .find(|binding| binding.as_str() == s) + .with_context(|| { + format!( + "unknown bindings type `{s}`, expected one of {}", + Bindings::VARIANTS.join(", ") + ) + }) + } +} + +impl Serialize for Bindings { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(self.as_str()) + } +} + +impl<'de> Deserialize<'de> for Bindings { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct BindingsVisitor; + + impl serde::de::Visitor<'_> for BindingsVisitor { + type Value = Bindings; + + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("a bindings type") + } + + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + Bindings::ALL + .into_iter() + .find(|binding| binding.as_str() == value) + .ok_or_else(|| E::unknown_variant(value, &Bindings::VARIANTS)) + } + } + + deserializer.deserialize_str(BindingsVisitor) + } +} + +impl clap::ValueEnum for Bindings { + fn value_variants<'a>() -> &'a [Self] { + &Bindings::ALL + } + + fn to_possible_value(&self) -> Option { + Some(clap::builder::PossibleValue::new(self.as_str())) + } +} + +#[cfg(feature = "schemars")] +impl schemars::JsonSchema for Bindings { + fn schema_name() -> std::borrow::Cow<'static, str> { + "Bindings".into() + } + + fn schema_id() -> std::borrow::Cow<'static, str> { + "maturin::Bindings".into() + } + + fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { + // Document the string form (e.g. "pyo3", "cffi") so the schema matches + // serde Deserialize/Serialize rather than an externally-tagged object. + // Emit one `const` per variant with a description, mirroring the house + // style of the other documented string enums (e.g. `CargoCrateType`). + let one_of: Vec = Bindings::ALL + .iter() + .map(|binding| { + serde_json::json!({ + "description": binding.description(), + "type": "string", + "const": binding.as_str(), + }) + }) + .collect(); + schemars::json_schema!({ + "description": "The kind of bindings to use.", + "oneOf": one_of, + }) + } +} + #[derive(Debug, Clone, Deserialize)] pub struct PyO3VersionMetadataRaw { #[serde(rename = "min-version")] @@ -439,4 +611,34 @@ mod tests { assert_eq!(abi3t.kind, StableAbiKind::Abi3t); assert_eq!(abi3t.version, StableAbiVersion::Version(3, 15)); } + + #[test] + fn bindings_spellings_roundtrip() { + // The exact accepted spellings, in the historical order. These are a + // public contract (CLI values, `[tool.maturin] bindings`, JSON schema) + // and are not something the type can guarantee: only pinning the + // literals catches an accidental edit to `as_str`. Hardcoded here (not + // derived from `as_str`/`VARIANTS`) so the round-trip is meaningful. + let spellings = [ + (Bindings::PyO3, "pyo3"), + (Bindings::PyO3Ffi, "pyo3-ffi"), + (Bindings::Cffi, "cffi"), + (Bindings::UniFfi, "uniffi"), + (Bindings::Bin, "bin"), + ]; + for (binding, spelling) in spellings { + assert_eq!(binding.as_str(), spelling); + assert_eq!(spelling.parse::().unwrap(), binding); + } + assert_eq!(Bindings::VARIANTS, spellings.map(|(_, spelling)| spelling)); + } + + #[test] + fn bindings_fromstr_rejects_unknown() { + let err = "foo".parse::().unwrap_err().to_string(); + assert_eq!( + err, + "unknown bindings type `foo`, expected one of pyo3, pyo3-ffi, cffi, uniffi, bin" + ); + } } diff --git a/src/build_context/builder.rs b/src/build_context/builder.rs index 5e9a44ca5..9941219c3 100644 --- a/src/build_context/builder.rs +++ b/src/build_context/builder.rs @@ -104,7 +104,7 @@ impl BuildContextBuilder { )?; let pyproject = pyproject_toml.as_ref(); - let bindings = build_options.python.bindings.as_deref().or_else(|| { + let bindings = build_options.python.bindings.or_else(|| { pyproject.and_then(|x| { if x.bindings().is_some() { pyproject_toml_maturin_options.push("bindings"); diff --git a/src/build_options.rs b/src/build_options.rs index 6b2affdad..d8643c6ed 100644 --- a/src/build_options.rs +++ b/src/build_options.rs @@ -1,4 +1,5 @@ use crate::auditwheel::{AuditWheelMode, CompatibilityTag}; +use crate::bridge::Bindings; use crate::build_context::BuildContextBuilder; pub use crate::cargo_options::{CargoOptions, TargetTriple}; use crate::compression::CompressionOptions; @@ -22,8 +23,8 @@ pub struct PythonOptions { pub find_interpreter: bool, /// Which kind of bindings to use. - #[arg(short, long, value_parser = ["pyo3", "pyo3-ffi", "cffi", "uniffi", "bin"])] - pub bindings: Option, + #[arg(short, long)] + pub bindings: Option, } /// Options for configuring platform tags and binary compatibility. @@ -141,7 +142,8 @@ impl BuildOptions { mod tests { use super::*; use crate::bridge::{ - CrateDependencies, PyO3, PyO3Crate, StableAbi, StableAbiKind, StableAbiVersion, find_bridge, + Bindings, CrateDependencies, PyO3, PyO3Crate, StableAbi, StableAbiKind, StableAbiVersion, + find_bridge, }; use crate::python_interpreter::InterpreterResolver; use crate::test_utils::test_crate_path; @@ -150,6 +152,41 @@ mod tests { use insta::assert_snapshot; use pretty_assertions::assert_eq; + #[test] + fn test_cli_bindings_parses_every_spelling() { + use clap::Parser; + + // `--bindings ` accepts each valid spelling and maps it to the + // matching `Bindings` variant. + for binding in Bindings::ALL { + let opts = + PythonOptions::try_parse_from(["prog", "--bindings", binding.as_str()]).unwrap(); + assert_eq!(opts.bindings, Some(binding)); + } + + // Omitting `--bindings` leaves auto-detection untouched (None). + let opts = PythonOptions::try_parse_from(["prog"]).unwrap(); + assert_eq!(opts.bindings, None); + } + + #[test] + fn test_cli_bindings_rejects_unknown() { + use clap::Parser; + + let err = PythonOptions::try_parse_from(["prog", "--bindings", "foo"]).unwrap_err(); + let rendered = err.to_string(); + // Same diagnostic quality as the previous `value_parser = [...]`: the + // invalid value plus the list of accepted spellings. + assert!( + rendered.contains("invalid value 'foo' for '--bindings '"), + "unexpected error: {rendered}" + ); + assert!( + rendered.contains("[possible values: pyo3, pyo3-ffi, cffi, uniffi, bin]"), + "unexpected error: {rendered}" + ); + } + #[test] fn test_find_bridge_pyo3() { let pyo3_mixed = MetadataCommand::new() @@ -162,7 +199,7 @@ mod tests { Ok(BridgeModel::PyO3 { .. }) )); assert!(matches!( - find_bridge(&pyo3_mixed, Some("pyo3"), &CargoOptions::default()), + find_bridge(&pyo3_mixed, Some(Bindings::PyO3), &CargoOptions::default()), Ok(BridgeModel::PyO3 { .. }) )); } @@ -196,7 +233,7 @@ mod tests { bridge ); assert_eq!( - find_bridge(&pyo3_pure, Some("pyo3"), &CargoOptions::default()).unwrap(), + find_bridge(&pyo3_pure, Some(Bindings::PyO3), &CargoOptions::default()).unwrap(), bridge ); } @@ -242,7 +279,7 @@ mod tests { assert_eq!(pyo3.crate_name, PyO3Crate::PyO3); let bridge_explicit = - find_bridge(&pyo3_abi3t, Some("pyo3"), &CargoOptions::default()).unwrap(); + find_bridge(&pyo3_abi3t, Some(Bindings::PyO3), &CargoOptions::default()).unwrap(); assert_eq!(bridge_explicit, bridge); } @@ -490,7 +527,7 @@ mod tests { .unwrap(); assert_eq!( - find_bridge(&cffi_pure, Some("cffi"), &CargoOptions::default()).unwrap(), + find_bridge(&cffi_pure, Some(Bindings::Cffi), &CargoOptions::default()).unwrap(), BridgeModel::Cffi ); assert_eq!( @@ -498,7 +535,7 @@ mod tests { BridgeModel::Cffi ); - assert!(find_bridge(&cffi_pure, Some("pyo3"), &CargoOptions::default()).is_err()); + assert!(find_bridge(&cffi_pure, Some(Bindings::PyO3), &CargoOptions::default()).is_err()); } #[test] @@ -509,7 +546,7 @@ mod tests { .unwrap(); assert_eq!( - find_bridge(&hello_world, Some("bin"), &CargoOptions::default()).unwrap(), + find_bridge(&hello_world, Some(Bindings::Bin), &CargoOptions::default()).unwrap(), BridgeModel::Bin(None) ); assert_eq!( @@ -517,14 +554,14 @@ mod tests { BridgeModel::Bin(None) ); - assert!(find_bridge(&hello_world, Some("pyo3"), &CargoOptions::default()).is_err()); + assert!(find_bridge(&hello_world, Some(Bindings::PyO3), &CargoOptions::default()).is_err()); let pyo3_bin = MetadataCommand::new() .manifest_path(test_crate_path("pyo3-bin").join("Cargo.toml")) .exec() .unwrap(); assert!(matches!( - find_bridge(&pyo3_bin, Some("bin"), &CargoOptions::default()).unwrap(), + find_bridge(&pyo3_bin, Some(Bindings::Bin), &CargoOptions::default()).unwrap(), BridgeModel::Bin(Some(_)) )); assert!(matches!( @@ -543,7 +580,12 @@ mod tests { .exec() .unwrap(); assert_eq!( - find_bridge(&workspace_bin, Some("bin"), &CargoOptions::default()).unwrap(), + find_bridge( + &workspace_bin, + Some(Bindings::Bin), + &CargoOptions::default() + ) + .unwrap(), BridgeModel::Bin(None) ); @@ -564,7 +606,7 @@ mod tests { ..Default::default() }; assert!(matches!( - find_bridge(&workspace_bin_python, Some("bin"), &cargo_options).unwrap(), + find_bridge(&workspace_bin_python, Some(Bindings::Bin), &cargo_options).unwrap(), BridgeModel::Bin(Some(_)) )); } diff --git a/src/develop/mod.rs b/src/develop/mod.rs index 3dff835a4..07e0edcbb 100644 --- a/src/develop/mod.rs +++ b/src/develop/mod.rs @@ -4,6 +4,7 @@ use crate::PlatformTag; use crate::PythonInterpreter; use crate::Target; use crate::auditwheel::AuditWheelMode; +use crate::bridge::Bindings; use crate::build_options::CargoOptions; use crate::compression::CompressionOptions; use crate::target::detect_arch_from_python; @@ -29,13 +30,8 @@ use url::Url; #[derive(Debug, clap::Parser)] pub struct DevelopOptions { /// Which kind of bindings to use - #[arg( - short = 'b', - long = "bindings", - alias = "binding-crate", - value_parser = ["pyo3", "pyo3-ffi", "cffi", "uniffi", "bin"] - )] - pub bindings: Option, + #[arg(short = 'b', long = "bindings", alias = "binding-crate")] + pub bindings: Option, /// Pass --release to cargo #[arg(short = 'r', long, help_heading = heading::COMPILATION_OPTIONS, conflicts_with = "profile")] pub release: bool, diff --git a/src/lib.rs b/src/lib.rs index 4c681d720..ac11099c7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,7 +42,9 @@ #![deny(missing_docs)] -pub use crate::bridge::{BridgeModel, PyO3, PyO3Crate, StableAbi, StableAbiKind, StableAbiVersion}; +pub use crate::bridge::{ + Bindings, BridgeModel, PyO3, PyO3Crate, StableAbi, StableAbiKind, StableAbiVersion, +}; pub use crate::build_context::{ ArtifactContext, BuildContext, BuiltArtifactTag, BuiltWheel, ProjectContext, PythonContext, }; diff --git a/src/pyproject_toml.rs b/src/pyproject_toml.rs index 4e656bb4d..2b1c96f18 100644 --- a/src/pyproject_toml.rs +++ b/src/pyproject_toml.rs @@ -2,6 +2,7 @@ use crate::CompatibilityTag; use crate::auditwheel::AuditWheelMode; +use crate::bridge::Bindings; use anyhow::{Context, Result}; use fs_err as fs; use pep440_rs::{Version, VersionSpecifiers}; @@ -366,7 +367,7 @@ pub struct ToolMaturin { /// Patterns are resolved relative to the directory containing `pyproject.toml`. pub exclude: Option>, /// Bindings type - pub bindings: Option, + pub bindings: Option, /// Platform compatibility #[serde(alias = "manylinux")] pub compatibility: Option, @@ -590,8 +591,8 @@ impl PyProjectToml { } /// Returns the value of `[tool.maturin.bindings]` in pyproject.toml - pub fn bindings(&self) -> Option<&str> { - self.maturin()?.bindings.as_deref() + pub fn bindings(&self) -> Option { + self.maturin()?.bindings } /// Returns the PGO training command from `[tool.maturin]` @@ -779,7 +780,7 @@ impl PyProjectToml { #[cfg(test)] mod tests { use crate::{ - CompatibilityTag, PyProjectToml, + Bindings, CompatibilityTag, PyProjectToml, pyproject_toml::{ FeatureConditionEnv, FeatureSpec, Format, Formats, GlobPattern, ToolMaturin, }, @@ -1168,6 +1169,31 @@ mod tests { assert_eq!(maturin.compatibility, Some(CompatibilityTag::Pypi)); } + #[test] + fn test_bindings_deserializes_every_spelling() { + for binding in Bindings::ALL { + let toml_str = format!("bindings = {:?}\n", binding.as_str()); + let maturin: ToolMaturin = toml::from_str(&toml_str).unwrap(); + assert_eq!(maturin.bindings, Some(binding)); + } + + // Omitting `bindings` keeps auto-detection (None). + let maturin: ToolMaturin = toml::from_str("").unwrap(); + assert_eq!(maturin.bindings, None); + } + + #[test] + fn test_bindings_rejects_unknown() { + let err = toml::from_str::("bindings = \"foo\"\n").unwrap_err(); + // serde's default unknown-variant message names every accepted spelling. + assert!( + err.to_string().contains( + "unknown variant `foo`, expected one of `pyo3`, `pyo3-ffi`, `cffi`, `uniffi`, `bin`" + ), + "unexpected error: {err}" + ); + } + #[test] fn test_feature_spec_deserialize_with_implementation() { let toml_str = r#" diff --git a/tests/common/develop.rs b/tests/common/develop.rs index 730467440..25a3ef57f 100644 --- a/tests/common/develop.rs +++ b/tests/common/develop.rs @@ -103,7 +103,9 @@ pub fn test_develop(case: &DevelopCase<'_>) -> Result<()> { TestEnvKind::Conda { .. } => (case_target_dir(case.id), None), }; let develop_options = DevelopOptions { - bindings: case.bindings.map(|binding| binding.to_owned()), + bindings: case + .bindings + .map(|binding| binding.parse().expect("valid bindings spelling")), release: false, pgo: false, strip: false,