search

budziq / rust-skeptic

Test your Rust Markdown documentation via Cargo
Apache License 2.0
286 stars 43 forks source link

multiple matching crates when deps are duplicated #18

Open colin-kiegel opened 7 years ago

colin-kiegel commented 7 years ago

I get a weird error when I use the combination of:

This looks like a skeptic (or cargo check) bug and it is dependent on the order of cargo check and cargo test.

error[E0464]: multiple matching crates for `cargo_check_bug`
 --> /tmp/rust-skeptic.U89a9gBDNE36/test.rs:1:1
  |
1 | extern crate cargo_check_bug;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: candidates:
  = note: path: /home/colin/projects/rust/cargo_check_bug/target/debug/deps/libcargo_check_bug-0b76af5b42a4f158.so
  = note: crate name: cargo_check_bug
  = note: crate name: cargo_check_bug

Steps to reprodroduce

  1. git clone https://github.com/colin-kiegel/cargo_check_bug.git
  2. cd cargo_check_bug
  3. rustup override set nightly-2017-03-03
  4. cargo clean && cargo test && cargo check
  5. cargo clean && cargo check && cargo test

Result of Step 4

Everything OK! 
   Compiling getopts v0.2.14
   Compiling libc v0.2.21
   Compiling bitflags v0.5.0
   Compiling rand v0.3.15
   Compiling pulldown-cmark v0.0.8
   Compiling tempdir v0.3.5
   Compiling skeptic v0.7.1
   Compiling cargo_check_bug v0.1.0 (file:///home/colin/projects/rust/cargo_check_bug)
    Finished dev [unoptimized + debuginfo] target(s) in 5.40 secs
     Running target/debug/deps/cargo_check_bug-bc124838e21e68e1

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

     Running target/debug/deps/skeptic-c3b458a20ebee641

running 1 test
warning: proc macro crates and `#[no_link]` crates have no effect without `#[macro_use]`
 --> /tmp/rust-skeptic.9IqFqNCvwCPZ/test.rs:1:1
  |
1 | extern crate cargo_check_bug;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

test readme_0 ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

   Doc-tests cargo_check_bug

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

   Compiling cargo_check_bug v0.1.0 (file:///home/colin/projects/rust/cargo_check_bug)
    Finished dev [unoptimized + debuginfo] target(s) in 0.12 secs

Result of Step 5

error[E0464]: multiple matching crates for `cargo_check_bug` 
   Compiling bitflags v0.5.0
   Compiling getopts v0.2.14
   Compiling libc v0.2.21
   Compiling rand v0.3.15
   Compiling pulldown-cmark v0.0.8
   Compiling tempdir v0.3.5
   Compiling skeptic v0.7.1
   Compiling cargo_check_bug v0.1.0 (file:///home/colin/projects/rust/cargo_check_bug)
    Finished dev [unoptimized + debuginfo] target(s) in 4.54 secs
   Compiling cargo_check_bug v0.1.0 (file:///home/colin/projects/rust/cargo_check_bug)
    Finished dev [unoptimized + debuginfo] target(s) in 0.97 secs
     Running target/debug/deps/cargo_check_bug-bc124838e21e68e1

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

     Running target/debug/deps/skeptic-c3b458a20ebee641

running 1 test
error[E0464]: multiple matching crates for `cargo_check_bug`
 --> /tmp/rust-skeptic.U89a9gBDNE36/test.rs:1:1
  |
1 | extern crate cargo_check_bug;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: candidates:
  = note: path: /home/colin/projects/rust/cargo_check_bug/target/debug/deps/libcargo_check_bug-0b76af5b42a4f158.so
  = note: crate name: cargo_check_bug
  = note: crate name: cargo_check_bug

error[E0463]: can't find crate for `cargo_check_bug`
 --> /tmp/rust-skeptic.U89a9gBDNE36/test.rs:1:1
  |
1 | extern crate cargo_check_bug;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to 2 previous errors

test readme_0 ... FAILED

failures:

---- readme_0 stdout ----
    thread 'readme_0' panicked at 'Command failed:
"rustc" "/tmp/rust-skeptic.U89a9gBDNE36/test.rs" "--verbose" "-o" "/tmp/rust-skeptic.U89a9gBDNE36/out.exe" "--crate-type=bin" "-L" "/home/colin/projects/rust/cargo_check_bug/target/debug" "-L" "/home/colin/projects/rust/cargo_check_bug/target/debug/deps" "--extern" "pulldown_cmark=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/libpulldown_cmark-ecfb67e72dcb6fb2.rlib" "--extern" "skeptic=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/libskeptic-74b6fcba1ef2d3dc.rlib" "--extern" "getopts=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/libgetopts-3facdbd0235704b0.rlib" "--extern" "tempdir=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/libtempdir-0ee757a585f719dd.rlib" "--extern" "libc=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/liblibc-5dc7b85e748840b4.rlib" "--extern" "bitflags=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/libbitflags-8510a47beebe00a6.rlib" "--extern" "rand=/home/colin/projects/rust/cargo_check_bug/target/debug/deps/librand-c9d9fbdab2355ee4.rlib"', /home/colin/.cargo/registry/src/github.com-1ecc6299db9ec823/skeptic-0.7.1/lib.rs:391
note: Run with `RUST_BACKTRACE=1` for a backtrace.

failures:
    readme_0

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured

error: test failed

Cargo.toml

[package]
name = "cargo_check_bug"
version = "0.1.0"
build = "build.rs"

[lib]
proc-macro = true

[dependencies]

[build-dependencies]
skeptic = "0.7"

[dev-dependencies]
skeptic = "0.7"

build.rs

extern crate skeptic;

fn main() {
    skeptic::generate_doc_tests(&["README.md"]);
}

README.md

```rust
extern crate cargo_check_bug;

fn main() {
}
```

tests/skeptic.rs

include!(concat!(env!("OUT_DIR"), "/skeptic-tests.rs"));

src/lib.rs

#![crate_type = "proc-macro"]
extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_derive(Foo)]
pub fn derive(_input: TokenStream) -> TokenStream {
    unimplemented!()
}
colin-kiegel commented 7 years ago

Do you think this might be a bug in cargo check or in rust-skeptic?

PS: I can't locate the file /tmp/rust-skeptic.ocUVTbTQ1DCk/test.rs:1:1 from the error message above. All I can find is this

./target/debug/build/cargo_check_bug-899f203356c46842/out/skeptic-tests.rs

extern crate skeptic;
#[test] fn readme_0() {
    let ref s = format!("{}", r####"extern crate cargo_check_bug;

fn main() {
}
"####);
    skeptic::rt::run_test(r#"/home/colin/projects/rust/cargo_check_bug/target/debug/build/cargo_check_bug-899f203356c46842/out"#, s);
}
colin-kiegel commented 7 years ago

ok, looks like something has changed between rust-skeptic v0.6.x and v0.7.1.

https://github.com/brson/rust-skeptic/compare/master@%7B2016-08-12%7D...master@%7B2017-02-26%7D

It looks like this commit https://github.com/brson/rust-skeptic/commit/205f426928408dcd6bebc719b376e11acbee0906 from @oli-obk fixed it for ordinary libs. But I can still reproduce this error if the library is a proc-macro!

Since an update of skeptic changed the error, I believe the remaining error is also related to skeptic (and not cargo check).

PS: I can confirm, that removing test/skeptic.rs eliminates the error - both in this toy crate and in derive_builder.

colin-kiegel commented 7 years ago

Any news on this?

It's a bit problematic that I get nonsense failing tests, if I activate skeptic tests (+cargo cache) on travis https://travis-ci.org/colin-kiegel/rust-derive-builder/jobs/219789090#L644

I'm already using skeptic 0.8.1.

The only workaround I know is to always run cargo clean before cargo test, but I would prefer to benefit from faster travis builds using its cargo cache feature.

brson commented 7 years ago

@colin-kiegel Thanks for the great report and investigation, and again sorry I haven't responded earlier.

This kind of sporadic crate resolution failure is because skeptic's tests aren't passing the --extern flags to rustc needed to disambiguate between multiple crate candidates, like

--extern skeptic_readme=/mnt2/dev/rust-skeptic/target/debug/deps/libskeptic_readme-77286c6085960cab.rlib

This happens when versions change and cargo leaves old deps around. Normally this doesn't matter because cargo properly passes --extern flags to everything it builds. Skeptic though runs rustc itself as part of cargo test, and doesn't actually know the correct values (cargo does not communicate these values to build scripts).

This is a pretty big limitation of skeptic presently, and the workaround is to cargo clean beforehand.

I'm surprised this looks like a regression to you, since I consider it a long-standing bug, put probably something about skeptic and/or cargo changed to exacerbate the problem, and there may be especially weird stuff going on with proc macros.

The best solution would be for cargo could to pass the names of various libraries to the build script, assuming it knows them that far ahead of time.

In the meantime, I think it would be possible for skeptic to maintain a cache of knowledge about the deps folder and use heuristics to make a pretty good guess about which libs are the most fresh.

brson commented 7 years ago

PS: I can't locate the file /tmp/rust-skeptic.ocUVTbTQ1DCk/test.rs:1:1 from the error message above. All I can find is this

This is a tempfile that is deleted during test execution. It would be nice if there were some way to tell skeptic not to delete them for debugging purposes.

colin-kiegel commented 7 years ago

Ok, it's already good to know that this is a known limitation and cargo clean is the only known workaround.

Hm, this would probably benefit a lot if cargo introduced some testing plugin slot, or some other query mechanism (maybe on nightly only). Compiletests seem to have similar problems. There it already starts with finding the right target directory, which can be a problem in workspaces.

I'm not familiar with cargo plugins. But could it help to introduce a cargo skeptic or cargo query subcommand? I could imagine that cargo plugins might already have some access to these kinds of information, no?

colin-kiegel commented 7 years ago

PS: I'm not sure how involved that is (or if it is even comparable), but cargo clippy seems to be doing a good job here. I never experienced these problems with clippy.

oli-obk commented 7 years ago

Cargo clippy forwards to cargo rustc and injects a custom compiler through the RUSTC env var. This way we avert these issues entirety. But it has the downside of overwriting the RUSTC env var and thus breaking builds that set it. This is an open issue in clippy

colin-kiegel commented 7 years ago

that sounds like this might work for skeptic, too. And I think skeptic wouldn't need to inject a custom compiler, right?

brson commented 7 years ago

I'm not familiar with cargo plugins. But could it help to introduce a cargo skeptic or cargo query subcommand? I could imagine that cargo plugins might already have some access to these kinds of information, no?

Unfortunately cargo plugins have no special information. They are just executables that cargo runs - there's no info passed between them.

brson commented 7 years ago

Cargo clippy forwards to cargo rustc and injects a custom compiler through the RUSTC env var. This way we avert these issues entirety. But it has the downside of overwriting the RUSTC env var and thus breaking builds that set it. This is an open issue in clippy

I can't quite imagine a way to fix this issue for skeptic by intercepting rustc itself, but if there is such a solution it may be worth pursuing.

brson commented 7 years ago

One plausible way to fix this would be to have skeptic synthesize an entire cargo project from the original cargo project, for every test it generates, sharing target directories. Then run cargo instead of rustc, and have cargo figure out the deps.

brson commented 7 years ago

I'm marking this help wanted, to try to implement the 'synthesize-a-cargo-project' solution I suggested previously. It doesn't seem all that complex to do.

budziq commented 7 years ago

@brson I have a very dirty POC implemented (cargo project synthesized and cargo build / run for each example) and it works in solving this particular issue but there are two problems:

I guess that another approach would be needed. I was wondering if there would be a way to generate separate test files for the project that would just share the original Cargo.toml and could be run concurrently with single cargo test, but I don't know if it would be ok to overwrite users tests dir or if it's possible to add another dir to cargo.toml that would be treated with the same semantics as the tests dir.

Edit: Writing separate test files (with contents of each playpen) to tests dir might work but the usage would be severely limited:

On the other hand:

budziq commented 7 years ago

I'm thinking about alternative approach. Parsing projects Cargo.lock to find all dependencies and their versions from "dependencies" section. Then match these with metadata from ./target/debug/.fingerprint/

like "local" -> "precalculated" section for lib-bitflags-40c9799a6d297c75.json

"local": {
    "Precalculated": "0.9.1"
},

And link tests only to the direct dependencies from Cargo.lock. In case there are duplicated rlibs for given semver (which happens in some cases when we do not do cargo clean) we would just chose the rlib with latest mtime.

Unfortunately I don't know how stable are the .fingerprint contents between cargo versions (I suspect there might be no guarantees). I'll try make a POC sometime this week.

brson commented 7 years ago

@budziq if that route is viable then it sounds awesome. I did not know it was possible to correlate specified dependencies with the resulting crate names.

epage commented 6 years ago

While it is more rare for me to it this, I still do from time to time.

For example, right now with assert_cli:

running 4 tests
error[E0464]: multiple matching crates for `assert_cli`
 --> /tmp/rust-skeptic.FmBU3mlPuvYC/test.rs:2:14
  |
2 | #[macro_use] extern crate assert_cli;
  |              ^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: candidates:
  = note: crate name: assert_cli
  = note: path: /home/epage/git/assert_cli/target/debug/deps/libassert_cli-d4ab2d90a07c5712.rlib
  = note: crate name: assert_cli

I confirmed in my Cargo.lock that I am using 0.13.2

budziq commented 6 years ago

Thanks @epage, I'll try to look into it later this weekend. Do you have some reliable way to reproduce it? I did not encounter this problem for quite a while.

epage commented 6 years ago

Looks like I'm reproducing it consistently with assert_cli.

If anyone familiar with skeptic is at RustBelt, they can catch me and we can look at it together.

Marwes commented 6 years ago

If anyone is having trouble with this issue and does not need compile-fail tests I create a fork with https://crates.io/crates/little-skeptic . (See #78 for a full explanation)

epage commented 6 years ago

Oops, forgot to past back here about docmatic, my lite version based on people's hacks here https://github.com/assert-rs/docmatic

budziq commented 6 years ago

Yeah I've seen both of your crates today. I'm sorry that you had to seek an alternative solution due to me being unable to commit more time to skeptic :/ Hope to free some bandwidth in the upcoming months ...

brson commented 5 years ago

I've started hacking on a fix for this that emits every test case as an individual cargo project, sharing the target directory with the "master" project.

brson commented 5 years ago

Another way to tackle this problem is to use cargo's unstable --build-plan flag, which looks like it contains enough metadata to find the correct libraries. I'm not pursuing that right now, because I think the every-test-case-a-cargo-project will be more reliable, not depending on skeptic to invoke rustc correctly.

budziq commented 5 years ago

Hi @brson nice to see you!

I've started hacking on a fix for this that emits every test case as an individual cargo project, sharing the target directory with the "master" project.

I've tried this approach way back, the problem was that it much slower and concurrent cargo instances working on the same target directory tended to congest due to internal file locking and the execution became sequential (with addition of the much longer cargo calls). It was especially painful on largeish testsets like cookbook. But the situation might have improved recently.

The --build-plan approach was what Alex Crichton suggested some time ago once it is stable.

Anyhow I'm game for any changes that improve the situation.

brson commented 5 years ago

Thanks for the feedback @budziq! I'm pretty far down this rabbit hole now, so I'll see for myself the problems you encountered.

I do have an idea I mentioned in #8 for combining every test case into a single binary such that cargo build only needs to be executed once for the entire test set. That would hopefully be much faster than running cargo build for every test.

brson commented 5 years ago

Ok, it's not in a mergable state yet, but here's a branch that overhauls how tests are run, using cargo to deal with resolution: https://github.com/budziq/rust-skeptic/compare/master...brson:next

I think this approach is much cleaner than the current, and will behave in more cases like would be expected.

It fixes this issue, but it slows down testing of rust-cookbook by about 11% (on WSL - results might be better on linux) re https://github.com/budziq/rust-skeptic/issues/8.

The performance of this approach can be improved still. I'll keep hacking at it.