Prevent duplicate CI builds

[pkch]

Current settings on both Travis and AppVeyor result in 2 builds whenever a push is made to a branch on the main repo (not a fork) that is part of a PR:

• One build for the branch itself. E.g., for commit 0efc26d: AppVeyor 1.0.1302; Travis 5197
• One build for the temporary commit created by Github to represent a hypothetical merge of the branch into master. E.g. for commit 9258f8ef:
  AppVeyor 1.0.1303; Travis 5198

Note that the commit 9258f8ef in the second bullet point is no longer visible because Github doesn't preserve it (it only lives until a new push to the branch, or until the branch is deleted).

Sometimes people actually want to know whether tests pass on their feature branch, separately from whether they would pass after a hypothetical merge with master. But if we can live without such granularity of information, we can improve the CI throughput. The semi-duplicate builds affect everyone because they use the same pool of VMs as any other builds.

We can disable either branch or PR build; I'd recommend to disable branch builds because merge PR are more important. To do this, there's equivalent web UI settings and .yml config file settings. I'll submit a PR that implements this change using config file, since it's more transparent. Of course, if a branch on the main repo is created that's not used in a PR, there'll be no automatic builds for it at all; hopefully that's ok (edit: it's better than I thought; AppVeyor - but not Travis - is smart enough to always build on such a branch)

There's another reason there are extra builds:

• One build for the actual (not hypothetical) merge commit when the PR is merged. E.g., for commit 09b6d4b: AppVeyor 1.0.1306; Travis 5201

In this case, it was redundant because the hypothetical and the actual merge commits were the same; but in general, it may not be so, since the master could have changed since last time the PR was tested. There are settings to do that check automatically too (Github's required status checks), but let's not worry about it for now, especially since these builds happen relatively less often (we don't merge PRs as often as we push to PRs).

[gvanrossum]

IIRC there's extensive literature on this in the GitHub docs. Can you try to find that and reference it?

[pkch]

Github does not have any docs on the main topic. The reason is that this is really the domain of CI service providers. Travis, AppVeyor and CircleCI all behave differently and are all evolving; Github won't be able to update their docs as things change externally.

The references on CI vendors websites, their issue trackers, and SO are below:

• AppVeyor
  • docs
  • discussion
• Travis
  • doesn't have a perfect solution like AppVeyor
  • simply limiting builds to master branch only is good enough for our purposes
  • docs
  • explanation
  • discussion

The reason limiting builds to master branch is ok with us, is that we don't really use the main repo to test many features on branches without PRs. If we did, we'd need a more complex solution.

On the secondary topic (about avoiding building on master), there are docs on Github webiste, but that's for the future (it's more complex and less benefit than disabling PR/branch duplication).

[JukkaL]

I guess the duplicate builds (for branch and master merged into branch) are sometimes useful, but not very often. What if we limit them to Travis only? The duplicate Windows build very rarely catches any additional Windows-specific issues, so the extra AppVeyor build is only marginally useful, but since AppVeyor has less capacity the duplicate builds cause significant delays.

[pkch]

Sure, let's try with AppVeyor first. However, if we reduce AppVeyor from 2 to 1 builds, the two will be much closer (3.5 min / commit for AppVeyor, 2.5 min / commit for Travis).

Talking of which, do you want me to add a setting to skip one of the 2 builds on AppVeyor on non-tagged commits? The only reason we have 2 builds now is for wheels (see #2934 and
#3238 (comment)) and we don't need wheels on non-tagged builds.

Edit: I said Travis instead of AppVeyor.

[gvanrossum]

@ddfisher ^^