Skip to content

Commit

Permalink
Use the library name to decide whether the override should be allowed…
Browse files Browse the repository at this point in the history 
… from the top-level.

If there's no library, give a hard error unless features are
unconditionally allowed with RUSTC_BOOTSTRAP=1.
  • Loading branch information
@jyn514
jyn514 committed Apr 20, 2021
1 parent 156254c commit b7a1237
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 25 deletions.
46 changes: 32 additions & 14 deletions src/cargo/core/compiler/custom_build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,12 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
}
})
.collect::<Vec<_>>();
let crate_name = unit.target.crate_name();
let library_name = unit
.pkg
.targets()
.iter()
.find(|t| t.is_lib())
.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();
Expand All @@ -277,7 +282,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,
crate_name.clone(),
library_name.clone(),
pkg_descr.clone(),
Arc::clone(&build_script_outputs),
output_file.clone(),
Expand Down Expand Up @@ -397,7 +402,7 @@ 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,
crate_name,
library_name,
&pkg_descr,
&script_out_dir,
&script_out_dir,
Expand All @@ -419,12 +424,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, crate_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,
crate_name,
library_name,
&pkg_descr,
&prev_script_out_dir,
&script_out_dir,
Expand Down Expand Up @@ -476,7 +481,7 @@ fn insert_warnings_in_build_outputs(
impl BuildOutput {
pub fn parse_file(
path: &Path,
crate_name: String,
library_name: Option<String>,
pkg_descr: &str,
script_out_dir_when_generated: &Path,
script_out_dir: &Path,
Expand All @@ -486,7 +491,7 @@ impl BuildOutput {
let contents = paths::read_bytes(path)?;
BuildOutput::parse(
&contents,
crate_name,
library_name,
pkg_descr,
script_out_dir_when_generated,
script_out_dir,
Expand All @@ -497,11 +502,11 @@ impl BuildOutput {

// Parses the output of a script.
// The `pkg_descr` is used for error messages.
// The `crate_name` is used for determining if RUSTC_BOOTSTRAP should be allowed.
// The `library_name` is used for determining if RUSTC_BOOTSTRAP should be allowed.
pub fn parse(
input: &[u8],
// Takes String instead of InternedString so passing `unit.pkg.name()` will give a compile error.
crate_name: String,
library_name: Option<String>,
pkg_descr: &str,
script_out_dir_when_generated: &Path,
script_out_dir: &Path,
Expand Down Expand Up @@ -590,12 +595,21 @@ impl BuildOutput {
// behavior, so still only give a warning.
// 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=crate_name` will work)
let rustc_bootstrap_allows = |name: &str| {
// (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(&*crate_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
Expand All @@ -608,7 +622,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,
crate_name,
library_name.as_deref().unwrap_or("1"),
);
}
} else {
Expand Down Expand Up @@ -865,7 +879,11 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option<BuildOutp
(
BuildOutput::parse_file(
&output_file,
unit.target.crate_name(),
unit.pkg
.targets()
.iter()
.find(|t| t.is_lib())
.map(|t| t.crate_name()),
&unit.pkg.to_string(),
&prev_script_out_dir,
&script_out_dir,
Expand Down
43 changes: 32 additions & 11 deletions tests/testsuite/build_script_env.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,35 +110,35 @@ 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("Cargo.toml", &basic_manifest("has-dashes", "0.0.1"))
.file("src/main.rs", "fn main() {}")
.file(
"build.rs",
r#"
fn main() {
println!("cargo:rustc-env=RUSTC_BOOTSTRAP=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=has_dashes` [..]",
)
.with_status(101)
.run();
// RUSTC_BOOTSTRAP unset on nightly should warn
p.cargo("build")
.masquerade_as_nightly_cargo()
.with_stderr_contains("warning: Cannot set `RUSTC_BOOTSTRAP=1` [..]")
.run();
// RUSTC_BOOTSTRAP set to the name of the crate
// 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
// 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` [..]")
Expand All @@ -147,4 +147,25 @@ fn rustc_bootstrap() {
)
.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();
// RUSTC_BOOTSTRAP unconditionally set when there's no library should warn
p.cargo("build")
.masquerade_as_nightly_cargo()
.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_does_not_contain(
"help: [..] set the environment variable `RUSTC_BOOTSTRAP=1` [..]",
)
.with_status(101)
.run();
}

0 comments on commit b7a1237

Please sign in to comment.