From 68f18c89f6c9c36091a37fa707c2f421bd23e072 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Mon, 6 Oct 2025 19:35:42 -0400 Subject: [PATCH] Use BindgenLoader for the Python pipeline I'm liking this BindgenLoader and want to use it in more places, this is a small step towards that. Also added the `CrateConfigSupplier::from_cargo_metadata_command` utility function. --- .../tests/test_generated_bindings.rs | 5 +- uniffi/src/cli/uniffi_bindgen.rs | 29 ++++----- uniffi_bindgen/src/bindings/swift/mod.rs | 11 +--- uniffi_bindgen/src/cargo_metadata.rs | 17 ++++- uniffi_bindgen/src/loader.rs | 64 ++++++++++++++++++- uniffi_bindgen/src/pipeline/initial/mod.rs | 2 +- 6 files changed, 94 insertions(+), 34 deletions(-) diff --git a/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs b/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs index 6033650e74..f8fda7d6b9 100644 --- a/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs +++ b/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs @@ -52,10 +52,7 @@ mod tests { let config_supplier = { use uniffi_bindgen::cargo_metadata::CrateConfigSupplier; - let metadata = cargo_metadata::MetadataCommand::new() - .exec() - .expect("error running cargo metadata"); - CrateConfigSupplier::from(metadata) + CrateConfigSupplier::from_cargo_metadata_command(false).unwrap() }; uniffi_bindgen::library_mode::generate_bindings( diff --git a/uniffi/src/cli/uniffi_bindgen.rs b/uniffi/src/cli/uniffi_bindgen.rs index 2624abc7b7..bedd2c7f03 100644 --- a/uniffi/src/cli/uniffi_bindgen.rs +++ b/uniffi/src/cli/uniffi_bindgen.rs @@ -2,12 +2,13 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; use camino::Utf8PathBuf; use clap::{Args, Parser, Subcommand, ValueEnum}; use std::fmt; use uniffi_bindgen::bindings::*; use uniffi_bindgen::pipeline::initial; +use uniffi_bindgen::BindgenLoader; use uniffi_pipeline::PrintOptions; /// Enumeration of all foreign language targets currently supported by our CLI. @@ -187,15 +188,10 @@ fn config_supplier( metadata_no_deps: bool, ) -> Result { #[cfg(feature = "cargo-metadata")] - { - use uniffi_bindgen::cargo_metadata::CrateConfigSupplier; - let mut cmd = cargo_metadata::MetadataCommand::new(); - if metadata_no_deps { - cmd.no_deps(); - } - let metadata = cmd.exec().context("error running cargo metadata")?; - Ok(CrateConfigSupplier::from(metadata)) - } + return uniffi_bindgen::cargo_metadata::CrateConfigSupplier::from_cargo_metadata_command( + metadata_no_deps, + ); + #[cfg(not(feature = "cargo-metadata"))] Ok(uniffi_bindgen::EmptyCrateConfigSupplier) } @@ -330,11 +326,14 @@ pub fn run_main() -> anyhow::Result<()> { .as_ref() .expect("--out-dir is required when generating {language} bindings"); let config_supplier = config_supplier(metadata_no_deps)?; - let initial_root = if library_mode { - initial::Root::from_library(config_supplier, &source, crate_name.clone())? - } else { - initial::Root::from_udl(config_supplier, &source, crate_name.clone())? - }; + let loader = BindgenLoader::new(&config_supplier); + // Note: we ignore `library_mode` in this case, `BindgenLoader` is smart enough to + // auto-detect the source type + let mut metadata = loader.load_metadata(&source)?; + if let Some(crate_name) = &crate_name { + loader.filter_metadata_by_crate_name(&mut metadata, crate_name); + } + let initial_root = loader.load_pipeline_initial_root(metadata)?; match language { TargetLanguage::Python => python::run_pipeline(initial_root, out_dir)?, diff --git a/uniffi_bindgen/src/bindings/swift/mod.rs b/uniffi_bindgen/src/bindings/swift/mod.rs index e946e0c2c2..b556808d1b 100644 --- a/uniffi_bindgen/src/bindings/swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/mod.rs @@ -146,14 +146,9 @@ impl BindingGenerator for SwiftBindingGenerator { pub fn generate_swift_bindings(options: SwiftBindingsOptions) -> Result<()> { #[cfg(feature = "cargo-metadata")] let config_supplier = { - use crate::cargo_metadata::CrateConfigSupplier; - use anyhow::Context; - let mut cmd = cargo_metadata::MetadataCommand::new(); - if options.metadata_no_deps { - cmd.no_deps(); - } - let metadata = cmd.exec().context("error running cargo metadata")?; - CrateConfigSupplier::from(metadata) + crate::cargo_metadata::CrateConfigSupplier::from_cargo_metadata_command( + options.metadata_no_deps, + )? }; #[cfg(not(feature = "cargo-metadata"))] let config_supplier = crate::EmptyCrateConfigSupplier; diff --git a/uniffi_bindgen/src/cargo_metadata.rs b/uniffi_bindgen/src/cargo_metadata.rs index 2a0009cf7a..37593c46d9 100644 --- a/uniffi_bindgen/src/cargo_metadata.rs +++ b/uniffi_bindgen/src/cargo_metadata.rs @@ -5,7 +5,7 @@ //! Helpers for data returned by cargo_metadata. Note that this doesn't //! execute cargo_metadata, just parses its output. -use anyhow::{bail, Context}; +use anyhow::{bail, Context, Result}; use camino::Utf8PathBuf; use cargo_metadata::Metadata; use std::{collections::HashMap, fs}; @@ -17,8 +17,19 @@ pub struct CrateConfigSupplier { paths: HashMap, } +impl CrateConfigSupplier { + pub fn from_cargo_metadata_command(no_deps: bool) -> Result { + let mut cmd = cargo_metadata::MetadataCommand::new(); + if no_deps { + cmd.no_deps(); + } + let metadata = cmd.exec().context("error running cargo metadata")?; + Ok(Self::from(metadata)) + } +} + impl BindgenCrateConfigSupplier for CrateConfigSupplier { - fn get_toml(&self, crate_name: &str) -> anyhow::Result> { + fn get_toml(&self, crate_name: &str) -> Result> { crate::load_toml_file(self.get_toml_path(crate_name).as_deref()) } @@ -26,7 +37,7 @@ impl BindgenCrateConfigSupplier for CrateConfigSupplier { self.paths.get(crate_name).map(|p| p.join("uniffi.toml")) } - fn get_udl(&self, crate_name: &str, udl_name: &str) -> anyhow::Result { + fn get_udl(&self, crate_name: &str, udl_name: &str) -> Result { let path = self .paths .get(crate_name) diff --git a/uniffi_bindgen/src/loader.rs b/uniffi_bindgen/src/loader.rs index 788db42390..0bae2f22c0 100644 --- a/uniffi_bindgen/src/loader.rs +++ b/uniffi_bindgen/src/loader.rs @@ -15,8 +15,8 @@ use uniffi_meta::{ }; use crate::{ - crate_name_from_cargo_toml, macro_metadata, BindgenCrateConfigSupplier, Component, - ComponentInterface, Result, + crate_name_from_cargo_toml, interface, macro_metadata, pipeline, BindgenCrateConfigSupplier, + Component, ComponentInterface, Result, }; /// Load metadata, component interfaces, configuration, etc. for binding generators. @@ -61,7 +61,9 @@ impl<'config> BindgenLoader<'config> { match source_path.extension() { Some(ext) if ext.to_lowercase() == "udl" => { let crate_name = crate_name_from_cargo_toml(source_path)?; - let group = uniffi_udl::parse_udl(&fs::read_to_string(source_path)?, &crate_name)?; + let mut group = + uniffi_udl::parse_udl(&fs::read_to_string(source_path)?, &crate_name)?; + Self::add_checksums(&mut group); Ok(HashMap::from([(crate_name, group)])) } _ => { @@ -88,6 +90,39 @@ impl<'config> BindgenLoader<'config> { } } + /// Add checksums for metadata parsed from UDL + fn add_checksums(metadata_group: &mut MetadataGroup) { + // Need to temporarily take items BTree set, since we're technically mutating it's contents + // Each item will be added back at the bottom of the for loop. + let items = std::mem::take(&mut metadata_group.items); + for mut meta in items { + // Make sure metadata checksums are set + match &mut meta { + Metadata::Func(func) => { + func.checksum = Some(uniffi_meta::checksum(&interface::Function::from( + func.clone(), + ))); + } + Metadata::Method(meth) => { + // making a method is mildly tricky as we need a type for self. + // for the purposes of a checksum we ignore self info from udl. + let method_object = + interface::Method::from_metadata(meth.clone(), uniffi_meta::Type::UInt8); + meth.checksum = Some(uniffi_meta::checksum(&method_object)); + } + Metadata::Constructor(cons) => { + cons.checksum = Some(uniffi_meta::checksum(&interface::Constructor::from( + cons.clone(), + ))); + } + // Note: UDL-based callbacks don't have checksum functions, don't set the + // checksum for those. + _ => (), + } + metadata_group.items.insert(meta); + } + } + /// Load a [ComponentInterface] list /// /// This converts the metadata into `ComponentInterface` instances, which contains additional @@ -182,6 +217,25 @@ impl<'config> BindgenLoader<'config> { .collect() } + /// Load a [pipeline::initial::Root] value from the metadata + pub fn load_pipeline_initial_root( + &self, + metadata: MetadataGroupMap, + ) -> Result { + let mut metadata_converter = pipeline::initial::UniffiMetaConverter::default(); + for metadata_group in metadata.into_values() { + if let Some(docstring) = metadata_group.namespace_docstring { + metadata_converter + .add_module_docstring(metadata_group.namespace.name.clone(), docstring); + } + metadata_converter.add_metadata_item(Metadata::Namespace(metadata_group.namespace))?; + for meta in metadata_group.items { + metadata_converter.add_metadata_item(meta)?; + } + } + metadata_converter.try_into_initial_ir() + } + /// Get the basename for a source file /// /// This will remove any file extension. @@ -206,4 +260,8 @@ impl<'config> BindgenLoader<'config> { Some(ext) if ext.to_lowercase() == "udl" ) } + + pub fn filter_metadata_by_crate_name(&self, metadata: &mut MetadataGroupMap, crate_name: &str) { + metadata.retain(|_, metadata_group| metadata_group.namespace.crate_name == crate_name) + } } diff --git a/uniffi_bindgen/src/pipeline/initial/mod.rs b/uniffi_bindgen/src/pipeline/initial/mod.rs index c256e99242..9fb2d4d250 100644 --- a/uniffi_bindgen/src/pipeline/initial/mod.rs +++ b/uniffi_bindgen/src/pipeline/initial/mod.rs @@ -17,7 +17,7 @@ use anyhow::Result; use camino::Utf8Path; use crate::{crate_name_from_cargo_toml, interface, macro_metadata, BindgenCrateConfigSupplier}; -use from_uniffi_meta::UniffiMetaConverter; +pub use from_uniffi_meta::UniffiMetaConverter; impl Root { pub fn from_library(