From 6ef861da961c62361c0d34383c845849ba3a26fa Mon Sep 17 00:00:00 2001 From: bors Date: Tue, 22 Jun 2021 19:27:25 +0000 Subject: [PATCH] Auto merge of #9563 - ehuss:link-cdylib-warning, r=alexcrichton Change `rustc-cdylib-link-arg` error to a warning. In #9523, an error was added if `cargo:rustc-cdylib-link-arg` was issued in a build script without actually having a cdylib target. This uncovered that there was an unintentional change in #8441 to cause those link args to be applied to transitive dependencies. This changes it so that the error is now a warning, with a note that this may become an error in the future. It also changes it so that the unstable `rustc-link-arg*` instructions only apply to the package that emitted them. --- src/cargo/core/compiler/custom_build.rs | 20 ++- src/cargo/core/compiler/mod.rs | 7 +- .../testsuite/build_script_extra_link_arg.rs | 135 ++++++++++++++++-- 3 files changed, 145 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index ed13a9d3ed8..8fa6cb8b1da 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -38,6 +38,9 @@ pub struct BuildOutput { /// Environment variables which, when changed, will cause a rebuild. pub rerun_if_env_changed: Vec, /// 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, } @@ -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 \ + .", + key, pkg_descr + )); } linker_args.push((LinkType::Cdylib, value)) } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 0d72a9491a3..e5ea065983b 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -394,7 +394,12 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc) -> 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)); } } diff --git a/tests/testsuite/build_script_extra_link_arg.rs b/tests/testsuite/build_script_extra_link_arg.rs index 6e8384b8a27..79baee04d11 100644 --- a/tests/testsuite/build_script_extra_link_arg.rs +++ b/tests/testsuite/build_script_extra_link_arg.rs @@ -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() { @@ -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", @@ -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 . +[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(); +}