Check for duplicate output filenames.

This commit is contained in:
Eric Huss 2018-11-12 07:13:13 -08:00
parent 4e6bbd7044
commit fa0787aaf7
11 changed files with 247 additions and 45 deletions

View File

@ -111,7 +111,7 @@ pub enum MessageFormat {
/// `compile_ws` to tell it the general execution strategy. This influences
/// the default targets selected. The other use is in the `Unit` struct
/// to indicate what is being done with a specific target.
#[derive(Clone, Copy, PartialEq, Debug, Eq, Hash)]
#[derive(Clone, Copy, PartialEq, Debug, Eq, Hash, PartialOrd, Ord)]
pub enum CompileMode {
/// A target being built for a test.
Test,

View File

@ -38,11 +38,13 @@ pub struct CompilationFiles<'a, 'cfg: 'a> {
#[derive(Debug)]
pub struct OutputFile {
/// File name that will be produced by the build process (in `deps`).
/// Absolute path to the file that will be produced by the build process.
pub path: PathBuf,
/// If it should be linked into `target`, and what it should be called
/// (e.g. without metadata).
pub hardlink: Option<PathBuf>,
/// If `--out-dir` is specified, the absolute path to the exported file.
pub export_path: Option<PathBuf>,
/// Type of the file (library / debug symbol / else).
pub flavor: FileFlavor,
}
@ -251,6 +253,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
ret.push(OutputFile {
path,
hardlink: None,
export_path: None,
flavor: FileFlavor::Linkable,
});
} else {
@ -273,9 +276,19 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
let hardlink = link_stem
.as_ref()
.map(|&(ref ld, ref ls)| ld.join(file_type.filename(ls)));
let export_path = if unit.target.is_custom_build() {
None
} else {
self.export_dir.as_ref().and_then(|export_dir| {
hardlink.as_ref().and_then(|hardlink| {
Some(export_dir.join(hardlink.file_name().unwrap()))
})
})
};
ret.push(OutputFile {
path,
hardlink,
export_path,
flavor: file_type.flavor,
});
},

View File

@ -4,8 +4,8 @@ use std::ffi::OsStr;
use std::fmt::Write;
use std::path::PathBuf;
use std::sync::Arc;
use std::cmp::Ordering;
use failure::Error;
use jobserver::Client;
use core::{Package, PackageId, Resolve, Target};
@ -42,7 +42,7 @@ use self::compilation_files::CompilationFiles;
/// example, it needs to know the target architecture (OS, chip arch etc.) and it needs to know
/// whether you want a debug or release build. There is enough information in this struct to figure
/// all that out.
#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)]
pub struct Unit<'a> {
/// Information about available targets, which files to include/exclude, etc. Basically stuff in
/// `Cargo.toml`.
@ -72,18 +72,6 @@ impl<'a> Unit<'a> {
}
}
impl<'a> Ord for Unit<'a> {
fn cmp(&self, other: &Unit) -> Ordering {
self.buildkey().cmp(&other.buildkey())
}
}
impl<'a> PartialOrd for Unit<'a> {
fn partial_cmp(&self, other: &Unit) -> Option<Ordering> {
Some(self.cmp(other))
}
}
pub struct Context<'a, 'cfg: 'a> {
pub bcx: &'a BuildContext<'a, 'cfg>,
pub compilation: Compilation<'cfg>,
@ -150,6 +138,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
self.prepare_units(export_dir, units)?;
self.prepare()?;
custom_build::build_map(&mut self, units)?;
self.check_collistions()?;
for unit in units.iter() {
// Build up a list of pending jobs, each of which represent
@ -353,13 +342,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
self.files.as_mut().unwrap()
}
/// Return the filenames that the given target for the given profile will
/// generate as a list of 3-tuples (filename, link_dst, linkable)
///
/// - filename: filename rustc compiles to. (Often has metadata suffix).
/// - link_dst: Optional file to link/copy the result to (without metadata suffix)
/// - linkable: Whether possible to link against file (eg it's a library)
pub fn outputs(&mut self, unit: &Unit<'a>) -> CargoResult<Arc<Vec<OutputFile>>> {
/// Return the filenames that the given unit will generate.
pub fn outputs(&self, unit: &Unit<'a>) -> CargoResult<Arc<Vec<OutputFile>>> {
self.files.as_ref().unwrap().outputs(unit, self.bcx)
}
@ -468,6 +452,89 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
inputs.sort();
Ok(inputs)
}
fn check_collistions(&self) -> CargoResult<()> {
let mut output_collisions = HashMap::new();
let describe_collision = |unit: &Unit, other_unit: &Unit, path: &PathBuf| -> String {
format!(
"The {} target `{}` in package `{}` has the same output \
filename as the {} target `{}` in package `{}`.\n\
Colliding filename is: {}\n",
unit.target.kind().description(),
unit.target.name(),
unit.pkg.package_id(),
other_unit.target.kind().description(),
other_unit.target.name(),
other_unit.pkg.package_id(),
path.display()
)
};
let suggestion = "Consider changing their names to be unique or compiling them separately.";
let report_collision = |unit: &Unit, other_unit: &Unit, path: &PathBuf| -> Error {
if unit.target.name() == other_unit.target.name() {
format_err!(
"output filename collision.\n\
{}\
The targets must have unique names.\n\
{}",
describe_collision(unit, other_unit, path),
suggestion
)
} else {
format_err!(
"output filename collision.\n\
{}\
The output filenames must be unique.\n\
{}\n\
If this looks unexpected, it may be a bug in Cargo. Please file a bug report at\n\
https://github.com/rust-lang/cargo/issues/ with as much information as you\n\
can provide.\n\
{} running on `{}` target `{}`\n\
First unit: {:?}\n\
Second unit: {:?}",
describe_collision(unit, other_unit, path),
suggestion,
::version(), self.bcx.host_triple(), self.bcx.target_triple(),
unit, other_unit)
}
};
let mut keys = self
.unit_dependencies
.keys()
.filter(|unit| !unit.mode.is_run_custom_build())
.collect::<Vec<_>>();
// Sort for consistent error messages.
keys.sort_unstable();
for unit in keys {
for output in self.outputs(unit)?.iter() {
if let Some(other_unit) =
output_collisions.insert(output.path.clone(), unit)
{
return Err(report_collision(unit, &other_unit, &output.path));
}
if let Some(hardlink) = output.hardlink.as_ref() {
if let Some(other_unit) = output_collisions.insert(hardlink.clone(), unit)
{
return Err(report_collision(unit, &other_unit, hardlink));
}
}
if let Some(ref export_path) = output.export_path {
if let Some(other_unit) =
output_collisions.insert(export_path.clone(), unit)
{
bail!("`--out-dir` filename collision.\n\
{}\
The exported filenames must be unique.\n\
{}",
describe_collision(unit, &other_unit, &export_path),
suggestion
);
}
}
}
}
Ok(())
}
}
#[derive(Default)]

