From f1d5801d3b299fa2e87d176f03b605532f931cb6 Mon Sep 17 00:00:00 2001 From: Arthur Meyre Date: Thu, 4 Jan 2024 13:15:41 +0100 Subject: [PATCH] fix: select package for bindgen based on name and manifest path - the old code was selecting the first package that was found which meant that dependencies with the same name could be used for bindgen instead of the intended package - a fallback to the old behavior is kept for now in case manifest paths are not matching because of relative paths for example - this should be backwards compatible with all cbindgen usage, the selection bug may still trigger in case the manifest path does not match --- src/bindgen/cargo/cargo.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/bindgen/cargo/cargo.rs b/src/bindgen/cargo/cargo.rs index 542f8e330..d639a82d8 100644 --- a/src/bindgen/cargo/cargo.rs +++ b/src/bindgen/cargo/cargo.rs @@ -87,7 +87,7 @@ impl Cargo { } pub(crate) fn binding_crate_ref(&self) -> PackageRef { - match self.find_pkg_ref(&self.binding_crate_name) { + match self.find_pkg_to_generate_bindings_ref(&self.binding_crate_name) { Some(pkg_ref) => pkg_ref, None => panic!( "Unable to find {} for {:?}", @@ -180,15 +180,28 @@ impl Cargo { .collect() } - /// Finds the package reference in `cargo metadata` that has `package_name` - /// ignoring the version. - fn find_pkg_ref(&self, package_name: &str) -> Option { + /// Finds the package reference for which we want to generate bindings in `cargo metadata` + /// matching on `package_name` and verifying the manifest path matches so that we don't get a + /// a dependency with the same name (fix for https://github.com/mozilla/cbindgen/issues/900) + fn find_pkg_to_generate_bindings_ref(&self, package_name: &str) -> Option { + // Keep a list of candidates in case the manifest check fails, so that the old behavior + // still applies, returning the first package that was found + let mut candidates = vec![]; for package in &self.metadata.packages { if package.name_and_version.name == package_name { - return Some(package.name_and_version.clone()); + // If we are sure it is the right package return it + if Path::new(package.manifest_path.as_str()) == self.manifest_path { + return Some(package.name_and_version.clone()); + } + // Otherwise note that a package was found + candidates.push(package.name_and_version.clone()); } } - None + + // If we could not verify the manifest path but we found candidates return the first one if + // any, this is the old behavior which did not check for manifest path, kept for backwards + // compatibility + candidates.into_iter().next() } /// Finds the directory for a specified package reference.