Skip to content

Commit

Permalink
Auto merge of #9523 - ehuss:link-args-validate, r=alexcrichton
Browse files Browse the repository at this point in the history 
Add some validation to rustc-link-arg

This adds some validation, so that if a `cargo:rustc-link-arg-*` build script instruction specifies a target that doesn't exist, it will generate an error.  This also changes a parse warning to an error if the `=` is missing from BIN=ARG.

I intentionally did not bother to add the validation to config overrides, as it is a bit trickier to do, and that feature is very rarely used (AFAIK), and I'm uncertain if rustc-link-arg is really useful in that context.

cc #9426
  • Loading branch information
@bors
bors committed Jun 1, 2021
2 parents 5fb59b0 + 836e537 commit 6597523
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 19 deletions.
79 changes: 60 additions & 19 deletions src/cargo/core/compiler/custom_build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ 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::{profiles::ProfileRoot, PackageId};
use crate::core::{profiles::ProfileRoot, PackageId, Target};
use crate::util::errors::CargoResult;
use crate::util::machine_message::{self, Message};
use crate::util::{internal, profile};
use anyhow::Context as _;
use anyhow::{bail, Context as _};
use cargo_platform::Cfg;
use cargo_util::paths;
use std::collections::hash_map::{Entry, HashMap};
Expand Down Expand Up @@ -296,6 +296,9 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {

let extra_link_arg = cx.bcx.config.cli_unstable().extra_link_arg;
let nightly_features_allowed = cx.bcx.config.nightly_features_allowed;
let targets: Vec<Target> = unit.pkg.targets().iter().cloned().collect();
// Need a separate copy for the fresh closure.
let targets_fresh = targets.clone();

// Prepare the unit of "dirty work" which will actually run the custom build
// command.
Expand Down Expand Up @@ -405,6 +408,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
&script_out_dir,
extra_link_arg,
nightly_features_allowed,
&targets,
)?;

if json_messages {
Expand Down Expand Up @@ -432,6 +436,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
&script_out_dir,
extra_link_arg,
nightly_features_allowed,
&targets_fresh,
)?,
};

