From a524879220481652bc3637c9c48f801d2ff05821 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Thu, 18 Jul 2024 12:44:54 -0600 Subject: [PATCH 1/7] Thread `ExternalType` metadata into RustBuffer In service of Java bindgen being able to generate fully qualified `RustBuffer`s when necessary. --- .../src/bindings/kotlin/gen_kotlin/mod.rs | 5 +++-- .../src/bindings/python/gen_python/mod.rs | 8 ++++---- uniffi_bindgen/src/interface/ffi.rs | 17 +++++++++++++++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 2b99d8be3b..c909a437d4 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -431,8 +431,9 @@ impl KotlinCodeOracle { FfiType::Float64 => "Double".to_string(), FfiType::Handle => "Long".to_string(), FfiType::RustArcPtr(_) => "Pointer".to_string(), - FfiType::RustBuffer(maybe_suffix) => { - format!("RustBuffer{}", maybe_suffix.as_deref().unwrap_or_default()) + FfiType::RustBuffer(maybe_external) => match maybe_external { + Some(external_meta) => format!("RustBuffer{}", external_meta.name), + None => "RustBuffer".to_string() } FfiType::RustCallStatus => "UniffiRustCallStatus.ByValue".to_string(), FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(), diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index 61ab04166e..e869d6856b 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -377,8 +377,8 @@ impl PythonCodeOracle { FfiType::Float64 => "ctypes.c_double".to_string(), FfiType::Handle => "ctypes.c_uint64".to_string(), FfiType::RustArcPtr(_) => "ctypes.c_void_p".to_string(), - FfiType::RustBuffer(maybe_suffix) => match maybe_suffix { - Some(suffix) => format!("_UniffiRustBuffer{suffix}"), + FfiType::RustBuffer(maybe_external) => match maybe_external { + Some(external_meta) => format!("_UniffiRustBuffer{}", external_meta.name), None => "_UniffiRustBuffer".to_string(), }, FfiType::RustCallStatus => "_UniffiRustCallStatus".to_string(), @@ -407,8 +407,8 @@ impl PythonCodeOracle { | FfiType::Int64 => "0".to_owned(), FfiType::Float32 | FfiType::Float64 => "0.0".to_owned(), FfiType::RustArcPtr(_) => "ctypes.c_void_p()".to_owned(), - FfiType::RustBuffer(maybe_suffix) => match maybe_suffix { - Some(suffix) => format!("_UniffiRustBuffer{suffix}.default()"), + FfiType::RustBuffer(maybe_external) => match maybe_external { + Some(external_meta) => format!("_UniffiRustBuffer{}.default()", external_meta.name), None => "_UniffiRustBuffer.default()".to_owned(), }, _ => unimplemented!("FFI return type: {t:?}"), diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index a1dc29713a..33f8ebb4d4 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -43,7 +43,7 @@ pub enum FfiType { /// or pass it to someone that will. /// If the inner option is Some, it is the name of the external type. The bindings may need /// to use this name to import the correct RustBuffer for that type. - RustBuffer(Option), + RustBuffer(Option), /// A borrowed reference to some raw bytes owned by foreign language code. /// The provider of this reference must keep it alive for the duration of the receiving call. ForeignBytes, @@ -144,13 +144,26 @@ impl From<&Type> for FfiType { Type::External { name, kind: ExternalKind::DataClass, + module_path, + namespace, .. - } => FfiType::RustBuffer(Some(name.clone())), + } => FfiType::RustBuffer(Some(ExternalMetadata { + name: name.clone(), + module_path: module_path.clone(), + namespace: namespace.clone() + })), Type::Custom { builtin, .. } => FfiType::from(builtin.as_ref()), } } } +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct ExternalMetadata { + pub name: String, + pub module_path: String, + pub namespace: String +} + // Needed for rust scaffolding askama template impl From for FfiType { fn from(ty: Type) -> Self { From fecb169c8bafae5c16895773edea11061c4ba37a Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Fri, 19 Jul 2024 12:06:54 -0600 Subject: [PATCH 2/7] Switch to `test --no-run` from `build` for cdylib `uniffi-bindgen-java` is external to the uniffi repo, so the fixtures/examples are all `dev-dependencies`, which aren't built on a call to `cargo build`. `cargo test --no-run` causes them to be built but doesn't cause a run of tests in place. `.no_deps()` seems like it has a similar reaction as an external bindings generator, it ignores dependencies in the cargo metadata, which is where all the fixtures/examples used in tests cdylibs will be. --- uniffi_bindgen/src/library_mode.rs | 1 - uniffi_testing/src/lib.rs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/uniffi_bindgen/src/library_mode.rs b/uniffi_bindgen/src/library_mode.rs index 0c110dc722..5fcef6f806 100644 --- a/uniffi_bindgen/src/library_mode.rs +++ b/uniffi_bindgen/src/library_mode.rs @@ -149,7 +149,6 @@ impl CratePathMap { fn new() -> Result { Ok(Self::from( cargo_metadata::MetadataCommand::new() - .no_deps() .exec() .context("error running cargo metadata")?, )) diff --git a/uniffi_testing/src/lib.rs b/uniffi_testing/src/lib.rs index 54dc247d71..df6a4c457c 100644 --- a/uniffi_testing/src/lib.rs +++ b/uniffi_testing/src/lib.rs @@ -159,7 +159,8 @@ fn get_cargo_metadata() -> Metadata { fn get_cargo_build_messages() -> Vec { let mut child = Command::new(env!("CARGO")) - .arg("build") + .arg("test") + .arg("--no-run") .arg("--message-format=json") .stdout(Stdio::piped()) .spawn() From f75fc4718d4bda51fd3fcc70e36cfdc621a8e379 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Wed, 24 Jul 2024 18:51:00 -0600 Subject: [PATCH 3/7] `cargo fmt`, make `no_deps` optional in lib mode --- uniffi/src/cli.rs | 6 ++++++ .../src/bindings/kotlin/gen_kotlin/mod.rs | 4 ++-- uniffi_bindgen/src/bindings/kotlin/test.rs | 1 + .../src/bindings/python/gen_python/mod.rs | 4 +++- uniffi_bindgen/src/bindings/python/test.rs | 1 + uniffi_bindgen/src/bindings/ruby/test.rs | 1 + uniffi_bindgen/src/bindings/swift/test.rs | 1 + uniffi_bindgen/src/interface/ffi.rs | 4 ++-- uniffi_bindgen/src/library_mode.rs | 15 +++++++++------ 9 files changed, 26 insertions(+), 11 deletions(-) diff --git a/uniffi/src/cli.rs b/uniffi/src/cli.rs index 09f408d628..0ed0b95026 100644 --- a/uniffi/src/cli.rs +++ b/uniffi/src/cli.rs @@ -138,6 +138,7 @@ fn gen_library_mode( cfo: Option<&camino::Utf8Path>, out_dir: &camino::Utf8Path, fmt: bool, + no_deps: bool, ) -> anyhow::Result<()> { use uniffi_bindgen::library_mode::generate_bindings; for language in languages { @@ -158,6 +159,7 @@ fn gen_library_mode( cfo, out_dir, fmt, + no_deps, )? .len(), TargetLanguage::Python => generate_bindings( @@ -167,6 +169,7 @@ fn gen_library_mode( cfo, out_dir, fmt, + no_deps, )? .len(), TargetLanguage::Ruby => generate_bindings( @@ -176,6 +179,7 @@ fn gen_library_mode( cfo, out_dir, fmt, + no_deps, )? .len(), TargetLanguage::Swift => generate_bindings( @@ -185,6 +189,7 @@ fn gen_library_mode( cfo, out_dir, fmt, + no_deps, )? .len(), }; @@ -273,6 +278,7 @@ pub fn run_main() -> anyhow::Result<()> { config.as_deref(), &out_dir, !no_format, + true, )?; } else { gen_bindings( diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index c909a437d4..92f2f39c33 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -433,8 +433,8 @@ impl KotlinCodeOracle { FfiType::RustArcPtr(_) => "Pointer".to_string(), FfiType::RustBuffer(maybe_external) => match maybe_external { Some(external_meta) => format!("RustBuffer{}", external_meta.name), - None => "RustBuffer".to_string() - } + None => "RustBuffer".to_string(), + }, FfiType::RustCallStatus => "UniffiRustCallStatus.ByValue".to_string(), FfiType::ForeignBytes => "ForeignBytes.ByValue".to_string(), FfiType::Callback(name) => self.ffi_callback_name(name), diff --git a/uniffi_bindgen/src/bindings/kotlin/test.rs b/uniffi_bindgen/src/bindings/kotlin/test.rs index 48c3d06cf1..d2b3197f76 100644 --- a/uniffi_bindgen/src/bindings/kotlin/test.rs +++ b/uniffi_bindgen/src/bindings/kotlin/test.rs @@ -42,6 +42,7 @@ pub fn run_script( None, &out_dir, false, + true, )?; let jar_file = build_jar(crate_name, &out_dir, options)?; diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index e869d6856b..58dda3c16b 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -408,7 +408,9 @@ impl PythonCodeOracle { FfiType::Float32 | FfiType::Float64 => "0.0".to_owned(), FfiType::RustArcPtr(_) => "ctypes.c_void_p()".to_owned(), FfiType::RustBuffer(maybe_external) => match maybe_external { - Some(external_meta) => format!("_UniffiRustBuffer{}.default()", external_meta.name), + Some(external_meta) => { + format!("_UniffiRustBuffer{}.default()", external_meta.name) + } None => "_UniffiRustBuffer.default()".to_owned(), }, _ => unimplemented!("FFI return type: {t:?}"), diff --git a/uniffi_bindgen/src/bindings/python/test.rs b/uniffi_bindgen/src/bindings/python/test.rs index 0392c324d4..881289a640 100644 --- a/uniffi_bindgen/src/bindings/python/test.rs +++ b/uniffi_bindgen/src/bindings/python/test.rs @@ -43,6 +43,7 @@ pub fn run_script( None, &out_dir, false, + true, )?; let pythonpath = env::var_os("PYTHONPATH").unwrap_or_else(|| OsString::from("")); diff --git a/uniffi_bindgen/src/bindings/ruby/test.rs b/uniffi_bindgen/src/bindings/ruby/test.rs index 86965fe2ff..d0cf6b0a24 100644 --- a/uniffi_bindgen/src/bindings/ruby/test.rs +++ b/uniffi_bindgen/src/bindings/ruby/test.rs @@ -40,6 +40,7 @@ pub fn test_script_command( None, &out_dir, false, + true, )?; let rubypath = env::var_os("RUBYLIB").unwrap_or_else(|| OsString::from("")); diff --git a/uniffi_bindgen/src/bindings/swift/test.rs b/uniffi_bindgen/src/bindings/swift/test.rs index 2bc98b6163..68a22fed56 100644 --- a/uniffi_bindgen/src/bindings/swift/test.rs +++ b/uniffi_bindgen/src/bindings/swift/test.rs @@ -133,6 +133,7 @@ impl GeneratedSources { None, out_dir, false, + true, )?; let main_source = sources .iter() diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 33f8ebb4d4..363dc95723 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -150,7 +150,7 @@ impl From<&Type> for FfiType { } => FfiType::RustBuffer(Some(ExternalMetadata { name: name.clone(), module_path: module_path.clone(), - namespace: namespace.clone() + namespace: namespace.clone(), })), Type::Custom { builtin, .. } => FfiType::from(builtin.as_ref()), } @@ -161,7 +161,7 @@ impl From<&Type> for FfiType { pub struct ExternalMetadata { pub name: String, pub module_path: String, - pub namespace: String + pub namespace: String, } // Needed for rust scaffolding askama template diff --git a/uniffi_bindgen/src/library_mode.rs b/uniffi_bindgen/src/library_mode.rs index 5fcef6f806..65eb17d178 100644 --- a/uniffi_bindgen/src/library_mode.rs +++ b/uniffi_bindgen/src/library_mode.rs @@ -40,8 +40,9 @@ pub fn generate_bindings( config_file_override: Option<&Utf8Path>, out_dir: &Utf8Path, try_format_code: bool, + no_dep_metadata: bool, ) -> Result>> { - let path_map = CratePathMap::new()?; + let path_map = CratePathMap::new(no_dep_metadata)?; let mut components = find_components(&path_map, library_path)? .into_iter() .map(|ci| { @@ -146,16 +147,18 @@ impl CratePathMap { // `config_file_override` - what are the semantics of that?) // Anyway: for now we just do this. #[cfg(feature = "cargo_metadata")] - fn new() -> Result { + fn new(no_dep_metadata: bool) -> Result { + let mut command = cargo_metadata::MetadataCommand::new(); + if no_dep_metadata { + command.no_deps(); + } Ok(Self::from( - cargo_metadata::MetadataCommand::new() - .exec() - .context("error running cargo metadata")?, + command.exec().context("error running cargo metadata")?, )) } #[cfg(not(feature = "cargo_metadata"))] - fn new() -> Result { + fn new(_no_dep_metadata: bool) -> Result { Ok(Self::default()) } From 310aa55765c51946cee7fa72e8cb1ccdb97d4b2f Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Wed, 24 Jul 2024 19:21:41 -0600 Subject: [PATCH 4/7] Add changelog entry per contributing guidelines --- CHANGELOG.md | 7 +++++++ .../docstring-proc-macro/tests/test_generated_bindings.rs | 1 + 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a1b47eed4..5e2fa1f6ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ ## [[UnreleasedUniFFIVersion]] (backend crates: [[UnreleasedBackendVersion]]) - (_[[ReleaseDate]]_) +### What's changed? + +- The library mode internal bindings generation has changed to not fetch dependencies when discovering components. + Fetching dependency metadata may optionally be toggled back on, making this a **breaking change** for bindings + generators. + No consumers of any languages are impacted, only the maintainers of these language bindings. + [All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.28.0...HEAD). ## v0.28.0 (backend crates: v0.28.0) - (_2024-06-11_) diff --git a/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs b/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs index 61c3d6e89d..4d571fe3a9 100644 --- a/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs +++ b/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs @@ -57,6 +57,7 @@ mod tests { None, &out_dir, false, + true, ) .unwrap(); From 294bee7c524f9e3eb779728aa7ce3af270921543 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Wed, 31 Jul 2024 12:12:24 -0600 Subject: [PATCH 5/7] Rename FfiMetadata struct --- uniffi_bindgen/src/interface/ffi.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/uniffi_bindgen/src/interface/ffi.rs b/uniffi_bindgen/src/interface/ffi.rs index 363dc95723..91343e1759 100644 --- a/uniffi_bindgen/src/interface/ffi.rs +++ b/uniffi_bindgen/src/interface/ffi.rs @@ -43,7 +43,7 @@ pub enum FfiType { /// or pass it to someone that will. /// If the inner option is Some, it is the name of the external type. The bindings may need /// to use this name to import the correct RustBuffer for that type. - RustBuffer(Option), + RustBuffer(Option), /// A borrowed reference to some raw bytes owned by foreign language code. /// The provider of this reference must keep it alive for the duration of the receiving call. ForeignBytes, @@ -147,7 +147,7 @@ impl From<&Type> for FfiType { module_path, namespace, .. - } => FfiType::RustBuffer(Some(ExternalMetadata { + } => FfiType::RustBuffer(Some(ExternalFfiMetadata { name: name.clone(), module_path: module_path.clone(), namespace: namespace.clone(), @@ -158,7 +158,7 @@ impl From<&Type> for FfiType { } #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] -pub struct ExternalMetadata { +pub struct ExternalFfiMetadata { pub name: String, pub module_path: String, pub namespace: String, From 1561db33c9b1c3ccea0f8252a89490a856ca2698 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Wed, 31 Jul 2024 13:27:30 -0600 Subject: [PATCH 6/7] Touch up missed spots --- .../docstring-proc-macro/tests/test_generated_bindings.rs | 1 - uniffi/src/cli.rs | 4 ---- uniffi_bindgen/src/bindings/kotlin/test.rs | 1 - uniffi_bindgen/src/bindings/python/test.rs | 1 - uniffi_bindgen/src/bindings/ruby/test.rs | 1 - uniffi_bindgen/src/bindings/swift/test.rs | 1 - uniffi_bindgen/src/library_mode.rs | 1 - 7 files changed, 10 deletions(-) diff --git a/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs b/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs index 90512ee419..e06bfdc6b3 100644 --- a/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs +++ b/fixtures/docstring-proc-macro/tests/test_generated_bindings.rs @@ -66,7 +66,6 @@ mod tests { None, &out_dir, false, - true, ) .unwrap(); diff --git a/uniffi/src/cli.rs b/uniffi/src/cli.rs index 7ebffac3fc..e3d5cf41e9 100644 --- a/uniffi/src/cli.rs +++ b/uniffi/src/cli.rs @@ -182,7 +182,6 @@ fn gen_library_mode( cfo, out_dir, fmt, - no_deps, )? .len(), TargetLanguage::Python => generate_bindings( @@ -193,7 +192,6 @@ fn gen_library_mode( cfo, out_dir, fmt, - no_deps, )? .len(), TargetLanguage::Ruby => generate_bindings( @@ -204,7 +202,6 @@ fn gen_library_mode( cfo, out_dir, fmt, - no_deps, )? .len(), TargetLanguage::Swift => generate_bindings( @@ -215,7 +212,6 @@ fn gen_library_mode( cfo, out_dir, fmt, - no_deps, )? .len(), }; diff --git a/uniffi_bindgen/src/bindings/kotlin/test.rs b/uniffi_bindgen/src/bindings/kotlin/test.rs index cc174bf384..5c7f0c9943 100644 --- a/uniffi_bindgen/src/bindings/kotlin/test.rs +++ b/uniffi_bindgen/src/bindings/kotlin/test.rs @@ -45,7 +45,6 @@ pub fn run_script( None, &out_dir, false, - true, )?; let jar_file = build_jar(crate_name, &out_dir, options)?; diff --git a/uniffi_bindgen/src/bindings/python/test.rs b/uniffi_bindgen/src/bindings/python/test.rs index 6f8e84ec06..39dbc9c136 100644 --- a/uniffi_bindgen/src/bindings/python/test.rs +++ b/uniffi_bindgen/src/bindings/python/test.rs @@ -45,7 +45,6 @@ pub fn run_script( None, &out_dir, false, - true, )?; let pythonpath = env::var_os("PYTHONPATH").unwrap_or_else(|| OsString::from("")); diff --git a/uniffi_bindgen/src/bindings/ruby/test.rs b/uniffi_bindgen/src/bindings/ruby/test.rs index c7689e6e45..47a3b68401 100644 --- a/uniffi_bindgen/src/bindings/ruby/test.rs +++ b/uniffi_bindgen/src/bindings/ruby/test.rs @@ -42,7 +42,6 @@ pub fn test_script_command( None, &out_dir, false, - true, )?; let rubypath = env::var_os("RUBYLIB").unwrap_or_else(|| OsString::from("")); diff --git a/uniffi_bindgen/src/bindings/swift/test.rs b/uniffi_bindgen/src/bindings/swift/test.rs index 1cb080bde5..9a2a5a7de9 100644 --- a/uniffi_bindgen/src/bindings/swift/test.rs +++ b/uniffi_bindgen/src/bindings/swift/test.rs @@ -145,7 +145,6 @@ impl GeneratedSources { None, out_dir, false, - true, )?; let main_source = sources .iter() diff --git a/uniffi_bindgen/src/library_mode.rs b/uniffi_bindgen/src/library_mode.rs index 295ad0797e..b8e919bfbf 100644 --- a/uniffi_bindgen/src/library_mode.rs +++ b/uniffi_bindgen/src/library_mode.rs @@ -41,7 +41,6 @@ pub fn generate_bindings( config_file_override: Option<&Utf8Path>, out_dir: &Utf8Path, try_format_code: bool, - no_dep_metadata: bool, ) -> Result>> { let mut components = find_components(config_supplier, library_path)? .into_iter() From 87948d9c5f0a5c305cdec7c56460152257c33afb Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Wed, 31 Jul 2024 15:55:26 -0600 Subject: [PATCH 7/7] Remove outdated changelog --- CHANGELOG.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b64f702b1..73097deae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,6 @@ ## [[UnreleasedUniFFIVersion]] (backend crates: [[UnreleasedBackendVersion]]) - (_[[ReleaseDate]]_) -### What's changed? - -- The library mode internal bindings generation has changed to not fetch dependencies when discovering components. - Fetching dependency metadata may optionally be toggled back on, making this a **breaking change** for bindings - generators. - No consumers of any languages are impacted, only the maintainers of these language bindings. - [All changes in [[UnreleasedUniFFIVersion]]](https://github.com/mozilla/uniffi-rs/compare/v0.28.0...HEAD). ## v0.28.0 (backend crates: v0.28.0) - (_2024-06-11_)