Skip to content

Commit

Permalink
fix: select package for bindgen based on name and manifest path
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
IceTDrinker authored and emilio committed Feb 26, 2024
1 parent 95047db commit f1d5801
Showing 1 changed file with 19 additions and 6 deletions.
25 changes: 19 additions & 6 deletions src/bindgen/cargo/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {:?}",
Expand Down Expand Up @@ -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<PackageRef> {
/// 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<PackageRef> {
// 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.
Expand Down

0 comments on commit f1d5801

Please sign in to comment.