Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize -Zfeatures and -Zpackage-features. #8997

Merged
merged 8 commits into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,12 +518,12 @@ fn dep_build_script(
// build.rs unit use the same features. This is because some
// people use `cfg!` and `#[cfg]` expressions to check for enabled
// features instead of just checking `CARGO_FEATURE_*` at runtime.
// In the case with `-Zfeatures=host_dep`, and a shared
// dependency has different features enabled for normal vs. build,
// then the build.rs script will get compiled twice. I believe it
// is not feasible to only build it once because it would break a
// large number of scripts (they would think they have the wrong
// set of features enabled).
// In the case with the new feature resolver (decoupled host
// deps), and a shared dependency has different features enabled
// for normal vs. build, then the build.rs script will get
// compiled twice. I believe it is not feasible to only build it
// once because it would break a large number of scripts (they
// would think they have the wrong set of features enabled).
let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features());
new_unit_dep_with_profile(
state,
Expand Down
4 changes: 1 addition & 3 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ features! {
[unstable] named_profiles: bool,

// Opt-in new-resolver behavior.
[unstable] resolver: bool,
[stable] resolver: bool,

// Allow to specify whether binaries should be stripped.
[unstable] strip: bool,
Expand Down Expand Up @@ -338,7 +338,6 @@ pub struct CliUnstable {
pub no_index_update: bool,
pub avoid_dev_deps: bool,
pub minimal_versions: bool,
pub package_features: bool,
pub advanced_env: bool,
pub config_include: bool,
pub dual_proc_macros: bool,
Expand Down Expand Up @@ -445,7 +444,6 @@ impl CliUnstable {
"no-index-update" => self.no_index_update = parse_empty(k, v)?,
"avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
"minimal-versions" => self.minimal_versions = parse_empty(k, v)?,
"package-features" => self.package_features = parse_empty(k, v)?,
"advanced-env" => self.advanced_env = parse_empty(k, v)?,
"config-include" => self.config_include = parse_empty(k, v)?,
"dual-proc-macros" => self.dual_proc_macros = parse_empty(k, v)?,
Expand Down
13 changes: 5 additions & 8 deletions src/cargo/core/resolver/features.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
//! Feature resolver.
//!
//! This is a new feature resolver that runs independently of the main
//! dependency resolver. It is intended to make it easier to experiment with
//! new behaviors. When `-Zfeatures` is not used, it will fall back to using
//! the original `Resolve` feature computation. With `-Zfeatures` enabled,
//! this will walk the dependency graph and compute the features using a
//! different algorithm.
//! dependency resolver. It is enabled when the user specifies `resolver =
//! "2"` in `Cargo.toml`.
//!
//! One of its key characteristics is that it can avoid unifying features for
//! shared dependencies in some situations. See `FeatureOpts` for the
Expand Down Expand Up @@ -61,11 +58,11 @@ pub struct ResolvedFeatures {
///
/// The value is the `name_in_toml` of the dependencies.
activated_dependencies: ActivateMap,
/// This is only here for legacy support when `-Zfeatures` is not enabled.
/// This is only here for legacy support when the new resolver is not enabled.
///
/// This is the set of features enabled for each package.
legacy_features: Option<HashMap<PackageId, Vec<InternedString>>>,
/// This is only here for legacy support when `-Zfeatures` is not enabled.
/// This is only here for legacy support when the new resolver is not enabled.
///
/// This is the set of optional dependencies enabled for each package.
legacy_dependencies: Option<HashMap<PackageId, HashSet<InternedString>>>,
Expand All @@ -75,7 +72,7 @@ pub struct ResolvedFeatures {
/// Options for how the feature resolver works.
#[derive(Default)]
struct FeatureOpts {
/// -Zfeatures is enabled, use new resolver.
/// Use the new resolver instead of the old one.
new_resolver: bool,
/// Build deps and proc-macros will not share share features with other dep kinds.
decouple_host_deps: bool,
Expand Down
81 changes: 64 additions & 17 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,15 @@ impl<'cfg> Workspace<'cfg> {
self.resolve_behavior.unwrap_or(ResolveBehavior::V1)
}

pub fn allows_unstable_package_features(&self) -> bool {
self.config().cli_unstable().package_features
/// Returns `true` if this workspace uses the new CLI features behavior.
///
/// The old behavior only allowed choosing the features from the package
/// in the current directory, regardless of which packages were chosen
/// with the -p flags. The new behavior allows selecting features from the
/// packages chosen on the command line (with -p or --workspace flags),
/// ignoring whatever is in the current directory.
pub fn allows_new_cli_feature_behavior(&self) -> bool {
self.is_virtual()
|| match self.resolve_behavior() {
ResolveBehavior::V1 => false,
ResolveBehavior::V2 => true,
Expand Down Expand Up @@ -947,15 +954,16 @@ impl<'cfg> Workspace<'cfg> {
.map(|m| (m, RequestedFeatures::new_all(true)))
.collect());
}
if self.allows_unstable_package_features() {
self.members_with_features_pf(specs, requested_features)
if self.allows_new_cli_feature_behavior() {
self.members_with_features_new(specs, requested_features)
} else {
self.members_with_features_stable(specs, requested_features)
self.members_with_features_old(specs, requested_features)
}
}

/// New command-line feature selection with -Zpackage-features.
fn members_with_features_pf(
/// New command-line feature selection behavior with resolver = "2" or the
/// root of a virtual workspace. See `allows_new_cli_feature_behavior`.
fn members_with_features_new(
&self,
specs: &[PackageIdSpec],
requested_features: &RequestedFeatures,
Expand Down Expand Up @@ -1053,30 +1061,69 @@ impl<'cfg> Workspace<'cfg> {
Ok(members)
}

/// This is the current "stable" behavior for command-line feature selection.
fn members_with_features_stable(
/// This is the "old" behavior for command-line feature selection.
/// See `allows_new_cli_feature_behavior`.
fn members_with_features_old(
&self,
specs: &[PackageIdSpec],
requested_features: &RequestedFeatures,
) -> CargoResult<Vec<(&Package, RequestedFeatures)>> {
// Split off any features with the syntax `member-name/feature-name` into a map
// so that those features can be applied directly to those workspace-members.
let mut member_specific_features: HashMap<&str, BTreeSet<InternedString>> = HashMap::new();
// Features for the member in the current directory.
let mut cwd_features = BTreeSet::new();
for feature in requested_features.features.iter() {
if let Some(index) = feature.find('/') {
let name = &feature[..index];
if specs.iter().any(|spec| spec.name() == name) {
member_specific_features
.entry(name)
.or_default()
.insert(InternedString::new(&feature[index + 1..]));
} else {
cwd_features.insert(*feature);
}
} else {
cwd_features.insert(*feature);
};
}

let ms = self.members().filter_map(|member| {
let member_id = member.package_id();
match self.current_opt() {
// The features passed on the command-line only apply to
// the "current" package (determined by the cwd).
Some(current) if member_id == current.package_id() => {
Some((member, requested_features.clone()))
let feats = RequestedFeatures {
features: Rc::new(cwd_features.clone()),
all_features: requested_features.all_features,
uses_default_features: requested_features.uses_default_features,
};
Some((member, feats))
}
_ => {
// Ignore members that are not enabled on the command-line.
if specs.iter().any(|spec| spec.matches(member_id)) {
// -p for a workspace member that is not the
// "current" one, don't use the local
// `--features`, only allow `--all-features`.
Some((
member,
RequestedFeatures::new_all(requested_features.all_features),
))
// -p for a workspace member that is not the "current"
// one.
//
// The odd behavior here is due to backwards
// compatibility. `--features` and
// `--no-default-features` used to only apply to the
// "current" package. As an extension, this allows
// member-name/feature-name to set member-specific
// features, which should be backwards-compatible.
let feats = RequestedFeatures {
features: Rc::new(
member_specific_features
.remove(member.name().as_str())
.unwrap_or_default(),
),
uses_default_features: true,
all_features: requested_features.all_features,
};
Some((member, feats))
} else {
// This member was not requested on the command-line, skip.
None
Expand Down
14 changes: 0 additions & 14 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,20 +310,6 @@ pub trait ArgMatchesExt {
if config.cli_unstable().avoid_dev_deps {
ws.set_require_optional_deps(false);
}
if ws.is_virtual() && !ws.allows_unstable_package_features() {
// --all-features is actually honored. In general, workspaces and
// feature flags are a bit of a mess right now.
for flag in &["features", "no-default-features"] {
if self._is_present(flag) {
bail!(
"--{} is not allowed in the root of a virtual workspace\n\
note: while this was previously accepted, it didn't actually do anything\n\
help: change the current directory to the package directory, or use the --manifest-path flag to the path of the package",
flag
);
}
}
}
Ok(ws)
}

Expand Down
14 changes: 1 addition & 13 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,19 +883,7 @@ impl TomlManifest {
.unwrap()
.clone();
package.workspace = None;
let mut cargo_features = self.cargo_features.clone();
package.resolver = ws.resolve_behavior().to_manifest();
if package.resolver.is_some() {
// This should be removed when stabilizing.
match &mut cargo_features {
None => cargo_features = Some(vec!["resolver".to_string()]),
Some(feats) => {
if !feats.iter().any(|feat| feat == "resolver") {
feats.push("resolver".to_string());
}
}
}
}
if let Some(license_file) = &package.license_file {
let license_path = Path::new(&license_file);
let abs_license_path = paths::normalize_path(&package_root.join(license_path));
Expand Down Expand Up @@ -977,7 +965,7 @@ impl TomlManifest {
patch: None,
workspace: None,
badges: self.badges.clone(),
cargo_features,
cargo_features: self.cargo_features.clone(),
});

fn map_deps(
Expand Down
21 changes: 9 additions & 12 deletions src/doc/man/generated_txt/cargo-bench.txt
Original file line number Diff line number Diff line change
Expand Up @@ -163,28 +163,25 @@ OPTIONS
--tests --benches --examples.

Feature Selection
The feature flags allow you to control the enabled features for the
"current" package. The "current" package is the package in the current
directory, or the one specified in --manifest-path. If running in the
root of a virtual workspace, then the default features are selected for
all workspace members, or all features if --all-features is specified.
The feature flags allow you to control which features are enabled. When
no feature options are given, the default feature is activated for every
selected package.

When no feature options are given, the default feature is activated for
every selected package.
See the features documentation
<https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options>
for more details.

--features features
Space or comma separated list of features to activate. These
features only apply to the current directory's package. Features of
direct dependencies may be enabled with <dep-name>/<feature-name>
Space or comma separated list of features to activate. Features of
workspace members may be enabled with package-name/feature-name
syntax. This flag may be specified multiple times, which enables all
specified features.

--all-features
Activate all available features of all selected packages.

--no-default-features
Do not activate the default feature of the current directory's
package.
Do not activate the default feature of the selected packages.

Compilation Options
--target triple
Expand Down
21 changes: 9 additions & 12 deletions src/doc/man/generated_txt/cargo-build.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,28 +106,25 @@ OPTIONS
--tests --benches --examples.

Feature Selection
The feature flags allow you to control the enabled features for the
"current" package. The "current" package is the package in the current
directory, or the one specified in --manifest-path. If running in the
root of a virtual workspace, then the default features are selected for
all workspace members, or all features if --all-features is specified.
The feature flags allow you to control which features are enabled. When
no feature options are given, the default feature is activated for every
selected package.

When no feature options are given, the default feature is activated for
every selected package.
See the features documentation
<https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options>
for more details.

--features features
Space or comma separated list of features to activate. These
features only apply to the current directory's package. Features of
direct dependencies may be enabled with <dep-name>/<feature-name>
Space or comma separated list of features to activate. Features of
workspace members may be enabled with package-name/feature-name
syntax. This flag may be specified multiple times, which enables all
specified features.

--all-features
Activate all available features of all selected packages.

--no-default-features
Do not activate the default feature of the current directory's
package.
Do not activate the default feature of the selected packages.

Compilation Options
--target triple
Expand Down
21 changes: 9 additions & 12 deletions src/doc/man/generated_txt/cargo-check.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,28 +112,25 @@ OPTIONS
--tests --benches --examples.

Feature Selection
The feature flags allow you to control the enabled features for the
"current" package. The "current" package is the package in the current
directory, or the one specified in --manifest-path. If running in the
root of a virtual workspace, then the default features are selected for
all workspace members, or all features if --all-features is specified.
The feature flags allow you to control which features are enabled. When
no feature options are given, the default feature is activated for every
selected package.

When no feature options are given, the default feature is activated for
every selected package.
See the features documentation
<https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options>
for more details.

--features features
Space or comma separated list of features to activate. These
features only apply to the current directory's package. Features of
direct dependencies may be enabled with <dep-name>/<feature-name>
Space or comma separated list of features to activate. Features of
workspace members may be enabled with package-name/feature-name
syntax. This flag may be specified multiple times, which enables all
specified features.

--all-features
Activate all available features of all selected packages.

--no-default-features
Do not activate the default feature of the current directory's
package.
Do not activate the default feature of the selected packages.

Compilation Options
--target triple
Expand Down
Loading