Skip to content

Commit

Permalink
refactor(features2): new variant FeaturesFor::ArtifactDep
Browse files Browse the repository at this point in the history
As the existence of `FeaturesFor` is acting as a key that discriminate
activated features for the same package with different circumstances.
Artifact dependencies deserve its own variant and it makes things clear to me.

- ✏️ `NormalOrDevOrArtifactTarget(CompileTarget)` ->  `NormalOrDev`
- 🆕 `ArtifactDep(target)`
  • Loading branch information
weihanglo committed Oct 7, 2022
1 parent 0b84a35 commit 5ea964a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 31 deletions.
5 changes: 1 addition & 4 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,7 @@ pub fn generate_std_roots(
// in time is minimal, and the difference in caching is
// significant.
let mode = CompileMode::Build;
let features = std_features.activated_features(
pkg.package_id(),
FeaturesFor::NormalOrDevOrArtifactTarget(None),
);
let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev);
for kind in kinds {
let list = ret.entry(*kind).or_insert_with(Vec::new);
let unit_for = UnitFor::new_normal(*kind);
Expand Down
45 changes: 21 additions & 24 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,30 +95,29 @@ pub enum ForceAllTargets {
No,
}

/// Flag to indicate if features are requested for a build dependency or not.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)]
/// Flag to indicate if features are requested for a certain type of dependency.
///
/// This is primarily used for constructing a [`PackageFeaturesKey`] to decouple
/// activated features of the same package with different types of dependency.
#[derive(Default, Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)]
pub enum FeaturesFor {
/// If `Some(target)` is present, we represent an artifact target.
/// Otherwise any other normal or dev dependency.
NormalOrDevOrArtifactTarget(Option<CompileTarget>),
/// Normal or dev dependency.
#[default]
NormalOrDev,
/// Build dependency or proc-macro.
HostDep,
}

impl Default for FeaturesFor {
fn default() -> Self {
FeaturesFor::NormalOrDevOrArtifactTarget(None)
}
/// Any dependency with both artifact and target specified.
///
/// That is, `dep = { …, artifact = <crate-type>, target = <triple> }`
ArtifactDep(CompileTarget),
}

impl std::fmt::Display for FeaturesFor {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
FeaturesFor::HostDep => f.write_str("host"),
FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)) => {
f.write_str(&target.rustc_target())
}
FeaturesFor::NormalOrDevOrArtifactTarget(None) => Ok(()),
FeaturesFor::ArtifactDep(target) => f.write_str(&target.rustc_target()),
FeaturesFor::NormalOrDev => Ok(()),
}
}
}
Expand All @@ -128,7 +127,7 @@ impl FeaturesFor {
if for_host {
FeaturesFor::HostDep
} else {
FeaturesFor::NormalOrDevOrArtifactTarget(None)
FeaturesFor::NormalOrDev
}
}

Expand All @@ -137,12 +136,12 @@ impl FeaturesFor {
artifact_target: Option<CompileTarget>,
) -> FeaturesFor {
match artifact_target {
Some(target) => FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)),
Some(target) => FeaturesFor::ArtifactDep(target),
None => {
if for_host {
FeaturesFor::HostDep
} else {
FeaturesFor::NormalOrDevOrArtifactTarget(None)
FeaturesFor::NormalOrDev
}
}
}
Expand Down Expand Up @@ -769,11 +768,11 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
self.target_data
.dep_platform_activated(dep, CompileKind::Host)
}
(_, FeaturesFor::NormalOrDevOrArtifactTarget(None)) => self
(_, FeaturesFor::NormalOrDev) => self
.requested_targets
.iter()
.any(|kind| self.target_data.dep_platform_activated(dep, *kind)),
(_, FeaturesFor::NormalOrDevOrArtifactTarget(Some(target))) => self
(_, FeaturesFor::ArtifactDep(target)) => self
.target_data
.dep_platform_activated(dep, CompileKind::Target(target)),
}
Expand Down Expand Up @@ -832,17 +831,15 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
artifact.is_lib(),
artifact.target().map(|target| match target {
ArtifactTarget::Force(target) => {
vec![FeaturesFor::NormalOrDevOrArtifactTarget(Some(target))]
vec![FeaturesFor::ArtifactDep(target)]
}
ArtifactTarget::BuildDependencyAssumeTarget => self
.requested_targets
.iter()
.filter_map(|kind| match kind {
CompileKind::Host => None,
CompileKind::Target(target) => {
Some(FeaturesFor::NormalOrDevOrArtifactTarget(
Some(*target),
))
Some(FeaturesFor::ArtifactDep(*target))
}
})
.collect(),
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,7 @@ pub fn resolve_all_features(
package_id: PackageId,
) -> HashSet<String> {
let mut features: HashSet<String> = resolved_features
.activated_features(package_id, FeaturesFor::NormalOrDevOrArtifactTarget(None))
.activated_features(package_id, FeaturesFor::NormalOrDev)
.iter()
.map(|s| s.to_string())
.collect();
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/tree/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ fn add_pkg(
let node_features = resolved_features.activated_features(package_id, features_for);
let node_kind = match features_for {
FeaturesFor::HostDep => CompileKind::Host,
FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)) => CompileKind::Target(target),
FeaturesFor::NormalOrDevOrArtifactTarget(None) => requested_kind,
FeaturesFor::ArtifactDep(target) => CompileKind::Target(target),
FeaturesFor::NormalOrDev => requested_kind,
};
let node = Node::Package {
package_id,
Expand Down

0 comments on commit 5ea964a

Please sign in to comment.