Move compiletest out of tree

[Manishearth]

See also: Manishearth/compiletest-rs#238

cc @rust-lang/compiler @rust-lang/clippy @rust-lang/devtools

Currently compiletest is used by Rust in two different forms: It's used by rustc itself in rust-lang/rust as a testsuite, and it's also used by clippy via https://github.com/Manishearth/compiletest-rs/ . Compiletest-rs is used by both clippy and some proc macro libraries (like syn) for testing diagnostics.

This is suboptimal. The clippy team already primarily maintains this fork (and recently the repo owner transferred it to my account). We've been considering moving it into rust-lang, but it feels super weird for Rust to maintain multiple compiletest forks.

It would be nice if compiletest-rs could be moved out of tree. A blocker to doing this is moving libtest out of tree -- this was tried before and had problems -- but I think we might be able to move the relevant parts of libtest out of tree since compiletest really only uses the parts of libtest that handle output formatting.

I propose the following steps:

• Identify the "test output formatting" parts of libtest, put them in a separate crate, make libtest and compiletest depend on them
• Identify features in compiletest-rs that compiletest does not have (it will likely be very minor stuff -- compiletest-rs largely does not add new features), try to add them
• Move compiletest out of tree into a temporary branch on compiletest-rs
• Try to make clippy use this temporary test branch, make sure it works. Upstream any necessary fixes.
• Try to make syn/serde use this temporary test branch, make sure it works. Upstream any necessary fixes.
• Release this as a major version bump of compiletest-rs
• Update clippy, rustc, and syn to use this new version

I feel like this will be a win-win for everyone. The resultant repo will be able to get the maintenance efforts of the clippy team and the compiler team, and will also get polish from community crates like syn slowly turning it into a general purpose tool.

[Mark-Simulacrum]

Hm, so the worry I've had in the past is that compiletest does.. a lot of fairly ad-hoc things that are pretty specific to the rustc tooling (e.g., LLVM version flags, support for MIR-opt diffing, etc.) -- and some of that is nice to add in PRs to rust-lang/rust alongside other changes, rather than needing to go through the version bump dance.

And I'm not sure we'd want to maintain the public API surface of compiletest as it stands today in-tree -- currently, we don't care about breaking changes, but if we were to maintain it we'd need to care.

The trybuild crate (https://github.com/dtolnay/trybuild) is one option for out-of-tree users that want a more limited set of functionality -- maybe it would be sufficient to use that? We could also attempt to split out compiletest into a "core" crate that handles things like output formats, etc., but then that is a dependency of the in-tree one that handles parsing our custom ignore rules and such.

[Manishearth]

Hm, so the worry I've had in the past is that compiletest does.. a lot of fairly ad-hoc things that are pretty specific to the rustc tooling (e.g., LLVM version flags, support for MIR-opt diffing, etc.) -- and some of that is nice to add in PRs to rust-lang/rust alongside other changes, rather than needing to go through the version bump dance.

I don't think these get added that often though. And we could totally have an unstable for_rustc feature that enables a lot of these things, or have a way to pass in additional "hooks" when you set it up from your test runner main().

But worth pointing out: clippy needs many of these flags too. Clippy has very similar testing needs as the compiler, though perhaps not as much of the LLVM/MIR stuff.

And I'm not sure we'd want to maintain the public API surface of compiletest as it stands today in-tree -- currently, we don't care about breaking changes, but if we were to maintain it we'd need to care.

Again, I don't think it gets changed that often. But we can continue to use v0.x.0 version bumps; this is what compiletest-rs does already since it sometimes pulls from rustc (though it hasn't for a while).

We can document it as not attempting to be super stable (though we should still follow semver), and that its main clients are rustc and clippy. Though honestly I think there's a really good opportunity for everyone if compiletest were evolved as something other people use as well, in my experience that leads to better APIs for everyone including the original users.

It's worth mentioning: the clippy team already kinda-sorta has to do this for its fork, and every breaking change in in-tree compiletest-rs is more work for us. This is not a new problem, this is a preexisting problem.

