Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* Fix generated WHEEL Tag metadata to be spec compliant.
* Ensure parent directory is created before attempting to write a file in ModuleWriter

## [1.9.6]

Expand Down
21 changes: 19 additions & 2 deletions src/module_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ pub trait ModuleWriter {
bytes: &[u8],
) -> Result<()> {
debug!("Adding {}", target.as_ref().display());

if let Some(parent_dir) = target.as_ref().parent() {
self.add_directory(parent_dir)
.with_context(|| format!("Failed to create directory {}", parent_dir.display()))?;
}

// 0o644 is the default from the zip crate
self.add_bytes_with_permissions(target, source, bytes, 0o644)
}
Expand Down Expand Up @@ -86,6 +92,11 @@ pub trait ModuleWriter {
let source = source.as_ref();
debug!("Adding {} from {}", target.display(), source.display());

if let Some(parent_dir) = target.parent() {
self.add_directory(parent_dir)
.with_context(|| format!("Failed to create directory {}", parent_dir.display()))?;
}

let read_failed_context = format!("Failed to read {}", source.display());
let mut file = File::open(source).context(read_failed_context.clone())?;
let mut buffer = Vec::new();
Expand Down Expand Up @@ -181,8 +192,10 @@ 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)?;
if !target.exists() {
debug!("Adding directory {}", target.display());
fs::create_dir_all(target)?;
}
Ok(())
}

Expand Down Expand Up @@ -1388,6 +1401,10 @@ if __name__ == '__main__':
let launcher_path = Path::new(&metadata.get_distribution_escaped())
.join(bin_name.replace('-', "_"))
.with_extension("py");

if let Some(parent_dir) = launcher_path.parent() {
writer.add_directory(parent_dir)?;
}
writer.add_bytes_with_permissions(&launcher_path, None, entrypoint_script.as_bytes(), 0o755)?;
Ok(())
}
Expand Down
34 changes: 27 additions & 7 deletions tests/common/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
use anyhow::Context;
use anyhow::Result;
use insta::assert_snapshot;
use itertools::Itertools;
use maturin::{write_dist_info, BuildOptions, CargoOptions, ModuleWriter};
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::collections::HashSet;
use std::path::{Path, PathBuf, MAIN_SEPARATOR};

#[derive(Default)]
struct MockWriter {
directories: HashSet<String>,
files: Vec<String>,
contents: HashMap<String, String>,
}

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());
let mut dir: String = "".into();
for component in path.as_ref().components() {
dir.push_str(&component.as_os_str().to_string_lossy());
self.directories.insert(dir.clone());
dir.push(MAIN_SEPARATOR);
}
Ok(())
}

Expand All @@ -23,10 +32,18 @@ impl ModuleWriter for MockWriter {
bytes: &[u8],
_permissions: u32,
) -> Result<()> {
self.files
.push(target.as_ref().to_string_lossy().to_string());
let target = target.as_ref();
if let Some(parent_dir) = target.parent() {
self.directories
.get(parent_dir.to_string_lossy().as_ref())
.with_context(|| {
format!("Parent directory does not exist: {}", parent_dir.display())
})?;
}

self.files.push(target.to_string_lossy().to_string());
self.contents.insert(
target.as_ref().to_string_lossy().into(),
target.to_string_lossy().into(),
std::str::from_utf8(bytes).unwrap().to_string(),
);
Ok(())
Expand Down Expand Up @@ -58,13 +75,16 @@ 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
");
assert_snapshot!(writer.directories.iter().sorted().join("\n").replace("\\", "/"), @r"
hello_world-0.1.0.dist-info
hello_world-0.1.0.dist-info/licenses
hello_world-0.1.0.dist-info/licenses/licenses
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra level of licenses nesting being missing from the previous snapshot output is the exact issue I had

");
let metadata_path = Path::new("hello_world-0.1.0.dist-info")
.join("METADATA")
.to_str()
Expand Down
Loading