Skip to content

Commit

Permalink
Fix optional dependency check when crate is renamed (#2265)
Browse files Browse the repository at this point in the history
Fix #2263 

I think this also fix #2262, because with this change the renamed
optional dependency wouldn't be pulled in at all, rendering the issue
moot.

cc @pamaury

---------

Co-authored-by: UebelAndre <[email protected]>
  • Loading branch information
nbdd0121 and UebelAndre authored Nov 17, 2023
1 parent 40e1a92 commit 8d3705c
Show file tree
Hide file tree
Showing 8 changed files with 20,218 additions and 2 deletions.
34 changes: 32 additions & 2 deletions crate_universe/src/metadata/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ fn is_optional_crate_enabled(
.filter(|&d| d.kind == kind)
.filter(|&d| d.target.as_ref() == target)
.filter(|&d| d.optional)
.find(|&d| sanitize_module_name(&d.name) == dep.name)
.find(|&d| sanitize_module_name(d.rename.as_ref().unwrap_or(&d.name)) == dep.name)
{
enabled_deps.any(|d| d == toml_dep.name.as_str())
enabled_deps.any(|d| d == toml_dep.rename.as_ref().unwrap_or(&toml_dep.name))
} else {
true
}
Expand Down Expand Up @@ -647,6 +647,19 @@ mod test {
.any(|dep| { dep.target_name == "is-terminal" || dep.target_name == "termcolor" }));
}

#[test]
fn renamed_optional_deps_disabled() {
let metadata = metadata::renamed_optional_deps_disabled();

let serde_with = find_metadata_node("serde_with", &metadata);
let serde_with_depset = DependencySet::new_for_node(serde_with, &metadata);
assert!(!serde_with_depset
.normal_deps
.get_iter(None)
.expect("Iterating over known keys should never panic")
.any(|dep| { dep.target_name == "indexmap" }));
}

#[test]
fn optional_deps_enabled() {
let metadata = metadata::optional_deps_enabled();
Expand Down Expand Up @@ -709,4 +722,21 @@ mod test {
.expect("Iterating over known keys should never panic")
.any(|dep| dep.target_name == "serde"));
}

#[test]
fn renamed_optional_deps_enabled() {
let metadata = metadata::renamed_optional_deps_enabled();

let p256 = find_metadata_node("p256", &metadata);
let p256_depset = DependencySet::new_for_node(p256, &metadata);
assert_eq!(
p256_depset
.normal_deps
.get_iter(None)
.expect("Iterating over known keys should never panic")
.filter(|dep| { dep.target_name == "ecdsa" })
.count(),
1
);
}
}
16 changes: 16 additions & 0 deletions crate_universe/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ pub mod metadata {
.unwrap()
}

pub fn renamed_optional_deps_disabled() -> cargo_metadata::Metadata {
serde_json::from_str(include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/test_data/metadata/crate_renamed_optional_deps_disabled/metadata.json"
)))
.unwrap()
}

pub fn optional_deps_disabled_build_dep_enabled() -> cargo_metadata::Metadata {
serde_json::from_str(include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
Expand All @@ -115,6 +123,14 @@ pub mod metadata {
.unwrap()
}

pub fn renamed_optional_deps_enabled() -> cargo_metadata::Metadata {
serde_json::from_str(include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/test_data/metadata/crate_renamed_optional_deps_enabled/metadata.json"
)))
.unwrap()
}

pub fn common() -> cargo_metadata::Metadata {
serde_json::from_str(include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
Expand Down
Loading

0 comments on commit 8d3705c

Please sign in to comment.