Expand Down Expand Up @@ -484,6 +489,7 @@ impl BuildOutput {
script_out_dir: &Path,
extra_link_arg: bool,
nightly_features_allowed: bool,
targets: &[Target],
) -> CargoResult<BuildOutput> {
let contents = paths::read_bytes(path)?;
BuildOutput::parse(
Expand All @@ -494,6 +500,7 @@ impl BuildOutput {
script_out_dir,
extra_link_arg,
nightly_features_allowed,
targets,
)
}

Expand All @@ -509,6 +516,7 @@ impl BuildOutput {
script_out_dir: &Path,
extra_link_arg: bool,
nightly_features_allowed: bool,
targets: &[Target],
) -> CargoResult<BuildOutput> {
let mut library_paths = Vec::new();
let mut library_links = Vec::new();
Expand Down Expand Up @@ -543,7 +551,7 @@ impl BuildOutput {
let (key, value) = match (key, value) {
(Some(a), Some(b)) => (a, b.trim_end()),
// Line started with `cargo:` but didn't match `key=value`.
_ => anyhow::bail!("Wrong output in {}: `{}`", whence, line),
_ => bail!("Wrong output in {}: `{}`", whence, line),
};

// This will rewrite paths if the target directory has been moved.
Expand All @@ -552,7 +560,7 @@ impl BuildOutput {
script_out_dir.to_str().unwrap(),
);

// Keep in sync with TargetConfig::new.
// Keep in sync with TargetConfig::parse_links_overrides.
match key {
"rustc-flags" => {
let (paths, links) = BuildOutput::parse_rustc_flags(&value, &whence)?;
Expand All @@ -562,29 +570,61 @@ impl BuildOutput {
"rustc-link-lib" => library_links.push(value.to_string()),
"rustc-link-search" => library_paths.push(PathBuf::from(value)),
"rustc-link-arg-cdylib" | "rustc-cdylib-link-arg" => {
if !targets.iter().any(|target| target.is_cdylib()) {
bail!(
"invalid instruction `cargo:{}` from {}\n\
The package {} does not have a cdylib target.",
key,
whence,
pkg_descr
);
}
linker_args.push((LinkType::Cdylib, value))
}
"rustc-link-arg-bins" => {
if extra_link_arg {
if !targets.iter().any(|target| target.is_bin()) {
bail!(
"invalid instruction `cargo:{}` from {}\n\
The package {} does not have a bin target.",
key,
whence,
pkg_descr
);
}
linker_args.push((LinkType::Bin, value));
} else {
warnings.push(format!("cargo:{} requires -Zextra-link-arg flag", key));
}
}
"rustc-link-arg-bin" => {
if extra_link_arg {
let parts = value.splitn(2, "=").collect::<Vec<_>>();
if parts.len() == 2 {
linker_args.push((
LinkType::SingleBin(parts[0].to_string()),
parts[1].to_string(),
));
} else {
warnings.push(format!(
"cargo:{} has invalid syntax: expected `cargo:{}=BIN=ARG`",
key, key
));
let mut parts = value.splitn(2, '=');
let bin_name = parts.next().unwrap().to_string();
let arg = parts.next().ok_or_else(|| {
anyhow::format_err!(
"invalid instruction `cargo:{}={}` from {}\n\
The instruction should have the form cargo:{}=BIN=ARG",
key,
value,
whence,
key
)
})?;
if !targets
.iter()
.any(|target| target.is_bin() && target.name() == bin_name)
{
bail!(
"invalid instruction `cargo:{}` from {}\n\
The package {} does not have a bin target with the name `{}`.",
key,
whence,
pkg_descr,
bin_name
);
}
linker_args.push((LinkType::SingleBin(bin_name), arg.to_string()));
} else {
warnings.push(format!("cargo:{} requires -Zextra-link-arg flag", key));
}
Expand Down Expand Up @@ -632,7 +672,7 @@ impl BuildOutput {
} else {
// Setting RUSTC_BOOTSTRAP would change the behavior of the crate.
// Abort with an error.
anyhow::bail!("Cannot set `RUSTC_BOOTSTRAP={}` from {}.\n\
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,
Expand Down Expand Up @@ -683,7 +723,7 @@ impl BuildOutput {
if value.is_empty() {
value = match flags_iter.next() {
Some(v) => v,
None => anyhow::bail! {
None => bail! {
"Flag in rustc-flags has no value in {}: {}",
whence,
value
Expand All @@ -699,7 +739,7 @@ impl BuildOutput {
_ => unreachable!(),
};
} else {
anyhow::bail!(
bail!(
"Only `-l` and `-L` flags are allowed in {}: `{}`",
whence,
value
Expand All @@ -715,7 +755,7 @@ impl BuildOutput {
let val = iter.next();
match (name, val) {
(Some(n), Some(v)) => Ok((n.to_owned(), v.to_owned())),
_ => anyhow::bail!("Variable rustc-env has no value in {}: {}", whence, value),
_ => bail!("Variable rustc-env has no value in {}: {}", whence, value),
}
}
}
Expand Down Expand Up @@ -900,6 +940,7 @@ fn prev_build_output(cx: &mut Context<'_, '_>, unit: &Unit) -> (Option<BuildOutp
&script_out_dir,
extra_link_arg,
cx.bcx.config.nightly_features_allowed,
unit.pkg.targets(),
)
.ok(),
prev_script_out_dir,
Expand Down
70 changes: 70 additions & 0 deletions tests/testsuite/build_script_extra_link_arg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,73 @@ fn build_script_extra_link_arg_warn_without_flag() {
.with_stderr_contains("warning: cargo:rustc-link-arg requires -Zextra-link-arg flag")
.run();
}

#[cargo_test]
fn link_arg_missing_target() {
// Errors when a given target doesn't exist.
let p = project()
.file("src/lib.rs", "")
.file(
"build.rs",
r#"fn main() { println!("cargo:rustc-link-arg-cdylib=--bogus"); }"#,
)
.build();

p.cargo("check")
.with_status(101)
.with_stderr("\
[COMPILING] foo [..]
error: invalid instruction `cargo:rustc-link-arg-cdylib` from build script of `foo v0.0.1 ([ROOT]/foo)`
The package foo v0.0.1 ([ROOT]/foo) does not have a cdylib target.
")
.run();

p.change_file(
"build.rs",
r#"fn main() { println!("cargo:rustc-link-arg-bins=--bogus"); }"#,
);

p.cargo("check -Zextra-link-arg")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr("\
[COMPILING] foo [..]
error: invalid instruction `cargo:rustc-link-arg-bins` from build script of `foo v0.0.1 ([ROOT]/foo)`
The package foo v0.0.1 ([ROOT]/foo) does not have a bin target.
")
.run();

p.change_file(
"build.rs",
r#"fn main() { println!("cargo:rustc-link-arg-bin=abc=--bogus"); }"#,
);

p.cargo("check -Zextra-link-arg")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr(
"\
[COMPILING] foo [..]
error: invalid instruction `cargo:rustc-link-arg-bin` from build script of `foo v0.0.1 ([ROOT]/foo)`
The package foo v0.0.1 ([ROOT]/foo) does not have a bin target with the name `abc`.
",
)
.run();

p.change_file(
"build.rs",
r#"fn main() { println!("cargo:rustc-link-arg-bin=abc"); }"#,
);

p.cargo("check -Zextra-link-arg")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr(
"\
[COMPILING] foo [..]
error: invalid instruction `cargo:rustc-link-arg-bin=abc` from build script of `foo v0.0.1 ([ROOT]/foo)`
The instruction should have the form cargo:rustc-link-arg-bin=BIN=ARG
",
)
.run();
}

0 comments on commit 6597523

Please sign in to comment.