Repository navigation
Add infrastructure for disabling submodules easily #44077
Description
We'd like to expand the number of submodules and tools that we build as part of the main Rust distribution, including the RLS, Clippy, and Rustfmt. In doing so however these tools all depend on nightly features in the compiler which means that they're highly susceptible to breakage. I believe that historically, at least, Clippy has broken quite regularly due to changes in the compiler.
In order to ease the rust-lang/rust contributor developer experience, we're thinking that we'll want a way to easily, and temporarily, disable building/testing the tool in CI. This means that if a contributors accidentally breaks the submodule there's no required pressure for them to do a submodule dance and update things, but rather it can happen after the PR has landed. This in turn means that there will be nights where not all tools are built, but the hope is that this is only periodically and only for nightlies, not for stable/beta releases.
The requirements of this infrastructure would look like:
- There's a file in the repository with a list of tools that are "known to fail".
- All tools are always built unconditionally.
- If the tool fails to build and it's in the "known to fail list", then the build continues
- If the tool fails to build and it's not in the "known to fail list", then the built halts with an error. The build also prints out a diagnostic indicating the presence of the "known to fail" list and suggests adding the tool to that list.
- If the tool succeeds to build and is in the "known to fail list" then the build halts with an error. The build also prints out how to correct this by updating the file in question
- If the tool succeeds to build and is not in the "known to fail list", the build continues.
- Tools which fail to build are not packaged up. We don't build tarballs. They're listed as
available = falsein the manifest.
The thinking is that this file would also have contributors and maintainers to cc whenever a tool is added to the file, so they get a heads up that breakage is happening. As soon as there's a new nightly then the tool can get updated, fix its own CI, ensure tests pass, and then finally send a PR to update the submodule in rust-lang/rust and remove the tool from the "known to fail list' as well.
cc @rust-lang/dev-tools
Activity
alexcrichton commentedon Aug 24, 2017
Oh it's also worth mentioning that this assumes that rustup can handle missing components on nightlies. AFAIK there's support for that, but there's been bugs in it historically and it's not well tested, so we may have some bumps to work out.
Mark-Simulacrum commentedon Aug 26, 2017
I think we'd also want support in rustup to not update to a new nightly if some tools aren't built for it.
As an aside, I'm not sure I'm happy with the approach we're taking here. It's concerning that we're willing to ship incomplete toolchains for nightlies; I worry that we'll have to also implement (even if on a human basis) some form of lock a week or so in advance of beta so that we guarantee that beta ships with all tools built. To be clear, I'm fine with doing this -- but I think we should be careful to avoid delays in beta or incomplete betas (and hence, stable). I'm not sure I have any real alternative suggestions though.
alexcrichton commentedon Aug 26, 2017
I believe rustup has at least some support for this in the sense that manifests can have
available = falsein them. That code path has been only very lightly tested and buggy in teh past s well though!You're right though that we'll need to be vigilant around releases and otherwise see how this pans out. Tools like the RLS and rustfmt have empirically broken very rarely so far, but it's expected that tools like Clippy break quite often, so I'm not quite sure how this'd pan out.
That being said, this came out of a lot of discussions, so it's unclear how we'd do better...
Mark-Simulacrum commentedon Aug 26, 2017
I agree that there doesn't seem to be a better alternative--I just wanted to voice my worry about this approach.
arielb1 commentedon Aug 29, 2017
Shouldn't we instead make the "lockstep upgrade" experience easier? I think the PR author is often in the best place to fix Rust breakage. How big of a concern is this in practice?
Manishearth commentedon Aug 29, 2017
Agreed. For clippy at least, most breakages are of the kind where the PR author has actually already made similar fixes to
librustc_*so fixing clippy is ~no effort. The first option should be fixing clippy, but if it's nontrivial the option to disable it should be there.oli-obk commentedon Aug 30, 2017
While there's no one file to rule them all right now, it would be as easy as flipping this boolean in order to disable it (right now it is disabled).
We could move that associated constant to be a trait method, and offer a simple way to query the central "known to fail" file.
Mark-Simulacrum commentedon Aug 30, 2017
The constant is supplemented by the default_condition method on the
ShouldRunstruct.oli-obk commentedon Aug 30, 2017
That is only called if the constant is
false(and a path was passed tox.py) (see https://github.com/rust-lang/rust/blob/master/src/bootstrap/builder.rs#L71-L74)Mark-Simulacrum commentedon Aug 30, 2017
No? The should_run is called for all
Steps and information is stored; the information is read here when no paths are passed; ifDEFAULT && default_conditionthen we run the step by-default.oli-obk commentedon Aug 30, 2017
Oh. I totally misread those docs then. Thanks.
oli-obk commentedon Sep 1, 2017
I mostly implemented this in #43628.
I did not implement that. I don't think anyone will be accidentally fixing tools.