Auto merge of #9181 - jyn514:computer-says-no, r=ehuss

Forbid setting `RUSTC_BOOTSTRAP` from a build script on stable

Instead, recommend `RUSTC_BOOTSTRAP=crate_name`. If cargo is using a nightly toolchain, or if RUSTC_BOOTSTRAP was set in *cargo*'s build environment, the error is downgraded to a warning, since the variable won't affect the build.

This is mostly the same as suggested in #7088 (comment), except that `RUSTC_BOOTSTRAP=` values other than 1 are treated the same as `RUSTC_BOOTSTRAP=1`. My reasoning was that rust-lang/rust#77802 is now on 1.50 stable, so some crates may have started using it, and I would still prefer not to give hard errors when there's no workaround.

Closes #7088.

r? `@joshtriplett`

Author: bors

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::{
@@ -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();
+        config.nightly_features_allowed = true;
         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.
+    config.nightly_features_allowed = true;
     config
         .configure(
             1,

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 !config.nightly_features_allowed {
             drop_println!(
                 config,
                 "\nUnstable flags are only available on the nightly channel \

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 config.nightly_features_allowed {
             return Err(format_err!(
                 "the `cargo logout` command is unstable, pass `-Z unstable-options` to enable it\n\
                 {}",

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,
@@ -32,7 +31,7 @@ fn main() {
         }
     };
 
-    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();

src/cargo/core/compiler/custom_build.rs

@@ -4,6 +4,7 @@ use crate::core::compiler::context::Metadata;
 use crate::core::compiler::job_queue::JobState;
 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;
@@ -267,7 +268,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
             }
         })
         .collect::<Vec<_>>();
-    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");
@@ -276,7 +278,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
     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(),
@@ -291,6 +294,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
     paths::create_dir_all(&script_out_dir)?;
 
     let extra_link_arg = cx.bcx.config.cli_unstable().extra_link_arg;
+    let nightly_features_allowed = cx.bcx.config.nightly_features_allowed;
 
     // Prepare the unit of "dirty work" which will actually run the custom build
     // command.
@@ -365,7 +369,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
                 },
                 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(
@@ -394,10 +398,12 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
         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,
+            nightly_features_allowed,
         )?;
 
         if json_messages {
@@ -414,15 +420,17 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
     // 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,
+                nightly_features_allowed,
             )?,
         };
 
@@ -469,29 +477,35 @@ 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,
+        nightly_features_allowed: bool,
     ) -> CargoResult<BuildOutput> {
         let contents = paths::read_bytes(path)?;
         BuildOutput::parse(
             &contents,
             pkg_name,
+            pkg_descr,
             script_out_dir_when_generated,
             script_out_dir,
             extra_link_arg,
+            nightly_features_allowed,
         )
     }
 
     // Parses the output of a script.
     // 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,
+        nightly_features_allowed: bool,
     ) -> CargoResult<BuildOutput> {
         let mut library_paths = Vec::new();
         let mut library_links = Vec::new();
@@ -502,7 +516,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) {
@@ -562,7 +576,37 @@ 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)?;
+                    // Build scripts aren't allowed to set RUSTC_BOOTSTRAP.
+                    // See https://github.com/rust-lang/cargo/issues/7088.
+                    if key == "RUSTC_BOOTSTRAP" {
+                        // 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.",
+                                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, set the environment variable `RUSTC_BOOTSTRAP={}` before running cargo instead.",
+                                val,
+                                whence,
+                                pkg_name,
+                            );
+                        }
+                    } else {
+                        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()),
@@ -813,10 +857,12 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option<BuildOutp
     (
         BuildOutput::parse_file(
             &output_file,
+            unit.pkg.name(),
             &unit.pkg.to_string(),
             &prev_script_out_dir,
             &script_out_dir,
             extra_link_arg,
+            cx.bcx.config.nightly_features_allowed,
         )
         .ok(),
         prev_script_out_dir,

src/cargo/core/compiler/mod.rs

@@ -49,7 +49,6 @@ pub use self::lto::Lto;
 use self::output_depinfo::output_depinfo;
 use self::unit_graph::UnitDep;
 pub use crate::core::compiler::unit::{Unit, UnitInterner};
-use crate::core::features::nightly_features_allowed;
 use crate::core::manifest::TargetSourcePath;
 use crate::core::profiles::{PanicStrategy, Profile, Strip};
 use crate::core::{Feature, PackageId, Target};
@@ -700,8 +699,8 @@ fn add_error_format_and_color(cx: &Context<'_, '_>, cmd: &mut ProcessBuilder, pi
     }
     cmd.arg(json);
 
-    if nightly_features_allowed() {
-        let config = cx.bcx.config;
+    let config = cx.bcx.config;
+    if config.nightly_features_allowed {
         match (
             config.cli_unstable().terminal_width,
             config.shell().err_width().diagnostic_terminal_width(),

src/cargo/core/compiler/unit_graph.rs

@@ -1,9 +1,10 @@
 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;
 use std::collections::HashMap;
 use std::io::Write;
 
@@ -62,8 +63,11 @@ 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 mut units: Vec<(&Unit, &Vec<UnitDep>)> = unit_graph.iter().collect();
     units.sort_unstable();
     // Create a map for quick lookup for dependencies.
@@ -80,7 +84,7 @@ pub fn emit_serialized_unit_graph(root_units: &[Unit], unit_graph: &UnitGraph) -
                 .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)