Auto merge of rust-lang#9563 - ehuss:link-cdylib-warning, r=alexcrichton

Change `rustc-cdylib-link-arg` error to a warning.

In rust-lang#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 rust-lang#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

@@ -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>,
 }
 
@@ -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))
                 }

src/cargo/core/compiler/mod.rs

@@ -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));
                 }
             }

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 <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();
+}