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

refactor: factor out Config from core::features #13278

Closed
wants to merge 1 commit into from
Closed
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
40 changes: 30 additions & 10 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ use serde::{Deserialize, Serialize};
use crate::core::resolver::ResolveBehavior;
use crate::util::errors::CargoResult;
use crate::util::indented_lines;
use crate::Config;

pub const SEE_CHANNELS: &str =
"See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information \
Expand Down Expand Up @@ -532,15 +531,16 @@ impl Features {
/// Creates a new unstable features context.
pub fn new(
features: &[String],
config: &Config,
ctx: &dyn UnstableFeatureContext,
warnings: &mut Vec<String>,
is_local: bool,
) -> CargoResult<Features> {
let mut ret = Features::default();
ret.nightly_features_allowed = config.nightly_features_allowed;
let allow_features = ctx.allow_features();
ret.nightly_features_allowed = ctx.nightly_features_allowed();
ret.is_local = is_local;
for feature in features {
ret.add(feature, config, warnings)?;
ret.add(feature, allow_features, warnings)?;
ret.activated.push(feature.to_string());
}
Ok(ret)
Expand All @@ -549,7 +549,7 @@ impl Features {
fn add(
&mut self,
feature_name: &str,
config: &Config,
allow_features: Option<&AllowFeatures>,
warnings: &mut Vec<String>,
) -> CargoResult<()> {
let nightly_features_allowed = self.nightly_features_allowed;
Expand Down Expand Up @@ -598,7 +598,7 @@ impl Features {
see_docs()
),
Status::Unstable => {
if let Some(allow) = &config.cli_unstable().allow_features {
if let Some(allow) = allow_features {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo this change loses some important context of which set of allowed features this is. This is specifically the -Z / CLI features. Maybe cli_allow_features for the trait method and variable??

if !allow.contains(feature_name) {
bail!(
"the feature `{}` is not in the list of allowed features: [{}]",
Expand Down Expand Up @@ -693,6 +693,25 @@ impl Features {
}
}

/// A trait offering methods for [`Features`] to determine which feature is
/// allowed to activate.
pub trait UnstableFeatureContext {
/// This is the top-level gate that when false,
/// no nightly feature would be allowed in this context.
///
/// Generally true when on the nightly channel, and false otherwise.
fn nightly_features_allowed(&self) -> bool;

/// Gets a list of features allowed to use in this context.
///
/// The value normally comes from `-Zallow-features` from the CLI,
/// or `cargo-features` list from the Cargo.toml manifest file.
///
/// Althought being an allow-list, the actual features activated are gated
/// by [`UnstableFeatureContext::nightly_features_allowed`].
fn allow_features(&self) -> Option<&AllowFeatures>;
}

/// Generates `-Z` flags as fields of [`CliUnstable`].
///
/// See the [module-level documentation](self#-z-options) for details.
Expand Down Expand Up @@ -943,8 +962,9 @@ fn parse_gitoxide(
}

impl CliUnstable {
/// Parses `-Z` flags from the command line, and returns messages that warn
/// if any flag has alreardy been stabilized.
/// Parses `-Z` flags from the command line, and adds flags onto itself.
///
/// Returns warnings of unstable flag issues, e.g. already stabilized.
pub fn parse(
&mut self,
flags: &[String],
Expand Down Expand Up @@ -1184,7 +1204,7 @@ impl CliUnstable {
/// unstable subcommand.
pub fn fail_if_stable_command(
&self,
config: &Config,
ctx: &dyn UnstableFeatureContext,
command: &str,
issue: u32,
z_name: &str,
Expand All @@ -1198,7 +1218,7 @@ impl CliUnstable {
information about the `cargo {}` command.",
issue, command
);
if config.nightly_features_allowed {
if ctx.nightly_features_allowed() {
bail!(
"the `cargo {command}` command is unstable, pass `-Z {z_name}` \
to enable it\n\
Expand Down
13 changes: 13 additions & 0 deletions src/cargo/util/config/contexts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use crate::core::features::AllowFeatures;
use crate::core::features::UnstableFeatureContext;
use crate::Config;

impl UnstableFeatureContext for Config {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not usually a fan of trait impls being separate from either the trait or the struct definition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(annotate-snippets took this to an extreme)

fn nightly_features_allowed(&self) -> bool {
self.nightly_features_allowed
}

fn allow_features(&self) -> Option<&AllowFeatures> {
self.unstable_flags.allow_features.as_ref()
}
}
2 changes: 2 additions & 0 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ pub use target::{TargetCfgConfig, TargetConfig};
mod environment;
use environment::Env;

mod contexts;

use super::auth::RegistryConfig;

// Helper macro for creating typed access methods.
Expand Down