Skip to content

Commit

Permalink
Change rustc-cdylib-link-arg error to a warning.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Jun 9, 2021
1 parent e21e03f commit 2bcece9
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 17 deletions.
20 changes: 13 additions & 7 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ pub struct BuildOutput {
/// Environment variables which, when changed, will cause a rebuild.
pub rerun_if_env_changed: Vec<String>,
/// Warnings generated by this build.
///
/// These are only displayed if this is a "local" package, `-vv` is used,
/// or there is a build error for any target in this package.
pub warnings: Vec<String>,
}

Expand Down Expand Up @@ -571,13 +574,16 @@ impl BuildOutput {
"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
);
warnings.push(format!(
"cargo:{} was specified in the build script of {}, \
but that package does not contain a cdylib target\n\
\n\
Allowing this was an unintended change in the 1.50 \
release, and may become an error in the future. \
For more information, see \
<https://github.com/rust-lang/cargo/issues/9562>.",
key, pkg_descr
));
}
linker_args.push((LinkType::Cdylib, value))
}
Expand Down
7 changes: 6 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,12 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
}

for (lt, arg) in &output.linker_args {
if lt.applies_to(target) {
// There was an unintentional change where cdylibs were
// allowed to be passed via transitive dependencies. This
// clause should have been kept in the `if` block above. For
// now, continue allowing it for cdylib only.
// See https://github.com/rust-lang/cargo/issues/9562
if lt.applies_to(target) && (key.0 == current_id || *lt == LinkType::Cdylib) {
rustc.arg("-C").arg(format!("link-arg={}", arg));
}
}
Expand Down
135 changes: 126 additions & 9 deletions tests/testsuite/build_script_extra_link_arg.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
//! Tests for -Zextra-link-arg.

use cargo_test_support::{basic_bin_manifest, project};
// NOTE: Many of these tests use `without_status()` when passing bogus flags
// because MSVC link.exe just gives a warning on unknown flags (how helpful!),
// and other linkers will return an error.

use cargo_test_support::registry::Package;
use cargo_test_support::{basic_bin_manifest, basic_manifest, project};

#[cargo_test]
fn build_script_extra_link_arg_bin() {
Expand Down Expand Up @@ -125,14 +130,16 @@ fn link_arg_missing_target() {
)
.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();
// TODO: Uncomment this if cdylib restriction is re-added (see
// cdylib_link_arg_transitive below).
// 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",
Expand Down Expand Up @@ -183,3 +190,113 @@ The instruction should have the form cargo:rustc-link-arg-bin=BIN=ARG
)
.run();
}

#[cargo_test]
fn cdylib_link_arg_transitive() {
// There was an unintended regression in 1.50 where rustc-link-arg-cdylib
// arguments from dependencies were being applied in the parent package.
// Previously it was silently ignored.
// See https://github.com/rust-lang/cargo/issues/9562
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[lib]
crate-type = ["cdylib"]
[dependencies]
bar = {path="bar"}
"#,
)
.file("src/lib.rs", "")
.file("bar/Cargo.toml", &basic_manifest("bar", "1.0.0"))
.file("bar/src/lib.rs", "")
.file(
"bar/build.rs",
r#"
fn main() {
println!("cargo:rustc-link-arg-cdylib=--bogus");
}
"#,
)
.build();
p.cargo("build -v")
.without_status()
.with_stderr_contains(
"\
[COMPILING] bar v1.0.0 [..]
[RUNNING] `rustc --crate-name build_script_build bar/build.rs [..]
[RUNNING] `[..]build-script-build[..]
warning: cargo:rustc-link-arg-cdylib was specified in the build script of bar v1.0.0 \
([ROOT]/foo/bar), but that package does not contain a cdylib target
Allowing this was an unintended change in the 1.50 release, and may become an error in \
the future. For more information, see <https://github.com/rust-lang/cargo/issues/9562>.
[RUNNING] `rustc --crate-name bar bar/src/lib.rs [..]
[COMPILING] foo v0.1.0 [..]
[RUNNING] `rustc --crate-name foo src/lib.rs [..]-C link-arg=--bogus[..]`
",
)
.run();
}

#[cargo_test]
fn link_arg_transitive_not_allowed() {
// Verify that transitive dependencies don't pass link args.
//
// Note that rustc-link-arg doesn't have any errors or warnings when it is
// unused. Perhaps that could be more aggressive, but it is difficult
// since it could be used for test binaries.
Package::new("bar", "1.0.0")
.file("src/lib.rs", "")
.file(
"build.rs",
r#"
fn main() {
println!("cargo:rustc-link-arg=--bogus");
}
"#,
)
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[lib]
crate-type = ["cdylib"]
[dependencies]
bar = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("build -v -Zextra-link-arg")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[UPDATING] [..]
[DOWNLOADING] [..]
[DOWNLOADED] [..]
[COMPILING] bar v1.0.0
[RUNNING] `rustc --crate-name build_script_build [..]
[RUNNING] `[..]/build-script-build[..]
[RUNNING] `rustc --crate-name bar [..]
[COMPILING] foo v0.1.0 [..]
[RUNNING] `rustc --crate-name foo src/lib.rs [..]
[FINISHED] dev [..]
",
)
.with_stderr_does_not_contain("--bogus")
.run();
}

0 comments on commit 2bcece9

Please sign in to comment.