Fix interpreter selection for abi3 bindings (#2392)

Include bindings crate version info in `BridgeModel::BindingsAbi3` so we
can determine minimal pypy version.

.github/workflows/test.yml

@@ -12,7 +12,7 @@ on:
   workflow_dispatch:
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.ref_name }}-${{ github.event.action }}-${{ github.event.pull_request.number }}
+  group: ${{ github.workflow }}-${{ github.ref_name }}-${{ github.event.pull_request.number }}
   cancel-in-progress: true
 
 jobs:

src/bridge.rs

@@ -60,8 +60,14 @@ pub enum BridgeModel {
     Bindings(Bindings),
     /// `Bindings`, but specifically for pyo3 with feature flags that allow building a single wheel
     /// for all cpython versions (pypy & graalpy still need multiple versions).
-    /// The numbers are the minimum major and minor version
-    BindingsAbi3(u8, u8),
+    BindingsAbi3 {
+        /// The bindings crate
+        bindings: Bindings,
+        /// Minimal abi3 major version
+        major: u8,
+        /// Minimal abi3 minor version
+        minor: u8,
+    },
     /// A native module with c bindings, i.e. `#[no_mangle] extern "C" <some item>`
     Cffi,
     /// A native module generated from uniffi
@@ -74,6 +80,7 @@ impl BridgeModel {
         match self {
             BridgeModel::Bin(Some(bindings)) => Some(bindings),
             BridgeModel::Bindings(bindings) => Some(bindings),
+            BridgeModel::BindingsAbi3 { bindings, .. } => Some(bindings),
             _ => None,
         }
     }
@@ -82,6 +89,7 @@ impl BridgeModel {
     pub fn unwrap_bindings_name(&self) -> &str {
         match self {
             BridgeModel::Bindings(bindings) => &bindings.name,
+            BridgeModel::BindingsAbi3 { bindings, .. } => &bindings.name,
             _ => panic!("Expected Bindings"),
         }
     }
@@ -91,6 +99,7 @@ impl BridgeModel {
         match self {
             BridgeModel::Bin(Some(bindings)) => bindings.name == name,
             BridgeModel::Bindings(bindings) => bindings.name == name,
+            BridgeModel::BindingsAbi3 { bindings, .. } => bindings.name == name,
             _ => false,
         }
     }
@@ -107,7 +116,7 @@ impl Display for BridgeModel {
             BridgeModel::Bin(Some(bindings)) => write!(f, "{} bin", bindings.name),
             BridgeModel::Bin(None) => write!(f, "bin"),
             BridgeModel::Bindings(bindings) => write!(f, "{}", bindings.name),
-            BridgeModel::BindingsAbi3(..) => write!(f, "pyo3"),
+            BridgeModel::BindingsAbi3 { bindings, .. } => write!(f, "{}", bindings.name),
             BridgeModel::Cffi => write!(f, "cffi"),
             BridgeModel::UniFfi => write!(f, "uniffi"),
         }

src/build_context.rs

@@ -153,7 +153,7 @@ impl BuildContext {
             BridgeModel::Bin(None) => self.build_bin_wheel(None)?,
             BridgeModel::Bin(Some(..)) => self.build_bin_wheels(&self.interpreter)?,
             BridgeModel::Bindings { .. } => self.build_binding_wheels(&self.interpreter)?,
-            BridgeModel::BindingsAbi3(major, minor) => {
+            BridgeModel::BindingsAbi3 { major, minor, .. } => {
                 let abi3_interps: Vec<_> = self
                     .interpreter
                     .iter()

src/build_options.rs

@@ -354,7 +354,7 @@ impl BuildOptions {
                 Ok(vec![interpreter])
             }
             BridgeModel::Bin(None) | BridgeModel::UniFfi => Ok(vec![]),
-            BridgeModel::BindingsAbi3(major, minor) => {
+            BridgeModel::BindingsAbi3 { major, minor, .. } => {
                 let found_interpreters =
                     find_interpreter_in_host(bridge, interpreter, target, requires_python)
                         .or_else(|err| {
@@ -1143,7 +1143,16 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
 
             return if let Some((major, minor)) = has_abi3(cargo_metadata)? {
                 eprintln!("🔗 Found {lib} bindings with abi3 support for Python ≥ {major}.{minor}");
-                Ok(BridgeModel::BindingsAbi3(major, minor))
+                let version = packages[lib].version.clone();
+                let bindings = Bindings {
+                    name: lib.to_string(),
+                    version,
+                };
+                Ok(BridgeModel::BindingsAbi3 {
+                    bindings,
+                    major,
+                    minor,
+                })
             } else {
                 eprintln!("🔗 Found {lib} bindings");
                 Ok(bridge)
@@ -1512,14 +1521,16 @@ mod test {
             .exec()
             .unwrap();
 
-        assert!(matches!(
-            find_bridge(&pyo3_pure, None),
-            Ok(BridgeModel::BindingsAbi3(3, 7))
-        ));
-        assert!(matches!(
-            find_bridge(&pyo3_pure, Some("pyo3")),
-            Ok(BridgeModel::BindingsAbi3(3, 7))
-        ));
+        let bridge = BridgeModel::BindingsAbi3 {
+            bindings: Bindings {
+                name: "pyo3".to_string(),
+                version: semver::Version::new(0, 23, 3),
+            },
+            major: 3,
+            minor: 7,
+        };
+        assert_eq!(find_bridge(&pyo3_pure, None).unwrap(), bridge);
+        assert_eq!(find_bridge(&pyo3_pure, Some("pyo3")).unwrap(), bridge);
     }
 
     #[test]

src/ci.rs

@@ -169,14 +169,14 @@ impl GenerateCI {
         bridge_model: &BridgeModel,
         sdist: bool,
     ) -> Result<String> {
-        let is_abi3 = matches!(bridge_model, BridgeModel::BindingsAbi3(..));
+        let is_abi3 = matches!(bridge_model, BridgeModel::BindingsAbi3 { .. });
         let is_bin = bridge_model.is_bin();
         let setup_python = self.pytest
             || matches!(
                 bridge_model,
                 BridgeModel::Bin(Some(_))
                     | BridgeModel::Bindings { .. }
-                    | BridgeModel::BindingsAbi3(..)
+                    | BridgeModel::BindingsAbi3 { .. }
                     | BridgeModel::Cffi
                     | BridgeModel::UniFfi
             );
@@ -868,7 +868,18 @@ mod tests {
     #[test]
     fn test_generate_github_abi3() {
         let conf = GenerateCI::default()
-            .generate_github("example", &BridgeModel::BindingsAbi3(3, 7), false)
+            .generate_github(
+                "example",
+                &BridgeModel::BindingsAbi3 {
+                    bindings: Bindings {
+                        name: "pyo3".to_string(),
+                        version: Version::new(0, 23, 0),
+                    },
+                    major: 3,
+                    minor: 7,
+                },
+                false,
+            )
             .unwrap()
             .lines()
             .skip(5)
@@ -1071,7 +1082,18 @@ mod tests {
             skip_attestation: true,
             ..Default::default()
         }
-        .generate_github("example", &BridgeModel::BindingsAbi3(3, 7), false)
+        .generate_github(
+            "example",
+            &BridgeModel::BindingsAbi3 {
+                bindings: Bindings {
+                    name: "pyo3".to_string(),
+                    version: Version::new(0, 23, 0),
+                },
+                major: 3,
+                minor: 7,
+            },
+            false,
+        )
         .unwrap()
         .lines()
         .skip(5)

src/compile.rs

@@ -199,7 +199,7 @@ fn cargo_build_command(
         BridgeModel::Cffi
         | BridgeModel::UniFfi
         | BridgeModel::Bindings { .. }
-        | BridgeModel::BindingsAbi3(..) => {
+        | BridgeModel::BindingsAbi3 { .. } => {
             cargo_rustc.lib = true;
             // https://github.com/rust-lang/rust/issues/59302#issue-422994250
             // We must only do this for libraries as it breaks binaries
@@ -219,13 +219,13 @@ fn cargo_build_command(
 
     // https://github.com/PyO3/pyo3/issues/88#issuecomment-337744403
     if target.is_macos() {
-        if let BridgeModel::Bindings { .. } | BridgeModel::BindingsAbi3(..) = bridge_model {
+        if let BridgeModel::Bindings { .. } | BridgeModel::BindingsAbi3 { .. } = bridge_model {
             // Change LC_ID_DYLIB to the final .so name for macOS targets to avoid linking with
             // non-existent library.
             // See https://github.com/PyO3/setuptools-rust/issues/106 for detail
             let module_name = &context.module_name;
             let so_filename = match bridge_model {
-                BridgeModel::BindingsAbi3(..) => format!("{module_name}.abi3.so"),
+                BridgeModel::BindingsAbi3 { .. } => format!("{module_name}.abi3.so"),
                 _ => python_interpreter
                     .expect("missing python interpreter for non-abi3 wheel build")
                     .get_library_name(module_name),
@@ -390,7 +390,7 @@ fn cargo_build_command(
         build_command.env("CARGO_ENCODED_RUSTFLAGS", rustflags.encode()?);
     }
 
-    if let BridgeModel::BindingsAbi3(_, _) = bridge_model {
+    if let BridgeModel::BindingsAbi3 { .. } = bridge_model {
         let is_pypy_or_graalpy = python_interpreter
             .map(|p| p.interpreter_kind.is_pypy() || p.interpreter_kind.is_graalpy())
             .unwrap_or(false);
@@ -411,7 +411,7 @@ fn cargo_build_command(
         if interpreter.runnable {
             if bridge_model.is_bindings("pyo3")
                 || bridge_model.is_bindings("pyo3-ffi")
-                || matches!(bridge_model, BridgeModel::BindingsAbi3(_, _))
+                || matches!(bridge_model, BridgeModel::BindingsAbi3 { .. })
             {
                 debug!(
                     "Setting PYO3_PYTHON to {}",
@@ -429,7 +429,7 @@ fn cargo_build_command(
             build_command.env("PYTHON_SYS_EXECUTABLE", &interpreter.executable);
         } else if (bridge_model.is_bindings("pyo3")
             || bridge_model.is_bindings("pyo3-ffi")
-            || (matches!(bridge_model, BridgeModel::BindingsAbi3(_, _))
+            || (matches!(bridge_model, BridgeModel::BindingsAbi3 { .. })
                 && (interpreter.interpreter_kind.is_pypy()
                     || interpreter.interpreter_kind.is_graalpy())))
             && env::var_os("PYO3_CONFIG_FILE").is_none()

src/main.rs

@@ -285,7 +285,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
                 BridgeModel::Bindings(..) | BridgeModel::Bin(Some(..)) => {
                     vec![context.interpreter[0].get_tag(&context, &[PlatformTag::Linux])?]
                 }
-                BridgeModel::BindingsAbi3(major, minor) => {
+                BridgeModel::BindingsAbi3 { major, minor, .. } => {
                     let platform = context.get_platform_tag(&[PlatformTag::Linux])?;
                     vec![format!("cp{major}{minor}-abi3-{platform}")]
                 }