From 8dbeda07ea48ae2b8c48435753947a7aeb874af1 Mon Sep 17 00:00:00 2001 From: Eashwar Ranganathan Date: Fri, 28 Nov 2025 13:27:05 -0500 Subject: [PATCH 1/3] Add `ArchiveSource` to track additional files --- src/archive_source.rs | 19 ++++++++++++++ src/binding_generator/cffi_binding.rs | 36 ++++++++++++++++++--------- src/binding_generator/mod.rs | 30 ++++++++++++++++++---- src/binding_generator/pyo3_binding.rs | 11 +++++--- src/lib.rs | 1 + 5 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 src/archive_source.rs diff --git a/src/archive_source.rs b/src/archive_source.rs new file mode 100644 index 000000000..6c5d0b94a --- /dev/null +++ b/src/archive_source.rs @@ -0,0 +1,19 @@ +use std::path::PathBuf; + +#[derive(Debug, Clone)] +pub(crate) enum ArchiveSource { + Generated(GeneratedSourceData), + File(FileSourceData), +} + +#[derive(Debug, Clone)] +pub(crate) struct GeneratedSourceData { + pub(crate) data: Vec, + pub(crate) executable: bool, +} + +#[derive(Debug, Clone)] +pub(crate) struct FileSourceData { + pub(crate) path: PathBuf, + pub(crate) executable: bool, +} diff --git a/src/binding_generator/cffi_binding.rs b/src/binding_generator/cffi_binding.rs index 7bfe507b2..571f500c4 100644 --- a/src/binding_generator/cffi_binding.rs +++ b/src/binding_generator/cffi_binding.rs @@ -20,6 +20,8 @@ use tracing::debug; use crate::BuildArtifact; use crate::BuildContext; use crate::PythonInterpreter; +use crate::archive_source::ArchiveSource; +use crate::archive_source::GeneratedSourceData; use crate::target::Os; use super::BindingGenerator; @@ -55,22 +57,32 @@ 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"), - cffi_init_file(&cffi_module_file_name).into(), - ); - additional_files.insert( - base_path.join("ffi.py"), - generate_cffi_declarations( - context.manifest_path.parent().unwrap(), - &context.target_dir, - &interpreter - .ok_or_else(|| anyhow!("A python interpreter is required for cffi builds but one was not provided"))? - .executable, - )? - .into(), + ArchiveSource::Generated(source), ); + let declarations = generate_cffi_declarations( + context.manifest_path.parent().unwrap(), + &context.target_dir, + &interpreter + .ok_or_else(|| { + anyhow!( + "A python interpreter is required for cffi builds but one was not provided" + ) + })? + .executable, + )?; + let source = GeneratedSourceData { + data: declarations.into(), + executable: false, + }; + additional_files.insert(base_path.join("ffi.py"), ArchiveSource::Generated(source)); + Ok(GeneratorOutput { artifact_target, artifact_source_override: None, diff --git a/src/binding_generator/mod.rs b/src/binding_generator/mod.rs index ad3a9e554..b663da578 100644 --- a/src/binding_generator/mod.rs +++ b/src/binding_generator/mod.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::io; use std::io::Write as _; use std::path::Path; use std::path::PathBuf; @@ -16,6 +17,7 @@ use crate::BuildContext; use crate::Metadata24; use crate::ModuleWriter; use crate::PythonInterpreter; +use crate::archive_source::ArchiveSource; use crate::module_writer::ModuleWriterExt; use crate::module_writer::write_python_part; @@ -56,7 +58,7 @@ pub(crate) struct GeneratorOutput { /// Additional files to be installed (e.g. __init__.py) /// The provided PathBuf should be relative to the archive root - additional_files: Option>>, + additional_files: Option>, } /// Every binding generator ultimately has to install the following: @@ -139,7 +141,7 @@ pub fn generate_binding( // 3. Install additional files if let Some(additional_files) = additional_files { - for (target, data) in additional_files { + for (target, source) in additional_files { let target = base_path.join(target); fs::create_dir_all(target.parent().unwrap())?; debug!("Generating file {}", target.display()); @@ -148,7 +150,13 @@ pub fn generate_binding( .create(true) .truncate(true) .open(&target)?; - file.write_all(data.as_slice())?; + match source { + ArchiveSource::Generated(source) => file.write_all(&source.data)?, + ArchiveSource::File(source) => { + let mut source = File::options().read(true).open(source.path)?; + io::copy(&mut source, &mut file)?; + } + } } } } @@ -164,9 +172,21 @@ pub fn generate_binding( // 3. Install additional files if let Some(additional_files) = additional_files { - for (target, data) in additional_files { + for (target, source) in additional_files { debug!("Generating archive entry {}", target.display()); - writer.add_bytes(target, None, data.as_slice(), false)?; + 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)?; + } + } } } } diff --git a/src/binding_generator/pyo3_binding.rs b/src/binding_generator/pyo3_binding.rs index 4d9dc2d5d..73dd29ec1 100644 --- a/src/binding_generator/pyo3_binding.rs +++ b/src/binding_generator/pyo3_binding.rs @@ -13,6 +13,8 @@ use crate::BuildArtifact; use crate::BuildContext; use crate::PythonInterpreter; use crate::Target; +use crate::archive_source::ArchiveSource; +use crate::archive_source::GeneratedSourceData; use super::BindingGenerator; use super::GeneratorOutput; @@ -80,9 +82,8 @@ impl BindingGenerator for Pyo3BindingGenerator { Some(_) => None, None => { let mut files = HashMap::new(); - files.insert( - module.join("__init__.py"), - format!( + let source = GeneratedSourceData { + data: format!( r#"from .{ext_name} import * __doc__ = {ext_name}.__doc__ @@ -90,7 +91,9 @@ if hasattr({ext_name}, "__all__"): __all__ = {ext_name}.__all__"# ) .into(), - ); + executable: false, + }; + files.insert(module.join("__init__.py"), ArchiveSource::Generated(source)); Some(files) } }; diff --git a/src/lib.rs b/src/lib.rs index 78d9c780e..14110eb2f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,6 +46,7 @@ pub use crate::upload::{PublishOpt, Registry, UploadError, upload, upload_ui}; pub use auditwheel::PlatformTag; pub use target::Target; +mod archive_source; mod auditwheel; mod binding_generator; mod bridge; From 7b201326e1854ca8b82b8f179edd284423687ff3 Mon Sep 17 00:00:00 2001 From: Eashwar Ranganathan Date: Fri, 28 Nov 2025 13:43:55 -0500 Subject: [PATCH 2/3] Migrate uniffi bindings to new `BindingGenerator` trait --- src/binding_generator/mod.rs | 2 +- src/binding_generator/uniffi_binding.rs | 166 +++++++++++------------- src/build_context.rs | 19 ++- 3 files changed, 83 insertions(+), 104 deletions(-) diff --git a/src/binding_generator/mod.rs b/src/binding_generator/mod.rs index b663da578..822310340 100644 --- a/src/binding_generator/mod.rs +++ b/src/binding_generator/mod.rs @@ -28,7 +28,7 @@ mod wasm_binding; pub use cffi_binding::CffiBindingGenerator; pub use pyo3_binding::Pyo3BindingGenerator; -pub use uniffi_binding::write_uniffi_module; +pub use uniffi_binding::UniFfiBindingGenerator; pub use wasm_binding::write_wasm_launcher; ///A trait to generate the binding files to be included in the built module diff --git a/src/binding_generator/uniffi_binding.rs b/src/binding_generator/uniffi_binding.rs index d6a7d20b8..4a2281572 100644 --- a/src/binding_generator/uniffi_binding.rs +++ b/src/binding_generator/uniffi_binding.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -use std::io::Write as _; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -8,17 +7,85 @@ use anyhow::Context as _; use anyhow::Result; use anyhow::bail; use fs_err as fs; -use fs_err::File; use normpath::PathExt as _; +use tempfile::TempDir; use tracing::debug; -use crate::ModuleWriter; -use crate::PyProjectToml; -use crate::module_writer::ModuleWriterExt; -use crate::module_writer::write_python_part; -use crate::project_layout::ProjectLayout; +use crate::BuildArtifact; +use crate::BuildContext; +use crate::PythonInterpreter; +use crate::archive_source::ArchiveSource; +use crate::archive_source::FileSourceData; +use crate::archive_source::GeneratedSourceData; use crate::target::Os; +use super::BindingGenerator; +use super::GeneratorOutput; + +/// A generator for producing UniFFI bindings. +#[derive(Default)] +pub struct UniFfiBindingGenerator {} + +impl BindingGenerator for UniFfiBindingGenerator { + fn generate_bindings( + &self, + context: &BuildContext, + _interpreter: Option<&PythonInterpreter>, + artifact: &BuildArtifact, + module: &Path, + _temp_dir: &TempDir, + ) -> Result { + let base_path = if context.project_layout.python_module.is_some() { + module.join(&context.project_layout.extension_name) + } else { + module.to_path_buf() + }; + + let UniFfiBindings { + names: binding_names, + cdylib, + path: binding_dir, + } = generate_uniffi_bindings( + context.manifest_path.parent().unwrap(), + &context.target_dir, + &context.module_name, + context.target.target_os(), + &artifact.path, + )?; + let artifact_target = base_path.join(cdylib); + let mut additional_files = HashMap::new(); + + let py_init = binding_names + .iter() + .map(|name| format!("from .{name} import * # NOQA\n")) + .collect::>() + .join(""); + let source = GeneratedSourceData { + data: py_init.into(), + executable: false, + }; + additional_files.insert( + base_path.join("__init__.py"), + ArchiveSource::Generated(source), + ); + + for binding in binding_names { + let filename = format!("{binding}.py"); + let source = FileSourceData { + path: binding_dir.join(&filename), + executable: false, + }; + additional_files.insert(base_path.join(filename), ArchiveSource::File(source)); + } + + Ok(GeneratorOutput { + artifact_target, + artifact_source_override: None, + additional_files: Some(additional_files), + }) + } +} + /// uniffi.toml #[derive(Debug, serde::Deserialize)] struct UniFfiToml { @@ -176,88 +243,3 @@ fn generate_uniffi_bindings( path: binding_dir, }) } - -/// Creates the uniffi module with the shared library -#[allow(clippy::too_many_arguments)] -pub fn write_uniffi_module( - writer: &mut impl ModuleWriter, - project_layout: &ProjectLayout, - crate_dir: &Path, - target_dir: &Path, - module_name: &str, - artifact: &Path, - target_os: Os, - editable: bool, - pyproject_toml: Option<&PyProjectToml>, -) -> Result<()> { - let UniFfiBindings { - names: binding_names, - cdylib, - path: binding_dir, - } = generate_uniffi_bindings(crate_dir, target_dir, module_name, target_os, artifact)?; - - let py_init = binding_names - .iter() - .map(|name| format!("from .{name} import * # NOQA\n")) - .collect::>() - .join(""); - - if !editable { - write_python_part(writer, project_layout, pyproject_toml) - .context("Failed to add the python module to the package")?; - } - - let module; - if let Some(python_module) = &project_layout.python_module { - if editable { - let base_path = python_module.join(&project_layout.extension_name); - fs::create_dir_all(&base_path)?; - let target = base_path.join(&cdylib); - fs::copy(artifact, &target).context(format!( - "Failed to copy {} to {}", - artifact.display(), - target.display() - ))?; - - File::create(base_path.join("__init__.py"))?.write_all(py_init.as_bytes())?; - - for binding_name in binding_names.iter() { - let target: PathBuf = base_path.join(binding_name).with_extension("py"); - fs::copy(binding_dir.join(binding_name).with_extension("py"), &target) - .with_context(|| { - format!("Failed to copy {:?} to {:?}", binding_dir.display(), target) - })?; - } - } - - let relative = project_layout - .rust_module - .strip_prefix(python_module.parent().unwrap()) - .unwrap(); - module = relative.join(&project_layout.extension_name); - } else { - module = PathBuf::from(module_name); - let type_stub = project_layout - .rust_module - .join(format!("{module_name}.pyi")); - if type_stub.exists() { - eprintln!("📖 Found type stub file at {module_name}.pyi"); - writer.add_file(module.join("__init__.pyi"), type_stub, false)?; - writer.add_empty_file(module.join("py.typed"))?; - } - }; - - if !editable || project_layout.python_module.is_none() { - writer.add_bytes(module.join("__init__.py"), None, py_init.as_bytes(), false)?; - for binding in binding_names.iter() { - writer.add_file( - module.join(binding).with_extension("py"), - binding_dir.join(binding).with_extension("py"), - false, - )?; - } - writer.add_file(module.join(cdylib), artifact, true)?; - } - - Ok(()) -} diff --git a/src/build_context.rs b/src/build_context.rs index d8ddb4ac8..77b08cfc1 100644 --- a/src/build_context.rs +++ b/src/build_context.rs @@ -1,8 +1,8 @@ use crate::auditwheel::{AuditWheelMode, get_policy_and_libs, patchelf, relpath}; use crate::auditwheel::{PlatformTag, Policy}; use crate::binding_generator::{ - CffiBindingGenerator, Pyo3BindingGenerator, generate_binding, write_bin, write_uniffi_module, - write_wasm_launcher, + CffiBindingGenerator, Pyo3BindingGenerator, UniFfiBindingGenerator, generate_binding, + write_bin, write_wasm_launcher, }; use crate::bridge::Abi3Version; use crate::build_options::CargoOptions; @@ -1049,16 +1049,13 @@ impl BuildContext { )?; self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?; - write_uniffi_module( + let generator = UniFfiBindingGenerator::default(); + generate_binding( &mut writer, - &self.project_layout, - self.manifest_path.parent().unwrap(), - &self.target_dir, - &self.module_name, - &artifact.path, - self.target.target_os(), - self.editable, - self.pyproject_toml.as_ref(), + &generator, + self, + self.interpreter.first(), + &artifact, )?; self.add_pth(&mut writer)?; From f8e5f0bcbbfca4fce180a0ed43a4b44175ff7c1a Mon Sep 17 00:00:00 2001 From: Eashwar Ranganathan Date: Fri, 28 Nov 2025 23:42:00 -0500 Subject: [PATCH 3/3] Address PR feedback --- src/archive_source.rs | 9 +++++++++ src/binding_generator/mod.rs | 16 +++++++++++----- src/module_writer/mod.rs | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/archive_source.rs b/src/archive_source.rs index 6c5d0b94a..3dacd97a8 100644 --- a/src/archive_source.rs +++ b/src/archive_source.rs @@ -6,6 +6,15 @@ pub(crate) enum ArchiveSource { File(FileSourceData), } +impl ArchiveSource { + pub(crate) fn executable(&self) -> bool { + match self { + Self::Generated(source) => source.executable, + Self::File(source) => source.executable, + } + } +} + #[derive(Debug, Clone)] pub(crate) struct GeneratedSourceData { pub(crate) data: Vec, diff --git a/src/binding_generator/mod.rs b/src/binding_generator/mod.rs index 822310340..a040d0e75 100644 --- a/src/binding_generator/mod.rs +++ b/src/binding_generator/mod.rs @@ -8,6 +8,8 @@ use anyhow::Context as _; use anyhow::Result; use fs_err as fs; use fs_err::File; +#[cfg(unix)] +use fs_err::os::unix::fs::OpenOptionsExt as _; use tempfile::TempDir; use tempfile::tempdir; use tracing::debug; @@ -19,6 +21,7 @@ use crate::ModuleWriter; use crate::PythonInterpreter; use crate::archive_source::ArchiveSource; use crate::module_writer::ModuleWriterExt; +use crate::module_writer::default_permission; use crate::module_writer::write_python_part; mod cffi_binding; @@ -145,11 +148,14 @@ pub fn generate_binding( let target = base_path.join(target); fs::create_dir_all(target.parent().unwrap())?; debug!("Generating file {}", target.display()); - let mut file = File::options() - .write(true) - .create(true) - .truncate(true) - .open(&target)?; + let mut options = File::options(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + { + options.mode(default_permission(source.executable())); + } + + let mut file = options.open(&target)?; match source { ArchiveSource::Generated(source) => file.write_all(&source.data)?, ArchiveSource::File(source) => { diff --git a/src/module_writer/mod.rs b/src/module_writer/mod.rs index 326cae891..c3b612785 100644 --- a/src/module_writer/mod.rs +++ b/src/module_writer/mod.rs @@ -332,7 +332,7 @@ fn permission_is_executable(mode: u32) -> bool { } #[inline] -fn default_permission(executable: bool) -> u32 { +pub(crate) fn default_permission(executable: bool) -> u32 { match executable { true => 0o755, false => 0o644,