Skip to content

Commit

Permalink
feat: support specifying version for expansion.
Browse files Browse the repository at this point in the history
See: [mozilla#900].

Previously, cbindgen might sometimes match the wrong version of a crate
if the crate occurs multiple times in the dependency list produced by
`cargo metadata`. This meant that you'd observe transient errors where
_sometimes_ the right output would be produced (when the intended
version is macro-expanded), and sometimes it would not (when the wrong
version is macro-expanded).

This commit modifies the configuration to permit optionally specifying
name and version separately instead of solely specifying version.

This is an initial draft, as I have not yet been able to test it.

[mozilla#900]: mozilla#900

Signed-off-by: Andrew Lilley Brinker <alilleybrinker@gmail.com>
  • Loading branch information
alilleybrinker committed Feb 15, 2024
1 parent c9c90bf commit 9880234
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 8 deletions.
18 changes: 15 additions & 3 deletions src/bindgen/builder.rs
Expand Up @@ -6,7 +6,7 @@ use std::path;

use crate::bindgen::bindings::Bindings;
use crate::bindgen::cargo::Cargo;
use crate::bindgen::config::{Braces, Config, Language, Profile, Style};
use crate::bindgen::config::{Braces, Config, CrateMatcher, Language, Profile, Style};
use crate::bindgen::error::Error;
use crate::bindgen::library::Library;
use crate::bindgen::parser::{self, Parse};
Expand Down Expand Up @@ -214,8 +214,20 @@ impl Builder {
}

#[allow(unused)]
pub fn with_parse_expand<S: AsRef<str>>(mut self, expand: &[S]) -> Builder {
self.config.parse.expand.crates = expand.iter().map(|x| String::from(x.as_ref())).collect();
pub fn with_parse_expand<S1: AsRef<str>, S2: AsRef<str>>(
mut self,
expand: &[(S1, Option<S2>)],
) -> Builder {
self.config.parse.expand.crates = expand
.iter()
.map(|(s1, s2)| match s2 {
Some(s2) => CrateMatcher::NameWithVersion {
name: String::from(s1.as_ref()),
version: String::from(s2.as_ref()),
},
None => CrateMatcher::Name(String::from(s1.as_ref())),
})
.collect();
self
}

Expand Down
48 changes: 45 additions & 3 deletions src/bindgen/config.rs
Expand Up @@ -10,10 +10,10 @@ use std::{fmt, fs, path::Path as StdPath, path::PathBuf as StdPathBuf};
use serde::de::value::{MapAccessDeserializer, SeqAccessDeserializer};
use serde::de::{Deserialize, Deserializer, MapAccess, SeqAccess, Visitor};

use crate::bindgen::ir::annotation::AnnotationSet;
use crate::bindgen::ir::path::Path;
use crate::bindgen::ir::repr::ReprAlign;
pub use crate::bindgen::rename::RenameRule;
use crate::{bindgen::cargo::PackageRef, bindgen::ir::annotation::AnnotationSet};

pub const VERSION: &str = env!("CARGO_PKG_VERSION");

Expand Down Expand Up @@ -757,7 +757,7 @@ deserialize_enum_str!(Profile);
#[serde(default)]
pub struct ParseExpandConfig {
/// The names of crates to parse with `rustc -Zunpretty=expanded`
pub crates: Vec<String>,
pub crates: Vec<CrateMatcher>,
/// Whether to enable all the features when expanding.
pub all_features: bool,
/// Whether to use the default feature set when expanding.
Expand All @@ -769,6 +769,48 @@ pub struct ParseExpandConfig {
pub profile: Profile,
}

impl ParseExpandConfig {
/// Decide if a crate must be expanded or not based on matching.
pub fn must_expand(&self, pkg: &PackageRef) -> bool {
for matcher in &self.crates {
match matcher {
CrateMatcher::Name(name) => {
if *name == pkg.name {
return true;
}
}
CrateMatcher::NameWithVersion { name, version } => {
if let Some(pkg_version) = &pkg.version {
if *name == pkg.name && *version == *pkg_version {
return true;
}
}
}
}
}

false
}
}

#[derive(Debug, Clone, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "snake_case")]
#[serde(deny_unknown_fields)]
#[serde(untagged)]
pub enum CrateMatcher {
Name(String),
NameWithVersion { name: String, version: String },
}

impl CrateMatcher {
pub fn matches_name(&self, other: &str) -> bool {
match self {
CrateMatcher::Name(name) => name == other,
CrateMatcher::NameWithVersion { name, .. } => name == other,
}
}
}

impl Default for ParseExpandConfig {
fn default() -> ParseExpandConfig {
ParseExpandConfig {
Expand Down Expand Up @@ -802,7 +844,7 @@ fn retrocomp_parse_expand_config_deserialize<'de, D: Deserializer<'de>>(

fn visit_seq<A: SeqAccess<'de>>(self, seq: A) -> Result<Self::Value, A::Error> {
let crates =
<Vec<String> as Deserialize>::deserialize(SeqAccessDeserializer::new(seq))?;
<Vec<CrateMatcher> as Deserialize>::deserialize(SeqAccessDeserializer::new(seq))?;
Ok(ParseExpandConfig {
crates,
all_features: true,
Expand Down
4 changes: 2 additions & 2 deletions src/bindgen/parser.rs
Expand Up @@ -116,7 +116,7 @@ impl<'a> Parser<'a> {
.expand
.crates
.iter()
.any(|name| name == pkg_name)
.any(|name| name.matches_name(pkg_name))
{
return true;
}
Expand Down Expand Up @@ -145,7 +145,7 @@ impl<'a> Parser<'a> {
self.parsed_crates.insert(pkg.name.clone());

// Check if we should use cargo expand for this crate
if self.config.parse.expand.crates.contains(&pkg.name) {
if self.config.parse.expand.must_expand(pkg) {
self.parse_expand_crate(pkg)?;
} else {
// Parse the crate before the dependencies otherwise the same-named idents we
Expand Down

0 comments on commit 9880234

Please sign in to comment.