Skip to content

Commit

Permalink
Better error message for bad manifest with cargo install.
Browse files Browse the repository at this point in the history
The old code assumed that any error loading a manifest meant that the
manifest didn't exist, but there are many other reasons it may fail.
Add a few helpful messages for some common cases.
  • Loading branch information
ehuss committed Jan 17, 2019
1 parent 513d230 commit 3378a5a
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 16 deletions.
28 changes: 22 additions & 6 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,30 @@ fn install_one(
)?
} else if source_id.is_path() {
let mut src = path_source(source_id, config)?;
src.update().chain_err(|| {
failure::format_err!(
"`{}` is not a crate root; specify a crate to \
install from crates.io, or use --path or --git to \
specify an alternate source",
if !src.path().is_dir() {
failure::bail!(
"`{}` is not a directory. \
--path must point to a directory containing a Cargo.toml file.",
src.path().display()
)
})?;
}
if !src.path().join("Cargo.toml").exists() {
if from_cwd {
failure::bail!(
"`{}` is not a crate root; specify a crate to \
install from crates.io, or use --path or --git to \
specify an alternate source",
src.path().display()
);
} else {
failure::bail!(
"`{}` does not contain a Cargo.toml file. \
--path must point to a directory containing a Cargo.toml file.",
src.path().display()
)
}
}
src.update()?;
select_pkg(src, krate, vers, config, false, &mut |path| {
path.read_packages()
})?
Expand Down
27 changes: 17 additions & 10 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,27 @@ fn bad_version() {
}

#[test]
fn no_crate() {
fn bad_paths() {
cargo_process("install")
.with_status(101)
.with_stderr(
"\
[ERROR] `[..]` is not a crate root; specify a crate to install [..]
.with_stderr("[ERROR] `[CWD]` is not a crate root; specify a crate to install [..]")
.run();

Caused by:
failed to read `[..]Cargo.toml`
cargo_process("install --path .")
.with_status(101)
.with_stderr("[ERROR] `[CWD]` does not contain a Cargo.toml file[..]")
.run();

Caused by:
[..] (os error [..])
",
)
let toml = paths::root().join("Cargo.toml");
fs::write(toml, "").unwrap();
cargo_process("install --path Cargo.toml")
.with_status(101)
.with_stderr("[ERROR] `[CWD]/Cargo.toml` is not a directory[..]")
.run();

cargo_process("install --path .")
.with_status(101)
.with_stderr_contains("[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`")
.run();
}

Expand Down
2 changes: 2 additions & 0 deletions tests/testsuite/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1249,6 +1249,8 @@ enum MatchKind {
/// - There is a wide range of macros (such as `[COMPILING]` or `[WARNING]`)
/// to match cargo's "status" output and allows you to ignore the alignment.
/// See `substitute_macros` for a complete list of macros.
/// - `[ROOT]` is `/` or `[..]:\` on Windows.
/// - `[CWD]` is the working directory of the process that was run.
pub fn lines_match(expected: &str, actual: &str) -> bool {
// Let's not deal with / vs \ (windows...)
// First replace backslash-escaped backslashes with forward slashes
Expand Down

0 comments on commit 3378a5a

Please sign in to comment.