[WIP] Move compiletest out of tree

[djrenren]

This commit attempts to move compiletest to an external dependency. This was recommended as a first step to externalizing libtest itself.

Currently everything is in personal git repos until we can get it all aligned and working.

The goal is to depend on an out-of-tree compiletest crate that depends on an externalized libtest. I'm currently hosting these under djrenren/compiletest and djrenren/libtest, but these will need to be migrated to official locations once everything is up and running.

It appears to be mostly working with a few spurious test failures on my machine that seem unrelated. I've created the PR so we can run bors and such.

Steps before merging

• Pass all appropriate tests
• Move libtest and compiletest into official locations
• Depend on them rather than git URLs.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[rust-highfive]

⚠️ Warning ⚠️

• These commits modify submodules.

[rust-highfive]

The job mingw-check of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2019-08-27T00:41:26.6101642Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-08-27T00:41:26.6335981Z ##[command]git config gc.auto 0
2019-08-27T00:41:26.6416524Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-08-27T00:41:26.6483297Z ##[command]git config --get-all http.proxy
2019-08-27T00:41:26.6633920Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63929/merge:refs/remotes/pull/63929/merge
---
2019-08-27T00:42:03.1190607Z do so (now or later) by using -b with the checkout command again. Example:
2019-08-27T00:42:03.1190636Z 
2019-08-27T00:42:03.1190828Z   git checkout -b <new-branch-name>
2019-08-27T00:42:03.1190857Z 
2019-08-27T00:42:03.1191112Z HEAD is now at e0c087ea9 Merge bd4b6712181b49c5110208d1a5c4378e2788fd95 into 9b91b9c10e3c87ed333a1e34c4f46ed68f1eee06
2019-08-27T00:42:03.1354258Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-08-27T00:42:03.1359004Z ==============================================================================
2019-08-27T00:42:03.1359078Z Task         : Bash
2019-08-27T00:42:03.1359144Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-08-27T00:45:33.4439117Z ##################################################                        69.5%
2019-08-27T00:45:33.4439254Z ######################################################################## 100.0%
2019-08-27T00:45:33.8680762Z extracting /checkout/obj/build/cache/2019-08-13/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
2019-08-27T00:45:33.9511740Z     Updating crates.io index
2019-08-27T00:46:07.9760438Z     Updating git repository `https://github.com/djrenren/libtest.git`
2019-08-27T00:46:08.7939251Z error: failed to write /checkout/Cargo.lock
2019-08-27T00:46:08.7943324Z Caused by:
2019-08-27T00:46:08.7943374Z   failed to open: /checkout/Cargo.lock
2019-08-27T00:46:08.7943405Z 
2019-08-27T00:46:08.7944038Z Caused by:
2019-08-27T00:46:08.7944038Z Caused by:
2019-08-27T00:46:08.7944814Z   Read-only file system (os error 30)
2019-08-27T00:46:08.7984410Z failed to run: /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo build --manifest-path /checkout/src/bootstrap/Cargo.toml
2019-08-27T00:46:08.8038831Z == clock drift check ==
2019-08-27T00:46:08.8048520Z   local time: Tue Aug 27 00:46:08 UTC 2019
2019-08-27T00:46:08.9522107Z   network time: Tue, 27 Aug 2019 00:46:08 GMT
2019-08-27T00:46:08.9526209Z == end clock drift check ==
2019-08-27T00:46:08.9526209Z == end clock drift check ==
2019-08-27T00:46:10.1458443Z ##[error]Bash exited with code '1'.
2019-08-27T00:46:10.1491669Z ##[section]Starting: Checkout
2019-08-27T00:46:10.1496496Z ==============================================================================
2019-08-27T00:46:10.1496578Z Task         : Get sources
2019-08-27T00:46:10.1496621Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[Mark-Simulacrum]

I personally feel like moving compiletest out of the tree isn't a great idea at this point -- it's pretty frequently changed or altered in concert with changes to our tests, and while in theory it could be "just a dependency" or similar it's also tied very closely to bootstrap via all of its arguments.

I'm also not entirely sure that there are enough wins from moving it out of tree: if the goal here is to decouple it from in-tree libtest, that's great, but should be orthogonal to moving it out of tree?

Overall I'd like to see some justification for why we're taking this route -- it feels like there should be at least a link to some explanation comment (or one here), but I wasn't able to find one (admittedly, somewhat quickly).

[djrenren]

This was first discussed as part of removing libtest:

#59775 (comment)

Ultimately I'm much more interested in getting libtest moved out, so if there's a better way I'm all for it

[Centril]

At minimum, I think if we move compiletest out of tree then it

• needs to be under control of the compiler team,
• must accomodate breaking changes whenever rust-lang/rust needs it
  • and I think crates on crates.io would not like that.

See https://rust-lang.github.io/compiler-team/procedures/crates/ for the compiler team's policy re. out of tree crates.

[Mark-Simulacrum]

I see no justification in #63929 (comment) beyond "why not" -- could you clarify if there's some inherent issue I'm missing? I would be surprised if compiletest's dependency on in-tree libtest caused any problems with moving to an external copy of libtest; if it does, we can temporarily keep two versions around or some similar situation -- I'm willing to put in legwork in bootstrap to make things work, if it'd be helpful.

So far, it seems like there's multiple downsides to moving compiletest out and little to no benefit.

[djrenren]

Sounds reasonable. I'll close for now then. And take a crack at just doing the external libtest thing.

Personally, I think compiletest is so painfully easy to externalize that it should be, but a PR probably isn't the place to discuss that.

[Mark-Simulacrum]

