Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Small simplifications to the library mode implementation. #2078

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Small simplifications to the library mode implementation. #2078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions uniffi_bindgen/src/interface/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ impl ComponentInterface {
self.types.namespace_docstring.as_deref()
}

/// The crate this interfaces lives in.
pub fn crate_name(&self) -> &str {
&self.types.namespace.crate_name
}

pub fn uniffi_contract_version(&self) -> u32 {
// This is set by the scripts in the version-mismatch fixture
let force_version = std::env::var("UNIFFI_FORCE_CONTRACT_VERSION");
Expand Down
62 changes: 28 additions & 34 deletions uniffi_bindgen/src/library_mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,27 @@ pub fn generate_external_bindings<T: BindingGenerator>(
let cdylib_name = calc_cdylib_name(library_path);
binding_generator.check_library_path(library_path, cdylib_name)?;

let mut sources = find_sources(
binding_generator,
&cargo_metadata,
library_path,
cdylib_name,
config_file_override,
)?;
let mut sources = find_components(&cargo_metadata, library_path)?
.into_iter()
.map(|(ci, package)| {
let crate_root = package
.manifest_path
.parent()
.context("manifest path has no parent")?;
let mut config = binding_generator
.new_config(&load_initial_config(crate_root, config_file_override)?)?;
if let Some(cdylib_name) = cdylib_name {
config.update_from_cdylib_name(cdylib_name);
}
config.update_from_ci(&ci);
Ok(Source {
config,
ci,
package,
})
})
.collect::<Result<Vec<_>>>()?;
Copy link
Contributor

Choose a reason for hiding this comment

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

This step is also needed for non-external bindings, but I guess that's fine since generate_bindings just calls generate_external_bindings anyway. What's stopping us from merging those 2 functions together?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing really - this is just a small step to make future steps easier to rationalize about.


for i in 0..sources.len() {
// Partition up the sources list because we're eventually going to call
// `update_from_dependency_configs()` which requires an exclusive reference to one source and
Expand All @@ -89,7 +103,7 @@ pub fn generate_external_bindings<T: BindingGenerator>(
.filter_map(|s| {
dependencies
.contains(s.package.name.as_str())
.then_some((s.crate_name.as_str(), &s.config))
.then_some((s.ci.crate_name(), &s.config))
})
.collect();
// We can finally call update_from_dependency_configs
Expand All @@ -99,7 +113,7 @@ pub fn generate_external_bindings<T: BindingGenerator>(
if let Some(crate_name) = &crate_name {
let old_elements = sources.drain(..);
let mut matches: Vec<_> = old_elements
.filter(|s| &s.crate_name == crate_name)
.filter(|s| s.ci.crate_name() == crate_name)
.collect();
match matches.len() {
0 => bail!("Crate {crate_name} not found in {library_path}"),
Expand All @@ -119,7 +133,6 @@ pub fn generate_external_bindings<T: BindingGenerator>(
#[derive(Debug)]
pub struct Source<Config: BindingsConfig> {
pub package: Package,
pub crate_name: String,
pub ci: ComponentInterface,
pub config: Config,
}
Expand All @@ -137,13 +150,10 @@ pub fn calc_cdylib_name(library_path: &Utf8Path) -> Option<&str> {
None
}

fn find_sources<T: BindingGenerator>(
generator: &T,
fn find_components(
cargo_metadata: &cargo_metadata::Metadata,
library_path: &Utf8Path,
cdylib_name: Option<&str>,
config_file_override: Option<&Utf8Path>,
) -> Result<Vec<Source<T::Config>>> {
) -> Result<Vec<(ComponentInterface, Package)>> {
let items = macro_metadata::extract_from_library(library_path)?;
let mut metadata_groups = create_metadata_groups(&items);
group_metadata(&mut metadata_groups, items)?;
Expand Down Expand Up @@ -178,28 +188,12 @@ fn find_sources<T: BindingGenerator>(
.into_values()
.map(|group| {
let package = find_package_by_crate_name(cargo_metadata, &group.namespace.crate_name)?;
let crate_root = package
.manifest_path
.parent()
.context("manifest path has no parent")?;
let crate_name = group.namespace.crate_name.clone();
let mut ci = ComponentInterface::new(&crate_name);
if let Some(metadata) = udl_items.remove(&crate_name) {
let mut ci = ComponentInterface::new(&group.namespace.crate_name);
if let Some(metadata) = udl_items.remove(&group.namespace.crate_name) {
ci.add_metadata(metadata)?;
};
ci.add_metadata(group)?;
let mut config =
generator.new_config(&load_initial_config(crate_root, config_file_override)?)?;
if let Some(cdylib_name) = cdylib_name {
config.update_from_cdylib_name(cdylib_name);
}
config.update_from_ci(&ci);
Ok(Source {
config,
crate_name,
ci,
package,
})
Ok((ci, package))
})
.collect()
}
Expand Down