View File

@ -9,7 +9,7 @@ use serde::de::{self, Deserialize};
use serde::ser;
use serde_json;
use core::{Edition, Package, TargetKind};
use core::{Edition, Package};
use util;
use util::errors::{CargoResult, CargoResultExt};
use util::paths;
@ -780,14 +780,7 @@ fn filename<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> String {
// fingerprint for every metadata hash version. This works because
// even if the package is fresh, we'll still link the fresh target
let file_stem = cx.files().file_stem(unit);
let kind = match *unit.target.kind() {
TargetKind::Lib(..) => "lib",
TargetKind::Bin => "bin",
TargetKind::Test => "integration-test",
TargetKind::ExampleBin | TargetKind::ExampleLib(..) => "example",
TargetKind::Bench => "bench",
TargetKind::CustomBuild => "build-script",
};
let kind = unit.target.kind().description();
let flavor = if unit.mode.is_any_test() {
"test-"
} else if unit.mode.is_doc() {

View File

@ -436,14 +436,13 @@ fn link_targets<'a, 'cfg>(
};
destinations.push(dst.display().to_string());
hardlink_or_copy(src, dst)?;
if let Some(ref path) = export_dir {
if !target.is_custom_build() {
if !path.exists() {
fs::create_dir_all(path)?;
}
hardlink_or_copy(src, &path.join(dst.file_name().unwrap()))?;
if let Some(ref path) = output.export_path {
let export_dir = export_dir.as_ref().unwrap();
if !export_dir.exists() {
fs::create_dir_all(export_dir)?;
}
hardlink_or_copy(src, path)?;
}
}

View File

@ -181,9 +181,22 @@ impl fmt::Debug for TargetKind {
}
}
impl TargetKind {
pub fn description(&self) -> &'static str {
match self {
TargetKind::Lib(..) => "lib",
TargetKind::Bin => "bin",
TargetKind::Test => "integration-test",
TargetKind::ExampleBin | TargetKind::ExampleLib(..) => "example",
TargetKind::Bench => "bench",
TargetKind::CustomBuild => "build-script",
}
}
}
/// Information about a binary, a library, an example, etc. that is part of the
/// package.
#[derive(Clone, Hash, PartialEq, Eq)]
#[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct Target {
kind: TargetKind,
name: String,
@ -202,7 +215,7 @@ pub struct Target {
edition: Edition,
}
#[derive(Clone, PartialEq, Eq)]
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum TargetSourcePath {
Path(PathBuf),
Metabuild,

View File

@ -1,4 +1,5 @@
use std::cell::{Ref, RefCell, Cell};
use std::cmp::Ordering;
use std::collections::{HashMap, HashSet};
use std::fmt;
use std::hash;
@ -38,6 +39,18 @@ pub struct Package {
manifest_path: PathBuf,
}
impl Ord for Package {
fn cmp(&self, other: &Package) -> Ordering {
self.package_id().cmp(other.package_id())
}
}
impl PartialOrd for Package {
fn partial_cmp(&self, other: &Package) -> Option<Ordering> {
Some(self.cmp(other))
}
}
/// A Package in a form where `Serialize` can be derived.
#[derive(Serialize)]
struct SerializedPackage<'a> {

View File

@ -371,7 +371,7 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
/// Profile settings used to determine which compiler flags to use for a
/// target.
#[derive(Clone, Copy, Eq)]
#[derive(Clone, Copy, Eq, PartialOrd, Ord)]
pub struct Profile {
pub name: &'static str,
pub opt_level: InternedString,
@ -523,7 +523,7 @@ impl Profile {
}
/// The link-time-optimization setting.
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)]
pub enum Lto {
/// False = no LTO
/// True = "Fat" LTO

View File

@ -4327,8 +4327,8 @@ fn target_filters_workspace() {
.file("a/src/lib.rs", "")
.file("a/examples/ex1.rs", "fn main() {}")
.file("b/Cargo.toml", &basic_bin_manifest("b"))
.file("b/src/lib.rs", "")
.file("b/src/main.rs", "fn main() {}")
.file("b/examples/ex1.rs", "fn main() {}")
.build();
ws.cargo("build -v --example ex")
@ -4343,12 +4343,12 @@ Did you mean `ex1`?",
ws.cargo("build -v --lib")
.with_status(0)
.with_stderr_contains("[RUNNING] `rustc [..]a/src/lib.rs[..]")
.with_stderr_contains("[RUNNING] `rustc [..]b/src/lib.rs[..]")
.run();
ws.cargo("build -v --example ex1")
.with_status(0)
.with_stderr_contains("[RUNNING] `rustc [..]a/examples/ex1.rs[..]")
.with_stderr_contains("[RUNNING] `rustc [..]b/examples/ex1.rs[..]")
.run();
}

View File

@ -0,0 +1,103 @@
use std::env;
use support::{basic_manifest, project};
#[test]
fn collision_dylib() {
// Path dependencies don't include metadata hash in filename for dylibs.
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["a", "b"]
"#,
)
.file(
"a/Cargo.toml",
r#"
[package]
name = "a"
version = "1.0.0"
[lib]
crate-type = ["dylib"]
"#,
)
.file("a/src/lib.rs", "")
.file(
"b/Cargo.toml",
r#"
[package]
name = "b"
version = "1.0.0"
[lib]
crate-type = ["dylib"]
name = "a"
"#,
)
.file("b/src/lib.rs", "")
.build();
p.cargo("build")
.with_stderr(&format!("\
[ERROR] output filename collision.
The lib target `a` in package `b v1.0.0 ([..]/foo/b)` has the same output filename as the lib target `a` in package `a v1.0.0 ([..]/foo/a)`.
Colliding filename is: [..]/foo/target/debug/deps/{}a{}
The targets must have unique names.
Consider changing their names to be unique or compiling them separately.
", env::consts::DLL_PREFIX, env::consts::DLL_SUFFIX))
.with_status(101)
.run();
}
#[test]
fn collision_example() {
// Examples in a workspace can easily collide.
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["a", "b"]
"#,
)
.file("a/Cargo.toml", &basic_manifest("a", "1.0.0"))
.file("a/examples/ex1.rs", "fn main() {}")
.file("b/Cargo.toml", &basic_manifest("b", "1.0.0"))
.file("b/examples/ex1.rs", "fn main() {}")
.build();
p.cargo("build --examples")
.with_stderr("\
[ERROR] output filename collision.
The example target `ex1` in package `b v1.0.0 ([..]/foo/b)` has the same output filename as the example target `ex1` in package `a v1.0.0 ([..]/foo/a)`.
Colliding filename is: [..]/foo/target/debug/examples/ex1[EXE]
The targets must have unique names.
Consider changing their names to be unique or compiling them separately.
")
.with_status(101)
.run();
}
#[test]
fn collision_export() {
// --out-dir combines some things which can cause conflicts.
let p = project()
.file("Cargo.toml", &basic_manifest("foo", "1.0.0"))
.file("examples/foo.rs", "fn main() {}")
.file("src/main.rs", "fn main() {}")
.build();
p.cargo("build --out-dir=out -Z unstable-options --bins --examples")
.masquerade_as_nightly_cargo()
.with_stderr("\
[ERROR] `--out-dir` filename collision.
The example target `foo` in package `foo v1.0.0 ([..]/foo)` has the same output filename as the bin target `foo` in package `foo v1.0.0 ([..]/foo)`.
Colliding filename is: [..]/foo/out/foo[EXE]
The exported filenames must be unique.
Consider changing their names to be unique or compiling them separately.
")
.with_status(101)
.run();
}

View File

@ -43,6 +43,7 @@ mod cargo_features;
mod cfg;
mod check;
mod clean;
mod collisions;
mod concurrent;
mod config;
mod corrupt_git;