There's a huge difference between being easy to externalize and that being a good idea -- compiletest is too tightly coupled to the tests in this repository for that to be feasible. But I agree that such discussion is best suited for internals or Zulip, perhaps.

Let me know if I can help with libtest extraction in terms of bootstrap work.

[Manishearth]

I would be surprised if compiletest's dependency on in-tree libtest caused any problems with moving to an external copy of libtest;

This already happened. There's a history here you're missing, clippy uses (its own out of tree) compiletest which uses unstable features, and the whole thing comes crashing down if libtest is moved out without rustc's compiletest. Out of tree compiletest is something others use too, and it's tricky to keep maintaining it on stable and nightly simultaneously for clippy and for other consumers. The nightly dependency is necessary for features that require libtest, but when we tried moving libtest out of tree everything broke.

Moving compiletest (and its libtest dependency) out first reduces the largest potential source of conflicts here. Plus, people actually use the existing out of tree compiletest, and having a single official compiletest (as opposed to a somewhat-mirrored somewhat-broken compiletest clone) would be much nicer for all.

[gnzlbg]

I'm also not entirely sure that there are enough wins from moving it out of tree: if the goal here is to decouple it from in-tree libtest, that's great, but should be orthogonal to moving it out of tree?

FWIW the goal here is to share the exact same compiletest version between rust-lang/rust, and other rust-lang projects like clippy.

An alternative would be to leave the source code in this repo, publish it to crates.io, and have clippy and rust bootstrap pull it from crates.io.

[Mark-Simulacrum]

So I've taken a brief look at the build failures in the PR which moved libtest out of tree and they seem ... unexpected to me? That is, it looks like unstable surface area changed in that PR which isn't what I'd expect at all. Maybe we were trying to do too many things at once?

Basically it is my opinion that if we're causing downstream breakage for clippy, we're probably causing it for many libtest-using crates and then the moving out bit is probably doing something not quite right. It should be possible to change where the source code of libtest is without affecting anyone.

[Mark-Simulacrum]

FWIW the goal here is to share the exact same compiletest version between rust-lang/rust, and other rust-lang projects like clippy.

That's ... a bit of an odd goal IMO? It's my opinion that there certainly exists space for something simpler than compiletest in the ecosystem (perhaps just targeting UI tests) but our version of compiletest is probably pretty clunky and such. Bits and pieces of it are of course fine to publish for reuse but the whole crate does a lot of rustc-specific magic that would feel pretty odd as a crates.io crate at least to me.

[gnzlbg]

That's ... a bit of an odd goal IMO? It's my opinion that there certainly exists space for something simpler than compiletest in the ecosystem (perhaps just targeting UI tests) but our version of compiletest is probably pretty clunky and such.

Maybe? What compiler plugins like clippy currently use is the compiletest-rs crate, which is a fork of rust-lang/rust compiletest . There might be a better solution to their problem.

---

So I've taken a brief look at the build failures in the PR which moved libtest out of tree and they seem ... unexpected to me? That is, it looks like unstable surface area changed in that PR which isn't what I'd expect at all.

Which PR did you look at? IIRC the PR that extracted libtest worked fine and passed all tests. What didn't work was the one tried to update the libtest dependency to a different libtest version and ran into linker issues.

[gnzlbg]

Basically it is my opinion that if we're causing downstream breakage for clippy, we're probably causing it for many libtest-using crates and then the moving out bit is probably doing something not quite right.

I think some crates started to use libtest 0.0.0 from crates.io which also caused them some issues, or issues mixing the libtest from crates.io with the libtest from another rust toolchain. Can't remember.

It should be possible to change where the source code of libtest is without affecting anyone.

Sure, this can be added by just moving the code as is to a repo, and using a submodule. But the intent was for rust itself to pull in a libtest crate from crates.io. And that interacted badly with how Rust build process treats libtest specially. This changed a lot recently, from a 3 step to a 2 step process, so maybe the linking issues that appeared back then do not appear now.

[Mark-Simulacrum]

Basically I think if this PR moves us closer to some goal people want, then we can proceed with it -- I just feel like this should be easier than this PR (i.e., we should be able to skip changes to compiletest as part of moving libtest). If that's not the case for whatever reason then fine, and we can reopen, but those reasons would ideally be documented in the PR (at least a link to relevant build logs would be great!)

[RalfJung]

Miri is also using compiletest-rs, and the fact that it lacks some features compared to rustc's compiletest is causing pain on a regular basis. On other occasions we'd also like to be able to customize things more, e.g. using a different rustc for aux builds vs the final build (only the final build should use Miri).

Maybe we should switch to trybuild entirely, though I am not sure if that is feasible.

[Centril]

Possibly crazy idea: move Miri in-tree?

[RalfJung]

That's not really the direction we have been taking with tools lately. And Miri is just the second example listed here, clippy being another.

[Centril]

@RalfJung The reason Clippy isn't being moved in tree is that @Manishearth wants to keep it out-of-tree for better on-boarding through shorter build times. Other than that, I believe the bors queue is equipped to take on Clippy. How is the situation re. on-boarding re. Miri? Would moving Miri in-tree be detrimental to on-boarding efforts?

[RalfJung]

How is the situation re. on-boarding re. Miri? Would moving Miri in-tree be detrimental to on-boarding efforts?

TBH I have no idea. We don't exactly have many contributors.

But speaking just for myself, I am quite happy every time hacking on Miri does not involve having to build rustc.

[gnzlbg]

But speaking just for myself, I am quite happy every time hacking on Miri does not involve having to build rustc.

Is there anybody working on a component that's out of tree unhappy with that? 😄