From ad035798de532c0d9e20629d2707c3809e76ba83 Mon Sep 17 00:00:00 2001 From: messense Date: Sun, 8 Mar 2026 22:15:25 +0800 Subject: [PATCH 1/4] refactor: eliminate IIFE anti-pattern in module_writer Extract the closure-immediately-invoked-for-error-context pattern in add_data() into a proper named function add_data_subdir(). This replaces the '(|| { ... })().with_context(...)' idiom with a clean function call that is easier to read and debug. --- src/module_writer/mod.rs | 96 ++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/src/module_writer/mod.rs b/src/module_writer/mod.rs index b092384cc..dbd7c6e5f 100644 --- a/src/module_writer/mod.rs +++ b/src/module_writer/mod.rs @@ -230,52 +230,60 @@ pub fn add_data( ); } debug!("Adding data from {}", subdir.path().display()); - (|| { - for file in WalkBuilder::new(subdir.path()) - .standard_filters(false) - .build() - { - let file = file?; - let relative_path = file.path().strip_prefix(data).with_context(|| { + add_data_subdir(writer, &subdir.path(), data, metadata24) + .with_context(|| format!("Failed to include data from {}", data.display()))? + } + } + Ok(()) +} + +/// Walk a single data subdirectory and add its files to the writer. +fn add_data_subdir( + writer: &mut impl ModuleWriter, + subdir_path: &Path, + data: &Path, + metadata24: &Metadata24, +) -> Result<()> { + for file in WalkBuilder::new(subdir_path) + .standard_filters(false) + .build() + { + let file = file?; + let relative_path = file.path().strip_prefix(data).with_context(|| { + format!( + "Data file {} is not under data dir {}", + file.path().display(), + data.display() + ) + })?; + let relative = metadata24.get_data_dir().join(relative_path); + + if file.path_is_symlink() { + // Copy the actual file contents, not the link, so that you can create a + // data directory by joining different data sources + let link_target = fs::read_link(file.path())?; + let source = if link_target.is_absolute() { + link_target + } else { + file.path() + .parent() + .with_context(|| { format!( - "Data file {} is not under data dir {}", - file.path().display(), - data.display() + "Data symlink {} has no parent directory", + file.path().display() ) - })?; - let relative = metadata24.get_data_dir().join(relative_path); - - if file.path_is_symlink() { - // Copy the actual file contents, not the link, so that you can create a - // data directory by joining different data sources - let link_target = fs::read_link(file.path())?; - let source = if link_target.is_absolute() { - link_target - } else { - file.path() - .parent() - .with_context(|| { - format!( - "Data symlink {} has no parent directory", - file.path().display() - ) - })? - .join(link_target) - }; - let mode = file_permission_mode(&source)?; - writer.add_file(relative, source, permission_is_executable(mode))?; - } else if file.path().is_file() { - let mode = file_permission_mode(file.path())?; - writer.add_file(relative, file.path(), permission_is_executable(mode))?; - } else if file.path().is_dir() { - // Intentionally ignored - } else { - bail!("Can't handle data dir entry {}", file.path().display()); - } - } - Ok(()) - })() - .with_context(|| format!("Failed to include data from {}", data.display()))? + })? + .join(link_target) + }; + let mode = file_permission_mode(&source)?; + writer.add_file(relative, source, permission_is_executable(mode))?; + } else if file.path().is_file() { + let mode = file_permission_mode(file.path())?; + writer.add_file(relative, file.path(), permission_is_executable(mode))?; + } else if file.path().is_dir() { + // Intentionally ignored + } else { + bail!("Can't handle data dir entry {}", file.path().display()); } } Ok(()) From 4c5ca7bd46da41c0766d9de3eb8f1d20eb0bfbb2 Mon Sep 17 00:00:00 2001 From: messense Date: Sun, 8 Mar 2026 22:16:44 +0800 Subject: [PATCH 2/4] refactor: cleanup anti-patterns - main.rs: Extract ensure_release_profile() to deduplicate PEP 517 profile defaulting in WriteDistInfo and BuildWheel handlers - auditwheel/musllinux.rs: Fall back to /usr/bin/ls when /bin/ls doesn't exist, fixing compatibility with distros that don't symlink /bin -> /usr/bin --- src/auditwheel/musllinux.rs | 9 ++++++++- src/main.rs | 22 ++++++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/auditwheel/musllinux.rs b/src/auditwheel/musllinux.rs index ce4746e83..afb1f282c 100644 --- a/src/auditwheel/musllinux.rs +++ b/src/auditwheel/musllinux.rs @@ -8,7 +8,14 @@ use std::process::{Command, Stdio}; /// Find musl libc path from executable's ELF header pub fn find_musl_libc() -> Result> { - let buffer = fs::read("/bin/ls")?; + // Try /bin/ls first; fall back to /usr/bin/ls for distros that don't + // symlink /bin -> /usr/bin. + let ls_path = if Path::new("/bin/ls").exists() { + Path::new("/bin/ls") + } else { + Path::new("/usr/bin/ls") + }; + let buffer = fs::read(ls_path)?; let elf = Elf::parse(&buffer)?; Ok(elf.interpreter.map(PathBuf::from)) } diff --git a/src/main.rs b/src/main.rs index 7f020a10b..9c8d6c913 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,8 +12,9 @@ use clap::CommandFactory; use clap::{Parser, Subcommand}; use ignore::overrides::Override; use maturin::{ - BridgeModel, BuildOptions, CargoOptions, DevelopOptions, PathWriter, Target, TargetTriple, - UnpackedSdist, VirtualWriter, develop, find_path_deps, unpack_sdist, write_dist_info, + BridgeModel, BuildContext, BuildOptions, CargoOptions, DevelopOptions, PathWriter, Target, + TargetTriple, UnpackedSdist, VirtualWriter, develop, find_path_deps, unpack_sdist, + write_dist_info, }; #[cfg(feature = "schemars")] use maturin::{GenerateJsonSchemaOptions, generate_json_schema}; @@ -280,6 +281,13 @@ fn detect_venv(target: &Target) -> Result { /// /// The last line of stdout is used as return value from the python part of the implementation fn pep517(subcommand: Pep517Command) -> Result<()> { + /// PEP 517 builds default to release profile. + fn ensure_release_profile(context: &mut BuildContext) { + if context.cargo_options.profile.is_none() { + context.cargo_options.profile = Some("release".to_string()); + } + } + match subcommand { Pep517Command::WriteDistInfo { build_options, @@ -292,11 +300,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> { .strip(strip_opt.strip) .editable(false) .build()?; - - // TBD: does `--profile release` do anything here? - if context.cargo_options.profile.is_none() { - context.cargo_options.profile = Some("release".to_string()); - } + ensure_release_profile(&mut context); let mut writer = VirtualWriter::new(PathWriter::from_path(metadata_directory), Override::empty()); @@ -319,9 +323,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> { .strip(strip_opt.strip) .editable(editable) .build()?; - if build_context.cargo_options.profile.is_none() { - build_context.cargo_options.profile = Some("release".to_string()); - } + ensure_release_profile(&mut build_context); let wheels = build_context.build_wheels()?; assert_eq!(wheels.len(), 1); println!("{}", wheels[0].0.to_str().unwrap()); From bd0b578e3f0d3af3d742856efa666a160a4c226d Mon Sep 17 00:00:00 2001 From: messense Date: Mon, 9 Mar 2026 20:15:51 +0800 Subject: [PATCH 3/4] Update src/main.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 9c8d6c913..df4905fb6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -281,7 +281,7 @@ fn detect_venv(target: &Target) -> Result { /// /// The last line of stdout is used as return value from the python part of the implementation fn pep517(subcommand: Pep517Command) -> Result<()> { - /// PEP 517 builds default to release profile. + // PEP 517 builds default to release profile. fn ensure_release_profile(context: &mut BuildContext) { if context.cargo_options.profile.is_none() { context.cargo_options.profile = Some("release".to_string()); From 03bb0eaafa25b61b6b1e8e54b6032fbb1d154f42 Mon Sep 17 00:00:00 2001 From: messense Date: Mon, 9 Mar 2026 20:16:05 +0800 Subject: [PATCH 4/4] Update src/module_writer/mod.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/module_writer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/module_writer/mod.rs b/src/module_writer/mod.rs index dbd7c6e5f..365c0069f 100644 --- a/src/module_writer/mod.rs +++ b/src/module_writer/mod.rs @@ -230,7 +230,7 @@ pub fn add_data( ); } debug!("Adding data from {}", subdir.path().display()); - add_data_subdir(writer, &subdir.path(), data, metadata24) + add_data_subdir(writer, subdir.path().as_path(), data, metadata24) .with_context(|| format!("Failed to include data from {}", data.display()))? } }