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.

Sign up for GitHub

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

Jump to bottom

Catch filename output collisions in rustdoc. #6998

Merged
merged 3 commits into from
Jun 3, 2019
Merged

Catch filename output collisions in rustdoc. #6998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6909178
Catch filename output collisions in rustdoc.
ehuss May 31, 2019
935adb3
Clean up how doc tests are treated.
ehuss May 31, 2019
de44054
Remove hard-coded index.html in fingerprint function.
ehuss May 31, 2019
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
11 changes: 7 additions & 4 deletions src/cargo/core/compiler/build_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,19 @@ impl CompileMode {
}
}

/// Returns `true` if this is a doc or doc test. Be careful using this.
/// Although both run rustdoc, the dependencies for those two modes are
/// very different.
/// Returns `true` if this is generating documentation.
pub fn is_doc(self) -> bool {
match self {
CompileMode::Doc { .. } | CompileMode::Doctest => true,
CompileMode::Doc { .. } => true,
_ => false,
}
}

/// Returns `true` if this a doc test.
pub fn is_doc_test(self) -> bool {
self == CompileMode::Doctest
}

/// Returns `true` if this is any type of test (test, benchmark, doc test, or
/// check test).
pub fn is_any_test(self) -> bool {
Expand Down
224 changes: 130 additions & 94 deletions src/cargo/core/compiler/context/compilation_files.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use lazycell::LazyCell;
use log::info;

use super::{BuildContext, Context, FileFlavor, Kind, Layout};
use crate::core::compiler::Unit;
use crate::core::compiler::{CompileMode, Unit};
use crate::core::{TargetKind, Workspace};
use crate::util::{self, CargoResult};

Expand Down Expand Up @@ -146,6 +146,8 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
pub fn out_dir(&self, unit: &Unit<'a>) -> PathBuf {
if unit.mode.is_doc() {
self.layout(unit.kind).root().parent().unwrap().join("doc")
} else if unit.mode.is_doc_test() {
panic!("doc tests do not have an out dir");
} else if unit.target.is_custom_build() {
self.build_script_dir(unit)
} else if unit.target.is_example() {
Expand Down Expand Up @@ -293,107 +295,139 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
unit: &Unit<'a>,
bcx: &BuildContext<'a, 'cfg>,
) -> CargoResult<Arc<Vec<OutputFile>>> {
let ret = match unit.mode {
CompileMode::Check { .. } => {
// This may be confusing. rustc outputs a file named `lib*.rmeta`
// for both libraries and binaries.
let file_stem = self.file_stem(unit);
let path = self.out_dir(unit).join(format!("lib{}.rmeta", file_stem));
vec![OutputFile {
path,
hardlink: None,
export_path: None,
flavor: FileFlavor::Linkable { rmeta: false },
}]
}
CompileMode::Doc { .. } => {
let path = self
.out_dir(unit)
.join(unit.target.crate_name())
.join("index.html");
vec![OutputFile {
path,
hardlink: None,
export_path: None,
flavor: FileFlavor::Normal,
}]
}
CompileMode::RunCustomBuild => {
// At this time, this code path does not handle build script
// outputs.
vec![]
}
CompileMode::Doctest => {
// Doctests are built in a temporary directory and then
// deleted. There is the `--persist-doctests` unstable flag,
// but Cargo does not know about that.
vec![]
}
CompileMode::Test | CompileMode::Build | CompileMode::Bench => {
self.calc_outputs_rustc(unit, bcx)?
}
};
info!("Target filenames: {:?}", ret);

Ok(Arc::new(ret))
}

fn calc_outputs_rustc(
&self,
unit: &Unit<'a>,
bcx: &BuildContext<'a, 'cfg>,
) -> CargoResult<Vec<OutputFile>> {
let mut ret = Vec::new();
let mut unsupported = Vec::new();

let out_dir = self.out_dir(unit);
let file_stem = self.file_stem(unit);
let link_stem = self.link_stem(unit);
let info = if unit.kind == Kind::Host {
&bcx.host_info
} else {
&bcx.target_info
};
let file_stem = self.file_stem(unit);

let mut ret = Vec::new();
let mut unsupported = Vec::new();
{
if unit.mode.is_check() {
// This may be confusing. rustc outputs a file named `lib*.rmeta`
// for both libraries and binaries.
let path = out_dir.join(format!("lib{}.rmeta", file_stem));
ret.push(OutputFile {
path,
hardlink: None,
export_path: None,
flavor: FileFlavor::Linkable { rmeta: false },
});
let mut add = |crate_type: &str, flavor: FileFlavor| -> CargoResult<()> {
let crate_type = if crate_type == "lib" {
"rlib"
} else {
let mut add = |crate_type: &str, flavor: FileFlavor| -> CargoResult<()> {
let crate_type = if crate_type == "lib" {
"rlib"
} else {
crate_type
};
let file_types = info.file_types(
crate_type,
flavor,
unit.target.kind(),
bcx.target_triple(),
)?;

match file_types {
Some(types) => {
for file_type in types {
let path = out_dir.join(file_type.filename(&file_stem));
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,
});
}
}
// Not supported; don't worry about it.
None => {
unsupported.push(crate_type.to_string());
}
}
Ok(())
};
// info!("{:?}", unit);
match *unit.target.kind() {
TargetKind::Bin
| TargetKind::CustomBuild
| TargetKind::ExampleBin
| TargetKind::Bench
| TargetKind::Test => {
add("bin", FileFlavor::Normal)?;
}
TargetKind::Lib(..) | TargetKind::ExampleLib(..) if unit.mode.is_any_test() => {
add("bin", FileFlavor::Normal)?;
}
TargetKind::ExampleLib(ref kinds) | TargetKind::Lib(ref kinds) => {
for kind in kinds {
add(
kind.crate_type(),
if kind.linkable() {
FileFlavor::Linkable { rmeta: false }
} else {
FileFlavor::Normal
},
)?;
}
let path = out_dir.join(format!("lib{}.rmeta", file_stem));
if !unit.target.requires_upstream_objects() {
ret.push(OutputFile {
path,
hardlink: None,
export_path: None,
flavor: FileFlavor::Linkable { rmeta: true },
});
}
crate_type
};
let file_types =
info.file_types(crate_type, flavor, unit.target.kind(), bcx.target_triple())?;

match file_types {
Some(types) => {
for file_type in types {
let path = out_dir.join(file_type.filename(&file_stem));
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,
});
}
}
// Not supported; don't worry about it.
None => {
unsupported.push(crate_type.to_string());
}
}
Ok(())
};
match *unit.target.kind() {
TargetKind::Bin
| TargetKind::CustomBuild
| TargetKind::ExampleBin
| TargetKind::Bench
| TargetKind::Test => {
add("bin", FileFlavor::Normal)?;
}
TargetKind::Lib(..) | TargetKind::ExampleLib(..) if unit.mode.is_any_test() => {
add("bin", FileFlavor::Normal)?;
}
TargetKind::ExampleLib(ref kinds) | TargetKind::Lib(ref kinds) => {
for kind in kinds {
add(
kind.crate_type(),
if kind.linkable() {
FileFlavor::Linkable { rmeta: false }
} else {
FileFlavor::Normal
},
)?;
}
let path = out_dir.join(format!("lib{}.rmeta", file_stem));
if !unit.target.requires_upstream_objects() {
ret.push(OutputFile {
path,
hardlink: None,
export_path: None,
flavor: FileFlavor::Linkable { rmeta: true },
});
}
}
}
if ret.is_empty() {
Expand All @@ -413,9 +447,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
bcx.target_triple()
);
}
info!("Target filenames: {:?}", ret);

Ok(Arc::new(ret))
Ok(ret)
}
}