The trybuild crate (https://github.com/dtolnay/trybuild) is one option for out-of-tree users that want a more limited set of functionality -- maybe it would be sufficient to use that?

Not for clippy. As I said, our needs are similar to the compiler -- we use rustfix tests, we use those commenty flags, and a lot of other things.

[petrochenkov]

Maybe some more generic parts can be factored out moved out to a separate crate(s), but moving compiletest as a whole as it exists now goes against all my experience with the test suite infra.

Compiletest is effectively a part of rustbuild now, it is indeed changed together with the test suite and rustbuild, and these changes need to be made together.

EDIT: In other words, I'm not against gradual librarification of compiletest, but against putting rustc-specific parts under crates.io and semver.

[Manishearth]

Can you expand on what rustc-specific parts you're talking about? Again, clippy needs most of this too, and as it stands we just are maintaining a weird fork anyway. I'm even okay doing away with the semver requirement, but I really don't want to keep having to maintain a fork for clippy.

I also don't see why semver is that big a deal. It's totally okay to make breaking changes often here.

How about this: compiletest comes with a lot of Step implementations for e.g. rustdoc-gui, linkcheck, tidy, etc. What if we refactored it so that it comes with some Step implementations by default, and rustc "adds in" more of them for miri, rustdoc-gui, etc.

[Manishearth]

Basically, I'm worried about the reluctance of putting rustc-specific stuff there because clippy needs a lot of that too. I'm happy to gate off stuff that clippy doesn't need: We could use a cargo feature that's guaranteed to be unstable, or we could split the crate via Step impls, etc. But I do think we should try to move the generally useful parts out of tree.

[Manishearth]

I'm happy to do more work into investigating how hard it would be to create such a separation, but I'd still need to know what line you're trying to draw when it comes to "rustc specific"

[ehuss]

compiletest really only uses the parts of libtest that handle output formatting

I'm confused about this. compiletest uses the entire libtest infrastructure for driving tests. Can you clarify what this means?

I have a question about why compiletest-rs exists. Why not use compiletest directly from the rust-lang/rust repository? Why does it need to move out in order to share it? If clippy needs to make a change to it, couldn't they post PRs to rust-lang/rust?

[Manishearth]

I'm confused about this. compiletest uses the entire libtest infrastructure for driving tests. Can you clarify what this means?

Yeah but it doesn't use a lot of the other stuff. But it might be best to just try to move libtest out of tree anyway, the libs team has wanted to do this previously.

Why not use compiletest directly from the rust-lang/rust repository? Why does it need to move out in order to share it?

How do we depend on it in a way that doesn't randomly break? Especially when faced with Cargo's unification -- we might have a lockfile, but that won't be respected when rust-lang/rust is building clippy.

Also I don't think the in-tree compiletest works for other things without tweaks.

[ehuss]

How do we depend on it in a way that doesn't randomly break? Especially when faced with Cargo's unification -- we might have a lockfile, but that won't be respected when rust-lang/rust is building clippy.

The way I see it, due to CI, rust-lang/rust would always be in a functional state, and so would rust-lang/rust-clippy. The only pain would be the periodic sync from rust-lang/rust to rust-clippy. If there is a breaking change, someone would need to manually bump the version on the rust-clippy side (and maybe deal with publishing a new version of compiletest if that isn't automated). I wouldn't expect breaking changes to happen often, but maybe that is a bad assumption? Clippy's API surface for compiletest seems very small, and scanning through the last few months of changes I don't see much that would break that.

[XAMPPRocky]

Hey, just want to voice that having an out-of-tree repo for compiletest would also be useful for rust-gpu, as we're currently trying to move our end-to-end testing code over to using it. It has been great so far, however we have encountered a couple of minor bugs and it would be nice to have a single place to contribute those fixes to. Like clippy we also use those compiler comment flags, and have similar needs to the compiler, being a compiler backend.

With the growing number rustc specific tooling being developed out-of-tree these days, having a single shared compiletest implementation would be great benefit for all of these tools including rust-lang/rust.

[jyn514]

It has been great so far, however we have encountered a couple of minor bugs and it would be nice to have a single place to contribute those fixes to.

Does that mean you're currently using compiletest-rs (out-of-tree) but you'd prefer to be using the in-tree version? If you're using the in-tree version, I think we'd take bug fixes (not sure about new features though).

[XAMPPRocky]

Does that mean you're currently using compiletest-rs (out-of-tree) but you'd prefer to be using the in-tree version? If you're using the in-tree version, I think we'd take bug fixes (not sure about new features though).

Well I'd prefer to have one version, regardless of location. I also have contributed the fixes to both versions. However I don't really think using the in-tree version is really viable for out-of-tree tools. Fetching rust as a git dependency alone takes minutes, longer than it takes to fetch, compile, and run out-of-tree compiletest.

Not to get into a tangent, there are few features as well that would beneficial for both projects. For example having a test mode for performing Codegen style FileCheck testing against disassembly of rustc's output. This would be very useful for testing WebAssembly or SPIR-V code output.

[jyn514]

Basically, I'm worried about the reluctance of putting rustc-specific stuff there because clippy needs a lot of that too. I'm happy to gate off stuff that clippy doesn't need: We could use a cargo feature that's guaranteed to be unstable, or we could split the crate via Step impls, etc. But I do think we should try to move the generally useful parts out of tree.

I don't think compiletest currently distinguishes between "generally useful" and "useful to rustc" at all, and I'm worried it would be very difficult to try and retrofit that. Here are some example changes I've made recently, none of which help clippy at all:

• Rename the tidy binary to rust-tidy #82507
• Don't abort rustdoc tests if tidy isn't installed #79370
• Give a better error when rustdoc tests fail #78752

If compiletest were completely out-of-tree with a submodule setup, like rust-analyzer, it would have been extremely painful to test and debug those changes. I think a subtree setup could be useful to avoid having to download all of rust-lang/rust's history, but I don't know about setting compiletest up as a generic library with semver, etc. - that means that it's much harder to make changes that are specific to rustc. You mentioned a plugin system briefly during the devtools meeting today and I guess that could work, but it seems like quite a lot of man-hours for something that's already working fine in-tree.

[XAMPPRocky]

but I don't know about setting compiletest up as a generic library with semver, etc. - that means that it's much harder to make changes that are specific to rustc.

Why that would be the case?. Semver and library scope are matter of policy, the request is just to move it out of tree, not convert it into a general purpose library. I don't know about clippy but it wouldn't matter for rust-gpu if every update was always required a major version bump, and the scope only included things that were useful for rustc and its tools. The only change for devs working on compiletest would be that a change can't introduced that is incompatible with running out-of-tree.

[GuillaumeGomez]

It means that instead of having one PR to make the changes on both sides, you'll require two. Also, if multiple people are making changes into compiletest, better hope that they don't break rust build, otherwise it'll be a nightmare.

[wesleywiser]

I'm not sure what team or teams own compiletest but since this will have an impact on the compiler team, I would ask that once we have consensus on the course of action for this idea, please file a Major Change Proposal before starting implementation. Thanks!

[jyn514]

I actually think this should be an MCP right now - I think that format works better than github issues for discussions.

[Manishearth]

@ehuss

The way I see it, due to CI, rust-lang/rust would always be in a functional state, and so would rust-lang/rust-clippy. The only pain would be the periodic sync from rust-lang/rust to rust-clippy. If there is a breaking change, someone would need to manually bump the version on the rust-clippy side (and maybe deal with publishing a new version of compiletest if that isn't automated). I wouldn't expect breaking changes to happen often, but maybe that is a bad assumption? Clippy's API surface for compiletest seems very small, and scanning through the last few months of changes I don't see much that would break that.

This ... could work for clippy. cc @rust-lang/clippy ? We could do a thing where we have a dependency which is path but if not path is a git dependency. There may be some tricky bits associated with that.

I would still prefer it be external because I think that additional pressure to keep things stable and reusable is valuable. But maybe this is an acceptable solution for clippy.

[Manishearth]

If compiletest were completely out-of-tree with a submodule setup, like rust-analyzer, it would have been extremely painful to test and debug those changes. I think a subtree setup could be useful to avoid having to download all of rust-lang/rust's history, but I don't know about setting compiletest up as a generic library with semver, etc. - that means that it's much harder to make changes that are specific to rustc. You mentioned a plugin system briefly during the devtools meeting today and I guess that could work, but it seems like quite a lot of man-hours for something that's already working fine in-tree.

@jyn514 Honestly, I think that some of this pressure is good -- compiletest isn't really owned by anyone, and a shared library that's got a bit of a tragedy-of-the-commons thing going seems like a bad place to not version. There's already somewhat of a plugin API with the ability to plug in test runners, the rustdoc test runners could be moved out. I'd rather us move towards a more stable compiletest (not stabilized, just "written in a way that rustc can make its changes without constantly breaking everyone else, and vice versa"). I'm fine with using subtree, or whatever, but my high order point is that I'd really like it if semver were involved in some way, and the repo worked out of tree via crates.io. Even if there are breaking changes somewhat often.

[jyn514]

Clippy is now planning to use Oli's rewrite of compiletest: rust-lang/rust-clippy#10426

@Manishearth does it make sense to close this issue?