Auto merge of #9365 - jyn514:rustc-bootstrap-crate-name, r=ehuss

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name

Fixes #9362.
The whole point of rust-lang/rust#77802 was to allow specifying this granularly, giving a hard error defeats the point.

I didn't know how to check what targets were reverse-dependencies of build.rs, so I just unconditionally use the library name (and give a hard error for anything else, even if it's the name of one of the binaries). End-users can still opt-in with RUSTC_BOOTSTRAP=1, and no public binaries use RUSTC_BOOTSTRAP=1, so I don't think this a big deal in practice.

<details><summary>Script to verify all crates using RUSTC_BOOTSTRAP=1 have a library</summary>

```sh
curl https://pastebin.com/raw/fGQ97xP6 | cut -d / -f1 | grep -v shnatsel | grep -v cargo- | sed 's#-\([0-9]\)#/\1#' | xargs -i curl -s -I -L "https://docs.rs/{}/" -w "%{http_code}\n" -o/dev/null
```

It should output 20 200s in a row.

</details>

r? `@ehuss` cc `@mark-simulacrum`

I don't know what cargo's policy is for backports, but this should be backported to 1.52.

Author: bors

src/cargo/core/compiler/custom_build.rs

@@ -4,7 +4,6 @@ use crate::core::compiler::context::Metadata;
 use crate::core::compiler::job_queue::JobState;
 use crate::core::{profiles::ProfileRoot, PackageId};
 use crate::util::errors::CargoResult;
-use crate::util::interning::InternedString;
 use crate::util::machine_message::{self, Message};
 use crate::util::{internal, profile};
 use anyhow::Context as _;
@@ -270,7 +269,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
             }
         })
         .collect::<Vec<_>>();
-    let pkg_name = unit.pkg.name();
+    let library_name = unit.pkg.library().map(|t| t.crate_name());
     let pkg_descr = unit.pkg.to_string();
     let build_script_outputs = Arc::clone(&cx.build_script_outputs);
     let id = unit.pkg.package_id();
@@ -280,7 +279,7 @@ 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,
+        library_name.clone(),
         pkg_descr.clone(),
         Arc::clone(&build_script_outputs),
         output_file.clone(),
