Putting bors on a PIP

[Manishearth]

IIRC it is possible to just build std with nightly, but you need to flip a bunch of cfgs to make it work. I did something like this whilst hacking on core::str::pattern (core is trickier). Having this as a dedicated build mode would be fantastic.

[johnthagen]

Is it possible to approach Travis/AppVeyor about paying some amount of money for more powerful build servers that could be dedicated to the rust repository? I’m not sure what the current limitations are on open source builds, but I’d imagine builds could potentially get more cores/memory/time out or more builds could go in parallel. For example, if rust builds went from 2 to 16 cores, maybe that dramatically helps this problem? Could Mozilla or some other entity sponsor this?

I bring it up because the overall cost could be dramatically less than the total man-hours (probably of core devs) needed to split up repos or allowing untested PRs to accidentally slip into master.

[Eh2406]

I was independently coming up with similar ideas for automatic roll ups at:

Issue: Add support for optimistic concurrency testing

But in the discussion of:

Proposal: move to bors-ng

The described batching algorithm does seem like it has a high potential to not work well for Rust, with our long turnaround times, and high intermittent rates. I’d be more comfortable adopting it if it supported our existing workflow and let us gradually experiment with others. If bors-ng is going to be maintained, and it can support our workflow, and somebody is willing to do the work of the transition, and preferably if it can remain under the venerable ‘bors’ account, then it might be worth …

2 things seemed to be made clearer.

1. brson, and presumably others would need convincing before accepting the automatic roll ups.
2. There was interest in switching to bors-ng, that already has an implementation this stufe.

[mark-i-m]

Question: are there times of the day/week when bors tends to be idle? For example, if most of the people with bors permission are asleep or not working at some time, maybe that would be a good time run rollups, since they are low-risk and (hopefully) should just work. For example, if bors tends to be idle on weekends or at midnight in the USA, then perhaps rollup jobs should be deferred until then. That way, the waking/working hours can be better utilized for harder PRs. The benefit is that it doesn’t really worsen average rollup merge time because you would just be waiting for maybe half a day extra…

[davidtwco]

Question: Is there any data available on what percentage of jobs in the queue (on average) are documentation, standard library, compiler, etc.?

That might help identify the optimizations to make - if we found that documentation changes were 20% of the queue (I’ve not got a clue how far off this guess is) then finding a way to optimize that by not running the compiler build and testing might show significant improvements; similarly with any other parts of the project that we can test independently.

[arielb1]

Measure, Don’t Guess

Second, do we have some statistics on why PRs fail bors?

IIRC failures can be divided into 4 kinds:

1. spurious failures (including timeout disease) - these cause random capacity loss and latency increase, and have to be removed.
2. PRs that don’t pass travis - if this still hasn’t been done, we might want to prevent them from getting into bors.
3. failures on odd platforms.
4. merge conflicts.

The latter 2 are the “interesting” cases, and have different solutions. I don’t see an easy way of handling merge conflicts, but I suspect they are fairly rare. I would prefer to measure rather than guess, however.

As for handling failures on odd platforms - we should have a way of running tests on several platforms on a PR without blocking the bors queue.

We might be able to find a representative subset of builders that can be used to try large PRs to shake out any trouble, so you could have @bors test-large-pr which would test your PR on, say, Windows Linux and wasm, 32-bit and 64-bit - basically we want a set of builders that covers say 90% of all non-spurious non-travis failures.

Maybe also have a similar suite for e.g. buildsystem changes, if they trigger a separate segment of tests.

If we could culturally not land large PRs without doing such an “enhanced regression run”, that would reduce the queue capacity loss due to “odd platform” failures.

[arielb1]

Network Access

On the purely technical side, IME bors accesses a lot of network sites outside of our control, and that causes a ton of outages. So, we could find a way to proxy these and only say refresh our “caches” once per day - so that we only access github and our cache - while being able to revert the network cache.

That would prevent these sort of outages. Maybe ask some OS packager (@cuviper ?) or CI expert for how they handle network dependencies in buildscripts?

[cuviper]

We “handle” network dependencies by forbidding them. Fedora and RHEL builders have no network access at all once they enter the build chroot, and I expect other distros are similar.

[arielb1]

A vision for the build system

Alice wants to fix a bug or improve documentation. This change probably does not cause failures on weird platforms. She pushes her PR, waits until travis is green, and gets an r+. Eventually, her PR is picked up in an automatic roll-up and is merged.

If one of the rollups has a failure, the failure is nicely displayed on the rollup PR, and she or a maintainer marks her PR as failed. Meanwhile, bors might start processing a larger PR (is this a good idea?).

A regression had landed on nightly, and Bob wants to get a fix up quickly. He puts his PR up with priority, and after he gets green on travis and the current PR succeeds, his PR is up next.

Charlie is writing a sweeping reform of the MIR optimization pipeline. This will obviously have weird failures in all sorts of weird platforms, so he wants to see that there are no weird failures before merging. Also, sweeping reforms cause regressions in both performance and functionality, so he wants to have a tracking commit.

Charlie can ask bors to run a testsuite on the 90%-percentile of platforms that cause genuine test failures - we keep analytics on platforms that cause test failures in order to compute that. Afterwards and after he gets reviewed, he can get his PR merged in a separate commit.

David wants to add a now function on a less-common platform. Therefore, travis being green won’t tell him much. He can give a bors command to make travis run tests on his platforms. If that travis is green, his PRs will also get rolled up.

Are there any user stories I missed?

[arielb1]

