Allow ui tests to have dependencies in a reliable way

[oli-obk]

This completely sidesteps the issue that compiletest-rs has where old artifacts of a dependency cause multiple available crates of name XXX errors. At this point I think we've reached feature parity for clippy, too, so I'm going to try publishing a version once this is merged.

[RalfJung]

Could you describe the design at a high level? That would help with reviewing. :)

[oli-obk]

1. There is a crate that we build once to generate all dependencies
2. we grab all the artifacts that were built for that crate
3. we link every test against all those artifacts

coincidentally this removes the implicit trick of using dev-dependencies from the main miri crate for tests via extern crate imports. the dev-deps scheme can require cargo clean when updating dependencies as you'd then get "multiple imports found" when there are two versions available

[RalfJung]

this removes the implicit trick of using dev-dependencies from the main miri crate for tests via extern crate imports

I never heard of this, no idea what you are talking about here.

[oli-obk]

I never heard of this, no idea what you are talking about here.

we have lots of tests that use extern crate libc; which just accidentally works by picking the libc from the sysroot. But with compiletest-rs you could have picked any dev-dependency and used that (clippy tests against usages of serde types and derives in its ui tests this way)

[RalfJung]

Oh lol, I had no idea. Like, taking the sysroot libc seems fine, but I didn't know the other thing would have worked. Good thing I didn't.^^

[bors]

☔ The latest upstream changes (presumably #2380) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

Do you want to review this? Or can I merge myself?

[RalfJung]

It's in my queue :)

[RalfJung]

Wow this is clever, I like it a lot. :)

[RalfJung]

We will probably want to move some tests from test-cargo-miri to here, in particular the getrandom tests. Those use the renaming feature of cargo:

getrandom_1 = { package = "getrandom", version = "0.1" }
getrandom_2 = { package = "getrandom", version = "0.2" }

Will that work?

[RalfJung]

Oh also please check that this works in the rustc workspace via x.py. The ./miri will probably not be the right call there...

[oli-obk]

Will that work?

I was afraid of this ^^ it doesn't work with my current scheme. The fix is obvious and I knew I should've done it this way, but 🤷

Basically what I need to do is to also run cargo metadata to get these renames and how they map to the artifacts

EDIT: works now correctly

[oli-obk]

Oh also please check that this works in the rustc workspace via x.py. The ./miri will probably not be the right call there...

even the latest version doesn't work from rustc yet, so this PR is not ready yet

[RalfJung]

even the latest version doesn't work from rustc yet, so this PR is not ready yet

@rustbot author

[oli-obk]

status update

I can reproduce the failure with x.py now: running

cargo run --manifest-path=cargo-miri/Cargo.toml -- miri run --manifest-path=test_dependencies/Cargo.toml

via the bootstrap machinery will suceed to build cargo-miri (that already worked for cargo miri setup), but fail when cargo-miri tries to build build scripts (or proc macros), as they can't find the libstd.

edit: manually adding --sysroot = build/...../stage1 makes it compile, now I gotta figure out why/where we're losing that

[oli-obk]

@rustbot ready

[RalfJung]

@rustbot author

[oli-obk]

@rustbot ready

[RalfJung]

r=me with run-test.py fixed and commits squashed

[oli-obk]

@bors r=RalfJung

[bors]

📌 Commit ab6fb9d has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit ab6fb9d with merge 1366bf6...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 1366bf6 to master...

[RalfJung]

Btw would be nice to also have a 'pass' test for #602 (comment).

Should we have these tests of non-stdlib crates in a subdir in our test folders? The root dirs are already quite messy, so something like {pass,fail}/crates or so might make sense?

[oli-obk]

yea grouping them into a custom folder makes sense.

[RalfJung]

#2408 does these changes.