Cargo: Can't build ripgrep with cargo nightly

Created on 17 Jul 2017  路  7Comments  路  Source: rust-lang/cargo

Doing this on nightly:

$ git clone https://github.com/BurntSushi/ripgrep
$ cd ripgrep
$ cargo build

Gives you an error:

error: unable to get packages from source
Caused by:
  failed to parse manifest at `~/.cargo/registry/src/github.com-1ecc6299db9ec823/bytecount-0.1.6/Cargo.toml`

Caused by:
  can't find `bench` bench, specify bench.path

On beta everything is fine.

cc @BurntSushi @llogiq

Most helpful comment

Huh, turns out the actual cause is https://github.com/rust-lang/cargo/pull/3947 from April, and https://github.com/rust-lang/cargo/pull/4259 only exposed it.

In #3947 we've intentionally removed src/bench.rs from the set of conventions. In #4259 we actually start checking that if some target is present in Cargo.toml, then it actually exists on the hard drive. I wonder if this is the correct thing to do...

@alexcrichton, a question: suppose we have a target in Cargo.toml with an explicit or an implicit path, which does not exist. Should we bail with an error when we parse the manifest, or should we wait until we actually compile the target? The former is in general more user-friendly, but the latter behavior might sometimes be useful if, for example, the target is generated by build.rs or if you exclude the target from uploading on crates.io.

The current behavior is to evaluate explicit paths lazily, but bail early for implicit paths. That is, if you have [[bench]] name=foo and no benches/foo.rs, we error when parsing the manifest. If you add path = "some/path.rs", we'll wait until cargo bench to show an error.

All 7 comments

Using bisect-rust I managed to narrow down the regression to this commit range (rust PR https://github.com/rust-lang/rust/pull/43207):
eb6cf012a6cc23c9c89c4009564de9fccc38b9cb...f709c35a35687b2665eb290695825375be91410b

cc @matklad

Earlier rustcs looked for bench.rs in the src directory, current search in benches. Unfortunately I forgot to publish when I changed this, and I'm travelling currently.

When I arrive, I'll publish a new bytecount version. Sorry for the inconvenience.

Huh, turns out the actual cause is https://github.com/rust-lang/cargo/pull/3947 from April, and https://github.com/rust-lang/cargo/pull/4259 only exposed it.

In #3947 we've intentionally removed src/bench.rs from the set of conventions. In #4259 we actually start checking that if some target is present in Cargo.toml, then it actually exists on the hard drive. I wonder if this is the correct thing to do...

@alexcrichton, a question: suppose we have a target in Cargo.toml with an explicit or an implicit path, which does not exist. Should we bail with an error when we parse the manifest, or should we wait until we actually compile the target? The former is in general more user-friendly, but the latter behavior might sometimes be useful if, for example, the target is generated by build.rs or if you exclude the target from uploading on crates.io.

The current behavior is to evaluate explicit paths lazily, but bail early for implicit paths. That is, if you have [[bench]] name=foo and no benches/foo.rs, we error when parsing the manifest. If you add path = "some/path.rs", we'll wait until cargo bench to show an error.

So we need to fix it in Cargo somehow, because bytecount is present in dependencies of various projects, including ripgrep and xi-rope.

Option 1: allow "src/bench.rs" with a warning. This basically tonnes down #3947 (when accessing impact of that PR, I've search for [test] name = "test", but forgot to search for bench as well TT).

Option 2: restore the behavior of warning about missing targets only during compilation of said targets. This could be implemented by fallback to some default path, like benches/{bench-name}.rs. This is probably the easiest approach, but I don't feel super excited about it, because if we move forward with allowing benches/{bench-name}/main.rs as well, then it won't be obvious which default path we should pick.

Option 3: use the same trick for missing targets as we do for warnings: show error only if you are compiling the crate itself, and hide errors for dependencies.

I find the first option most compelling, because it's the simplest one, although it seems like a funny step backwards from #3947.

The #4288 PR implements the first option.

Thanks @matklad for fixing!

Thank you @est31 for bisecting and finding out who's to blame :) It made fixing the issue so much easier!

Was this page helpful?
0 / 5 - 0 ratings