Skip to content

Commit

Permalink
Auto merge of #6560 - ehuss:better-install-error, r=Eh2406
Browse files Browse the repository at this point in the history
Better error message for bad manifest with `cargo install`.

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.

First noticed at #5654 (comment)
  • Loading branch information
bors committed Jan 17, 2019
2 parents 10e3230 + 3378a5a commit ffe6587
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 ffe6587

Please sign in to comment.