Skip to content

Commit 9034e48

Browse files
ehussehuss
committed
Unify weak and namespaced features.
1 parent 81537ee commit 9034e48

File tree

8 files changed

+74
-205
lines changed

8 files changed

+74
-205
lines changed

‎src/cargo/core/resolver/dep_cache.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -405,12 +405,12 @@ impl Requirements<'_> {
405405
&mut self,
406406
package: InternedString,
407407
feat: InternedString,
408-
dep_prefix: bool,
408+
weak: bool,
409409
) -> Result<(), RequirementError> {
410410
// If `package` is indeed an optional dependency then we activate the
411411
// feature named `package`, but otherwise if `package` is a required
412412
// dependency then there's no feature associated with it.
413-
if !dep_prefix
413+
if !weak
414414
&& self
415415
.summary
416416
.dependencies()
@@ -456,12 +456,11 @@ impl Requirements<'_> {
456456
FeatureValue::DepFeature {
457457
dep_name,
458458
dep_feature,
459-
dep_prefix,
460459
// Weak features are always activated in the dependency
461460
// resolver. They will be narrowed inside the new feature
462461
// resolver.
463-
weak: _,
464-
} => self.require_dep_feature(*dep_name, *dep_feature, *dep_prefix)?,
462+
weak,
463+
} => self.require_dep_feature(*dep_name, *dep_feature, *weak)?,
465464
};
466465
Ok(())
467466
}

