libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.83k stars 1.82k forks source link

Combine all ci into one workflow #10200

Closed madebr closed 3 weeks ago

madebr commented 3 months ago

Does anyone object to the idea of combining all workflows into one yml?

This allows to:

The disadvantage is a big, somewhat more complicated yml

slouken commented 3 months ago

I like the advantages. I get zillions of e-mails about failed builds while iterating on a PR.

slouken commented 3 months ago

Will we lose the parallelism that we have with the current setup?

icculus commented 3 months ago

I was going to ask about parallelism, too; GitHub Actions will run 20 things in parallel atm, and we currently have 39.

If it still thinks of these as separate jobs (so we don't have to wait hours for a final result), then the listed benefits are really desirable. But they aren't if each build runs in serial.

madebr commented 3 months ago

Will we lose the parallelism that we have with the current setup?

No, for a first iteration I was thinking about merging all workflows into one yml. All jobs would run in parallel just like the main workflow does now. The only difference would be more items in the left column.

Introducing some way of serialization in the form of a canary build, would be to reduce the amount of lost work on a failed build. This is not so important on public GitHub, but would be when we'd run ci on our own infrastructure. There's a balance between fast feedback and energy here.

icculus commented 3 months ago

I'm sold, then. :)

slouken commented 3 months ago

I think we should have the canary builds be the ones that output shippable build artifacts, and then run the rest of the jobs once those succeed.

madebr commented 2 months ago

This has been done in #10372.

Would it make sense to also add one of n3ds, ps2, or psp to the initial canary build (or instead)? The reason I'm asking is because those builds fail when you forget about the SDL_PRI... integer format macros.

icculus commented 2 months ago

I'm not against it if one of those can build quickly (like, it doesn't have to spend forever installing build tools just to get started).

icculus commented 2 months ago

Just pushed a PR that was obviously broken, fixed something and force-pushed (still broken), and force-pushed again...the canary build was SUPER nice for this. Didn't have to wait for 40 builds to fail before I could deal with my obvious mistakes.

Nice work here, @madebr. :)

madebr commented 2 months ago

Happy to hear it works out!

Would it be interesting to (conditionally) add -k 0 to a build job? This ninja argument makes ninja continue building until it cannot continue, even when there are errors. Then you can view the build log, grep all build errors and fix them all in one go. This should (hopefully) result in less edit-push-ci iterations.

I'd conditionally enable it with a tag in the commit message. e.g. [sdl-ci-k0] (do you have a better suggestion?)

slouken commented 2 months ago

tags in the commit messages are problematic, because then you have to remember to strip them out of any final merge - which then triggers another CI flow. Also, grepping for errors in the logs is a pain, it's much easier to scroll to the bottom and see what failed. Usually it's just one or two failures before it proceeds to the main build flow.

slouken commented 2 months ago

Also, this is done! Can we do something similar for SDL2? It's less important since there are fewer commits there, but it would still be nice to have one action kicked off for one commit when looking at the action list.

icculus commented 2 months ago

(Tags aside, let's not do -k 0, usually a fast fail is better than complete error logs, and I'd bet most times you probably only have one error total anyhow.)

madebr commented 2 months ago

My use case for [sdl-ci-k0] is this SDL2 commit. I had to perform a few edit-commit-ci iterations to see the next -Wdeclaration-after-statement violations. So I hacked -k 0 in my wip branch, and could do it in fewer iterations. It's relatively easy to grep for errors in the raw logs, by looking for error: (error + colon).

The ci script currently already supports 2 commit tags: [sdl-ci-filter] and [sdl-ci-artifacts], so [sdl-ci-k0] would not be too big of a deal.

Also, this is done! Can we do something similar for SDL2? It's less important since there are fewer commits there, but it would still be nice to have one action kicked off for one commit when looking at the action list.

Sure, autoconf is the only big addition. I'll do this in a future SDL time slot :)

slouken commented 3 weeks ago

I'm going to call this done. Feel free to open other issues if you have more work you want to do here.