@@ -400,7 +399,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
         paths::write(&root_output_file, paths::path2bytes(&script_out_dir)?)?;
         let parsed_output = BuildOutput::parse(
             &output.stdout,
-            pkg_name,
+            library_name,
             &pkg_descr,
             &script_out_dir,
             &script_out_dir,
@@ -422,12 +421,12 @@ 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, pkg_descr, build_script_outputs, output_file, script_out_dir) = all;
+        let (id, library_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,
+                library_name,
                 &pkg_descr,
                 &prev_script_out_dir,
                 &script_out_dir,
@@ -479,7 +478,7 @@ fn insert_warnings_in_build_outputs(
 impl BuildOutput {
     pub fn parse_file(
         path: &Path,
-        pkg_name: InternedString,
+        library_name: Option<String>,
         pkg_descr: &str,
         script_out_dir_when_generated: &Path,
         script_out_dir: &Path,
@@ -489,7 +488,7 @@ impl BuildOutput {
         let contents = paths::read_bytes(path)?;
         BuildOutput::parse(
             &contents,
-            pkg_name,
+            library_name,
             pkg_descr,
             script_out_dir_when_generated,
             script_out_dir,
@@ -499,10 +498,12 @@ impl BuildOutput {
     }
 
     // Parses the output of a script.
-    // The `pkg_name` is used for error messages.
+    // The `pkg_descr` is used for error messages.
+    // The `library_name` is used for determining if RUSTC_BOOTSTRAP should be allowed.
     pub fn parse(
         input: &[u8],
-        pkg_name: InternedString,
+        // Takes String instead of InternedString so passing `unit.pkg.name()` will give a compile error.
+        library_name: Option<String>,
         pkg_descr: &str,
         script_out_dir_when_generated: &Path,
         script_out_dir: &Path,
@@ -589,7 +590,23 @@ 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 {
+                        // NOTE: cargo only allows nightly features on RUSTC_BOOTSTRAP=1, but we
+                        // want setting any value of RUSTC_BOOTSTRAP to downgrade this to a warning
+                        // (so that `RUSTC_BOOTSTRAP=library_name` will work)
+                        let rustc_bootstrap_allows = |name: Option<&str>| {
+                            let name = match name {
+                                // as of 2021, no binaries on crates.io use RUSTC_BOOTSTRAP, so
+                                // fine-grained opt-outs aren't needed. end-users can always use
+                                // RUSTC_BOOTSTRAP=1 from the top-level if it's really a problem.
+                                None => return false,
+                                Some(n) => n,
+                            };
+                            std::env::var("RUSTC_BOOTSTRAP")
+                                .map_or(false, |var| var.split(',').any(|s| s == name))
+                        };
+                        if nightly_features_allowed
+                            || rustc_bootstrap_allows(library_name.as_deref())
+                        {
                             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
@@ -602,7 +619,7 @@ impl BuildOutput {
                                 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,
+                                library_name.as_deref().unwrap_or("1"),
                             );
                         }
                     } else {
@@ -859,7 +876,7 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option<BuildOutp
     (
         BuildOutput::parse_file(
             &output_file,
-            unit.pkg.name(),
+            unit.pkg.library().map(|t| t.crate_name()),
             &unit.pkg.to_string(),
             &prev_script_out_dir,
             &script_out_dir,

src/cargo/core/package.rs

@@ -151,6 +151,10 @@ impl Package {
     pub fn targets(&self) -> &[Target] {
         self.manifest().targets()
     }
+    /// Gets the library crate for this package, if it exists.
+    pub fn library(&self) -> Option<&Target> {
+        self.targets().iter().find(|t| t.is_lib())
+    }
     /// Gets the current package version.
     pub fn version(&self) -> &Version {
         self.package_id().version()

tests/testsuite/build_script_env.rs

@@ -1,5 +1,6 @@
 //! Tests for build.rs rerun-if-env-changed and rustc-env
 
+use cargo_test_support::basic_manifest;
 use cargo_test_support::project;
 use cargo_test_support::sleep_ms;
 
@@ -109,24 +110,66 @@ fn rerun_if_env_or_file_changes() {
 
 #[cargo_test]
 fn rustc_bootstrap() {
+    let build_rs = r#"
+        fn main() {
+            println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1");
+        }
+    "#;
     let p = project()
-        .file("src/main.rs", "fn main() {}")
-        .file(
-            "build.rs",
-            r#"
-            fn main() {
-                println!("cargo:rustc-env=RUSTC_BOOTSTRAP=1");
-            }
-        "#,
-        )
+        .file("Cargo.toml", &basic_manifest("has-dashes", "0.0.1"))
+        .file("src/lib.rs", "#![feature(rustc_attrs)]")
+        .file("build.rs", build_rs)
         .build();
+    // RUSTC_BOOTSTRAP unset on stable should error
     p.cargo("build")
         .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
-        .with_stderr_contains("help: [..] set the environment variable `RUSTC_BOOTSTRAP=foo` [..]")
+        .with_stderr_contains(
+            "help: [..] set the environment variable `RUSTC_BOOTSTRAP=has_dashes` [..]",
+        )
         .with_status(101)
         .run();
+    // nightly should warn whether or not RUSTC_BOOTSTRAP is set
     p.cargo("build")
         .masquerade_as_nightly_cargo()
+        // NOTE: uses RUSTC_BOOTSTRAP so it will be propagated to rustc
+        // (this matters when tests are being run with a beta or stable cargo)
+        .env("RUSTC_BOOTSTRAP", "1")
         .with_stderr_contains("warning: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
         .run();
+    // RUSTC_BOOTSTRAP set to the name of the library should warn
+    p.cargo("build")
+        .env("RUSTC_BOOTSTRAP", "has_dashes")
+        .with_stderr_contains("warning: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
+        .run();
+    // RUSTC_BOOTSTRAP set to some random value should error
+    p.cargo("build")
+        .env("RUSTC_BOOTSTRAP", "bar")
+        .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
+        .with_stderr_contains(
+            "help: [..] set the environment variable `RUSTC_BOOTSTRAP=has_dashes` [..]",
+        )
+        .with_status(101)
+        .run();
+
+    // Tests for binaries instead of libraries
+    let p = project()
+        .file("Cargo.toml", &basic_manifest("foo", "0.0.1"))
+        .file("src/main.rs", "#![feature(rustc_attrs)] fn main() {}")
+        .file("build.rs", build_rs)
+        .build();
+    // nightly should warn when there's no library whether or not RUSTC_BOOTSTRAP is set
+    p.cargo("build")
+        .masquerade_as_nightly_cargo()
+        // NOTE: uses RUSTC_BOOTSTRAP so it will be propagated to rustc
+        // (this matters when tests are being run with a beta or stable cargo)
+        .env("RUSTC_BOOTSTRAP", "1")
+        .with_stderr_contains("warning: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
+        .run();
+    // RUSTC_BOOTSTRAP conditionally set when there's no library should error (regardless of the value)
+    p.cargo("build")
+        .env("RUSTC_BOOTSTRAP", "foo")
+        .with_stderr_contains("error: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
+        .with_stderr_contains("help: [..] set the environment variable `RUSTC_BOOTSTRAP=1` [..]")
+        .with_status(101)
+        .run();
 }