‎src/cargo/core/resolver/features.rs

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,7 @@ impl CliFeatures {
244244
match feature {
245245
// Maybe call validate_feature_name here once it is an error?
246246
FeatureValue::Feature(_) => {}
247-
FeatureValue::Dep { .. }
248-
| FeatureValue::DepFeature {
249-
dep_prefix: true, ..
250-
} => {
247+
FeatureValue::Dep { .. } => {
251248
bail!(
252249
"feature `{}` is not allowed to use explicit `dep:` syntax",
253250
feature
@@ -441,10 +438,8 @@ pub struct FeatureResolver<'a, 'cfg> {
441438
///
442439
/// The key is the `(package, for_host, dep_name)` of the package whose
443440
/// dependency will trigger the addition of new features. The value is the
444-
/// set of `(feature, dep_prefix)` features to activate (`dep_prefix` is a
445-
/// bool that indicates if `dep:` prefix was used).
446-
deferred_weak_dependencies:
447-
HashMap<(PackageId, bool, InternedString), HashSet<(InternedString, bool)>>,
441+
/// set of features to activate.
442+
deferred_weak_dependencies: HashMap<(PackageId, bool, InternedString), HashSet<InternedString>>,
448443
}
449444

450445
impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
@@ -591,17 +586,9 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
591586
FeatureValue::DepFeature {
592587
dep_name,
593588
dep_feature,
594-
dep_prefix,
595589
weak,
596590
} => {
597-
self.activate_dep_feature(
598-
pkg_id,
599-
for_host,
600-
*dep_name,
601-
*dep_feature,
602-
*dep_prefix,
603-
*weak,
604-
)?;
591+
self.activate_dep_feature(pkg_id, for_host, *dep_name, *dep_feature, *weak)?;
605592
}
606593
}
607594
Ok(())
@@ -675,17 +662,14 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
675662
continue;
676663
}
677664
if let Some(to_enable) = &to_enable {
678-
for (dep_feature, dep_prefix) in to_enable {
665+
for dep_feature in to_enable {
679666
log::trace!(
680667
"activate deferred {} {} -> {}/{}",
681668
pkg_id.name(),
682669
for_host,
683670
dep_name,
684671
dep_feature
685672
);
686-
if !dep_prefix {
687-
self.activate_rec(pkg_id, for_host, dep_name)?;
688-
}
689673
let fv = FeatureValue::new(*dep_feature);
690674
self.activate_fv(dep_pkg_id, dep_for_host, &fv)?;
691675
}
@@ -704,7 +688,6 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
704688
for_host: bool,
705689
dep_name: InternedString,
706690
dep_feature: InternedString,
707-
dep_prefix: bool,
708691
weak: bool,
709692
) -> CargoResult<()> {
710693
for (dep_pkg_id, deps) in self.deps(pkg_id, for_host) {
@@ -733,16 +716,16 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
733716
self.deferred_weak_dependencies
734717
.entry((pkg_id, for_host, dep_name))
735718
.or_default()
736-
.insert((dep_feature, dep_prefix));
719+
.insert(dep_feature);
737720
continue;
738721
}
739722

740723
// Activate the dependency on self.
741724
let fv = FeatureValue::Dep { dep_name };
742725
self.activate_fv(pkg_id, for_host, &fv)?;
743-
if !dep_prefix {
744-
// To retain compatibility with old behavior,
745-
// this also enables a feature of the same
726+
if !weak {
727+
// The old behavior before weak dependencies were
728+
// added is to also enables a feature of the same
746729
// name.
747730
self.activate_rec(pkg_id, for_host, dep_name)?;
748731
}

‎src/cargo/core/summary.rs

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -218,12 +218,7 @@ fn build_feature_map(
218218
.values()
219219
.flatten()
220220
.filter_map(|fv| match fv {
221-
Dep { dep_name }
222-
| DepFeature {
223-
dep_name,
224-
dep_prefix: true,
225-
..
226-
} => Some(*dep_name),
221+
Dep { dep_name } => Some(*dep_name),
227222
_ => None,
228223
})
229224
.collect();
@@ -391,9 +386,6 @@ pub enum FeatureValue {
391386
DepFeature {
392387
dep_name: InternedString,
393388
dep_feature: InternedString,
394-
/// If this is true, then the feature used the `dep:` prefix, which
395-
/// prevents enabling the feature named `dep_name`.
396-
dep_prefix: bool,
397389
/// If `true`, indicates the `?` syntax is used, which means this will
398390
/// not automatically enable the dependency unless the dependency is
399391
/// activated through some other means.
@@ -407,11 +399,6 @@ impl FeatureValue {
407399
Some(pos) => {
408400
let (dep, dep_feat) = feature.split_at(pos);
409401
let dep_feat = &dep_feat[1..];
410-
let (dep, dep_prefix) = if let Some(dep) = dep.strip_prefix("dep:") {
411-
(dep, true)
412-
} else {
413-
(dep, false)
414-
};
415402
let (dep, weak) = if let Some(dep) = dep.strip_suffix('?') {
416403
(dep, true)
417404
} else {
@@ -420,7 +407,6 @@ impl FeatureValue {
420407
FeatureValue::DepFeature {
421408
dep_name: InternedString::new(dep),
422409
dep_feature: InternedString::new(dep_feat),
423-
dep_prefix,
424410
weak,
425411
}
426412
}
@@ -438,14 +424,7 @@ impl FeatureValue {
438424

439425
/// Returns `true` if this feature explicitly used `dep:` syntax.
440426
pub fn has_dep_prefix(&self) -> bool {
441-
matches!(
442-
self,
443-
FeatureValue::Dep { .. }
444-
| FeatureValue::DepFeature {
445-
dep_prefix: true,
446-
..
447-
}
448-
)
427+
matches!(self, FeatureValue::Dep { .. })
449428
}
450429
}
451430

@@ -458,12 +437,10 @@ impl fmt::Display for FeatureValue {
458437
DepFeature {
459438
dep_name,
460439
dep_feature,
461-
dep_prefix,
462440
weak,
463441
} => {
464-
let dep_prefix = if *dep_prefix { "dep:" } else { "" };
465442
let weak = if *weak { "?" } else { "" };
466-
write!(f, "{}{}{}/{}", dep_prefix, dep_name, weak, dep_feature)
443+
write!(f, "{}{}/{}", dep_name, weak, dep_feature)
467444
}
468445
}
469446
}

‎src/cargo/core/workspace.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,14 +1163,10 @@ impl<'cfg> Workspace<'cfg> {
11631163
}
11641164
}
11651165
// This should be enforced by CliFeatures.
1166-
FeatureValue::Dep { .. }
1167-
| FeatureValue::DepFeature {
1168-
dep_prefix: true, ..
1169-
} => panic!("unexpected dep: syntax {}", feature),
1166+
FeatureValue::Dep { .. } => panic!("unexpected dep: syntax {}", feature),
11701167
FeatureValue::DepFeature {
11711168
dep_name,
11721169
dep_feature,
1173-
dep_prefix: _,
11741170
weak: _,
11751171
} => {
11761172
if dependencies.contains_key(dep_name) {
@@ -1283,14 +1279,10 @@ impl<'cfg> Workspace<'cfg> {
12831279
.map(|s| s.to_string())
12841280
.collect::<Vec<_>>()
12851281
}
1286-
FeatureValue::Dep { .. }
1287-
| FeatureValue::DepFeature {
1288-
dep_prefix: true, ..
1289-
} => panic!("unexpected dep: syntax {}", feature),
1282+
FeatureValue::Dep { .. } => panic!("unexpected dep: syntax {}", feature),
12901283
FeatureValue::DepFeature {
12911284
dep_name,
12921285
dep_feature,
1293-
dep_prefix: _,
12941286
weak: _,
12951287
} => {
12961288
// Finds set of `pkg/feat` that are very similar to current `pkg/feat`.
@@ -1446,14 +1438,10 @@ impl<'cfg> Workspace<'cfg> {
14461438
cwd_features.insert(feature.clone());
14471439
}
14481440
// This should be enforced by CliFeatures.
1449-
FeatureValue::Dep { .. }
1450-
| FeatureValue::DepFeature {
1451-
dep_prefix: true, ..
1452-
} => panic!("unexpected dep: syntax {}", feature),
1441+
FeatureValue::Dep { .. } => panic!("unexpected dep: syntax {}", feature),
14531442
FeatureValue::DepFeature {
14541443
dep_name,
14551444
dep_feature,
1456-
dep_prefix: _,
14571445
weak: _,
14581446
} => {
14591447
// I think weak can be ignored here.

‎src/cargo/ops/cargo_compile.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,10 +1225,7 @@ fn validate_required_features(
12251225
))?;
12261226
}
12271227
}
1228-
FeatureValue::Dep { .. }
1229-
| FeatureValue::DepFeature {
1230-
dep_prefix: true, ..
1231-
} => {
1228+
FeatureValue::Dep { .. } => {
12321229
anyhow::bail!(
12331230
"invalid feature `{}` in required-features of target `{}`: \
12341231
`dep:` prefixed feature values are not allowed in required-features",
@@ -1248,7 +1245,6 @@ fn validate_required_features(
12481245
FeatureValue::DepFeature {
12491246
dep_name,
12501247
dep_feature,
1251-
dep_prefix: false,
12521248
weak: false,
12531249
} => {
12541250
match resolve

‎src/cargo/ops/tree/graph.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,6 @@ fn add_cli_features(
488488
FeatureValue::DepFeature {
489489
dep_name,
490490
dep_feature,
491-
dep_prefix: _,
492491
weak,
493492
} => {
494493
let dep_connections = match graph.dep_name_map[&package_index].get(&dep_name) {
@@ -596,12 +595,12 @@ fn add_feature_rec(
596595
FeatureValue::DepFeature {
597596
dep_name,
598597
dep_feature,
599-
dep_prefix,
600-
// `weak` is ignored, because it will be skipped if the
601-
// corresponding dependency is not found in the map, which
602-
// means it wasn't activated. Skipping is handled by
603-
// `is_dep_activated` when the graph was built.
604-
weak: _,
598+
// Note: `weak` is mostly handled when the graph is built in
599+
// `is_dep_activated` which is responsible for skipping
600+
// unactivated weak dependencies. Here it is only used to
601+
// determine if the feature of the dependency name is
602+
// activated on self.
603+
weak,
605604
} => {
606605
let dep_indexes = match graph.dep_name_map[&package_index].get(dep_name) {
607606
Some(indexes) => indexes.clone(),
@@ -619,7 +618,7 @@ fn add_feature_rec(
619618
};
620619
for (dep_index, is_optional) in dep_indexes {
621620
let dep_pkg_id = graph.package_id_for_index(dep_index);
622-
if is_optional && !dep_prefix {
621+
if is_optional && !weak {
623622
// Activate the optional dep on self.
624623
add_feature(
625624
graph,

0 commit comments

Comments
 (0)