rust
/
cargo
mirror of https://github.com/rust-lang/cargo
0
0
Fork 0
Code Issues Releases Wiki Activity

Auto merge of #9611 - ehuss:beta-1.54-backports, r=alexcrichton 

Beta 1.54 backports

This backports the following changes to 1.54 beta:

* #9595 — Relax doc collision error.  This was an unintended regression where a warning was switched to an error in 1.54.
* #9563 — Change rustc-cdylib-link-arg error to a warning.  Validation was added in #9523 (1.54) that generated an error. However, it was unknown at the time that use of link args via dependencies was being used. Rather than regress those projects (which has been on stable since 1.50), this switches the validation to a warning to indicate this was not intended.
* #9604 — Disambiguate is_symlink. Required to get CI to pass.
This commit is contained in:
bors 2021-06-22 22:02:10 +00:00
parent aa8b09297b 09ed3d2fcd
commit 5ae8d74b3b
8 changed files with 193 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
crates/cargo-test-support/src/paths.rs
View File

@ -125,8 +125,6 @@ pub trait CargoPathExt {
fn move_in_time<F>(&self, travel_amount: F)
where
F: Fn(i64, u32) -> (i64, u32);
fn is_symlink(&self) -> bool;
}
impl CargoPathExt for Path {
@ -199,12 +197,14 @@ impl CargoPathExt for Path {
});
}
}
}
fn is_symlink(&self) -> bool {
fs::symlink_metadata(self)
.map(|m| m.file_type().is_symlink())
.unwrap_or(false)
}
// Replace with std implementation when stabilized, see
// https://github.com/rust-lang/rust/issues/85748
pub fn is_symlink(path: &Path) -> bool {
fs::symlink_metadata(path)
.map(|m| m.file_type().is_symlink())
.unwrap_or(false)
}
fn do_op<F>(path: &Path, desc: &str, mut f: F)

28
src/cargo/core/compiler/context/mod.rs
View File

@ -507,17 +507,35 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
.collect::<Vec<_>>();
// Sort for consistent error messages.
keys.sort_unstable();
// These are kept separate to retain compatibility with older
// versions, which generated an error when there was a duplicate lib
// or bin (but the old code did not check bin<->lib collisions). To
// retain backwards compatibility, this only generates an error for
// duplicate libs or duplicate bins (but not both). Ideally this
// shouldn't be here, but since there isn't a complete workaround,
// yet, this retains the old behavior.
let mut doc_libs = HashMap::new();
let mut doc_bins = HashMap::new();
for unit in keys {
if unit.mode.is_doc() && self.is_primary_package(unit) {
// These situations have been an error since before 1.0, so it
// is not a warning like the other situations.
if unit.target.is_lib() {
if let Some(prev) = doc_libs.insert((unit.target.crate_name(), unit.kind), unit)
{
doc_collision_error(unit, prev)?;
}
} else if let Some(prev) =
doc_bins.insert((unit.target.crate_name(), unit.kind), unit)
{
doc_collision_error(unit, prev)?;
}
}
for output in self.outputs(unit)?.iter() {
if let Some(other_unit) = output_collisions.insert(output.path.clone(), unit) {
if unit.mode.is_doc() {
// See https://github.com/rust-lang/rust/issues/56169
// and https://github.com/rust-lang/rust/issues/61378
if self.is_primary_package(unit) {
// This has been an error since before 1.0, so it
// is not a warning like the other situations.
doc_collision_error(unit, other_unit)?;
}
report_collision(unit, other_unit, &output.path, rustdoc_suggestion)?;
} else {
report_collision(unit, other_unit, &output.path, suggestion)?;

20
src/cargo/core/compiler/custom_build.rs
View File

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

7
src/cargo/core/compiler/mod.rs
View File

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

6
tests/testsuite/build.rs
View File

@ -4505,6 +4505,7 @@ fn building_a_dependent_crate_witout_bin_should_fail() {
#[cargo_test]
#[cfg(any(target_os = "macos", target_os = "ios"))]
fn uplift_dsym_of_bin_on_mac() {
use cargo_test_support::paths::is_symlink;
let p = project()
.file("src/main.rs", "fn main() { panic!(); }")
.file("src/bin/b.rs", "fn main() { panic!(); }")
@ -4517,7 +4518,7 @@ fn uplift_dsym_of_bin_on_mac() {
.run();
assert!(p.target_debug_dir().join("foo.dSYM").is_dir());
assert!(p.target_debug_dir().join("b.dSYM").is_dir());
assert!(p.target_debug_dir().join("b.dSYM").is_symlink());
assert!(is_symlink(&p.target_debug_dir().join("b.dSYM")));
assert!(p.target_debug_dir().join("examples/c.dSYM").is_dir());
assert!(!p.target_debug_dir().join("c.dSYM").exists());
assert!(!p.target_debug_dir().join("d.dSYM").exists());
@ -4526,6 +4527,7 @@ fn uplift_dsym_of_bin_on_mac() {
#[cargo_test]
#[cfg(any(target_os = "macos", target_os = "ios"))]
fn uplift_dsym_of_bin_on_mac_when_broken_link_exists() {
use cargo_test_support::paths::is_symlink;
let p = project()
.file("src/main.rs", "fn main() { panic!(); }")
.build();
@ -4544,7 +4546,7 @@ fn uplift_dsym_of_bin_on_mac_when_broken_link_exists() {
.join("foo-baaaaaadbaaaaaad.dSYM"),
&dsym,
);
assert!(dsym.is_symlink());
assert!(is_symlink(&dsym));
assert!(!dsym.exists());
p.cargo("build").enable_mac_dsym().run();

135
tests/testsuite/build_script_extra_link_arg.rs
View File

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

4
tests/testsuite/clean.rs
View File

@ -1,6 +1,6 @@
//! Tests for the `cargo clean` command.
use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::paths::is_symlink;
use cargo_test_support::registry::Package;
use cargo_test_support::{basic_bin_manifest, basic_manifest, git, main_file, project, rustc_host};
use std::env;
@ -438,7 +438,7 @@ fn assert_all_clean(build_dir: &Path) {
{
continue;
}
if path.is_symlink() || path.is_file() {
if is_symlink(path) || path.is_file() {
panic!("{:?} was not cleaned", path);
}
}

19
tests/testsuite/doc.rs
View File

@ -271,14 +271,19 @@ fn doc_multiple_targets_same_name() {
.build();
p.cargo("doc --workspace")
.with_status(101)
.with_stderr(
.with_stderr_unordered(
"\
error: document output filename collision
The bin `foo_lib` in package `foo v0.1.0 ([ROOT]/foo/foo)` has the same name as \
the lib `foo_lib` in package `bar v0.1.0 ([ROOT]/foo/bar)`.
Only one may be documented at once since they output to the same path.
Consider documenting only one, renaming one, or marking one with `doc = false` in Cargo.toml.
warning: output filename collision.
The bin target `foo_lib` in package `foo v0.1.0 ([ROOT]/foo/foo)` \
has the same output filename as the lib target `foo_lib` in package \
`bar v0.1.0 ([ROOT]/foo/bar)`.
Colliding filename is: [ROOT]/foo/target/doc/foo_lib/index.html
The targets should have unique names.
This is a known bug where multiple crates with the same name use
the same path; see <https://github.com/rust-lang/cargo/issues/6313>.
[DOCUMENTING] bar v0.1.0 ([ROOT]/foo/bar)
[DOCUMENTING] foo v0.1.0 ([ROOT]/foo/foo)
[FINISHED] [..]
",
)
.run();