Clean up BindingGenerator interface

src/binding_generator/bin_binding.rs

@@ -5,12 +5,10 @@ use std::str::FromStr as _;
 use anyhow::Context;
 use anyhow::Result;
 use pep508_rs::Requirement;
-use tempfile::TempDir;
 
 use crate::BuildArtifact;
 use crate::BuildContext;
 use crate::Metadata24;
-use crate::PythonInterpreter;
 use crate::archive_source::ArchiveSource;
 use crate::archive_source::GeneratedSourceData;
 
@@ -32,10 +30,8 @@ impl<'m> BindingGenerator for BinBindingGenerator<'m> {
     fn generate_bindings(
         &mut self,
         context: &BuildContext,
-        _interpreter: Option<&PythonInterpreter>,
         artifact: &BuildArtifact,
         _module: &Path,
-        _temp_dir: &TempDir,
     ) -> Result<GeneratorOutput> {
         // I wouldn't know of any case where this would be the wrong (and neither do
         // I know a better alternative)

src/binding_generator/cffi_binding.rs

@@ -10,7 +10,6 @@ use std::str;
 
 use anyhow::Context as _;
 use anyhow::Result;
-use anyhow::anyhow;
 use anyhow::bail;
 use fs_err as fs;
 use tempfile::TempDir;
@@ -28,17 +27,26 @@ use super::BindingGenerator;
 use super::GeneratorOutput;
 
 /// A generator for producing Cffi bindings.
-#[derive(Default)]
-pub struct CffiBindingGenerator {}
+pub struct CffiBindingGenerator<'a> {
+    interpreter: &'a PythonInterpreter,
+    tempdir: TempDir,
+}
 
-impl BindingGenerator for CffiBindingGenerator {
+impl<'a> CffiBindingGenerator<'a> {
+    pub fn new(interpreter: &'a PythonInterpreter) -> Result<Self> {
+        Ok(Self {
+            interpreter,
+            tempdir: tempdir()?,
+        })
+    }
+}
+
+impl<'a> BindingGenerator for CffiBindingGenerator<'a> {
     fn generate_bindings(
         &mut self,
         context: &BuildContext,
-        interpreter: Option<&PythonInterpreter>,
         _artifact: &BuildArtifact,
         module: &Path,
-        _temp_dir: &TempDir,
     ) -> Result<GeneratorOutput> {
         let cffi_module_file_name = {
             let extension_name = &context.project_layout.extension_name;
@@ -69,13 +77,8 @@ impl BindingGenerator for CffiBindingGenerator {
         let declarations = generate_cffi_declarations(
             context.manifest_path.parent().unwrap(),
             &context.target_dir,
-            &interpreter
-                .ok_or_else(|| {
-                    anyhow!(
-                        "A python interpreter is required for cffi builds but one was not provided"
-                    )
-                })?
-                .executable,
+            &self.interpreter.executable,
+            &self.tempdir,
         )?;
         let source = GeneratedSourceData {
             data: declarations.into(),
@@ -175,9 +178,9 @@ fn generate_cffi_declarations(
     crate_dir: &Path,
     target_dir: &Path,
     python: &Path,
+    tempdir: &TempDir,
 ) -> Result<String> {
-    let tempdir = tempdir()?;
-    let header = cffi_header(crate_dir, target_dir, &tempdir)?;
+    let header = cffi_header(crate_dir, target_dir, tempdir)?;
 
     let ffi_py = tempdir.as_ref().join("ffi.py");
 
@@ -231,7 +234,7 @@ recompiler.make_py_source(ffi, "ffi", r"{ffi_py}")
 
     // If there was success or an error that was not missing cffi, return here
     if !install_cffi {
-        return handle_cffi_call_result(python, tempdir, &ffi_py, &output);
+        return handle_cffi_call_result(python, &ffi_py, &output);
     }
 
     eprintln!("⚠️ cffi not found. Trying to install it");
@@ -260,16 +263,11 @@ recompiler.make_py_source(ffi, "ffi", r"{ffi_py}")
 
     // Try again
     let output = call_python(python, ["-c", &cffi_invocation])?;
-    handle_cffi_call_result(python, tempdir, &ffi_py, &output)
+    handle_cffi_call_result(python, &ffi_py, &output)
 }
 
 /// Extracted into a function because this is needed twice
-fn handle_cffi_call_result(
-    python: &Path,
-    tempdir: TempDir,
-    ffi_py: &Path,
-    output: &Output,
-) -> Result<String> {
+fn handle_cffi_call_result(python: &Path, ffi_py: &Path, output: &Output) -> Result<String> {
     if !output.status.success() {
         bail!(
             "Failed to generate cffi declarations using {}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}",
@@ -283,7 +281,6 @@ fn handle_cffi_call_result(
         io::stderr().write_all(&output.stderr)?;
 
         let ffi_py_content = fs::read_to_string(ffi_py)?;
-        tempdir.close()?;
         Ok(ffi_py_content)
     }
 }

src/binding_generator/mod.rs

@@ -11,14 +11,11 @@ use fs_err as fs;
 use fs_err::File;
 #[cfg(unix)]
 use fs_err::os::unix::fs::OpenOptionsExt as _;
-use tempfile::TempDir;
-use tempfile::tempdir;
 use tracing::debug;
 
 use crate::BuildArtifact;
 use crate::BuildContext;
 use crate::ModuleWriter;
-use crate::PythonInterpreter;
 use crate::archive_source::ArchiveSource;
 use crate::module_writer::ModuleWriterExt;
 #[cfg(unix)]
@@ -43,10 +40,8 @@ pub(crate) trait BindingGenerator {
     fn generate_bindings(
         &mut self,
         context: &BuildContext,
-        interpreter: Option<&PythonInterpreter>,
         artifact: &BuildArtifact,
         module: &Path,
-        temp_dir: &TempDir,
     ) -> Result<GeneratorOutput>;
 }
 
@@ -88,7 +83,6 @@ pub fn generate_binding<A>(
     writer: &mut impl ModuleWriter,
     generator: &mut impl BindingGenerator,
     context: &BuildContext,
-    interpreter: Option<&PythonInterpreter>,
     artifacts: &[A],
 ) -> Result<()>
 where
@@ -122,12 +116,11 @@ where
 
     for artifact in artifacts {
         let artifact = artifact.borrow();
-        let temp_dir = tempdir()?;
         let GeneratorOutput {
             artifact_target,
             artifact_source_override,
             additional_files,
-        } = generator.generate_bindings(context, interpreter, artifact, &module, &temp_dir)?;
+        } = generator.generate_bindings(context, artifact, &module)?;
 
         match (context.editable, &base_path) {
             (true, Some(base_path)) => {

src/binding_generator/pyo3_binding.rs

@@ -4,9 +4,12 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::process::Command;
 
+use anyhow::Context;
 use anyhow::Result;
+use anyhow::anyhow;
 use anyhow::bail;
 use tempfile::TempDir;
+use tempfile::tempdir;
 use tracing::debug;
 
 use crate::BuildArtifact;
@@ -22,58 +25,78 @@ use super::GeneratorOutput;
 /// A generator for producing PyO3 bindings.
 ///
 /// This struct is responsible for generating Python bindings for modules using PyO3.
-/// The `abi3` field determines whether the generated bindings use the stable PyO3 "abi3" interface,
-/// which allows compatibility with multiple Python versions.
-pub struct Pyo3BindingGenerator {
-    abi3: bool,
+/// The `binding_type` field determines whether the generated bindings use the stable PyO3 "abi3" interface,
+/// which allows compatibility with multiple Python versions and allows targeting a specific python
+/// interpreter.
+pub struct Pyo3BindingGenerator<'a> {
+    binding_type: BindingType<'a>,
+    tempdir: TempDir,
 }
 
-impl Pyo3BindingGenerator {
-    pub fn new(abi3: bool) -> Self {
-        Self { abi3 }
+enum BindingType<'a> {
+    Abi3(Option<&'a PythonInterpreter>),
+    NonAbi3(&'a PythonInterpreter),
+}
+
+impl<'a> Pyo3BindingGenerator<'a> {
+    pub fn new(abi3: bool, interpreter: Option<&'a PythonInterpreter>) -> Result<Self> {
+        let binding_type = match abi3 {
+            true => BindingType::Abi3(interpreter),
+            false => {
+                let interpreter = interpreter.ok_or_else(|| anyhow!(
+                    "A python interpreter is required for non-abi3 builds but one was not provided"
+                ))?;
+                BindingType::NonAbi3(interpreter)
+            }
+        };
+        Ok(Self {
+            binding_type,
+            tempdir: tempdir()?,
+        })
     }
 }
 
-impl BindingGenerator for Pyo3BindingGenerator {
+impl<'a> BindingGenerator for Pyo3BindingGenerator<'a> {
     fn generate_bindings(
         &mut self,
         context: &BuildContext,
-        interpreter: Option<&PythonInterpreter>,
         artifact: &BuildArtifact,
         module: &Path,
-        temp_dir: &TempDir,
     ) -> Result<GeneratorOutput> {
         let ext_name = &context.project_layout.extension_name;
         let target = &context.target;
 
-        let so_filename = if self.abi3 {
-            if target.is_unix() {
-                if target.is_cygwin() {
-                    format!("{ext_name}.abi3.dll")
+        let so_filename = match self.binding_type {
+            BindingType::Abi3(interpreter) => {
+                if target.is_unix() {
+                    if target.is_cygwin() {
+                        format!("{ext_name}.abi3.dll")
+                    } else {
+                        format!("{ext_name}.abi3.so")
+                    }
                 } else {
-                    format!("{ext_name}.abi3.so")
-                }
-            } else {
-                match interpreter {
-                    Some(interpreter) if interpreter.is_windows_debug() => {
-                        format!("{ext_name}_d.pyd")
+                    match interpreter {
+                        Some(interpreter) if interpreter.is_windows_debug() => {
+                            format!("{ext_name}_d.pyd")
+                        }
+                        // Apparently there is no tag for abi3 on windows
+                        _ => format!("{ext_name}.pyd"),
                     }
-                    // Apparently there is no tag for abi3 on windows
-                    _ => format!("{ext_name}.pyd"),
                 }
             }
-        } else {
-            let interpreter =
-                interpreter.expect("A python interpreter is required for non-abi3 build");
-            interpreter.get_library_name(ext_name)
+            BindingType::NonAbi3(interpreter) => interpreter.get_library_name(ext_name),
         };
         let artifact_target = module.join(so_filename);
 
         let artifact_is_big_ar = target.is_aix()
             && artifact.path.extension().unwrap_or(OsStr::new(" ")) == OsStr::new("a");
 
         let artifact_source_override = if artifact_is_big_ar {
-            Some(unpack_big_archive(target, &artifact.path, temp_dir.path())?)
+            Some(unpack_big_archive(
+                target,
+                &artifact.path,
+                self.tempdir.path(),
+            )?)
         } else {
             None
         };
@@ -120,7 +143,7 @@ fn unpack_big_archive(target: &Target, artifact: &Path, temp_dir_path: &Path) ->
         .arg("-X64")
         .arg("x")
         .arg(artifact);
-    let status = ar_command.status().expect("Failed to run ar");
+    let status = ar_command.status().context("Failed to run ar")?;
     if !status.success() {
         bail!(r#"ar finished with "{}": `{:?}`"#, status, ar_command,)
     }

src/binding_generator/uniffi_binding.rs

@@ -8,12 +8,10 @@ use anyhow::Result;
 use anyhow::bail;
 use fs_err as fs;
 use normpath::PathExt as _;
-use tempfile::TempDir;
 use tracing::debug;
 
 use crate::BuildArtifact;
 use crate::BuildContext;
-use crate::PythonInterpreter;
 use crate::archive_source::ArchiveSource;
 use crate::archive_source::FileSourceData;
 use crate::archive_source::GeneratedSourceData;
@@ -30,10 +28,8 @@ impl BindingGenerator for UniFfiBindingGenerator {
     fn generate_bindings(
         &mut self,
         context: &BuildContext,
-        _interpreter: Option<&PythonInterpreter>,
         artifact: &BuildArtifact,
         module: &Path,
-        _temp_dir: &TempDir,
     ) -> Result<GeneratorOutput> {
         let base_path = if context.project_layout.python_module.is_some() {
             module.join(&context.project_layout.extension_name)