From 6c422a2c0a4e7290b5d0f080f89233e7f9c692ff Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 28 Aug 2020 18:49:34 -0400 Subject: [PATCH 01/13] Restrict RUSTC_BOOTSTRAP in build.rs --- src/cargo/core/compiler/custom_build.rs | 14 +++++++++++++- tests/testsuite/build_script_env.rs | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 320f4f7ae70..7d6fde053b1 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -562,7 +562,19 @@ impl BuildOutput { } } "rustc-cfg" => cfgs.push(value.to_string()), - "rustc-env" => env.push(BuildOutput::parse_rustc_env(&value, &whence)?), + "rustc-env" => { + let (key, val) = BuildOutput::parse_rustc_env(&value, &whence)?; + // See https://github.com/rust-lang/cargo/issues/7088 + if key == "RUSTC_BOOTSTRAP" { + anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP` from {}.\n\ + note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project. + help: If you're sure you want to do this in your project, use `RUSTC_BOOTSTRAP=1 cargo build` instead. + help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", + whence + ); + } + env.push((key, val)); + } "warning" => warnings.push(value.to_string()), "rerun-if-changed" => rerun_if_changed.push(PathBuf::from(value)), "rerun-if-env-changed" => rerun_if_env_changed.push(value.to_string()), diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index 771c63b6e0b..14fabe20823 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -106,3 +106,22 @@ fn rerun_if_env_or_file_changes() { ) .run(); } + +#[cargo_test] +fn rustc_bootstrap_is_disallowed() { + let p = project() + .file("src/main.rs", "fn main() {}") + .file( + "build.rs", + r#" + fn main() { + println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1"); + } + "#, + ) + .build(); + p.cargo("build") + .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP` [..]") + .with_status(101) + .run(); +} From 418129daea649499558cfadce10afa391fa377d2 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 17 Feb 2021 12:14:28 -0500 Subject: [PATCH 02/13] Downgrade error to a warning when `RUSTC_BOOTSTRAP` is set or this is the nightly channel --- src/cargo/core/compiler/custom_build.rs | 36 +++++++++++++++++++------ src/cargo/core/features.rs | 2 +- tests/testsuite/build_script_env.rs | 9 +++++-- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 7d6fde053b1..198e7f194eb 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -2,6 +2,7 @@ use super::job::{Freshness, Job, Work}; use super::{fingerprint, Context, LinkType, Unit}; use crate::core::compiler::context::Metadata; use crate::core::compiler::job_queue::JobState; +use crate::core::nightly_features_allowed; use crate::core::{profiles::ProfileRoot, PackageId}; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::machine_message::{self, Message}; @@ -564,16 +565,35 @@ impl BuildOutput { "rustc-cfg" => cfgs.push(value.to_string()), "rustc-env" => { let (key, val) = BuildOutput::parse_rustc_env(&value, &whence)?; - // See https://github.com/rust-lang/cargo/issues/7088 + // Build scripts aren't allowed to set RUSTC_BOOTSTRAP. + // See https://github.com/rust-lang/cargo/issues/7088. if key == "RUSTC_BOOTSTRAP" { - anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP` from {}.\n\ - note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project. - help: If you're sure you want to do this in your project, use `RUSTC_BOOTSTRAP=1 cargo build` instead. - help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", - whence - ); + // If RUSTC_BOOTSTRAP is already set, the user of Cargo knows about + // bootstrap and still wants to override the channel. Give them a way to do + // so, but still emit a warning that the current crate shouldn't be trying + // to set RUSTC_BOOTSTRAP. + // If this is a nightly build, setting RUSTC_BOOTSTRAP wouldn't affect the + // behavior, so still only give a warning. + if nightly_features_allowed() { + warnings.push(format!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ + note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ + help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", + val, whence + )); + } else { + // Setting RUSTC_BOOTSTRAP would change the behavior of the crate. + // Abort with an error. + anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ + note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ + help: If you're sure you want to do this in your project, use `RUSTC_BOOTSTRAP=1 cargo build` instead.\n\ + help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", + val, + whence + ); + } + } else { + env.push((key, val)); } - env.push((key, val)); } "warning" => warnings.push(value.to_string()), "rerun-if-changed" => rerun_if_changed.push(PathBuf::from(value)), diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 377cf25fb58..867fc013bc5 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -822,7 +822,7 @@ pub fn nightly_features_allowed() -> bool { return true; } match &channel()[..] { - "nightly" | "dev" => NIGHTLY_FEATURES_ALLOWED.with(|c| c.get()), + "nightly" | "dev" => true, //NIGHTLY_FEATURES_ALLOWED.with(|c| c.get()), _ => false, } } diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index 14fabe20823..8633f8d16c7 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -108,7 +108,7 @@ fn rerun_if_env_or_file_changes() { } #[cargo_test] -fn rustc_bootstrap_is_disallowed() { +fn rustc_bootstrap() { let p = project() .file("src/main.rs", "fn main() {}") .file( @@ -121,7 +121,12 @@ fn rustc_bootstrap_is_disallowed() { ) .build(); p.cargo("build") - .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP` [..]") + .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP=1` [..]") + .with_stderr_contains("help: [..] use `RUSTC_BOOTSTRAP=1 cargo build` [..]") .with_status(101) .run(); + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_stderr_contains("warning: Cannot set `RUSTC_BOOTSTRAP=1` [..]") + .run(); } From e56417c8c898c8456d4028a45ae0b311a6c661c1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 17 Feb 2021 12:37:01 -0500 Subject: [PATCH 03/13] Suggest RUSTC_BOOTSTRAP=crate instead of RUSTC_BOOTSTRAP=1 This was the whole point of https://github.com/rust-lang/rust/pull/77802. - Pass `pkg.name()` to `parse()`. This can't pass the `Package` directly because `PackageInner` is an `Rc` and therefore not thread-safe. Note that `pkg_name` was previously a *description* of the package, not the name passed with `--crate-name`. --- src/cargo/core/compiler/custom_build.rs | 30 ++++++++++++++++--------- tests/testsuite/build_script_env.rs | 2 +- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 198e7f194eb..a384316842a 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -5,6 +5,7 @@ use crate::core::compiler::job_queue::JobState; use crate::core::nightly_features_allowed; use crate::core::{profiles::ProfileRoot, PackageId}; use crate::util::errors::{CargoResult, CargoResultExt}; +use crate::util::interning::InternedString; use crate::util::machine_message::{self, Message}; use crate::util::{self, internal, paths, profile}; use cargo_platform::Cfg; @@ -268,7 +269,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { } }) .collect::>(); - let pkg_name = unit.pkg.to_string(); + let pkg_name = unit.pkg.name(); + let pkg_descr = unit.pkg.to_string(); let build_script_outputs = Arc::clone(&cx.build_script_outputs); let id = unit.pkg.package_id(); let output_file = script_run_dir.join("output"); @@ -277,7 +279,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { let host_target_root = cx.files().host_dest().to_path_buf(); let all = ( id, - pkg_name.clone(), + pkg_name, + pkg_descr.clone(), Arc::clone(&build_script_outputs), output_file.clone(), script_out_dir.clone(), @@ -395,7 +398,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { paths::write(&root_output_file, util::path2bytes(&script_out_dir)?)?; let parsed_output = BuildOutput::parse( &output.stdout, - &pkg_name, + pkg_name, + &pkg_descr, &script_out_dir, &script_out_dir, extra_link_arg, @@ -415,12 +419,13 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { // itself to run when we actually end up just discarding what we calculated // above. let fresh = Work::new(move |state| { - let (id, pkg_name, build_script_outputs, output_file, script_out_dir) = all; + let (id, pkg_name, pkg_descr, build_script_outputs, output_file, script_out_dir) = all; let output = match prev_output { Some(output) => output, None => BuildOutput::parse_file( &output_file, - &pkg_name, + pkg_name, + &pkg_descr, &prev_script_out_dir, &script_out_dir, extra_link_arg, @@ -470,7 +475,8 @@ fn insert_warnings_in_build_outputs( impl BuildOutput { pub fn parse_file( path: &Path, - pkg_name: &str, + pkg_name: InternedString, + pkg_descr: &str, script_out_dir_when_generated: &Path, script_out_dir: &Path, extra_link_arg: bool, @@ -479,6 +485,7 @@ impl BuildOutput { BuildOutput::parse( &contents, pkg_name, + pkg_descr, script_out_dir_when_generated, script_out_dir, extra_link_arg, @@ -489,7 +496,8 @@ impl BuildOutput { // The `pkg_name` is used for error messages. pub fn parse( input: &[u8], - pkg_name: &str, + pkg_name: InternedString, + pkg_descr: &str, script_out_dir_when_generated: &Path, script_out_dir: &Path, extra_link_arg: bool, @@ -503,7 +511,7 @@ impl BuildOutput { let mut rerun_if_changed = Vec::new(); let mut rerun_if_env_changed = Vec::new(); let mut warnings = Vec::new(); - let whence = format!("build script of `{}`", pkg_name); + let whence = format!("build script of `{}`", pkg_descr); for line in input.split(|b| *b == b'\n') { let line = match str::from_utf8(line) { @@ -585,10 +593,11 @@ impl BuildOutput { // Abort with an error. anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ - help: If you're sure you want to do this in your project, use `RUSTC_BOOTSTRAP=1 cargo build` instead.\n\ + help: If you're sure you want to do this in your project, use `RUSTC_BOOTSTRAP={} cargo build` instead.\n\ help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", val, - whence + whence, + pkg_name, ); } } else { @@ -845,6 +854,7 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option Date: Thu, 18 Feb 2021 13:46:31 -0500 Subject: [PATCH 04/13] Fix `masquerade_as_nightly_cargo` in work threads Previously, since `ENABLE_NIGHTLY_FEATURES` and `NIGHTLY_FEATURES_ENABLED` were thread locals, reading them in any other thread would always say nightly features were disabled. Now, they are tied to the `Context` itself, so it is both more clear how the variables are being set and fixes the behavior within work threads. Note that `Context` is not thread-safe, so this passes a boolean through to `BuildOutput::parse`. --- crates/resolver-tests/tests/resolve.rs | 14 +++---- src/bin/cargo/cli.rs | 2 +- src/bin/cargo/commands/logout.rs | 2 +- src/bin/cargo/main.rs | 4 +- src/cargo/core/compiler/custom_build.rs | 12 +++++- src/cargo/core/compiler/mod.rs | 4 +- src/cargo/core/compiler/unit_graph.rs | 9 ++++- src/cargo/core/features.rs | 51 +++++++++++++++++-------- src/cargo/ops/cargo_compile.rs | 2 +- src/cargo/ops/fix.rs | 11 +++--- src/cargo/util/config/mod.rs | 14 +++++-- src/cargo/util/toml/mod.rs | 6 +-- tests/testsuite/config.rs | 24 +++++++----- tests/testsuite/profile_config.rs | 4 +- 14 files changed, 101 insertions(+), 58 deletions(-) diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index dfa78ac11e1..edaccff4ab1 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -55,9 +55,8 @@ proptest! { fn prop_minimum_version_errors_the_same( PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) ) { - enable_nightly_features(); - let mut config = Config::default().unwrap(); + enable_nightly_features(&mut config); config .configure( 1, @@ -553,11 +552,6 @@ fn test_resolving_maximum_version_with_transitive_deps() { #[test] fn test_resolving_minimum_version_with_transitive_deps() { - enable_nightly_features(); // -Z minimal-versions - // When the minimal-versions config option is specified then the lowest - // possible version of a package should be selected. "util 1.0.0" can't be - // selected because of the requirements of "bar", so the minimum version - // must be 1.1.1. let reg = registry(vec![ pkg!(("util", "1.2.2")), pkg!(("util", "1.0.0")), @@ -567,6 +561,12 @@ fn test_resolving_minimum_version_with_transitive_deps() { ]); let mut config = Config::default().unwrap(); + // -Z minimal-versions + // When the minimal-versions config option is specified then the lowest + // possible version of a package should be selected. "util 1.0.0" can't be + // selected because of the requirements of "bar", so the minimum version + // must be 1.1.1. + enable_nightly_features(&mut config); config .configure( 1, diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 243f6ac0773..4c605519c71 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -48,7 +48,7 @@ Available unstable (nightly-only) flags: Run with 'cargo -Z [FLAG] [SUBCOMMAND]'" ); - if !features::nightly_features_allowed() { + if !features::nightly_features_allowed(config) { drop_println!( config, "\nUnstable flags are only available on the nightly channel \ diff --git a/src/bin/cargo/commands/logout.rs b/src/bin/cargo/commands/logout.rs index 5fcc4ea64c6..4a2ecd5f317 100644 --- a/src/bin/cargo/commands/logout.rs +++ b/src/bin/cargo/commands/logout.rs @@ -16,7 +16,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { if !(unstable.credential_process || unstable.unstable_options) { const SEE: &str = "See https://github.com/rust-lang/cargo/issues/8933 for more \ information about the `cargo logout` command."; - if features::nightly_features_allowed() { + if features::nightly_features_allowed(config) { return Err(format_err!( "the `cargo logout` command is unstable, pass `-Z unstable-options` to enable it\n\ {}", diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index 85b45a360ea..ce6113e24ad 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -22,7 +22,6 @@ fn main() { pretty_env_logger::init_custom_env("CARGO_LOG"); #[cfg(not(feature = "pretty-env-logger"))] env_logger::init_from_env("CARGO_LOG"); - cargo::core::maybe_allow_nightly_features(); let mut config = match Config::default() { Ok(cfg) => cfg, @@ -31,8 +30,9 @@ fn main() { cargo::exit_with_error(e.into(), &mut shell) } }; + cargo::core::maybe_allow_nightly_features(&mut config); - let result = match cargo::ops::fix_maybe_exec_rustc() { + let result = match cargo::ops::fix_maybe_exec_rustc(&config) { Ok(true) => Ok(()), Ok(false) => { let _token = cargo::util::job::setup(); diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index a384316842a..8883dc48214 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -295,6 +295,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { paths::create_dir_all(&script_out_dir)?; let extra_link_arg = cx.bcx.config.cli_unstable().extra_link_arg; + let nightly_features_allowed = nightly_features_allowed(cx.bcx.config); // Prepare the unit of "dirty work" which will actually run the custom build // command. @@ -369,7 +370,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { }, true, ) - .chain_err(|| format!("failed to run custom build command for `{}`", pkg_name)); + .chain_err(|| format!("failed to run custom build command for `{}`", pkg_descr)); if let Err(error) = output { insert_warnings_in_build_outputs( @@ -403,6 +404,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { &script_out_dir, &script_out_dir, extra_link_arg, + nightly_features_allowed, )?; if json_messages { @@ -429,6 +431,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { &prev_script_out_dir, &script_out_dir, extra_link_arg, + nightly_features_allowed, )?, }; @@ -480,6 +483,7 @@ impl BuildOutput { script_out_dir_when_generated: &Path, script_out_dir: &Path, extra_link_arg: bool, + nightly_features_allowed: bool, ) -> CargoResult { let contents = paths::read_bytes(path)?; BuildOutput::parse( @@ -489,6 +493,7 @@ impl BuildOutput { script_out_dir_when_generated, script_out_dir, extra_link_arg, + nightly_features_allowed, ) } @@ -501,6 +506,7 @@ impl BuildOutput { script_out_dir_when_generated: &Path, script_out_dir: &Path, extra_link_arg: bool, + nightly_features_allowed: bool, ) -> CargoResult { let mut library_paths = Vec::new(); let mut library_links = Vec::new(); @@ -582,7 +588,7 @@ impl BuildOutput { // to set RUSTC_BOOTSTRAP. // If this is a nightly build, setting RUSTC_BOOTSTRAP wouldn't affect the // behavior, so still only give a warning. - if nightly_features_allowed() { + if nightly_features_allowed { warnings.push(format!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", @@ -850,6 +856,7 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option, unit: &Unit) -> (Option, cmd: &mut ProcessBuilder, pi } cmd.arg(json); - if nightly_features_allowed() { - let config = cx.bcx.config; + let config = cx.bcx.config; + if nightly_features_allowed(config) { match ( config.cli_unstable().terminal_width, config.shell().err_width().diagnostic_terminal_width(), diff --git a/src/cargo/core/compiler/unit_graph.rs b/src/cargo/core/compiler/unit_graph.rs index 5d7e4cde607..59ef89d5cd0 100644 --- a/src/cargo/core/compiler/unit_graph.rs +++ b/src/cargo/core/compiler/unit_graph.rs @@ -4,6 +4,7 @@ use crate::core::profiles::{Profile, UnitFor}; use crate::core::{nightly_features_allowed, PackageId, Target}; use crate::util::interning::InternedString; use crate::util::CargoResult; +use crate::Config; use std::collections::HashMap; use std::io::Write; @@ -62,8 +63,12 @@ struct SerializedUnitDep { // internal detail that is mostly used for building the graph. } -pub fn emit_serialized_unit_graph(root_units: &[Unit], unit_graph: &UnitGraph) -> CargoResult<()> { - let is_nightly = nightly_features_allowed(); +pub fn emit_serialized_unit_graph( + root_units: &[Unit], + unit_graph: &UnitGraph, + config: &Config, +) -> CargoResult<()> { + let is_nightly = nightly_features_allowed(config); let mut units: Vec<(&Unit, &Vec)> = unit_graph.iter().collect(); units.sort_unstable(); // Create a map for quick lookup for dependencies. diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 867fc013bc5..8550c5ff052 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -102,6 +102,7 @@ use serde::{Deserialize, Serialize}; use crate::util::errors::CargoResult; use crate::util::{indented_lines, ProcessBuilder}; +use crate::Config; pub const SEE_CHANNELS: &str = "See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information \ @@ -406,16 +407,25 @@ pub struct Feature { } impl Features { - pub fn new(features: &[String], warnings: &mut Vec) -> CargoResult { + pub fn new( + features: &[String], + config: &Config, + warnings: &mut Vec, + ) -> CargoResult { let mut ret = Features::default(); for feature in features { - ret.add(feature, warnings)?; + ret.add(feature, config, warnings)?; ret.activated.push(feature.to_string()); } Ok(ret) } - fn add(&mut self, feature_name: &str, warnings: &mut Vec) -> CargoResult<()> { + fn add( + &mut self, + feature_name: &str, + config: &Config, + warnings: &mut Vec, + ) -> CargoResult<()> { let (slot, feature) = match self.status(feature_name) { Some(p) => p, None => bail!("unknown cargo feature `{}`", feature_name), @@ -453,7 +463,7 @@ impl Features { ); warnings.push(warning); } - Status::Unstable if !nightly_features_allowed() => bail!( + Status::Unstable if !nightly_features_allowed(config) => bail!( "the cargo feature `{}` requires a nightly version of \ Cargo, but this is the `{}` channel\n\ {}\n{}", @@ -488,7 +498,9 @@ impl Features { let feature = feature.name.replace("_", "-"); let mut msg = format!("feature `{}` is required", feature); - if nightly_features_allowed() { + // TODO + let channel = channel(); + if channel == "nightly" || channel == "dev" { let s = format!( "\n\nconsider adding `cargo-features = [\"{0}\"]` \ to the manifest", @@ -602,8 +614,12 @@ where } impl CliUnstable { - pub fn parse(&mut self, flags: &[String]) -> CargoResult> { - if !flags.is_empty() && !nightly_features_allowed() { + pub fn parse( + &mut self, + flags: &[String], + nightly_features_allowed: bool, + ) -> CargoResult> { + if !flags.is_empty() && !nightly_features_allowed { bail!( "the `-Z` flag is only accepted on the nightly channel of Cargo, \ but this is the `{}` channel\n\ @@ -759,7 +775,9 @@ impl CliUnstable { information about the `{}` flag.", issue, flag ); - if nightly_features_allowed() { + // NOTE: a `config` isn't available here, check the channel directly + let channel = channel(); + if channel == "nightly" || channel == "dev" { bail!( "the `{}` flag is unstable, pass `-Z unstable-options` to enable it\n\ {}", @@ -773,7 +791,7 @@ impl CliUnstable { {}\n\ {}", flag, - channel(), + channel, SEE_CHANNELS, see ); @@ -817,12 +835,12 @@ thread_local!( /// - this is an `#[test]` that called `enable_nightly_features` /// - this is a integration test that uses `ProcessBuilder` /// that called `masquerade_as_nightly_cargo` -pub fn nightly_features_allowed() -> bool { - if ENABLE_NIGHTLY_FEATURES.with(|c| c.get()) { +pub fn nightly_features_allowed(config: &Config) -> bool { + if config.enable_nightly_features { return true; } match &channel()[..] { - "nightly" | "dev" => true, //NIGHTLY_FEATURES_ALLOWED.with(|c| c.get()), + "nightly" | "dev" => config.maybe_allow_nightly_features, _ => false, } } @@ -831,13 +849,14 @@ pub fn nightly_features_allowed() -> bool { /// development channel is nightly or dev. /// /// Used by cargo main to ensure that a cargo build from source has nightly features -pub fn maybe_allow_nightly_features() { - NIGHTLY_FEATURES_ALLOWED.with(|c| c.set(true)); +pub fn maybe_allow_nightly_features(config: &mut Config) { + config.maybe_allow_nightly_features = true; } /// Forcibly enables nightly features for this thread. /// /// Used by tests to allow the use of nightly features. -pub fn enable_nightly_features() { - ENABLE_NIGHTLY_FEATURES.with(|c| c.set(true)); +/// NOTE: this should be called before `configure()`. Consider using `ConfigBuilder::enable_nightly_features` instead. +pub fn enable_nightly_features(config: &mut Config) { + config.enable_nightly_features = true; } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 3d4cf8c96a1..dd15a970a18 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -287,7 +287,7 @@ pub fn compile_ws<'a>( let interner = UnitInterner::new(); let bcx = create_bcx(ws, options, &interner)?; if options.build_config.unit_graph { - unit_graph::emit_serialized_unit_graph(&bcx.roots, &bcx.unit_graph)?; + unit_graph::emit_serialized_unit_graph(&bcx.roots, &bcx.unit_graph, ws.config())?; return Compilation::new(&bcx); } let _p = profile::start("compiling"); diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index c1d1f20d2ad..e072cc04bb5 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -201,7 +201,7 @@ fn check_version_control(config: &Config, opts: &FixOptions) -> CargoResult<()> /// `true` if in `fix` proxy mode, and the fix was complete without any /// warnings or errors. If there are warnings or errors, this does not return, /// and the process exits with the corresponding `rustc` exit code. -pub fn fix_maybe_exec_rustc() -> CargoResult { +pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult { let lock_addr = match env::var(FIX_ENV) { Ok(s) => s, Err(_) => return Ok(false), @@ -216,7 +216,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { let rustc = util::process(&args.rustc).wrapped(workspace_rustc.as_ref()); trace!("start rustfixing {:?}", args.file); - let fixes = rustfix_crate(&lock_addr, &rustc, &args.file, &args)?; + let fixes = rustfix_crate(&lock_addr, &rustc, &args.file, &args, config)?; // Ok now we have our final goal of testing out the changes that we applied. // If these changes went awry and actually started to cause the crate to @@ -296,8 +296,9 @@ fn rustfix_crate( rustc: &ProcessBuilder, filename: &Path, args: &FixArgs, + config: &Config, ) -> Result { - args.check_edition_and_send_status()?; + args.check_edition_and_send_status(config)?; // First up, we want to make sure that each crate is only checked by one // process at a time. If two invocations concurrently check a crate then @@ -679,7 +680,7 @@ impl FixArgs { /// Validates the edition, and sends a message indicating what is being /// done. - fn check_edition_and_send_status(&self) -> CargoResult<()> { + fn check_edition_and_send_status(&self, config: &Config) -> CargoResult<()> { let to_edition = match self.prepare_for_edition { Some(s) => s, None => { @@ -699,7 +700,7 @@ impl FixArgs { // Unfortunately this results in a pretty poor error message when // multiple jobs run in parallel (the error appears multiple // times). Hopefully this doesn't happen often in practice. - if !to_edition.is_stable() && !nightly_features_allowed() { + if !to_edition.is_stable() && !nightly_features_allowed(config) { bail!( "cannot migrate {} to edition {to_edition}\n\ Edition {to_edition} is unstable and not allowed in this release, \ diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index a0dc0963c17..4e0024d988a 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -184,6 +184,8 @@ pub struct Config { doc_extern_map: LazyCell, progress_config: ProgressConfig, env_config: LazyCell, + pub(crate) enable_nightly_features: bool, + pub(crate) maybe_allow_nightly_features: bool, } impl Config { @@ -268,6 +270,8 @@ impl Config { doc_extern_map: LazyCell::new(), progress_config: ProgressConfig::default(), env_config: LazyCell::new(), + enable_nightly_features: false, + maybe_allow_nightly_features: false, } } @@ -755,7 +759,11 @@ impl Config { unstable_flags: &[String], cli_config: &[String], ) -> CargoResult<()> { - for warning in self.unstable_flags.parse(unstable_flags)? { + let nightly_features_allowed = nightly_features_allowed(&self); + for warning in self + .unstable_flags + .parse(unstable_flags, nightly_features_allowed)? + { self.shell().warn(warning)?; } if !unstable_flags.is_empty() { @@ -821,7 +829,7 @@ impl Config { fn load_unstable_flags_from_config(&mut self) -> CargoResult<()> { // If nightly features are enabled, allow setting Z-flags from config // using the `unstable` table. Ignore that block otherwise. - if nightly_features_allowed() { + if nightly_features_allowed(self) { self.unstable_flags = self .get::>("unstable")? .unwrap_or_default(); @@ -830,7 +838,7 @@ impl Config { // allows the CLI to override config files for both enabling // and disabling, and doing it up top allows CLI Zflags to // control config parsing behavior. - self.unstable_flags.parse(unstable_flags_cli)?; + self.unstable_flags.parse(unstable_flags_cli, true)?; } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 683f5e7c825..6fe3a0eac96 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1037,7 +1037,7 @@ impl TomlManifest { // Parse features first so they will be available when parsing other parts of the TOML. let empty = Vec::new(); let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); - let features = Features::new(cargo_features, &mut warnings)?; + let features = Features::new(cargo_features, config, &mut warnings)?; let project = me.project.as_ref().or_else(|| me.package.as_ref()); let project = project.ok_or_else(|| anyhow!("no `package` section found"))?; @@ -1076,7 +1076,7 @@ impl TomlManifest { let mut msg = "`rust-version` is not supported on this version of Cargo and will be ignored" .to_string(); - if nightly_features_allowed() { + if nightly_features_allowed(config) { msg.push_str( "\n\n\ consider adding `cargo-features = [\"rust-version\"]` to the manifest", @@ -1432,7 +1432,7 @@ impl TomlManifest { let mut deps = Vec::new(); let empty = Vec::new(); let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); - let features = Features::new(cargo_features, &mut warnings)?; + let features = Features::new(cargo_features, config, &mut warnings)?; let (replace, patch) = { let mut cx = Context { diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 1b45b65b307..9388c3c9d8d 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -20,6 +20,7 @@ pub struct ConfigBuilder { unstable: Vec, config_args: Vec, cwd: Option, + enable_nightly_features: bool, } impl ConfigBuilder { @@ -29,6 +30,7 @@ impl ConfigBuilder { unstable: Vec::new(), config_args: Vec::new(), cwd: None, + enable_nightly_features: false, } } @@ -44,6 +46,12 @@ impl ConfigBuilder { self } + /// Unconditionaly enable nightly features, even on stable channels. + pub fn enable_nightly_features(&mut self) -> &mut Self { + self.enable_nightly_features = true; + self + } + /// Passes a `--config` flag. pub fn config_arg(&mut self, arg: impl Into) -> &mut Self { if !self.unstable.iter().any(|s| s == "unstable-options") { @@ -67,15 +75,14 @@ impl ConfigBuilder { /// Creates the `Config`, returning a Result. pub fn build_err(&self) -> CargoResult { - if !self.unstable.is_empty() { - // This is unfortunately global. Some day that should be fixed. - enable_nightly_features(); - } let output = Box::new(fs::File::create(paths::root().join("shell.out")).unwrap()); let shell = Shell::from_write(output); let cwd = self.cwd.clone().unwrap_or_else(|| paths::root()); let homedir = paths::home(); let mut config = Config::new(shell, cwd, homedir); + if self.enable_nightly_features || !self.unstable.is_empty() { + enable_nightly_features(&mut config); + } config.set_env(self.env.clone()); config.set_search_stop_path(paths::root()); config.configure( @@ -1095,40 +1102,37 @@ Caused by: /// Assert that unstable options can be configured with the `unstable` table in /// cargo config files fn unstable_table_notation() { - cargo::core::enable_nightly_features(); write_config( "\ [unstable] print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().build(); + let config = ConfigBuilder::new().enable_nightly_features().build(); assert_eq!(config.cli_unstable().print_im_a_teapot, true); } #[cargo_test] /// Assert that dotted notation works for configuring unstable options fn unstable_dotted_notation() { - cargo::core::enable_nightly_features(); write_config( "\ unstable.print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().build(); + let config = ConfigBuilder::new().enable_nightly_features().build(); assert_eq!(config.cli_unstable().print_im_a_teapot, true); } #[cargo_test] /// Assert that Zflags on the CLI take precedence over those from config fn unstable_cli_precedence() { - cargo::core::enable_nightly_features(); write_config( "\ unstable.print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().build(); + let config = ConfigBuilder::new().enable_nightly_features().build(); assert_eq!(config.cli_unstable().print_im_a_teapot, true); let config = ConfigBuilder::new() diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index 2449e5a171e..e99188b27e6 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -343,12 +343,10 @@ fn named_config_profile() { // middle exists in Cargo.toml, the others in .cargo/config use super::config::ConfigBuilder; use cargo::core::compiler::CompileMode; - use cargo::core::enable_nightly_features; use cargo::core::profiles::{Profiles, UnitFor}; use cargo::core::{PackageId, Workspace}; use cargo::util::interning::InternedString; use std::fs; - enable_nightly_features(); paths::root().join(".cargo").mkdir_p(); fs::write( paths::root().join(".cargo/config"), @@ -394,7 +392,7 @@ fn named_config_profile() { "#, ) .unwrap(); - let config = ConfigBuilder::new().build(); + let config = ConfigBuilder::new().enable_nightly_features().build(); let profile_name = InternedString::new("foo"); let ws = Workspace::new(&paths::root().join("Cargo.toml"), &config).unwrap(); let profiles = Profiles::new(&ws, profile_name).unwrap(); From 8fc86e155c867c2903e0059c5bb0b2753fce69f9 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 14:19:09 -0500 Subject: [PATCH 05/13] Remove unused thread_locals --- src/cargo/core/features.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 8550c5ff052..1f9fc824e7e 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -92,7 +92,6 @@ //! of that page. Update the rest of the documentation to add the new //! feature. -use std::cell::Cell; use std::env; use std::fmt; use std::str::FromStr; @@ -817,11 +816,6 @@ pub fn channel() -> String { .unwrap_or_else(|| String::from("dev")) } -thread_local!( - static NIGHTLY_FEATURES_ALLOWED: Cell = Cell::new(false); - static ENABLE_NIGHTLY_FEATURES: Cell = Cell::new(false); -); - /// This is a little complicated. /// This should return false if: /// - this is an artifact of the rustc distribution process for "stable" or for "beta" From 169b09ce7e9264f831387a4616bfd590c409a9fb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 14:27:27 -0500 Subject: [PATCH 06/13] Compute `enable_nightly_features` once instead of on each call This avoids reparsing `channel()` over and over again. It also makes `maybe_enable_nightly_features` unnecessary and removes it. --- src/bin/cargo/main.rs | 1 - src/cargo/core/features.rs | 18 ++---------------- src/cargo/core/mod.rs | 2 +- src/cargo/util/config/mod.rs | 9 +++++---- 4 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index ce6113e24ad..e6a1c3f271b 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -30,7 +30,6 @@ fn main() { cargo::exit_with_error(e.into(), &mut shell) } }; - cargo::core::maybe_allow_nightly_features(&mut config); let result = match cargo::ops::fix_maybe_exec_rustc(&config) { Ok(true) => Ok(()), diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 1f9fc824e7e..71019b083b4 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -830,24 +830,10 @@ pub fn channel() -> String { /// - this is a integration test that uses `ProcessBuilder` /// that called `masquerade_as_nightly_cargo` pub fn nightly_features_allowed(config: &Config) -> bool { - if config.enable_nightly_features { - return true; - } - match &channel()[..] { - "nightly" | "dev" => config.maybe_allow_nightly_features, - _ => false, - } -} - -/// Allows nightly features to be enabled for this thread, but only if the -/// development channel is nightly or dev. -/// -/// Used by cargo main to ensure that a cargo build from source has nightly features -pub fn maybe_allow_nightly_features(config: &mut Config) { - config.maybe_allow_nightly_features = true; + config.enable_nightly_features } -/// Forcibly enables nightly features for this thread. +/// Forcibly enables nightly features for this config. /// /// Used by tests to allow the use of nightly features. /// NOTE: this should be called before `configure()`. Consider using `ConfigBuilder::enable_nightly_features` instead. diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 5abab1b9cd4..6ed25f39638 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,6 +1,6 @@ pub use self::dependency::Dependency; pub use self::features::{ - enable_nightly_features, maybe_allow_nightly_features, nightly_features_allowed, + enable_nightly_features, nightly_features_allowed, }; pub use self::features::{CliUnstable, Edition, Feature, Features}; pub use self::manifest::{EitherManifest, VirtualManifest}; diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 4e0024d988a..9cc4659300d 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -74,7 +74,7 @@ use url::Url; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; -use crate::core::{nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace}; +use crate::core::{features, nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace}; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::toml as cargo_toml; @@ -185,7 +185,6 @@ pub struct Config { progress_config: ProgressConfig, env_config: LazyCell, pub(crate) enable_nightly_features: bool, - pub(crate) maybe_allow_nightly_features: bool, } impl Config { @@ -270,8 +269,10 @@ impl Config { doc_extern_map: LazyCell::new(), progress_config: ProgressConfig::default(), env_config: LazyCell::new(), - enable_nightly_features: false, - maybe_allow_nightly_features: false, + enable_nightly_features: match &*features::channel() { + "nightly" | "dev" => true, + _ => false, + } } } From a5720117fedac84e0fd07aba51ee725077e56ba5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 14:35:51 -0500 Subject: [PATCH 07/13] Make `nightly_features_allowed` a field instead of a function `nightly_features_allowed()` is no longer doing any work, so it can be accessed directly. This also renames the `enable_nightly_features` field to `nightly_features_allowed`. --- crates/resolver-tests/tests/resolve.rs | 6 ++--- src/bin/cargo/cli.rs | 2 +- src/bin/cargo/commands/logout.rs | 2 +- src/cargo/core/compiler/custom_build.rs | 6 ++--- src/cargo/core/compiler/mod.rs | 3 +-- src/cargo/core/compiler/unit_graph.rs | 5 ++--- src/cargo/core/features.rs | 27 +---------------------- src/cargo/core/mod.rs | 3 --- src/cargo/ops/fix.rs | 4 ++-- src/cargo/util/config/mod.rs | 29 +++++++++++++++++-------- src/cargo/util/toml/mod.rs | 3 +-- tests/testsuite/config.rs | 4 ++-- 12 files changed, 36 insertions(+), 58 deletions(-) diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index edaccff4ab1..bd5437faedb 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -1,5 +1,5 @@ use cargo::core::dependency::DepKind; -use cargo::core::{enable_nightly_features, Dependency}; +use cargo::core::Dependency; use cargo::util::{is_ci, Config}; use resolver_tests::{ @@ -56,7 +56,7 @@ proptest! { PrettyPrintRegistry(input) in registry_strategy(50, 20, 60) ) { let mut config = Config::default().unwrap(); - enable_nightly_features(&mut config); + config.nightly_features_allowed = true; config .configure( 1, @@ -566,7 +566,7 @@ fn test_resolving_minimum_version_with_transitive_deps() { // possible version of a package should be selected. "util 1.0.0" can't be // selected because of the requirements of "bar", so the minimum version // must be 1.1.1. - enable_nightly_features(&mut config); + config.nightly_features_allowed = true; config .configure( 1, diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index 4c605519c71..ad8f1f763e6 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -48,7 +48,7 @@ Available unstable (nightly-only) flags: Run with 'cargo -Z [FLAG] [SUBCOMMAND]'" ); - if !features::nightly_features_allowed(config) { + if !config.nightly_features_allowed { drop_println!( config, "\nUnstable flags are only available on the nightly channel \ diff --git a/src/bin/cargo/commands/logout.rs b/src/bin/cargo/commands/logout.rs index 4a2ecd5f317..b3fc67958ce 100644 --- a/src/bin/cargo/commands/logout.rs +++ b/src/bin/cargo/commands/logout.rs @@ -16,7 +16,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult { if !(unstable.credential_process || unstable.unstable_options) { const SEE: &str = "See https://github.com/rust-lang/cargo/issues/8933 for more \ information about the `cargo logout` command."; - if features::nightly_features_allowed(config) { + if config.nightly_features_allowed { return Err(format_err!( "the `cargo logout` command is unstable, pass `-Z unstable-options` to enable it\n\ {}", diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 8883dc48214..fa09998541d 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -2,7 +2,6 @@ use super::job::{Freshness, Job, Work}; use super::{fingerprint, Context, LinkType, Unit}; use crate::core::compiler::context::Metadata; use crate::core::compiler::job_queue::JobState; -use crate::core::nightly_features_allowed; use crate::core::{profiles::ProfileRoot, PackageId}; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::interning::InternedString; @@ -295,7 +294,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { paths::create_dir_all(&script_out_dir)?; let extra_link_arg = cx.bcx.config.cli_unstable().extra_link_arg; - let nightly_features_allowed = nightly_features_allowed(cx.bcx.config); + let nightly_features_allowed = cx.bcx.config.nightly_features_allowed; // Prepare the unit of "dirty work" which will actually run the custom build // command. @@ -856,7 +855,6 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option, unit: &Unit) -> (Option, cmd: &mut ProcessBuilder, pi cmd.arg(json); let config = cx.bcx.config; - if nightly_features_allowed(config) { + if config.nightly_features_allowed { match ( config.cli_unstable().terminal_width, config.shell().err_width().diagnostic_terminal_width(), diff --git a/src/cargo/core/compiler/unit_graph.rs b/src/cargo/core/compiler/unit_graph.rs index 59ef89d5cd0..1357afb93f8 100644 --- a/src/cargo/core/compiler/unit_graph.rs +++ b/src/cargo/core/compiler/unit_graph.rs @@ -1,7 +1,7 @@ use crate::core::compiler::Unit; use crate::core::compiler::{CompileKind, CompileMode}; use crate::core::profiles::{Profile, UnitFor}; -use crate::core::{nightly_features_allowed, PackageId, Target}; +use crate::core::{PackageId, Target}; use crate::util::interning::InternedString; use crate::util::CargoResult; use crate::Config; @@ -68,7 +68,6 @@ pub fn emit_serialized_unit_graph( unit_graph: &UnitGraph, config: &Config, ) -> CargoResult<()> { - let is_nightly = nightly_features_allowed(config); let mut units: Vec<(&Unit, &Vec)> = unit_graph.iter().collect(); units.sort_unstable(); // Create a map for quick lookup for dependencies. @@ -85,7 +84,7 @@ pub fn emit_serialized_unit_graph( .iter() .map(|unit_dep| { // https://github.com/rust-lang/rust/issues/64260 when stabilized. - let (public, noprelude) = if is_nightly { + let (public, noprelude) = if config.nightly_features_allowed { (Some(unit_dep.public), Some(unit_dep.noprelude)) } else { (None, None) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 71019b083b4..6442dfd0da3 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -462,7 +462,7 @@ impl Features { ); warnings.push(warning); } - Status::Unstable if !nightly_features_allowed(config) => bail!( + Status::Unstable if !config.nightly_features_allowed => bail!( "the cargo feature `{}` requires a nightly version of \ Cargo, but this is the `{}` channel\n\ {}\n{}", @@ -815,28 +815,3 @@ pub fn channel() -> String { .map(|c| c.release_channel) .unwrap_or_else(|| String::from("dev")) } - -/// This is a little complicated. -/// This should return false if: -/// - this is an artifact of the rustc distribution process for "stable" or for "beta" -/// - this is an `#[test]` that does not opt in with `enable_nightly_features` -/// - this is a integration test that uses `ProcessBuilder` -/// that does not opt in with `masquerade_as_nightly_cargo` -/// This should return true if: -/// - this is an artifact of the rustc distribution process for "nightly" -/// - this is being used in the rustc distribution process internally -/// - this is a cargo executable that was built from source -/// - this is an `#[test]` that called `enable_nightly_features` -/// - this is a integration test that uses `ProcessBuilder` -/// that called `masquerade_as_nightly_cargo` -pub fn nightly_features_allowed(config: &Config) -> bool { - config.enable_nightly_features -} - -/// Forcibly enables nightly features for this config. -/// -/// Used by tests to allow the use of nightly features. -/// NOTE: this should be called before `configure()`. Consider using `ConfigBuilder::enable_nightly_features` instead. -pub fn enable_nightly_features(config: &mut Config) { - config.enable_nightly_features = true; -} diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 6ed25f39638..557f7f19b54 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,7 +1,4 @@ pub use self::dependency::Dependency; -pub use self::features::{ - enable_nightly_features, nightly_features_allowed, -}; pub use self::features::{CliUnstable, Edition, Feature, Features}; pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::manifest::{Manifest, Target, TargetKind}; diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index e072cc04bb5..8b6acb0d205 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -50,7 +50,7 @@ use log::{debug, trace, warn}; use rustfix::diagnostics::Diagnostic; use rustfix::{self, CodeFix}; -use crate::core::{nightly_features_allowed, Edition, Workspace}; +use crate::core::{Edition, Workspace}; use crate::ops::{self, CompileOptions}; use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer}; use crate::util::errors::CargoResult; @@ -700,7 +700,7 @@ impl FixArgs { // Unfortunately this results in a pretty poor error message when // multiple jobs run in parallel (the error appears multiple // times). Hopefully this doesn't happen often in practice. - if !to_edition.is_stable() && !nightly_features_allowed(config) { + if !to_edition.is_stable() && !config.nightly_features_allowed { bail!( "cannot migrate {} to edition {to_edition}\n\ Edition {to_edition} is unstable and not allowed in this release, \ diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 9cc4659300d..74f50d7be0e 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -74,7 +74,7 @@ use url::Url; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; -use crate::core::{features, nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace}; +use crate::core::{features, CliUnstable, Shell, SourceId, Workspace}; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::toml as cargo_toml; @@ -184,7 +184,22 @@ pub struct Config { doc_extern_map: LazyCell, progress_config: ProgressConfig, env_config: LazyCell, - pub(crate) enable_nightly_features: bool, + /// This should be false if: + /// - this is an artifact of the rustc distribution process for "stable" or for "beta" + /// - this is an `#[test]` that does not opt in with `enable_nightly_features` + /// - this is a integration test that uses `ProcessBuilder` + /// that does not opt in with `masquerade_as_nightly_cargo` + /// This should be true if: + /// - this is an artifact of the rustc distribution process for "nightly" + /// - this is being used in the rustc distribution process internally + /// - this is a cargo executable that was built from source + /// - this is an `#[test]` that called `enable_nightly_features` + /// - this is a integration test that uses `ProcessBuilder` + /// that called `masquerade_as_nightly_cargo` + /// It's public to allow tests use nightly features. + /// NOTE: this should be set before `configure()`. If calling this from an integration test, + /// consider using `ConfigBuilder::enable_nightly_features` instead. + pub nightly_features_allowed: bool, } impl Config { @@ -269,10 +284,7 @@ impl Config { doc_extern_map: LazyCell::new(), progress_config: ProgressConfig::default(), env_config: LazyCell::new(), - enable_nightly_features: match &*features::channel() { - "nightly" | "dev" => true, - _ => false, - } + nightly_features_allowed: matches!(&*features::channel(), "nightly" | "dev"), } } @@ -760,10 +772,9 @@ impl Config { unstable_flags: &[String], cli_config: &[String], ) -> CargoResult<()> { - let nightly_features_allowed = nightly_features_allowed(&self); for warning in self .unstable_flags - .parse(unstable_flags, nightly_features_allowed)? + .parse(unstable_flags, self.nightly_features_allowed)? { self.shell().warn(warning)?; } @@ -830,7 +841,7 @@ impl Config { fn load_unstable_flags_from_config(&mut self) -> CargoResult<()> { // If nightly features are enabled, allow setting Z-flags from config // using the `unstable` table. Ignore that block otherwise. - if nightly_features_allowed(self) { + if self.nightly_features_allowed { self.unstable_flags = self .get::>("unstable")? .unwrap_or_default(); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 6fe3a0eac96..65432b34518 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -15,7 +15,6 @@ use url::Url; use crate::core::dependency::DepKind; use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; -use crate::core::nightly_features_allowed; use crate::core::resolver::ResolveBehavior; use crate::core::{Dependency, Manifest, PackageId, Summary, Target}; use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace}; @@ -1076,7 +1075,7 @@ impl TomlManifest { let mut msg = "`rust-version` is not supported on this version of Cargo and will be ignored" .to_string(); - if nightly_features_allowed(config) { + if config.nightly_features_allowed { msg.push_str( "\n\n\ consider adding `cargo-features = [\"rust-version\"]` to the manifest", diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 9388c3c9d8d..fa2cd748a93 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1,6 +1,6 @@ //! Tests for config settings. -use cargo::core::{enable_nightly_features, Shell}; +use cargo::core::Shell; use cargo::util::config::{self, Config, SslVersionConfig, StringList}; use cargo::util::interning::InternedString; use cargo::util::toml::{self, VecStringOrBool as VSOB}; @@ -81,7 +81,7 @@ impl ConfigBuilder { let homedir = paths::home(); let mut config = Config::new(shell, cwd, homedir); if self.enable_nightly_features || !self.unstable.is_empty() { - enable_nightly_features(&mut config); + config.nightly_features_allowed = true; } config.set_env(self.env.clone()); config.set_search_stop_path(paths::root()); From eba5419946e0284a80d3d628ed8ec6ba028eebff Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 14:49:23 -0500 Subject: [PATCH 08/13] Update comment in build_script_env --- tests/testsuite/build_script_env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index 5ce71aec0d3..90620876d70 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -1,4 +1,4 @@ -//! Tests for build.rs rerun-if-env-changed. +//! Tests for build.rs rerun-if-env-changed and rustc-env use cargo_test_support::project; use cargo_test_support::sleep_ms; From ecfdced0d8accf829498ddbf26ff41b74b5dcc8b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 14:56:14 -0500 Subject: [PATCH 09/13] Fix test that assumed tests always were run on the stable channel Note that this has to be set in the builder, before `config.configure()` gets run. --- tests/testsuite/config.rs | 17 ++++++++--------- tests/testsuite/profile_config.rs | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index fa2cd748a93..da7775ae62a 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -47,8 +47,8 @@ impl ConfigBuilder { } /// Unconditionaly enable nightly features, even on stable channels. - pub fn enable_nightly_features(&mut self) -> &mut Self { - self.enable_nightly_features = true; + pub fn nightly_features_allowed(&mut self, allowed: bool) -> &mut Self { + self.enable_nightly_features = allowed; self } @@ -80,9 +80,7 @@ impl ConfigBuilder { let cwd = self.cwd.clone().unwrap_or_else(|| paths::root()); let homedir = paths::home(); let mut config = Config::new(shell, cwd, homedir); - if self.enable_nightly_features || !self.unstable.is_empty() { - config.nightly_features_allowed = true; - } + config.nightly_features_allowed = self.enable_nightly_features || !self.unstable.is_empty(); config.set_env(self.env.clone()); config.set_search_stop_path(paths::root()); config.configure( @@ -1108,7 +1106,7 @@ fn unstable_table_notation() { print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().enable_nightly_features().build(); + let config = ConfigBuilder::new().nightly_features_allowed(true).build(); assert_eq!(config.cli_unstable().print_im_a_teapot, true); } @@ -1120,7 +1118,7 @@ fn unstable_dotted_notation() { unstable.print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().enable_nightly_features().build(); + let config = ConfigBuilder::new().nightly_features_allowed(true).build(); assert_eq!(config.cli_unstable().print_im_a_teapot, true); } @@ -1132,7 +1130,7 @@ fn unstable_cli_precedence() { unstable.print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().enable_nightly_features().build(); + let config = ConfigBuilder::new().nightly_features_allowed(true).build(); assert_eq!(config.cli_unstable().print_im_a_teapot, true); let config = ConfigBuilder::new() @@ -1163,7 +1161,8 @@ fn unstable_flags_ignored_on_stable() { print-im-a-teapot = true ", ); - let config = ConfigBuilder::new().build(); + // Enforce stable channel even when testing on nightly. + let config = ConfigBuilder::new().nightly_features_allowed(false).build(); assert_eq!(config.cli_unstable().print_im_a_teapot, false); } diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs index e99188b27e6..a7c85e33f1f 100644 --- a/tests/testsuite/profile_config.rs +++ b/tests/testsuite/profile_config.rs @@ -392,7 +392,7 @@ fn named_config_profile() { "#, ) .unwrap(); - let config = ConfigBuilder::new().enable_nightly_features().build(); + let config = ConfigBuilder::new().nightly_features_allowed(true).build(); let profile_name = InternedString::new("foo"); let ws = Workspace::new(&paths::root().join("Cargo.toml"), &config).unwrap(); let profiles = Profiles::new(&ws, profile_name).unwrap(); From 09677c83c29aa5ae056bad71b6c0d0811b969565 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 15:14:39 -0500 Subject: [PATCH 10/13] Be less unix-centric in error messages Co-authored-by: Eric Huss --- src/cargo/core/compiler/custom_build.rs | 2 +- tests/testsuite/build_script_env.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index fa09998541d..8f36ed637ff 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -598,7 +598,7 @@ impl BuildOutput { // Abort with an error. anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ - help: If you're sure you want to do this in your project, use `RUSTC_BOOTSTRAP={} cargo build` instead.\n\ + help: If you're sure you want to do this in your project, set the environment variable `RUSTC_BOOTSTRAP={}` before running cargo instead.\n\ help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", val, whence, diff --git a/tests/testsuite/build_script_env.rs b/tests/testsuite/build_script_env.rs index 90620876d70..f03a3024a5a 100644 --- a/tests/testsuite/build_script_env.rs +++ b/tests/testsuite/build_script_env.rs @@ -122,7 +122,7 @@ fn rustc_bootstrap() { .build(); p.cargo("build") .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP=1` [..]") - .with_stderr_contains("help: [..] use `RUSTC_BOOTSTRAP=foo cargo build` [..]") + .with_stderr_contains("help: [..] set the environment variable `RUSTC_BOOTSTRAP=foo` [..]") .with_status(101) .run(); p.cargo("build") From 3a86ecf2d1cd7547858aba5273a3c82623be9905 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 24 Feb 2021 17:38:11 -0500 Subject: [PATCH 11/13] Fix TODO about nightly features --- src/cargo/core/features.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 6442dfd0da3..dba967f06d1 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -276,6 +276,7 @@ macro_rules! features { pub struct Features { $($feature: bool,)* activated: Vec, + nightly_features_allowed: bool, } impl Feature { @@ -416,6 +417,7 @@ impl Features { ret.add(feature, config, warnings)?; ret.activated.push(feature.to_string()); } + ret.nightly_features_allowed = config.nightly_features_allowed; Ok(ret) } @@ -497,9 +499,7 @@ impl Features { let feature = feature.name.replace("_", "-"); let mut msg = format!("feature `{}` is required", feature); - // TODO - let channel = channel(); - if channel == "nightly" || channel == "dev" { + if self.nightly_features_allowed { let s = format!( "\n\nconsider adding `cargo-features = [\"{0}\"]` \ to the manifest", From a6394bcc16d50319a9d0f3854b49c17305a90721 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 25 Feb 2021 09:18:52 -0500 Subject: [PATCH 12/13] Remove unnecessary `config` argument to `Features::add` The info was already present on `self`. Note that this uses a temporary variable to avoid a borrowck error from `slot`. --- src/cargo/core/features.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index dba967f06d1..2a2816e9ada 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -413,20 +413,16 @@ impl Features { warnings: &mut Vec, ) -> CargoResult { let mut ret = Features::default(); + ret.nightly_features_allowed = config.nightly_features_allowed; for feature in features { - ret.add(feature, config, warnings)?; + ret.add(feature, warnings)?; ret.activated.push(feature.to_string()); } - ret.nightly_features_allowed = config.nightly_features_allowed; Ok(ret) } - fn add( - &mut self, - feature_name: &str, - config: &Config, - warnings: &mut Vec, - ) -> CargoResult<()> { + fn add(&mut self, feature_name: &str, warnings: &mut Vec) -> CargoResult<()> { + let nightly_features_allowed = self.nightly_features_allowed; let (slot, feature) = match self.status(feature_name) { Some(p) => p, None => bail!("unknown cargo feature `{}`", feature_name), @@ -464,7 +460,7 @@ impl Features { ); warnings.push(warning); } - Status::Unstable if !config.nightly_features_allowed => bail!( + Status::Unstable if !nightly_features_allowed => bail!( "the cargo feature `{}` requires a nightly version of \ Cargo, but this is the `{}` channel\n\ {}\n{}", From 0b1816578161dbeb6900af7609428e71dd33615f Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 25 Feb 2021 09:21:35 -0500 Subject: [PATCH 13/13] Remove unhelpful link to Cargo book It didn't have any information about RUSTC_BOOTSTRAP itself, only the general `rustc-env` feature. --- src/cargo/core/compiler/custom_build.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 8f36ed637ff..55f628f66ed 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -589,8 +589,7 @@ impl BuildOutput { // behavior, so still only give a warning. if nightly_features_allowed { warnings.push(format!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ - note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ - help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", + note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.", val, whence )); } else { @@ -598,8 +597,7 @@ impl BuildOutput { // Abort with an error. anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\ note: Crates cannot set `RUSTC_BOOTSTRAP` themselves, as doing so would subvert the stability guarantees of Rust for your project.\n\ - help: If you're sure you want to do this in your project, set the environment variable `RUSTC_BOOTSTRAP={}` before running cargo instead.\n\ - help: See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env for details.", + help: If you're sure you want to do this in your project, set the environment variable `RUSTC_BOOTSTRAP={}` before running cargo instead.", val, whence, pkg_name,