We “handle” network dependencies by forbidding them. Fedora and RHEL builders have no network access at all once they enter the build chroot, and I expect other distros are similar.

You apply patches to the build-systems so that they download network dependencies from local sources. It might be worth investigating whether we can do that change on our own CI.

[mark-i-m]

As for handling failures on odd platforms - we should have a way of running tests on several platforms on a PR without blocking the bors queue.

We might be able to find a representative subset of builders that can be used to try large PRs to shake out any trouble, so you could have @bors test-large-pr which would test your PR on, say, Windows Linux and wasm, 32-bit and 64-bit - basically we want a set of builders that covers say 90% of all non-spurious non-travis failures.

I think we have a tendence to use @bors try for this, which is not very efficient because we tend to only use try for things that are not going to be merging immediately. Why should they be serialized with PRs that actually want to merge?

[leodasvacas]

A common and fixable cause of bors failure is lack of rebasing, when a patch doesn’t fail travis but the merged patch will fail. This can be fixed by somehow adding an intermediate step between travis and full bors. Instead of the two-step:

Travis tests single target unmerged -> bors tests all targets merged

We could do a three-step:

Travis tests single target unmerged -> bors tests single target merged -> bors tests all targets merged.

[sanxiyn]

I think “failure due to lack of rebasing” is basically merge conflict. That is, two PRs don’t conflict physically, but they do conflict semantically.

[nikomatsakis]

Most of my changes to the rust repo are in std or rustdoc or something ancillary to rustc like that. Its always seemed odd to me that I am rebuilding the compiler and competing for queue time for changes that don’t need that.

I just want to point out that changes to libstd can totally break rustc. rustdoc, perhaps not.

[cuviper]

You apply patches to the build-systems so that they download network dependencies from local sources. It might be worth investigating whether we can do that change on our own CI.

Sometimes we do have to patch build systems, but not in rust’s case. We just use ./configure --enable-local-rust --enable-vendor and that’s enough.

What happens for local sources is that the packager manually uploads the source tarball to Fedora’s own infrastructure, and it’s fetched from there before the build. When first bootstrapping rustc, we also include the upstream binaries as extra “sources” for stage0 (manually unpacked for --enable-local-rust), then we use rpm dependencies for that in subsequent builds.

[aidanhs]

Currently, spurious failures largely come down to timeouts; reducing absolute cycle time will help.

I’d like to get thoughts on this line - my belief is that this is a significant factor behind many of our current woes with CI. Timeouts are basically the worst case - CI has run for the maximum time possible, then it fails and the PR needs retrying and the only solution is “make the build faster”. We’ve been bumping into this for quite a while now, it’s just that as average time creeps up, so too does the frequency with which we hit the limit.

The scenario that really worries me is that we get to a state where 9/10 failures is caused by timeout, and we become unable to merge anything.

So how do we solve this? First off, caching is something to tread carefully around - it would be easy to get into a state where builds stop working if the cache isn’t primed (we already have this to some extent with the Docker images and llvm, but they both have some mitigating factors), so my focus has generally been around the ‘worst case build’.

I also feel like 3 hours is pushing the limit of what’s acceptable for a ‘complete’ build - for people who want to do the full build and tests themselves, it’s not a great experience.

My preferred long-term solution is to split the repo and have two-stage landing. This gets us the quick satisfaction of landing PRs without them blocking unrelated PRs, and it has the advantage that it scales.

To be clear, are you suggesting (say) moving out libstd from the main tree? We may sacrifice some possible ‘correctness’ here, if we don’t do libstd tests against every change to rustc. Of course, this is a spectrum where one end is doing a whole crater run on every commit and the other is just making sure rustc builds.

Perhaps this works if we promote crater to be a regular part of CI - @pietroalbini is doing some great work on automating crater that may help us walk this route. But it’s worth being explicit about what this means for the not rocket science rule and “automatically maintain a repository of code that always passes all the tests” - what does all mean?

[nrc]

To be clear, are you suggesting (say) moving out libstd from the main tree?

Yes. The way I envisage this working is similar to how tools like Cargo and the RLS work today - we run a subset of appropriate tests when a PR is merged to the std repo (for example), and then the full set of tests when we update the sub-module/crate dep in the main repo (which would take as long as today, but would happen much less frequently).

Of course merging updated std into the main repo might not work and someone would have to then fix up the std repo. So we’d be trading that bit of manual work for the improved turnover (and other benefits). I don’t think we sacrifice correctness. Of course the ‘fixup’ work is happening today, but it is happening in individual PRs, in the 2-stage model, it would happen in bigger batches and would require some dedicated attention from somebody.

[steveklabnik]

it would happen in bigger batches and would require some dedicated attention from somebody.

Yeah, this gets close to the “Lieutenant” model, and it’s their job to deal with integration stuff. This is what happens already with me and the books.

For example, when updating the books for 1.26, a bunch of links were broken: https://github.com/rust-lang/rust/pull/49318#issuecomment-375850762

So I went and fixed them, and then updated the integration PR to pass.

[aidanhs]

and then the full set of tests when we update the sub-module/crate dep in the main repo (which would take as long as today, but would happen much less frequently)

Ah I see. This indeed doesn’t sacrifice correctness, but also doesn’t try to tackle the build time timeouts problem (which is fair). I suppose this would potentially be used in combination with moving to a CI provider with longer timeouts - because there are less individual merges to master, longer builds aren’t a problem.

[gnzlbg]

This is also how std::arch, std::simd, and libc are set up and it is really nice.

I worry a bit about duplicating the CI infrastructure throughout all of these, but until now we have managed this just fine.