Remove add_directory() from ModuleWriter trait

Changelog.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 * Fix generated WHEEL Tag metadata to be spec compliant.
+* Remove `add_directory()` from ModuleWriter and make it an implementation detail for the specific impl
 
 ## [1.9.6]
 

src/build_context.rs

@@ -422,7 +422,6 @@ impl BuildContext {
             .unwrap_or_else(|| self.module_name.clone().into());
         libs_dir.push(".libs");
         let libs_dir = PathBuf::from(libs_dir);
-        writer.add_directory(&libs_dir)?;
 
         let temp_dir = tempfile::tempdir()?;
         let mut soname_map = BTreeMap::new();

src/module_writer.rs

@@ -40,9 +40,6 @@ use zip::{self, DateTime, ZipWriter};
 
 /// Allows writing the module to a wheel or add it directly to the virtualenv
 pub trait ModuleWriter {
-    /// Adds a directory relative to the module base path
-    fn add_directory(&mut self, path: impl AsRef<Path>) -> Result<()>;
-
     /// Adds a file with bytes as content in target relative to the module base path.
     ///
     /// For generated files, `source` is `None`.
@@ -179,13 +176,6 @@ impl PathWriter {
 }
 
 impl ModuleWriter for PathWriter {
-    fn add_directory(&mut self, path: impl AsRef<Path>) -> Result<()> {
-        let target = self.base_path.join(path);
-        debug!("Adding directory {}", target.display());
-        fs::create_dir_all(target)?;
-        Ok(())
-    }
-
     fn add_bytes_with_permissions(
         &mut self,
         target: impl AsRef<Path>,
@@ -200,6 +190,11 @@ impl ModuleWriter for PathWriter {
             return Ok(());
         }
 
+        if let Some(parent_dir) = path.parent() {
+            fs::create_dir_all(parent_dir)
+                .with_context(|| format!("Failed to create directory {}", parent_dir.display()))?;
+        }
+
         // We only need to set the executable bit on unix
         let mut file = {
             #[cfg(target_family = "unix")]
@@ -267,10 +262,6 @@ impl CompressionOptions {
 }
 
 impl ModuleWriter for WheelWriter {
-    fn add_directory(&mut self, _path: impl AsRef<Path>) -> Result<()> {
-        Ok(()) // We don't need to create directories in zip archives
-    }
-
     fn add_bytes_with_permissions(
         &mut self,
         target: impl AsRef<Path>,
@@ -435,10 +426,6 @@ pub struct SDistWriter {
 }
 
 impl ModuleWriter for SDistWriter {
-    fn add_directory(&mut self, _path: impl AsRef<Path>) -> Result<()> {
-        Ok(())
-    }
-
     fn add_bytes_with_permissions(
         &mut self,
         target: impl AsRef<Path>,
@@ -943,7 +930,6 @@ pub fn write_bindings_module(
         }
     } else {
         let module = PathBuf::from(ext_name);
-        writer.add_directory(&module)?;
         // Reexport the shared library as if it were the top level module
         writer.add_bytes(
             module.join("__init__.py"),
@@ -1020,12 +1006,8 @@ pub fn write_cffi_module(
             .strip_prefix(python_module.parent().unwrap())
             .unwrap();
         module = relative.join(&project_layout.extension_name);
-        if !editable {
-            writer.add_directory(&module)?;
-        }
     } else {
         module = PathBuf::from(module_name);
-        writer.add_directory(&module)?;
         let type_stub = project_layout
             .rust_module
             .join(format!("{module_name}.pyi"));
@@ -1287,12 +1269,8 @@ pub fn write_uniffi_module(
             .strip_prefix(python_module.parent().unwrap())
             .unwrap();
         module = relative.join(&project_layout.extension_name);
-        if !editable {
-            writer.add_directory(&module)?;
-        }
     } else {
         module = PathBuf::from(module_name);
-        writer.add_directory(&module)?;
         let type_stub = project_layout
             .rust_module
             .join(format!("{module_name}.pyi"));
@@ -1331,8 +1309,6 @@ pub fn write_bin(
     ))
     .join("scripts");
 
-    writer.add_directory(&data_dir)?;
-
     // We can't use add_file since we need to mark the file as executable
     writer.add_file_with_permissions(data_dir.join(bin_name), artifact, 0o755)?;
     Ok(())
@@ -1424,9 +1400,7 @@ pub fn write_python_part(
                 continue;
             }
             let relative = absolute.strip_prefix(python_dir).unwrap();
-            if absolute.is_dir() {
-                writer.add_directory(relative)?;
-            } else {
+            if !absolute.is_dir() {
                 // Ignore native libraries from develop, if any
                 if let Some(extension) = relative.extension() {
                     if extension.to_string_lossy() == "so" {
@@ -1460,9 +1434,7 @@ pub fn write_python_part(
                     .filter_map(Result::ok)
                 {
                     let target = source.strip_prefix(pyproject_dir)?.to_path_buf();
-                    if source.is_dir() {
-                        writer.add_directory(target)?;
-                    } else {
+                    if !source.is_dir() {
                         #[cfg(unix)]
                         let mode = source.metadata()?.permissions().mode();
                         #[cfg(not(unix))]
@@ -1486,8 +1458,6 @@ pub fn write_dist_info(
 ) -> Result<()> {
     let dist_info_dir = metadata24.get_dist_info_dir();
 
-    writer.add_directory(&dist_info_dir)?;
-
     writer.add_bytes(
         dist_info_dir.join("METADATA"),
         None,
@@ -1520,7 +1490,6 @@ pub fn write_dist_info(
 
     if !metadata24.license_files.is_empty() {
         let license_files_dir = dist_info_dir.join("licenses");
-        writer.add_directory(&license_files_dir)?;
         for path in &metadata24.license_files {
             writer.add_file(license_files_dir.join(path), pyproject_dir.join(path))?;
         }
@@ -1582,7 +1551,7 @@ pub fn add_data(
                     } else if file.path().is_file() {
                         writer.add_file_with_permissions(relative, file.path(), mode)?;
                     } else if file.path().is_dir() {
-                        writer.add_directory(relative)?;
+                        // Intentionally ignored
                     } else {
                         bail!("Can't handle data dir entry {}", file.path().display());
                     }

src/source_distribution.rs

@@ -267,8 +267,6 @@ fn add_crate_to_source_distribution(
         })
         .collect();
 
-    writer.add_directory(prefix)?;
-
     let cargo_toml_path = prefix.join(manifest_path.file_name().unwrap());
 
     let readme_name = readme
@@ -425,8 +423,6 @@ fn add_git_tracked_files_to_sdist(
     }
 
     let prefix = prefix.as_ref();
-    writer.add_directory(prefix)?;
-
     let file_paths = str::from_utf8(&output.stdout)
         .context("git printed invalid utf-8 ಠ_ಠ")?
         .split('\0')
@@ -647,9 +643,7 @@ fn add_cargo_package_files_to_sdist(
                 continue;
             }
             let target = root_dir.join(source.strip_prefix(pyproject_dir).unwrap());
-            if source.is_dir() {
-                writer.add_directory(target)?;
-            } else {
+            if !source.is_dir() {
                 writer.add_file(target, &source)?;
             }
         }
@@ -826,9 +820,7 @@ pub fn source_distribution(
             .filter_map(Result::ok)
         {
             let target = root_dir.join(source.strip_prefix(pyproject_dir).unwrap());
-            if source.is_dir() {
-                writer.add_directory(target)?;
-            } else {
+            if !source.is_dir() {
                 writer.add_file(target, source)?;
             }
         }

tests/common/metadata.rs

@@ -11,11 +11,6 @@ struct MockWriter {
 }
 
 impl ModuleWriter for MockWriter {
-    fn add_directory(&mut self, path: impl AsRef<Path>) -> Result<()> {
-        self.files.push(path.as_ref().to_string_lossy().to_string());
-        Ok(())
-    }
-
     fn add_bytes_with_permissions(
         &mut self,
         target: impl AsRef<Path>,
@@ -58,10 +53,8 @@ fn metadata_hello_world_pep639() {
     .unwrap();
 
     assert_snapshot!(writer.files.join("\n").replace("\\", "/"), @r"
-    hello_world-0.1.0.dist-info
     hello_world-0.1.0.dist-info/METADATA
     hello_world-0.1.0.dist-info/WHEEL
-    hello_world-0.1.0.dist-info/licenses
     hello_world-0.1.0.dist-info/licenses/LICENSE
     hello_world-0.1.0.dist-info/licenses/licenses/AUTHORS.txt
     ");