Update ModuleWriter logic to use ArchiveSource enum

src/archive_source.rs

@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 #[derive(Debug, Clone)]
 pub(crate) enum ArchiveSource {
@@ -13,11 +13,19 @@ impl ArchiveSource {
             Self::File(source) => source.executable,
         }
     }
+
+    pub(crate) fn path(&self) -> Option<&Path> {
+        match self {
+            Self::Generated(source) => source.path.as_deref(),
+            Self::File(source) => Some(&source.path),
+        }
+    }
 }
 
 #[derive(Debug, Clone)]
 pub(crate) struct GeneratedSourceData {
     pub(crate) data: Vec<u8>,
+    pub(crate) path: Option<PathBuf>,
     pub(crate) executable: bool,
 }
 

src/binding_generator/bin_binding.rs

@@ -60,6 +60,7 @@ impl<'m> BindingGenerator for BinBindingGenerator<'m> {
                     .with_extension("py"),
                 ArchiveSource::Generated(GeneratedSourceData {
                     data: generate_wasm_launcher(&bin_name).into(),
+                    path: None,
                     executable: false,
                 }),
             );

src/binding_generator/cffi_binding.rs

@@ -57,13 +57,13 @@ impl BindingGenerator for CffiBindingGenerator {
         let artifact_target = base_path.join(&cffi_module_file_name);
 
         let mut additional_files = HashMap::new();
-        let source = GeneratedSourceData {
-            data: cffi_init_file(&cffi_module_file_name).into(),
-            executable: false,
-        };
         additional_files.insert(
             base_path.join("__init__.py"),
-            ArchiveSource::Generated(source),
+            ArchiveSource::Generated(GeneratedSourceData {
+                data: cffi_init_file(&cffi_module_file_name).into(),
+                path: None,
+                executable: false,
+            }),
         );
 
         let declarations = generate_cffi_declarations(
@@ -77,11 +77,14 @@ impl BindingGenerator for CffiBindingGenerator {
                 })?
                 .executable,
         )?;
