[auto-toolstate][1/8] Always ignore build failure of failable tools (rls, rustfmt, clippy)

[kennytm]

If build failed for these tools, they will be automatically skipped from distribution, and will not fail the whole build.

Test failures are not ignored, nor build failure of other tools (e.g. cargo). Therefore it should have no observable effect to the current CI system.

This is step 1/8 of automatic management of broken tools #45861. The purpose is concentrate all failure detection about tools into a single CI job for easy management, while keeping the ability to distribute these tools in the nightlies.

r? @Mark-Simulacrum

[Mark-Simulacrum]

I believe this is good, but we should get more eyes on the approach here just in case. I'm happy with this.

cc @rust-lang/infra @alexcrichton @nrc

[alexcrichton]

Thanks for the PR! I'm a little confused though, mind explaining a bit more? So for example if I set the state of the rls to "broken" then the build completes on all bots successfully today, so I'm not entirely certain what this is enabling. Does this mean that if the state is set to "testing" and the build fails that we'll ignore it?

[kennytm]

@alexcrichton The idea is to make dist eventually ignore toolstate.toml. After #45861 is complete, there would be no need to keep toolstate.toml, instead the build failure of these non-essential external tools will just be logged and allowed.

The tools are currently referred in two kinds of jobs in the CI:

1. The existing aux jobs, which compile then and run tests, and
2. The dist jobs, which just compile them for distribution.

Before this PR, if the compile step failed, it will exit with code 1, and fail the whole process. Such failure can happen in the dist jobs or aux job.

After this PR, compile step failure is ignored, so dist jobs won't fail anymore (if it can be built, it will distributed, otherwise ignored). But for the aux job, the test step will still run cargo build once more and will still cause failure. Overall the whole CI build will still fail, thus "no observable effect to the current CI system".

After this PR and before toolstate is removed,

• "toolstate = broken" is same as today — nothing will be dist-ed because we still respect the toolstate check at the moment
• "toolstate = testing" is same as today — the aux job will still protect us from tool build failure
• "toolstate = compiling" becomes effectively the same as "broken" — compile failure is now ignored, so aux tests about the tools become useless. Distribution is not done due to toolstate.

The eventual intention would be,

• If a tool cannot be built — great, not distributed.

• If a tool can be built — distributed, but we don't guarantee the tools' tests will pass

  • The tests will be performed after a successful merge to avoid spamming the dev-tool team. This is intended.

We will need rustbuild to collect the build/test failure result of these tools, which will happen at step 3/8.

[alexcrichton]

Ok thanks for the info! One point mentioned on #45861 though is that the failure interpretation here is also channel specific in that we can't allow broken tools to reach beta. I think as-is it's fine for now but will we eventually want some channel checking in these locations as well?

[kennytm]

@alexcrichton Since we can't distinguish between a beta merge and master merge (every thing is run in the auto branch, and the [beta] tag in the PR title is not mandatory), the best we could do is to monitor the CI, i.e.

1. We will run the tools job in Travis CI when the merge target is not master. Not sure about AppVeyor.
2. After a successful merge, watch the Travis and AppVeyor CI badges and ensure they are green.

I have better ideas if we can easily distinguish between a beta merge and master merge ;)

[alexcrichton]

@kennytm oh the channel is configured via src/ci/run.sh and rustbuild has a config variable for the channel --

rust/src/bootstrap/config.rs

Line 112 in 33374fa

pub channel: String,

[nrc]

Looks good to me! Thanks for jumping on this @kennytm !

[kennytm]

@alexcrichton Thanks! That changes the final CI structure like this:

• the tools job will still be run as part of auto — so no spare jobs left, sorry 😛
• if the channel is nightly, the CI will exit 0 on failure, otherwise exit 1, thus able to trigger bors outside of nightly.
• if the channel is nightly, append the test result to the external GitHub repo as well.
• if the PR is merged successfully on master, a bot (a Travis CI job that runs on master branch only) will promote the test result of the latest commit, and notify people if the result changed.

[alexcrichton]

@kennytm sounds good to me!

[bors]

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

[carols10cents]

@kennytm sounds good to me!

@alexcrichton soooo do I hear an r+?

[alexcrichton]

@bors: r+

Sure, was going to wait on @Mark-Simulacrum but I will interpret this as pseudo approval!

[bors]

📌 Commit 8cc5cd4 has been approved by alexcrichton

[Mark-Simulacrum]

Yep, I was signing off there, just wanted to get a few more eyes on this just in case since it's a relatively major change.

[bors]

⌛ Testing commit 8cc5cd4 with merge 3e9a7f7...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 3e9a7f7 to master...