It is a good property for github history to have each commit buildable and runnable on its own. Bors only checks the PR as one big change before landing. Would it be possible in the future to ask bors to test each commit before merging?
Unfortunately, I don't think that this feature is compatible with how Bors works.
As far as I can tell, Bors does not use the CI statuses on any of the commits on your branch. Rather, it only uses at the CI statuses on the merge commits that it creates.
For example, suppose that your Git history looks like this, with a default branch named
master and one feature branch named
myfeature that contains a single commit:
A——B——C <——master \ D <——myfeature
If you open a pull request to merge
master and comment
bors r+ on that pull request, then Bors will create a merge commit
M by merging
master and will force-push the branch
A——B——C———M <—— staging \ / D
Now, your CI system will test the
staging branch, which is equivalent to testing the
M commit. Bors waits for your CI to finish. If CI fails on
staging, Bors posts a
Build failed comment. If all CI checks pass on
staging, Bors will fast-forward
master to point to
A——B——C———M <—— master \ / D
So, as you can see, the CI status on the single commit
D on the
myfeature branch is never used by Bors. Bors only uses the CI status on the
staging branch (i.e. the merge commit
M) to decide whether a build succeeds or fails.
Here is an example of a single feature branch with three commits:
A——B——C <——master \ D——E——F <——myfeature
A——B——C—————————M <——staging \ / D——E——F
A——B——C—————————M <——master \ / D——E——F
Here is an example of two feature branches, each with one commit:
A——B——C <——master |\ | \ | \ \ D <——feature—1 \ \ E <——feature—2
A——B——C———————M <—— staging |\ /| | \ | | \ / | \ D / \ / \ / E
A——B——C———————M <—— master |\ /| | \ | | \ / | \ D / \ / \ / E
And here is an example of two feature branches, each with three commits:
A——B——C <——master |\ | \ | \ \ D——E——F <——feature—1 \ \ G——H——I <——feature—2
A——B——C—————————————M <——staging |\ /| | \ / | | \ / | \ D——E——F / \ / \ / G——H——I
A——B——C—————————————M <——master |\ /| | \ / | | \ / | \ D——E——F / \ / \ / G——H——I
Notice in all of these examples that Bors only uses the CI statuses on the merge commit
@notriddle Can you take a look at this and make sure that I am explaining Bors correctly?
If you want to enforce the property that every commit is buildable and runnable on its own, you can accomplish this, but not within Bors. Also, it will be very inconvenient.
The basic idea is:
- Enforce the property that each pull request contains only a single commit.
- Require that both the
Travis CI - Branchand
Travis CI - Pull Requestchecks pass on each pull request.
- If a branch only contains one commit, and the
Travis CI - Branchcheck has passed on that branch, then we know that the single commit in that branch passes.
To implement number 1, add code like this to your Travis
if [[ "$TRAVIS_PULL_REQUEST" == "false" ]]; then else export NUMBER_OF_COMMITS=$(git rev-list --count $TRAVIS_COMMIT_RANGE) echo $NUMBER_OF_COMMITS if [ "$NUMBER_OF_COMMITS" -eq "1" ]; then echo "There is exactly one commit in this pull request. Success." else echo "There is NOT exactly one commit in this pull request. Failing..." exit 1 fi fi
To implement number 2, make sure that the
Travis CI - Branch,
Travis CI - Pull Request, and
bors checks are all required in your GitHub branch protection rule for the
Additionally, you should add the
Travis CI - Pull Request to the
pr_status list in
delete_merged_branches = true pr_status = ["Travis CI - Pull Request"] status = ["Travis CI - Branch"]
Also, you should make sure that your
.travis.yml is configured to build all branches except
branches: except: - staging.tmp - trying.tmp
That's all true, but I'm not sure it's relevant. Nothing prevents us from modifying bors so that it'll scan through all the old commits and check their statuses.
What really makes this inconvenient is that most GitHub CI products only build the tip commit when you push (you'd notice this if you push a pull request with multiple commits, only the last one has a status). So, to make this work, bors would have to take care of ensuring that all the intermediate commits got built.
If it did this as part of its existing queue, then the queue is now going to take twice as long or more. If it does this in parallel with its existing queue, then you risk causing a build timeout, because there's no way to make Travis prioritize staging the way we want.
How would this work? Bors would go through each commit on the branch, one by one, see if it had a status, and if not, call e.g. the Travis API to trigger a build on that commit?
Pretty much, yeah.
I am really interested in this feature as well. I don't think it needs to slow down testing too much, because unlike bors' final test of the merge, the test of the intermediate commits don't need to be re-run.
Restricting PRs to single commits seems like a mistake.
Would I just go and code up the featuer and create a PR to bors-ng itself?
Personally, I would rather not accept such a PR.
For one thing, it doesn't actually need to be a part of bors-ng. You could write something that tests all the intermediate commits on a PR, then posts a commit status on that PR, and set the
pr_status config option to require it.
For the other thing, it seems really complicated to actually do it right. As I said before, you really need to make sure that staging builds never wait on pull request builds, but there's no good API to do that.