-        let source = GeneratedSourceData {
-            data: declarations.into(),
-            executable: false,
-        };
-        additional_files.insert(base_path.join("ffi.py"), ArchiveSource::Generated(source));
+        additional_files.insert(
+            base_path.join("ffi.py"),
+            ArchiveSource::Generated(GeneratedSourceData {
+                data: declarations.into(),
+                path: None,
+                executable: false,
+            }),
+        );
 
         Ok(GeneratorOutput {
             artifact_target,

src/binding_generator/mod.rs

@@ -20,7 +20,7 @@ use crate::BuildContext;
 use crate::ModuleWriter;
 use crate::PythonInterpreter;
 use crate::archive_source::ArchiveSource;
-use crate::module_writer::ModuleWriterExt;
+use crate::module_writer::ModuleWriterInternal;
 #[cfg(unix)]
 use crate::module_writer::default_permission;
 use crate::module_writer::write_python_part;
@@ -85,7 +85,7 @@ pub(crate) struct GeneratorOutput {
 ///
 /// Note: Writing the pth to the archive is handled by [BuildContext], not here
 pub fn generate_binding<A>(
-    writer: &mut impl ModuleWriter,
+    writer: &mut impl ModuleWriterInternal,
     generator: &mut impl BindingGenerator,
     context: &BuildContext,
     interpreter: Option<&PythonInterpreter>,
@@ -187,19 +187,7 @@ where
                 if let Some(additional_files) = additional_files {
                     for (target, source) in additional_files {
                         debug!("Generating archive entry {}", target.display());
-                        match source {
-                            ArchiveSource::Generated(source) => {
-                                writer.add_bytes(
-                                    target,
-                                    None,
-                                    source.data.as_slice(),
-                                    source.executable,
-                                )?;
-                            }
-                            ArchiveSource::File(source) => {
-                                writer.add_file(target, source.path, source.executable)?;
-                            }
-                        }
+                        writer.add_entry(target, source)?;
                     }
                 }
             }

src/binding_generator/pyo3_binding.rs

@@ -91,6 +91,7 @@ if hasattr({ext_name}, "__all__"):
     __all__ = {ext_name}.__all__"#
                     )
                     .into(),
+                    path: None,
                     executable: false,
                 };
                 files.insert(module.join("__init__.py"), ArchiveSource::Generated(source));

src/binding_generator/uniffi_binding.rs

@@ -60,13 +60,13 @@ impl BindingGenerator for UniFfiBindingGenerator {
             .map(|name| format!("from .{name} import *  # NOQA\n"))
             .collect::<Vec<String>>()
             .join("");
-        let source = GeneratedSourceData {
-            data: py_init.into(),
-            executable: false,
-        };
         additional_files.insert(
             base_path.join("__init__.py"),
-            ArchiveSource::Generated(source),
+            ArchiveSource::Generated(GeneratedSourceData {
+                data: py_init.into(),
+                path: None,
+                executable: false,
+            }),
         );
 
         for binding in binding_names {

src/build_context.rs

@@ -8,14 +8,14 @@ use crate::bridge::Abi3Version;
 use crate::build_options::CargoOptions;
 use crate::compile::{CompileTarget, warn_missing_py_init};
 use crate::compression::CompressionOptions;
-use crate::module_writer::{ModuleWriterExt, WheelWriter, add_data};
+use crate::module_writer::{WheelWriter, add_data};
 use crate::project_layout::ProjectLayout;
 use crate::source_distribution::source_distribution;
 use crate::target::validate_wheel_filename_for_pypi;
 use crate::target::{Arch, Os};
 use crate::{
-    BridgeModel, BuildArtifact, Metadata24, PyProjectToml, PythonInterpreter, Target, compile,
-    pyproject_toml::Format,
+    BridgeModel, BuildArtifact, Metadata24, ModuleWriter, PyProjectToml, PythonInterpreter, Target,
+    compile, pyproject_toml::Format,
 };
 use anyhow::{Context, Result, anyhow, bail};
 use cargo_metadata::CrateType;

tests/common/metadata.rs → src/module_writer/mock_writer.rs

@@ -1,30 +1,48 @@
+use std::io::Read as _;
+use std::path::Path;
+use std::path::PathBuf;
+use std::str;
+
 use anyhow::Result;
+use anyhow::bail;
+use fs_err::File;
+use indexmap::IndexMap;
+use indexmap::map::Entry;
 use insta::assert_snapshot;
-use maturin::{BuildOptions, CargoOptions, ModuleWriter, write_dist_info};
-use std::collections::HashMap;
-use std::path::{Path, PathBuf};
+use itertools::Itertools as _;
+
+use crate::BuildOptions;
+use crate::CargoOptions;
+use crate::archive_source::ArchiveSource;
+use crate::write_dist_info;
+
+use super::ModuleWriterInternal;
 
 #[derive(Default)]
 struct MockWriter {
-    files: Vec<String>,
-    contents: HashMap<String, String>,
+    files: IndexMap<PathBuf, Vec<u8>>,
 }
 
-impl ModuleWriter for MockWriter {
-    fn add_bytes(
-        &mut self,
-        target: impl AsRef<Path>,
-        _source: Option<&Path>,
-        mut data: impl std::io::Read,
-        _executable: bool,
-    ) -> Result<()> {
-        let target = target.as_ref().to_string_lossy().to_string();
-        let mut buffer = String::new();
-        data.read_to_string(&mut buffer)?;
+impl super::private::Sealed for MockWriter {}
 
-        self.files.push(target.clone());
-        self.contents.insert(target, buffer);
+impl ModuleWriterInternal for MockWriter {
+    fn add_entry(&mut self, target: impl AsRef<Path>, source: ArchiveSource) -> Result<()> {
+        let target = target.as_ref().to_path_buf();
+        let Entry::Vacant(entry) = self.files.entry(target.clone()) else {
+            bail!("Duplicate file {target:?} written to mock writer");
+        };
 
+        let buffer = match source {
+            ArchiveSource::Generated(source) => source.data,
+            ArchiveSource::File(source) => {
+                let mut file = File::options().read(true).open(source.path)?;
+                let mut buffer = Vec::new();
+                file.read_to_end(&mut buffer)?;
+                buffer
+            }
+        };
+
+        entry.insert(buffer);
         Ok(())
     }
 }
@@ -53,19 +71,16 @@ fn metadata_hello_world_pep639() {
     )
     .unwrap();
 
-    assert_snapshot!(writer.files.join("\n").replace("\\", "/"), @r"
+    assert_snapshot!(writer.files.keys().map(|p| p.to_string_lossy()).collect_vec().join("\n").replace("\\", "/"), @r"
     hello_world-0.1.0.dist-info/METADATA
     hello_world-0.1.0.dist-info/WHEEL
     hello_world-0.1.0.dist-info/licenses/LICENSE
     hello_world-0.1.0.dist-info/licenses/licenses/AUTHORS.txt
     ");
-    let metadata_path = Path::new("hello_world-0.1.0.dist-info")
-        .join("METADATA")
-        .to_str()
-        .unwrap()
-        .to_string();
+    let metadata_path = Path::new("hello_world-0.1.0.dist-info").join("METADATA");
     // Remove the README in the body of the email
-    let metadata = writer.contents[&metadata_path]
+    let metadata = str::from_utf8(&writer.files[&metadata_path])
+        .unwrap()
         .split_once("\n\n")
         .unwrap()
         .0;

src/module_writer/mod.rs

@@ -1,6 +1,4 @@
 use std::fmt::Write as _;
-use std::io;
-use std::io::Read;
 #[cfg(unix)]
 use std::os::unix::fs::PermissionsExt as _;
 use std::path::Path;
@@ -9,17 +7,21 @@ use anyhow::Context as _;
 use anyhow::Result;
 use anyhow::bail;
 use fs_err as fs;
-use fs_err::File;
 use ignore::WalkBuilder;
 use indexmap::IndexMap;
 use itertools::Itertools as _;
 use tracing::debug;
 
 use crate::Metadata24;
 use crate::PyProjectToml;
+use crate::archive_source::ArchiveSource;
+use crate::archive_source::FileSourceData;
+use crate::archive_source::GeneratedSourceData;
 use crate::project_layout::ProjectLayout;
 use crate::pyproject_toml::Format;
 
+#[cfg(test)]
+mod mock_writer;
 mod path_writer;
 mod sdist_writer;
 mod util;
@@ -29,8 +31,20 @@ pub use path_writer::PathWriter;
 pub use sdist_writer::SDistWriter;
 pub use wheel_writer::WheelWriter;
 
+mod private {
+    pub trait Sealed {}
+}
+
+const EMPTY: Vec<u8> = vec![];
+
 /// Allows writing the module to a wheel or add it directly to the virtualenv
-pub trait ModuleWriter {
+pub(crate) trait ModuleWriterInternal: private::Sealed {
+    /// Adds an entry into the archive
+    fn add_entry(&mut self, target: impl AsRef<Path>, source: ArchiveSource) -> Result<()>;
+}
+
+/// Extension trait with convenience methods for interacting with a [ModuleWriterInternal]
+pub trait ModuleWriter: private::Sealed {
     /// Adds a file with data as content in target relative to the module base path while setting
     /// the appropriate unix permissions
     ///
@@ -39,15 +53,42 @@ pub trait ModuleWriter {
         &mut self,
         target: impl AsRef<Path>,
         source: Option<&Path>,
-        data: impl Read,
+        data: impl Into<Vec<u8>>,
         executable: bool,
     ) -> Result<()>;
-}
 
-/// Extension trait with convenience methods for interacting with a [ModuleWriter]
-pub trait ModuleWriterExt: ModuleWriter {
     /// Copies the source file the target path relative to the module base path while setting
     /// the given unix permissions
+    fn add_file(
+        &mut self,
+        target: impl AsRef<Path>,
+        source: impl AsRef<Path>,
+        executable: bool,
+    ) -> Result<()>;
+
+    /// Add an empty file to the target path
+    fn add_empty_file(&mut self, target: impl AsRef<Path>) -> Result<()>;
+}
+
+/// This blanket impl makes it impossible to overwrite the methods in [ModuleWriter]
+impl<T: ModuleWriterInternal> ModuleWriter for T {
+    fn add_bytes(
+        &mut self,
+        target: impl AsRef<Path>,
+        source: Option<&Path>,
+        data: impl Into<Vec<u8>>,
+        executable: bool,
+    ) -> Result<()> {
+        self.add_entry(
+            target,
+            ArchiveSource::Generated(GeneratedSourceData {
+                data: data.into(),
+                path: source.map(ToOwned::to_owned),
+                executable,
+            }),
+        )
+    }
+
     fn add_file(
         &mut self,
         target: impl AsRef<Path>,
@@ -58,22 +99,21 @@ pub trait ModuleWriterExt: ModuleWriter {
         let source = source.as_ref();
         debug!("Adding {} from {}", target.display(), source.display());
 
-        let file =
-            File::open(source).with_context(|| format!("Failed to open {}", source.display()))?;
-        self.add_bytes(target, Some(source), file, executable)
-            .with_context(|| format!("Failed to write to {}", target.display()))?;
-        Ok(())
+        self.add_entry(
+            target,
+            ArchiveSource::File(FileSourceData {
+                path: source.to_path_buf(),
+                executable,
+            }),
+        )
     }
 
-    /// Add an empty file to the target path
+    #[inline]
     fn add_empty_file(&mut self, target: impl AsRef<Path>) -> Result<()> {
-        self.add_bytes(target, None, io::empty(), false)
+        self.add_bytes(target, None, EMPTY, false)
     }
 }
 
-/// This blanket impl makes it impossible to overwrite the methods in [ModuleWriterExt]
-impl<T: ModuleWriter> ModuleWriterExt for T {}
-
 /// Adds the python part of a mixed project to the writer,
 pub fn write_python_part(
     writer: &mut impl ModuleWriter,