From b60eacc20206d38e03aa348786f6f59c0acb1153 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 16 Oct 2017 12:30:57 -0400 Subject: [PATCH 1/2] Document the lib if a lib and bin have the same name Fixes #4341. - Removes the check that bailed if there was a bin and lib with the same name - Exclude bins with the same name as libs from the proposed targets to build when compiling docs - Adjust tests to expect this behavior --- src/cargo/ops/cargo_compile.rs | 5 +++- src/cargo/ops/cargo_doc.rs | 14 ---------- tests/doc.rs | 49 +++++++++++++++++++++++++--------- tests/rustdoc.rs | 14 +++++++--- 4 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 685911203de..2de0739c628 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -489,7 +489,10 @@ fn generate_auto_targets<'a>(mode: CompileMode, targets: &'a [Target], } CompileMode::Doc { .. } => { targets.iter().filter(|t| { - t.documented() + t.documented() && ( + !t.is_bin() || + !targets.iter().any(|l| l.is_lib() && l.name() == t.name()) + ) }).map(|t| BuildProposal { target: t, profile: profile, diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index d4d5620367b..150a3a99608 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -52,20 +52,6 @@ pub fn doc(ws: &Workspace, options: &DocOptions) -> CargoResult<()> { } } } - for (bin, bin_package) in bin_names.iter() { - if let Some(lib_package) = lib_names.get(bin) { - bail!("The target `{}` is specified as a library {}. It can be \ - documented only once. Consider renaming or marking one \ - of the targets as `doc = false`.", - bin, - if lib_package == bin_package { - format!("and as a binary by package `{}`", lib_package) - } else { - format!("by package `{}` and as a binary by \ - package `{}`", lib_package, bin_package) - }); - } - } } ops::compile(ws, &options.compile_opts)?; diff --git a/tests/doc.rs b/tests/doc.rs index 0d215c91a98..c61428befc5 100644 --- a/tests/doc.rs +++ b/tests/doc.rs @@ -3,7 +3,8 @@ extern crate hamcrest; extern crate cargo; use std::str; -use std::fs; +use std::fs::{self, File}; +use std::io::Read; use cargotest::rustc_host; use cargotest::support::{project, execs, path2url}; @@ -255,6 +256,7 @@ fn doc_multiple_targets_same_name() { version = "0.1.0" [[bin]] name = "foo_lib" + path = "src/foo_lib.rs" "#) .file("foo/src/foo_lib.rs", "") .file("bar/Cargo.toml", r#" @@ -267,12 +269,17 @@ fn doc_multiple_targets_same_name() { .file("bar/src/lib.rs", "") .build(); + let root = path2url(p.root()); + assert_that(p.cargo("doc").arg("--all"), execs() - .with_status(101) - .with_stderr_contains("[..] target `foo_lib` [..]") - .with_stderr_contains("[..] binary by package `foo v0.1.0[..]`[..]") - .with_stderr_contains("[..] library by package `bar v0.1.0[..]` [..]")); + .with_status(0) + .with_stderr_contains(&format!("[DOCUMENTING] foo v0.1.0 ({}/foo)", root)) + .with_stderr_contains(&format!("[DOCUMENTING] bar v0.1.0 ({}/bar)", root)) + .with_stderr_contains("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]")); + assert_that(&p.root().join("target/doc"), existing_dir()); + let doc_file = p.root().join("target/doc/foo_lib/index.html"); + assert_that(&doc_file, existing_file()); } #[test] @@ -339,7 +346,7 @@ fn doc_multiple_targets_same_name_undoced() { } #[test] -fn doc_lib_bin_same_name() { +fn doc_lib_bin_same_name_documents_lib() { let p = project("foo") .file("Cargo.toml", r#" [package] @@ -347,15 +354,31 @@ fn doc_lib_bin_same_name() { version = "0.0.1" authors = [] "#) - .file("src/main.rs", "fn main() {}") - .file("src/lib.rs", "fn foo() {}") + .file("src/main.rs", r#" + //! Binary documentation + extern crate foo; + fn main() { + foo::foo(); + } + "#) + .file("src/lib.rs", r#" + //! Library documentation + pub fn foo() {} + "#) .build(); assert_that(p.cargo("doc"), - execs().with_status(101) - .with_stderr("\ -[ERROR] The target `foo` is specified as a library and as a binary by package \ -`foo [..]`. It can be documented[..]")); + execs().with_status(0).with_stderr(&format!("\ +[..] foo v0.0.1 ({dir}) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", dir = path2url(p.root())))); + assert_that(&p.root().join("target/doc"), existing_dir()); + let doc_file = p.root().join("target/doc/foo/index.html"); + assert_that(&doc_file, existing_file()); + let mut doc_html = String::new(); + File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap(); + assert!(doc_html.contains("Library")); + assert!(!doc_html.contains("Binary")); } #[test] @@ -503,7 +526,7 @@ fn output_not_captured() { if let CargoError(CargoErrorKind::ProcessErrorKind(perr), ..) = error { let output = perr.output.unwrap(); let stderr = str::from_utf8(&output.stderr).unwrap(); - + assert!(stderr.contains("☃"), "no snowman\n{}", stderr); assert!(stderr.contains("unknown start of token"), "no message{}", stderr); } else { diff --git a/tests/rustdoc.rs b/tests/rustdoc.rs index 371937ccb01..570ce6ad8fd 100644 --- a/tests/rustdoc.rs +++ b/tests/rustdoc.rs @@ -147,7 +147,7 @@ fn rustdoc_only_bar_dependency() { #[test] -fn rustdoc_same_name_err() { +fn rustdoc_same_name_documents_lib() { let p = project("foo") .file("Cargo.toml", r#" [package] @@ -164,7 +164,13 @@ fn rustdoc_same_name_err() { assert_that(p.cargo("rustdoc").arg("-v") .arg("--").arg("--cfg=foo"), execs() - .with_status(101) - .with_stderr("[ERROR] The target `foo` is specified as a \ -library and as a binary by package `foo [..]`. It can be documented[..]")); + .with_status(0) + .with_stderr(format!("\ +[DOCUMENTING] foo v0.0.1 ([..]) +[RUNNING] `rustdoc --crate-name foo src[/]lib.rs \ + -o {dir}[/]target[/]doc \ + --cfg=foo \ + -L dependency={dir}[/]target[/]debug[/]deps` +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", dir = p.root().display()))); } From e98b9dbec8342a552c93ca23332d949880d1ecc7 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 16 Oct 2017 16:05:47 -0400 Subject: [PATCH 2/2] Add more tests about documenting lib vs bins with the same name --- tests/doc.rs | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/tests/doc.rs b/tests/doc.rs index c61428befc5..32379c37bc8 100644 --- a/tests/doc.rs +++ b/tests/doc.rs @@ -369,7 +369,7 @@ fn doc_lib_bin_same_name_documents_lib() { assert_that(p.cargo("doc"), execs().with_status(0).with_stderr(&format!("\ -[..] foo v0.0.1 ({dir}) +[DOCUMENTING] foo v0.0.1 ({dir}) [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", dir = path2url(p.root())))); assert_that(&p.root().join("target/doc"), existing_dir()); @@ -381,6 +381,116 @@ fn doc_lib_bin_same_name_documents_lib() { assert!(!doc_html.contains("Binary")); } +#[test] +fn doc_lib_bin_same_name_documents_lib_when_requested() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + "#) + .file("src/main.rs", r#" + //! Binary documentation + extern crate foo; + fn main() { + foo::foo(); + } + "#) + .file("src/lib.rs", r#" + //! Library documentation + pub fn foo() {} + "#) + .build(); + + assert_that(p.cargo("doc").arg("--lib"), + execs().with_status(0).with_stderr(&format!("\ +[DOCUMENTING] foo v0.0.1 ({dir}) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", dir = path2url(p.root())))); + assert_that(&p.root().join("target/doc"), existing_dir()); + let doc_file = p.root().join("target/doc/foo/index.html"); + assert_that(&doc_file, existing_file()); + let mut doc_html = String::new(); + File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap(); + assert!(doc_html.contains("Library")); + assert!(!doc_html.contains("Binary")); +} + +#[test] +fn doc_lib_bin_same_name_documents_named_bin_when_requested() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + "#) + .file("src/main.rs", r#" + //! Binary documentation + extern crate foo; + fn main() { + foo::foo(); + } + "#) + .file("src/lib.rs", r#" + //! Library documentation + pub fn foo() {} + "#) + .build(); + + assert_that(p.cargo("doc").arg("--bin").arg("foo"), + execs().with_status(0).with_stderr(&format!("\ +[COMPILING] foo v0.0.1 ({dir}) +[DOCUMENTING] foo v0.0.1 ({dir}) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", dir = path2url(p.root())))); + assert_that(&p.root().join("target/doc"), existing_dir()); + let doc_file = p.root().join("target/doc/foo/index.html"); + assert_that(&doc_file, existing_file()); + let mut doc_html = String::new(); + File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap(); + assert!(!doc_html.contains("Library")); + assert!(doc_html.contains("Binary")); +} + +#[test] +fn doc_lib_bin_same_name_documents_bins_when_requested() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + "#) + .file("src/main.rs", r#" + //! Binary documentation + extern crate foo; + fn main() { + foo::foo(); + } + "#) + .file("src/lib.rs", r#" + //! Library documentation + pub fn foo() {} + "#) + .build(); + + assert_that(p.cargo("doc").arg("--bins"), + execs().with_status(0).with_stderr(&format!("\ +[COMPILING] foo v0.0.1 ({dir}) +[DOCUMENTING] foo v0.0.1 ({dir}) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", dir = path2url(p.root())))); + assert_that(&p.root().join("target/doc"), existing_dir()); + let doc_file = p.root().join("target/doc/foo/index.html"); + assert_that(&doc_file, existing_file()); + let mut doc_html = String::new(); + File::open(&doc_file).unwrap().read_to_string(&mut doc_html).unwrap(); + assert!(!doc_html.contains("Library")); + assert!(doc_html.contains("Binary")); +} + #[test] fn doc_dash_p() { let p = project("foo")