detect more failure cases

src/cmd/mod.rs

@@ -214,7 +214,7 @@ pub struct Command<'w, 'pl> {
     source_dir_mount_kind: MountKind,
 }
 
-impl<'w, 'pl> Command<'w, 'pl> {
+impl<'w> Command<'w, '_> {
     /// Create a new, unsandboxed command.
     pub fn new<R: Runnable>(workspace: &'w Workspace, binary: R) -> Self {
         binary.prepare_command(Self::new_inner(binary.name(), Some(workspace), None))
@@ -331,9 +331,14 @@ impl<'w, 'pl> Command<'w, 'pl> {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn process_lines(mut self, f: &'pl mut dyn FnMut(&str, &mut ProcessLinesActions)) -> Self {
-        self.process_lines = Some(f);
-        self
+    pub fn process_lines<'pl>(
+        self,
+        f: &'pl mut dyn FnMut(&str, &mut ProcessLinesActions),
+    ) -> Command<'w, 'pl> {
+        Command {
+            process_lines: Some(f),
+            ..self
+        }
     }
 
     /// Enable or disable logging all the output lines to the [`log` crate][log]. By default

src/prepare.rs

@@ -1,4 +1,4 @@
-use crate::cmd::{Command, CommandError};
+use crate::cmd::{Command, CommandError, ProcessLinesActions};
 use crate::{build::CratePatch, Crate, Toolchain, Workspace};
 use anyhow::Context as _;
 use log::info;
@@ -101,8 +101,6 @@ impl<'a> Prepare<'a> {
             return Ok(());
         }
 
-        let mut yanked_deps = false;
-        let mut missing_deps = false;
         let mut cmd = Command::new(self.workspace, self.toolchain.cargo()).args(&[
             "generate-lockfile",
             "--manifest-path",
@@ -114,28 +112,7 @@ impl<'a> Prepare<'a> {
                 .env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "nightly");
         }
 
-        match cmd
-            .cd(self.source_dir)
-            .process_lines(&mut |line, _| {
-                if line.contains("failed to select a version for the requirement") {
-                    yanked_deps = true;
-                } else if line.contains("failed to load source for dependency")
-                    || line.contains("no matching package named")
-                {
-                    missing_deps = true;
-                }
-            })
-            .run_capture()
-        {
-            Ok(_) => Ok(()),
-            Err(CommandError::ExecutionFailed { status: _, stderr }) if yanked_deps => {
-                Err(PrepareError::YankedDependencies(stderr).into())
-            }
-            Err(CommandError::ExecutionFailed { status: _, stderr }) if missing_deps => {
-                Err(PrepareError::MissingDependencies(stderr).into())
-            }
-            Err(err) => Err(err.into()),
-        }
+        run_command(cmd.cd(self.source_dir))
     }
 
     fn fetch_deps(&mut self) -> anyhow::Result<()> {
@@ -149,7 +126,6 @@ pub(crate) fn fetch_deps(
     source_dir: &Path,
     fetch_build_std_targets: &[&str],
 ) -> anyhow::Result<()> {
-    let mut missing_deps = false;
     let mut cmd = Command::new(workspace, toolchain.cargo())
         .args(&["fetch", "--manifest-path", "Cargo.toml"])
         .cd(source_dir);
@@ -163,18 +139,60 @@ pub(crate) fn fetch_deps(
         cmd = cmd.args(&["--target", target]);
     }
 
-    match cmd
-        .process_lines(&mut |line, _| {
-            if line.contains("failed to load source for dependency") {
-                missing_deps = true;
-            }
-        })
-        .run_capture()
-    {
+    run_command(cmd)
+}
+
+fn run_command(cmd: Command) -> anyhow::Result<()> {
+    let mut yanked_deps = false;
+    let mut missing_deps = false;
+    let mut broken_deps = false;
+    let mut broken_lockfile = false;
+
+    let mut process = |line: &str, _: &mut ProcessLinesActions| {
+        if line.contains("failed to select a version for the requirement") {
+            yanked_deps = true;
+        } else if line.contains("failed to load source for dependency")
+            || line.contains("no matching package named")
+            || line.contains("no matching package found")
+            || line.contains("no matching package for override ")
+            || (line.contains("The patch location ")
+                && line.contains(" does not appear to contain any packages matching the name "))
+        {
+            missing_deps = true;
+        } else if line.contains("failed to parse manifest at")
+            || line.contains("error: invalid table header")
+            || line.contains("error: invalid type: ")
+            || line.contains("error: cyclic feature dependency: feature ")
+            || line.contains("error: cyclic package dependency: package ")
+            || (line.contains("error: package collision in the lockfile: packages ")
+                && line.contains(
+                    " are different, but only one can be written to lockfile unambiguously",
+                ))
+        {
+            broken_deps = true;
+        } else if line.contains("error: failed to parse lock file at")
+            || line.contains(
+                "error: Attempting to resolve a dependency with more than one crate with links=",
+            )
+        {
+            broken_lockfile = true;
+        }
+    };
+
+    match cmd.process_lines(&mut process).run_capture() {
         Ok(_) => Ok(()),
+        Err(CommandError::ExecutionFailed { status: _, stderr }) if yanked_deps => {
+            Err(PrepareError::YankedDependencies(stderr).into())
+        }
         Err(CommandError::ExecutionFailed { status: _, stderr }) if missing_deps => {
             Err(PrepareError::MissingDependencies(stderr).into())
         }
+        Err(CommandError::ExecutionFailed { status: _, stderr }) if broken_deps => {
+            Err(PrepareError::BrokenDependencies(stderr).into())
+        }
+        Err(CommandError::ExecutionFailed { status: _, stderr }) if broken_lockfile => {
+            Err(PrepareError::InvalidCargoLock(stderr).into())
+        }
         Err(err) => Err(err.into()),
     }
 }
@@ -384,12 +402,18 @@ pub enum PrepareError {
     /// rejecting it.
     #[error("invalid Cargo.toml syntax")]
     InvalidCargoTomlSyntax,
+    /// Something about the crates dependencies is invalid
+    #[error("broken dependencies: \n\n{0}")]
+    BrokenDependencies(String),
     /// Some of this crate's dependencies were yanked, preventing Crater from fetching them.
     #[error("the crate depends on yanked dependencies: \n\n{0}")]
     YankedDependencies(String),
     /// Some of the dependencies do not exist anymore.
     #[error("the crate depends on missing dependencies: \n\n{0}")]
     MissingDependencies(String),
+    /// cargo rejected (generating) the lockfile
+    #[error("the crate has a broken lockfile: \n\n{0}")]
+    InvalidCargoLock(String),
     /// Uncategorized error
     #[doc(hidden)]
     #[error("uncategorized prepare error")]

tests/buildtest/mod.rs

@@ -261,42 +261,42 @@ test_prepare_error_stderr!(
     "error: no matching package named `macro` found"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_content_deps,
     "invalid-cargotoml-content-deps",
     BrokenDependencies,
     "failed to parse the version requirement `0.11\t` for dependency `parking_lot`"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_syntax_deps,
     "invalid-cargotoml-syntax-deps",
     BrokenDependencies,
     "error: invalid table header"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_lockfile_syntax,
     "invalid-lockfile-syntax",
     InvalidCargoLock,
     "error: failed to parse lock file at"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_missing_deps_typo,
     "missing-deps-typo",
     MissingDependencies,
     "error: no matching package found"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_cyclic_feature,
     "invalid-cargotoml-cyclic-feature",
     BrokenDependencies,
     "error: cyclic feature dependency: feature"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_cyclic_package,
     "invalid-cargotoml-cyclic-package",
     BrokenDependencies,
@@ -309,42 +309,42 @@ test_prepare_error!(
     InvalidCargoTomlSyntax
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_missing_override,
     "invalid-cargotoml-missing-override",
     MissingDependencies,
     "no matching package for override `https://github.com/rust-lang/crates.io-index#[email protected]` found"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_missing_deps_registry_version,
     "missing-deps-registry-version",
-    MissingDependencies,
+    YankedDependencies,
     "error: failed to select a version for the requirement `empty-library = \"=0.5.0\"`"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_content_type_in_deps,
     "invalid-cargotoml-content-type-in-deps",
     BrokenDependencies,
     "error: invalid type: map, expected a string"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_conflicting_links,
     "invalid-cargotoml-conflicting-links",
     InvalidCargoLock,
     "error: Attempting to resolve a dependency with more than one crate with links=ring-asm"
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_lockfile_collision,
     "lockfile-collision",
     BrokenDependencies,
     "error: package collision in the lockfile: packages lockfile-collision v0.1.0 "
 );
 
-test_prepare_uncategorized_err!(
+test_prepare_error_stderr!(
     test_invalid_cargotoml_missing_patch,
     "invalid-cargotoml-missing-patch",
     MissingDependencies,

tests/buildtest/runner.rs

@@ -96,8 +96,6 @@ macro_rules! test_prepare_error {
     };
 }
 
-pub(crate) use test_prepare_error;
-
 macro_rules! test_prepare_error_stderr {
     ($name:ident, $krate:expr, $expected:ident, $expected_output:expr) => {
         #[test]
@@ -130,9 +128,3 @@ macro_rules! test_prepare_error_stderr {
         }
     };
 }
-
-macro_rules! test_prepare_uncategorized_err {
-    ($name:ident, $krate:expr, $expected:ident $(,$expected_output:expr)?) => {
-        $crate::buildtest::runner::test_prepare_error!($name, $krate, Uncategorized);
-    };
-}