Skip to content

Commit 5ea017e

Browse files
committed
refactor: extract duplicated helpers and reduce code repetition
- main.rs: Extract unpack_sdist_for_build() and StripOption shared struct to eliminate duplicated sdist-then-build and --strip arg definitions - build_context/mod.rs: Unify build_cffi_wheel/build_uniffi_wheel into shared build_cdylib_wheel() pipeline - metadata.rs: Extract split_contacts() for duplicated author/maintainer processing - auditwheel: Make is_dynamic_linker() pub(crate), extract run_patchelf() - binding_generator: Extract cdylib_filename() shared helper - module_writer: Extract file_permission_mode() helper - pyproject_toml.rs: Extract CIConfigOverrides with serde(flatten) - source_distribution: Extract resolve_workspace_dep_entries()
1 parent 552e68a commit 5ea017e

16 files changed

Lines changed: 353 additions & 393 deletions

File tree

src/auditwheel/audit.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,13 @@ use tracing::debug;
2020
static IS_LIBPYTHON: Lazy<Regex> =
2121
Lazy::new(|| Regex::new(r"^libpython3\.\d+m?u?t?\.so\.\d+\.\d+$").unwrap());
2222

23-
fn is_dynamic_linker(name: &str) -> bool {
24-
name.starts_with("ld-linux") || name == "ld64.so.2" || name == "ld64.so.1"
23+
/// Returns `true` if the given shared-library name is a dynamic linker
24+
/// (e.g. `ld-linux-x86-64.so.2`, `ld64.so.2`, `ld-musl-*.so.1`).
25+
pub(crate) fn is_dynamic_linker(name: &str) -> bool {
26+
name.starts_with("ld-linux")
27+
|| name == "ld64.so.2"
28+
|| name == "ld64.so.1"
29+
|| name.starts_with("ld-musl")
2530
}
2631

2732
/// Error raised during auditing an elf file for manylinux/musllinux compatibility

src/auditwheel/patchelf.rs

Lines changed: 36 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,27 @@ use std::process::Command;
55

66
static MISSING_PATCHELF_ERROR: &str = "Failed to execute 'patchelf', did you install it? Hint: Try `pip install maturin[patchelf]` (or just `pip install patchelf`)";
77

8-
/// Verify patchelf version
9-
pub fn verify_patchelf() -> Result<()> {
8+
/// Run a patchelf command with the given arguments.
9+
///
10+
/// Returns `Ok(stdout)` on success, or an error with the stderr message.
11+
fn run_patchelf(args: &[&OsStr]) -> Result<Vec<u8>> {
1012
let output = Command::new("patchelf")
11-
.arg("--version")
13+
.args(args)
1214
.output()
1315
.context(MISSING_PATCHELF_ERROR)?;
14-
let version = String::from_utf8(output.stdout)
16+
if !output.status.success() {
17+
bail!(
18+
"patchelf failed: {}",
19+
String::from_utf8_lossy(&output.stderr)
20+
);
21+
}
22+
Ok(output.stdout)
23+
}
24+
25+
/// Verify patchelf version
26+
pub fn verify_patchelf() -> Result<()> {
27+
let stdout = run_patchelf(&[OsStr::new("--version")])?;
28+
let version = String::from_utf8(stdout)
1529
.context("Failed to parse patchelf version")?
1630
.trim()
1731
.to_string();
@@ -33,64 +47,42 @@ pub fn replace_needed<O: AsRef<OsStr>, N: AsRef<OsStr>>(
3347
file: impl AsRef<Path>,
3448
old_new_pairs: &[(O, N)],
3549
) -> Result<()> {
36-
let mut cmd = Command::new("patchelf");
50+
let mut args: Vec<&OsStr> = Vec::new();
3751
for (old, new) in old_new_pairs {
38-
cmd.arg("--replace-needed").arg(old).arg(new);
39-
}
40-
cmd.arg(file.as_ref());
41-
let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
42-
if !output.status.success() {
43-
bail!(
44-
"patchelf --replace-needed failed: {}",
45-
String::from_utf8_lossy(&output.stderr)
46-
);
52+
args.push(OsStr::new("--replace-needed"));
53+
args.push(old.as_ref());
54+
args.push(new.as_ref());
4755
}
56+
args.push(file.as_ref().as_os_str());
57+
run_patchelf(&args)?;
4858
Ok(())
4959
}
5060

5161
/// Change `SONAME` of a dynamic library
5262
pub fn set_soname<S: AsRef<OsStr>>(file: impl AsRef<Path>, soname: &S) -> Result<()> {
53-
let mut cmd = Command::new("patchelf");
54-
cmd.arg("--set-soname").arg(soname).arg(file.as_ref());
55-
let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
56-
if !output.status.success() {
57-
bail!(
58-
"patchelf --set-soname failed: {}",
59-
String::from_utf8_lossy(&output.stderr)
60-
);
61-
}
63+
run_patchelf(&[
64+
OsStr::new("--set-soname"),
65+
soname.as_ref(),
66+
file.as_ref().as_os_str(),
67+
])?;
6268
Ok(())
6369
}
6470

6571
/// Remove a `RPATH` from executables and libraries
6672
pub fn remove_rpath(file: impl AsRef<Path>) -> Result<()> {
67-
let mut cmd = Command::new("patchelf");
68-
cmd.arg("--remove-rpath").arg(file.as_ref());
69-
let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
70-
if !output.status.success() {
71-
bail!(
72-
"patchelf --remove-rpath failed: {}",
73-
String::from_utf8_lossy(&output.stderr)
74-
);
75-
}
73+
run_patchelf(&[OsStr::new("--remove-rpath"), file.as_ref().as_os_str()])?;
7674
Ok(())
7775
}
7876

7977
/// Change the `RPATH` of executables and libraries
8078
pub fn set_rpath<S: AsRef<OsStr>>(file: impl AsRef<Path>, rpath: &S) -> Result<()> {
8179
remove_rpath(&file)?;
82-
let mut cmd = Command::new("patchelf");
83-
cmd.arg("--force-rpath")
84-
.arg("--set-rpath")
85-
.arg(rpath)
86-
.arg(file.as_ref());
87-
let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
88-
if !output.status.success() {
89-
bail!(
90-
"patchelf --set-rpath failed: {}",
91-
String::from_utf8_lossy(&output.stderr)
92-
);
93-
}
80+
run_patchelf(&[
81+
OsStr::new("--force-rpath"),
82+
OsStr::new("--set-rpath"),
83+
rpath.as_ref(),
84+
file.as_ref().as_os_str(),
85+
])?;
9486
Ok(())
9587
}
9688

src/auditwheel/repair.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::audit::AuditWheelError;
1+
use super::audit::{AuditWheelError, is_dynamic_linker};
22
use crate::auditwheel::Policy;
33
use anyhow::Result;
44
use lddtree::DependencyAnalyzer;
@@ -19,12 +19,8 @@ pub fn find_external_libs(
1919
let mut ext_libs = Vec::new();
2020
for (_, lib) in deps.libraries {
2121
let name = &lib.name;
22-
// Skip dynamic linker/loader and white-listed libs
23-
if name.starts_with("ld-linux")
24-
|| name == "ld64.so.2"
25-
|| name == "ld64.so.1"
26-
// musl libc, eg: libc.musl-aarch64.so.1
27-
|| name.starts_with("ld-musl")
22+
// Skip dynamic linker/loader, musl libc, and white-listed libs
23+
if is_dynamic_linker(name)
2824
|| name.starts_with("libc.")
2925
|| policy.lib_whitelist.contains(name)
3026
{

src/binding_generator/cffi_binding.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use crate::PythonInterpreter;
2222
use crate::archive_source::ArchiveSource;
2323
use crate::archive_source::GeneratedSourceData;
2424
use crate::binding_generator::ArtifactTarget;
25-
use crate::target::Os;
2625

2726
use super::BindingGenerator;
2827
use super::GeneratorOutput;
@@ -52,11 +51,7 @@ impl<'a> BindingGenerator for CffiBindingGenerator<'a> {
5251
let cffi_module_file_name = {
5352
let extension_name = &context.project_layout.extension_name;
5453
// https://cffi.readthedocs.io/en/stable/embedding.html#issues-about-using-the-so
55-
match context.target.target_os() {
56-
Os::Macos => format!("lib{extension_name}.dylib"),
57-
Os::Windows => format!("{extension_name}.dll"),
58-
_ => format!("lib{extension_name}.so"),
59-
}
54+
super::cdylib_filename(extension_name, context.target.target_os())
6055
};
6156
let base_path = if context.project_layout.python_module.is_some() {
6257
module.join(&context.project_layout.extension_name)

src/binding_generator/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,19 @@ pub use cffi_binding::CffiBindingGenerator;
3636
pub use pyo3_binding::Pyo3BindingGenerator;
3737
pub use uniffi_binding::UniFfiBindingGenerator;
3838

39+
use crate::target::Os;
40+
41+
/// Returns the platform-specific cdylib filename for the given library name.
42+
///
43+
/// For example, `cdylib_filename("foo", Os::Macos)` returns `"libfoo.dylib"`.
44+
pub(crate) fn cdylib_filename(name: &str, os: Os) -> String {
45+
match os {
46+
Os::Macos => format!("lib{name}.dylib"),
47+
Os::Windows => format!("{name}.dll"),
48+
_ => format!("lib{name}.so"),
49+
}
50+
}
51+
3952
/// A trait to generate the binding files to be included in the built module
4053
///
4154
/// This trait is used to generate the support files necessary to build a python

src/binding_generator/uniffi_binding.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,7 @@ fn generate_uniffi_bindings(
226226
let cdylib = match cdylib_name {
227227
// this logic should match with uniffi's expected names, e.g.
228228
// https://github.com/mozilla/uniffi-rs/blob/86a34083dd18bdd33f420c602b4fad624cc1e404/uniffi_bindgen/src/bindings/python/templates/NamespaceLibraryTemplate.py#L14-L37
229-
Some(cdylib_name) => match target_os {
230-
Os::Macos => format!("lib{cdylib_name}.dylib"),
231-
Os::Windows => format!("{cdylib_name}.dll"),
232-
_ => format!("lib{cdylib_name}.so"),
233-
},
229+
Some(cdylib_name) => super::cdylib_filename(&cdylib_name, target_os),
234230
None => artifact.file_name().unwrap().to_str().unwrap().to_string(),
235231
};
236232

src/build_context/builder.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,7 @@ impl BuildContextBuilder {
238238
.unwrap_or_else(|| cargo_metadata.target_directory.clone().into_std_path_buf());
239239

240240
let config_targets = pyproject.and_then(|x| x.targets());
241-
let compile_targets =
242-
filter_cargo_targets(&cargo_metadata, bridge, config_targets.as_deref())?;
241+
let compile_targets = filter_cargo_targets(&cargo_metadata, bridge, config_targets)?;
243242
if compile_targets.is_empty() {
244243
bail!(
245244
"No Cargo targets to build, please check your bindings configuration in pyproject.toml."

src/build_context/mod.rs

Lines changed: 36 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -868,49 +868,50 @@ impl BuildContext {
868868
Ok(())
869869
}
870870

871-
fn write_cffi_wheel(
872-
&self,
873-
artifact: BuildArtifact,
874-
platform_tags: &[PlatformTag],
875-
ext_libs: Vec<Library>,
871+
/// Shared build pipeline for cdylib-based bindings (CFFI, UniFfi).
872+
///
873+
/// Compiles the cdylib, runs auditwheel, resolves platform tags, writes
874+
/// the wheel via `write_wheel`, and returns the built wheel metadata.
875+
#[allow(clippy::needless_lifetimes)] // false positive
876+
fn build_cdylib_wheel<'a, F>(
877+
&'a self,
878+
make_generator: F,
876879
sbom_data: &Option<SbomData>,
877-
out_dirs: &HashMap<String, PathBuf>,
878-
) -> Result<PathBuf> {
879-
let tag = self.get_universal_tag(platform_tags)?;
880-
881-
let interpreter = self.interpreter.first().ok_or_else(|| {
882-
anyhow!("A python interpreter is required for cffi builds but one was not provided")
883-
})?;
884-
self.write_wheel(
880+
) -> Result<(PathBuf, HashMap<String, PathBuf>)>
881+
where
882+
F: FnOnce(Rc<tempfile::TempDir>) -> Result<Box<dyn BindingGenerator + 'a>>,
883+
{
884+
let (artifact, out_dirs) = self.compile_cdylib(None, None)?;
885+
let (policy, external_libs) = self.auditwheel(&artifact, &self.platform_tag, None)?;
886+
let platform_tags = self.resolve_platform_tags(&policy);
887+
let tag = self.get_universal_tag(&platform_tags)?;
888+
let wheel_path = self.write_wheel(
885889
&tag,
886890
&[&artifact],
887-
&[ext_libs],
888-
|temp_dir| {
889-
Ok(Box::new(
890-
CffiBindingGenerator::new(interpreter, temp_dir)
891-
.context("Failed to initialize Cffi binding generator")?,
892-
))
893-
},
891+
&[external_libs],
892+
make_generator,
894893
sbom_data,
895-
out_dirs,
896-
)
894+
&out_dirs,
895+
)?;
896+
Ok((wheel_path, out_dirs))
897897
}
898898

899899
/// Builds a wheel with cffi bindings
900900
pub fn build_cffi_wheel(
901901
&self,
902902
sbom_data: &Option<SbomData>,
903903
) -> Result<Vec<BuiltWheelMetadata>> {
904-
let mut wheels = Vec::new();
905-
let (artifact, out_dirs) = self.compile_cdylib(None, None)?;
906-
let (policy, external_libs) = self.auditwheel(&artifact, &self.platform_tag, None)?;
907-
let platform_tags = self.resolve_platform_tags(&policy);
908-
let wheel_path = self.write_cffi_wheel(
909-
artifact,
910-
&platform_tags,
911-
external_libs,
904+
let interpreter = self.interpreter.first().ok_or_else(|| {
905+
anyhow!("A python interpreter is required for cffi builds but one was not provided")
906+
})?;
907+
let (wheel_path, _) = self.build_cdylib_wheel(
908+
|temp_dir| {
909+
Ok(Box::new(
910+
CffiBindingGenerator::new(interpreter, temp_dir)
911+
.context("Failed to initialize Cffi binding generator")?,
912+
))
913+
},
912914
sbom_data,
913-
&out_dirs,
914915
)?;
915916

916917
// Warn if cffi isn't specified in the requirements
@@ -927,52 +928,21 @@ impl BuildContext {
927928
}
928929

929930
eprintln!("📦 Built wheel to {}", wheel_path.display());
930-
wheels.push((wheel_path, "py3".to_string()));
931-
932-
Ok(wheels)
933-
}
934-
935-
fn write_uniffi_wheel(
936-
&self,
937-
artifact: BuildArtifact,
938-
platform_tags: &[PlatformTag],
939-
ext_libs: Vec<Library>,
940-
sbom_data: &Option<SbomData>,
941-
out_dirs: &HashMap<String, PathBuf>,
942-
) -> Result<PathBuf> {
943-
let tag = self.get_universal_tag(platform_tags)?;
944-
945-
self.write_wheel(
946-
&tag,
947-
&[&artifact],
948-
&[ext_libs],
949-
|_temp_dir| Ok(Box::new(UniFfiBindingGenerator::default())),
950-
sbom_data,
951-
out_dirs,
952-
)
931+
Ok(vec![(wheel_path, "py3".to_string())])
953932
}
954933

955934
/// Builds a wheel with uniffi bindings
956935
pub fn build_uniffi_wheel(
957936
&self,
958937
sbom_data: &Option<SbomData>,
959938
) -> Result<Vec<BuiltWheelMetadata>> {
960-
let mut wheels = Vec::new();
961-
let (artifact, out_dirs) = self.compile_cdylib(None, None)?;
962-
let (policy, external_libs) = self.auditwheel(&artifact, &self.platform_tag, None)?;
963-
let platform_tags = self.resolve_platform_tags(&policy);
964-
let wheel_path = self.write_uniffi_wheel(
965-
artifact,
966-
&platform_tags,
967-
external_libs,
939+
let (wheel_path, _) = self.build_cdylib_wheel(
940+
|_temp_dir| Ok(Box::new(UniFfiBindingGenerator::default())),
968941
sbom_data,
969-
&out_dirs,
970942
)?;
971943

972944
eprintln!("📦 Built wheel to {}", wheel_path.display());
973-
wheels.push((wheel_path, "py3".to_string()));
974-
975-
Ok(wheels)
945+
Ok(vec![(wheel_path, "py3".to_string())])
976946
}
977947

978948
fn write_bin_wheel(

0 commit comments

Comments
 (0)