Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change rustc-cdylib-link-arg error to a warning. #9563

Merged
merged 1 commit into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}