Expand All @@ -439,6 +471,10 @@ fn compute_metadata<'a, 'cfg>(
cx: &Context<'a, 'cfg>,
metas: &mut HashMap<Unit<'a>, Option<Metadata>>,
) -> Option<Metadata> {
if unit.mode.is_doc_test() {
// Doc tests do not have metadata.
return None;
}
// No metadata for dylibs because of a couple issues:
// - macOS encodes the dylib name in the executable,
// - Windows rustc multiple files of which we can't easily link all of them.
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
}
}

if unit.mode == CompileMode::Doctest {
if unit.mode.is_doc_test() {
// Note that we can *only* doc-test rlib outputs here. A
// staticlib output cannot be linked by the compiler (it just
// doesn't do that). A dylib output, however, can be linked by
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/unit_dependencies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ fn compute_deps<'a, 'cfg, 'tmp>(
) -> CargoResult<Vec<(Unit<'a>, UnitFor)>> {
if unit.mode.is_run_custom_build() {
return compute_deps_custom_build(unit, state.cx.bcx);
} else if unit.mode.is_doc() && !unit.mode.is_any_test() {
} else if unit.mode.is_doc() {
// Note: this does not include doc test.
return compute_deps_doc(unit, state);
}
Expand Down
24 changes: 10 additions & 14 deletions src/cargo/core/compiler/fingerprint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,8 @@ fn calculate<'a, 'cfg>(
}
let mut fingerprint = if unit.mode.is_run_custom_build() {
calculate_run_custom_build(cx, unit)?
} else if unit.mode.is_doc_test() {
panic!("doc tests do not fingerprint");
} else {
calculate_normal(cx, unit)?
};
Expand Down Expand Up @@ -1066,19 +1068,12 @@ fn calculate_normal<'a, 'cfg>(

// Figure out what the outputs of our unit is, and we'll be storing them
// into the fingerprint as well.
let outputs = if unit.mode.is_doc() {
vec![cx
.files()
.out_dir(unit)
.join(unit.target.crate_name())
.join("index.html")]
} else {
cx.outputs(unit)?
.iter()
.filter(|output| output.flavor != FileFlavor::DebugInfo)
.map(|output| output.path.clone())
.collect()
};
let outputs = cx
.outputs(unit)?
.iter()
.filter(|output| output.flavor != FileFlavor::DebugInfo)
.map(|output| output.path.clone())
.collect();

// Fill out a bunch more information that we'll be tracking typically
// hashed to take up less space on disk as we just need to know when things
Expand Down Expand Up @@ -1339,7 +1334,8 @@ fn write_fingerprint(loc: &Path, fingerprint: &Fingerprint) -> CargoResult<()> {
pub fn prepare_init<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<()> {
let new1 = cx.files().fingerprint_dir(unit);

if fs::metadata(&new1).is_err() {
// Doc tests have no output, thus no fingerprint.
if !new1.exists() && !unit.mode.is_doc_test() {
fs::create_dir(&new1)?;
}

Expand Down
10 changes: 4 additions & 6 deletions src/cargo/core/compiler/job_queue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,11 +596,10 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
// being a compiled package.
Dirty => {
if unit.mode.is_doc() {
self.documented.insert(unit.pkg.package_id());
config.shell().status("Documenting", unit.pkg)?;
} else if unit.mode.is_doc_test() {
// Skip doc test.
if !unit.mode.is_any_test() {
self.documented.insert(unit.pkg.package_id());
config.shell().status("Documenting", unit.pkg)?;
}
} else {
self.compiled.insert(unit.pkg.package_id());
if unit.mode.is_check() {
Expand All @@ -613,8 +612,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
Fresh => {
// If doc test are last, only print "Fresh" if nothing has been printed.
if self.counts[&unit.pkg.package_id()] == 0
&& !(unit.mode == CompileMode::Doctest
&& self.compiled.contains(&unit.pkg.package_id()))
&& !(unit.mode.is_doc_test() && self.compiled.contains(&unit.pkg.package_id()))
{
self.compiled.insert(unit.pkg.package_id());
config.shell().verbose(|c| c.status("Fresh", unit.pkg))?;
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fn compile<'a, 'cfg: 'a>(

let job = if unit.mode.is_run_custom_build() {
custom_build::prepare(cx, unit)?
} else if unit.mode == CompileMode::Doctest {
} else if unit.mode.is_doc_test() {
// We run these targets later, so this is just a no-op for now.
Job::new(Work::noop(), Freshness::Fresh)
} else if build_plan {
Expand Down
Loading