Get the targets from cargo itself

This also adds a `no_build` flag, prints errors on stderr instead of
stdout, and no longer hides errors from cargo.

CHANGELOG.md

@@ -16,6 +16,11 @@
 * `--debug` and `--verbose` are deprecated in favor of `RUST_LOG`. [PR#100]
 * Published Linux binaries are now built against musl libc, not glibc. This allows running deadlinks in an alpine docker container. [PR#103]
 
+#### Fixes
+
+* `doc = false` is now taken into account when running `cargo deadlinks`. It will still be ignored when running with `--no-build`. [PR#102]
+* `CARGO_BUILD_TARGET` and other cargo configuration is now taken into account when running `cargo deadlinks`. It will still be ignored when running with `--no-build`. [PR#102]
+
 [PR#87]: https://github.com/deadlinks/cargo-deadlinks/pull/87
 [PR#100]: https://github.com/deadlinks/cargo-deadlinks/pull/100
 [PR#101]: https://github.com/deadlinks/cargo-deadlinks/pull/101

Cargo.toml

@@ -23,7 +23,7 @@ default = ["cargo"]
 
 [dependencies]
 cached = { version = "0.20.0", default-features = false }
-cargo_metadata = { version = "0.9", optional = true }
+cargo_metadata = { version = "0.12", optional = true }
 serde_json = { version = "1.0.34", optional = true }
 docopt = "1"
 env_logger = "0.8"

src/bin/cargo-deadlinks.rs

@@ -1,8 +1,9 @@
 use std::env;
+use std::io::BufReader;
 use std::path::PathBuf;
 use std::process::{self, Command};
 
-use cargo_metadata::MetadataCommand;
+use cargo_metadata::{Message, MetadataCommand};
 use docopt::Docopt;
 use serde_derive::Deserialize;
 
@@ -61,15 +62,19 @@ fn main() {
 
     let ctx = CheckContext::from(&args);
     let mut errors = false;
-    for dir in dirs {
+    for dir in &dirs {
         let dir = match dir.canonicalize() {
             Ok(dir) => dir,
             Err(_) => {
-                println!("Could not find directory {:?}.", dir);
+                eprintln!("error: could not find directory {:?}.", dir);
                 if args.arg_directory.is_none() {
-                    println!();
-                    println!("deadlinks incorrectly guessed the target directory for the documentation.\
-                        This is a bug in deadlinks; please open an issue at https://github.com/deadlinks/cargo-deadlinks/issues/new/.");
+                    assert!(
+                        args.flag_no_build,
+                        "cargo said it built a directory it didn't build"
+                    );
+                    eprintln!(
+                        "help: consider removing `--no-build`, or running `cargo doc` yourself."
+                    );
                 }
                 process::exit(1);
             }
@@ -81,49 +86,88 @@ fn main() {
     }
     if errors {
         process::exit(1);
+    } else if dirs.is_empty() {
+        assert!(args.arg_directory.is_none());
+        eprintln!("warning: no directories were detected");
     }
 }
 
-/// Returns the directory to use as root of the documentation.
+/// Returns the directories to use as root of the documentation.
 ///
 /// If an directory has been provided as CLI argument that one is used.
-/// Otherwise we try to find the `Cargo.toml` and construct the documentation path
-/// from the package name found there.
+/// Otherwise, if `no_build` is passed, we try to find the `Cargo.toml` and
+/// construct the documentation path from the package name found there.
+/// Otherwise, build the documentation and have cargo itself tell us where it is.
 ///
 /// All *.html files under the root directory will be checked.
 fn determine_dir(no_build: bool) -> Vec<PathBuf> {
-    let manifest = MetadataCommand::new()
-        .no_deps()
-        .exec()
-        .unwrap_or_else(|err| {
-            println!("error: {}", err);
-            println!("help: if this is not a cargo directory, use `--dir`");
-            process::exit(1);
-        });
-    let doc = manifest.target_directory.join("doc");
-
-    // originally written with this impressively bad jq query:
-    // `.packages[] |select(.source == null) | .targets[] | select(.kind[] | contains("test") | not) | .name`
-    let paths = manifest
-        .packages
-        .into_iter()
-        .filter(|package| package.source.is_none())
-        .map(|package| package.targets)
-        .flatten()
-        .filter(has_docs)
-        .map(|target| doc.join(target.name.replace('-', "_")))
-        .collect();
+    if no_build {
+        eprintln!("warning: --no-build ignores `doc = false` and may have other bugs");
+        let manifest = MetadataCommand::new()
+            .no_deps()
+            .exec()
+            .unwrap_or_else(|err| {
+                println!("error: {}", err);
+                println!("help: if this is not a cargo directory, use `--dir`");
+                process::exit(1);
+            });
+        let doc = manifest.target_directory.join("doc");
+
+        // originally written with this impressively bad jq query:
+        // `.packages[] |select(.source == null) | .targets[] | select(.kind[] | contains("test") | not) | .name`
+        let iter = manifest
+            .packages
+            .into_iter()
+            .filter(|package| package.source.is_none())
+            .map(|package| package.targets)
+            .flatten()
+            .filter(has_docs)
+            .map(move |target| doc.join(target.name.replace('-', "_")));
+        return iter.collect();
+    }
 
-    // Finally, build the documentation.
+    // Build the documentation, collecting info about the build at the same time.
     log::info!("building documentation using cargo");
-    let cargo = env::var("CARGO").expect(
-        "`cargo-deadlinks` must be run as either `cargo deadlinks` or with the `--dir` flag",
-    );
-    if !no_build && !Command::new(cargo).arg("doc").status().unwrap().success() {
-        process::exit(2);
-    } else {
-        paths
+    let cargo = env::var("CARGO").unwrap_or_else(|_| {
+        println!("error: `cargo-deadlinks` must be run as either `cargo deadlinks` or with the `--dir` flag");
+        process::exit(1);
+    });
+    // Stolen from https://docs.rs/cargo_metadata/0.12.0/cargo_metadata/#examples
+    let mut cargo_process = Command::new(cargo)
+        .args(&["doc", "--no-deps", "--message-format", "json"])
+        .stdout(process::Stdio::piped())
+        // spawn instead of output() allows running deadlinks and cargo in parallel;
+        // this is helpful when you have many dependencies that take a while to document
+        .spawn()
+        .unwrap();
+    let reader = BufReader::new(cargo_process.stdout.take().unwrap());
+    // Originally written with jq:
+    // `select(.reason == "compiler-artifact") | .filenames[] | select(endswith("/index.html")) | rtrimstr("/index.html")`
+    let directories = Message::parse_stream(reader)
+        .filter_map(|message| match message {
+            Ok(Message::CompilerArtifact(artifact)) => Some(artifact.filenames),
+            _ => None,
+        })
+        .flatten()
+        .filter_map(|dir| {
+            dir.to_str()
+                .expect("non UTF-8 paths are not supported when building documentation with Cargo")
+                .strip_suffix("/index.html")
+                // needed in order to keep a streaming iterator
+                .map(|s| s.to_owned())
+        })
+        .map(|s| s.into())
+        // TODO: run this in parallel, which should speed up builds a fair bit.
+        // This will be hard because either cargo's progress bar will overlap with our output,
+        // or we'll have to recreate the progress bar somehow.
+        // See https://discord.com/channels/273534239310479360/335502067432947748/778636447154044948 for discussion.
+        .collect();
+    let status = cargo_process.wait().unwrap();
+    if !status.success() {
+        eprintln!("help: if this is not a cargo directory, use `--dir`");
+        process::exit(status.code().unwrap_or(2));
     }
+    directories
 }
 
 fn has_docs(target: &cargo_metadata::Target) -> bool {

tests/simple_project.rs

@@ -56,7 +56,7 @@ mod simple_project {
             .current_dir(env::temp_dir())
             .assert()
             .failure()
-            .stdout(
+            .stderr(
                 contains("help: if this is not a cargo directory, use `--dir`")
                     .and(contains("error: could not find `Cargo.toml`")),
             );
@@ -79,12 +79,11 @@ mod simple_project {
         assert_doc("./tests/simple_project", &[("CARGO_TARGET_DIR", "target2")]).success();
 
         remove_all("./tests/simple_project/target");
-        // This currently breaks due to a cargo bug: https://github.com/rust-lang/cargo/issues/8791
         assert_doc(
             "./tests/simple_project",
             &[("CARGO_BUILD_TARGET", "x86_64-unknown-linux-gnu")],
         )
-        .failure();
+        .success();
 
         // fn it_shortens_path_on_error
         remove_all("./tests/simple_project/target");