After #17586 will be fixed, should we update the merging policy to require successful AppVeyor build?
From IRC:
<wumpus> I mean the reason that appveyor breaks all the time is because people ignore appveyor before merging, because it breaks all the time, we kind of need to break that circle shrug <meshcollider> true, appveyor has failed on pretty much every single open PR so it kinda just becomes cry-wolf <meshcollider> we need to fix it and then rebase every PR at once ;)
we need to fix it and then rebase every PR at once ;)
While rebasing does fix this, a simple “re-run build” might be sufficient as well. Not sure who has the rights to re-run a build, but re-opening the pull request is a work around
should we update the merging policy to require successful AppVeyor build?
That is already the merging policy. However, sometimes there are intermittent failures, so we can’t enforce it.
Also, in this particular case the error came from a package that was upgraded below our feet. This has trade-offs:
Pick one, but not both ;)
For my 2 cents I find appveyor CI to be marginally beneficial. Mostly due to the build time and fragility or maintaining it. That being said for a project like Bitcoin Core where catching problems far outweighs rapid development cycles it does seem justified.
As I’m writing I have just succeeded in porting the appveyor job to Github Actions and have run a successful build (minus tests). It looks like build time and maintenance overhead will be approximately the same as appveyor but the usage limits do look a lot more appealing. If it does support 20 concurrent CI jobs that could significantly improve the situation for Bitcoin Core PR writers waiting for the build results.
An alternative to GitHub actions would be CirrusCi, which at least gives twice the amount of parallel builds: https://cirrus-ci.org/faq/#are-there-any-limits Another alternative would be to ask Appveyor to raise our limit.
Anyway, thanks for working on this. I am lacking the background on Windows to be of any use here.
An alternative to GitHub actions would be CirrusCi
Probably worth doing some benchmarking on. Once I’ve got GitHub actions working will take a look at CirrusCi.
Another alternative would be to ask Appveyor to raise our limit.
+1 for that. There are currently 13 appveyor jobs queued for PR’s. That’s an improvement from about 20 at this time yesterday but at approx an hour a job that’s a long wait.
Hmm, on https://www.appveyor.com/pricing/ it says that an additional machine is $25/month for FOSS. However, on the billing page they say 50$.
Edit: I guess on the billing page the price is for additional jobs and not for the total amount of jobs?
Ok, we just sent them 50$ and there are now two parallel builds on https://ci.appveyor.com/project/DrahtBot/bitcoin/history
We can upgrade/downgrade as needed in the future.
I’m agnostic on CI but the Github Actions first impressions are that it will facilitate a lot higher loads than AppVeyor and is free (of course the load could suffer once usage increases and pricing could also change).
I was able to run 10 consecutive Bitcoin Core build jobs (before I stopped adding more).
The jobs are almost identical to appveyor except for the unit tests which I’ve had some problems with.
A comparison of two jobs chosen at random for the msbuild stage:
github action: 16m 50s appveyor: 38m 33s
The github action job is contained in a single yml file, same as appveyor. I have it configured in a branch on my fork and will leave it running. As far as I can tell it couldn’t do any harm to enable it on the main Bitcoin repo. It doesn’t need to be integrated into the PR workflow. It would be interesting to see how the build queue performed with a trigger for PR’s.
If there’s interest I can create a PR for the job.
Why I don’t blame AppVeyor too much for these kind of temporary issues: today, it’s again the turn for Travis CI to fail due to a dependency issue outside our control (#17626).
If there’s interest I can create a PR for the job.
Well, the “faster for free” part is kind of nice. The essential thing is that it tests the MSVC build system and runs the unit/functional tests.