Add GitHub Actions CI workflow

[FranciscoPombal]

• Ubuntu 18.04/20.04, GUI ON/OFF
• Windows 2019
• macOS 10.15 GUI ON/OFF

---

Depends on #12746 #13327 (it's rebased on top of that one). master is now ready for this.

• The workflow runs on pushes to master or any events from PRs that target master.

• It takes 10 to 15 minutes to complete if cached.
  The very first time around (or if the caches are dropped due to not being used for a week) there will be no caches. Without caches, it takes 1:30 to 3 hours.

• It's quite readable and maintainable: only 230 240 lines for all OS/GUI combinations we currently have in CI, plus Ubuntu 20.04, which we currently don't have. It does not depend in any way on packages/archives from builds.shiki.hu. All dependencies are built from official sources, and it's easier to change out their versions.

• It only covers building with CMake. If something similar is desirable for qmake builds, I think it should be done in another workflow file.

• The build artifacts are published after the build is finished, which may come in handy for testing purposes.

Example cached run: https://github.com/FranciscoPombal/qBittorrent/actions/runs/223928661

---

Most of this is based on lessons learned from #13070. Both can coexist and are independent of each other, but this one is the one that will use caching (cache space is limited to 5 GiB), because nobody wants CI that takes hours to complete, whereas nightly builds with various combinations of dependency versions for user testing on Windows is not as time-sensitive.

[FranciscoPombal]

Additional notes:

• The Add CMakeGraphVizOptions.cmake file commit is so that the target_graph.dot files produced as output of the builds don't get as cluttered. It was previously part of CMake scripts overhaul #12746, and I thought this would be a good place to re-introduce it.

• The macOS builds on Travis have been broken for some time now, with no hope of getting fixed until either sledge returns and fixes the archives from builds.shiki.hu or someone spends time overhauling the Travis script to to work around it. This PR is an alternative way of fixing that.

[FranciscoPombal]

No change rebase, on top of the most recent CMake overhaul commit.

[FranciscoPombal]

Force push: removed the toolset specification for Windows builds, so that it is clear that everything is built with the latest stable VS 2019 Enterprise tools. Turns out GitHub Actions runners don't keep older point releases around, so specifying the tool versions with msvc-dev-cmd is misleading (the latest ones will get used).

[glassez]

The Add CMakeGraphVizOptions.cmake file commit is so that the target_graph.dot files produced as output of the builds don't get as cluttered.

I'm sorry, but I still don't understand why it's required to be added in this PR.
Your explanation sounds like if you say to people who know what you're talking about in advance.

[FranciscoPombal]

@glassez

The Add CMakeGraphVizOptions.cmake file commit is so that the target_graph.dot files produced as output of the builds don't get as cluttered.

I'm sorry, but I still don't understand why it's required to be added in this PR.
Your explanation sounds like if you say to people who know what you're talking about in advance.

I've explained its purpose before: #12746 (comment), #12746 (comment) (first 2 paragraphs). Try to build qBittorrent with the --graphviz flag without this file in the root dir to see the difference. The generated graph itself will be more cluttered with things that are not relevant and it will be split in many files, cluttering the output build directory.

[glassez]

Try to build qBittorrent with the --graphviz flag without this file in the root dir to see the difference.

How is it related to this PR? Is any graphs producing required here?

[FranciscoPombal]

@glassez

How is it related to this PR? Is any graphs producing required here?

Yes, it's useful to see and troubleshoot the actual resulting dependency graph of the build. This was already relevant for #12746, so that the user could produce uncluttered graphs if they chose to, when building. But it is even more relevant in this PR. Download one of the zips from https://github.com/FranciscoPombal/qBittorrent/actions/runs/223928661, and imagine how much more annoying it would be if the zip contained many little graph files instead. Thanks to this configuration file it contains just one.

[glassez]

Yes, it's useful to see and troubleshoot the actual resulting dependency graph of the build.

Really? I haven't seen any complaints about it not being in current CI builds.
Isn't CI just intended to make sure that new changes are still buildable on platforms other than the one they were created on?
"Dependency graph of the build" doesn't seem to be intended for everyone's use at all. Someone who actually cares about it can produce it locally.
In any case, you should not mix it in here (just as you shouldn't include creating graphs in CI environment). As already suggested in another PR, you can publish it in a separate PR, and try to provide a convincing argument that it makes sense at the project level.

[FranciscoPombal]

@glassez

Really? I haven't seen any complaints about it not being in current CI builds.

It's not a solution to some specific complaint, it's just a general improvement.

Isn't CI just intended to make sure that new changes are still buildable on platforms other than the one they were created on?

Yes, but not just that they are buildable, but also built the same way (or at least without surprising changes). I want to have confidence that changes I make which shouldn't impact the way qBittorrent is built on other platforms live up to that promise. This is especially useful when making changes/improvements to the build system.

"Dependency graph of the build" doesn't seem to be intended for everyone's use at all. Someone who actually cares about it can produce it locally.

Not all developers have one machine with each of the supported operating systems. If someone complains there is something wrong when they tried to build on macOS, it is much easier to troubleshoot if everyone (even those who do not own systems with macOS/Hackintoshes/don't have macOS VMs set up) can use the files produced by the macOS CI runs as a canonical point of reference. CI is meant to both verify that things still build fine and provide useful reference information for troubleshooting.

You could make a similar argument for the generated executable + pdb file. If you consider them useful, why not the graph? If you're concerned about performance, don't be, CMake generates this graph really fast.

In any case, you should not mix it in here (just as you shouldn't include creating graphs in CI environment). As already suggested in another PR, you can publish it in a separate PR, and try to provide a convincing argument that it makes sense at the project level.

I'll consider it, but for now I believe I've refuted the central premises of CI is _just_ for checking if it is still buildable and you shouldn't include creating graphs in CI environment in the previous paragraphs.

[glassez]

Okay, maybe producing dependency graph on CI really makes sense. I don't deal with CMake that closely, so it doesn't concern me.
But then I have another questions. Isn't this file for customizing CMake graph output according to someone's personal preferences? Do we impose our preferences on others? Does it have something that really depends on the individual characteristics of our application (or its build system)? If it's really better for everyone, why does it behave differently by default?

[glassez]

If it is approved, shouldn't we remove all similar(CMake based) configurations from other CI environments in the same PR?

[FranciscoPombal]

@glassez

These are the changes from default:

• GRAPHVIZ_CUSTOM_TARGETS: FALSE -> TRUE

To give more information about custom target by default; I think this will be quite useful if we ever add custom targets. A little more information is always welcome

• GRAPHVIZ_GENERATE_PER_TARGET and GRAPHVIZ_GENERATE_DEPENDERS: TRUE -> FALSE

When both of these are false, only the final overall graph is generated. This prevents spam of lots of little files.

• GRAPHVIZ_IGNORE_TARGETS: <empty list> -> .*autogen

This makes the .*autogen auto-generated targets (due to automatic {MOC, UIC, RCC}) not appear in the graph. I guess this would be the most controversial change, since this is the only one that removes information from what would otherwise be shown as the default. However, I don't think this is very useful 99% of the time.

---

Overall I think we should select the defaults that are most convenient for CI. If the options we choose for the CI don't suit someone's graph-generation preferences, they can always change it locally. I do wish there was a better way to pass these options than editing a specifically-named file, but sadly this is currently not possible.

However, thinking about it a little better now, I think it would be beneficial to open up discussion about exactly which graph-generation options are "most convenient for CI". From this standpoint, it is indeed better to factor this out to another PR. I don't expect it will be too different from this, though. I'll just take care of the other prerequisite PR first, before factoring out the CMake graphviz stuff.

[FranciscoPombal]

@glassez @Chocobo1

About this force push:

Seems like something changed with GitHub actions runners, such that when using Ninja, GCC is detected as the default compiler, which breaks the build. I came across this problem when reusing this PR for other things recently.

So I have made a minor change to the Windows job, it now uses the Visual Studio 16 2019 generator instead of Ninja, because this way, MSVC is always detected as the default compiler.

I have reported the bug here: actions/runner-images#1621

Sample run showing that it works now again: https://github.com/FranciscoPombal/qBittorrent/runs/1138166418

diff:

       run: |
-        cmake -B build -G "Ninja" `
+        cmake -B build -G "Visual Studio 16 2019" `
           -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=RelWithDebInfo `
           -DCMAKE_TOOLCHAIN_FILE=${{ env.VCPKG_DEST_WIN }}/scripts/buildsystems/vcpkg.cmake `
           -DVCPKG_TARGET_TRIPLET=x64-windows-static-release `
           -DVERBOSE_CONFIGURE=ON `
           --graphviz=build/target_graph.dot
-        cmake --build build
+        cmake --build build --config RelWithDebInfo -- /p:UseMultiToolTask=true
 
     - name: upload artifact as zip
       uses: actions/upload-artifact@v2.1.3

The /p:UseMultiToolTask=true flag is to make sure the build utilizes all cores.

[glassez]

@Chocobo1, @FranciscoPombal
Sorry, I don't have time to write extensive comments right now to express my opinion on each issue raised. So I will comment on my attitude to this PR in general.
I approve it without going into too much detail about it (although some aspects don't suit me either, or at least they should be further tweaked). And I want it to be merged as early as possible. I have two reasons:

1. We don't currently have a fully functional CI environment and this is really annoying.
2. It is better to continue discussing / improving it when it is already working and bringing some results, than to try to perfect something that is not yet there, at the risk of not getting anything in the end. These changes do not affect the source code of the app in any way, so the omitted flaws should not spoil anything very much. This will give us time to cool down a bit and look at it slowly and more objectively.

[glassez]

Seems like something changed with GitHub actions runners, such that when using Ninja, GCC is detected as the default compiler, which breaks the build. I came across this problem when reusing this PR for other things recently.
So I have made a minor change to the Windows job, it now uses the Visual Studio 16 2019 generator instead of Ninja, because this way, MSVC is always detected as the default compiler.

This is a very controversial decision. Instead of fixing script so that it uses the compiler we need with previously selected generator, you just changed the generator. I would agree with this if you made an equivalent substitution (for example, jom). But "Visual Studio"?.. Personally, I don't see any practical value for these generators other than loading CMake project to Visual Studio IDE. I think it's perverse to use these tools in a non-interactive environment.

[FranciscoPombal]

@glassez

You're right, I changed it to use Ninja + the ilammy/msvc-dev-cmd@v1.3.0 action, I think this is a better solution.

[FranciscoPombal]

I've given up completely on the upstream inclusion of a CMakeGraphVizOptions.cmake file, as I no longer think it makes sense. So I won't be making a future PR about it. If someone really wants it later, it's only about 5 lines anyway for the options that matter.

[glassez]

@Chocobo1, @FranciscoPombal
Hey, you guys still haven't come to any agreement about it? I want to have a working CI.

[Chocobo1]

Hey, you guys still haven't come to any agreement about it? I want to have a working CI.

@FranciscoPombal
I would like to approve this PR if you agree to look into this afterwards: #13288 (comment)
Do we have an agreement?

[FranciscoPombal]

@glassez

Hey, you guys still haven't come to any agreement about it? I want to have a working CI.

This has been ready for 3 days.

@Chocobo1

@FranciscoPombal
I would like to approve this PR if you agree to look into this afterwards: #13288 (comment)
Do we have an agreement?

The static builds? In case it wasn't clear before: sure! I also think it would be very useful. I thought it was understood that it was out of scope of this PR - but this PR was definitely made with such future extensions in mind.

[Chocobo1]

I think we can also add a badge in README.md, see: https://docs.github.com/en/actions/managing-workflow-runs/adding-a-workflow-status-badge

[FranciscoPombal]

@Chocobo1

I think we can also add a badge in README.md, see: https://docs.github.com/en/actions/managing-workflow-runs/adding-a-workflow-status-badge

#13432

[Chocobo1]

@FranciscoPombal
Thank you!

[oprypin]

Sorry for necro-posting and sorry for plugging my website. But,

Since this mentions "nightly" builds (and I'm not sure what exact purpose was in mind) --
Well, you can get a convenient link to the builds (either always-latest or a particular one) via this website:
https://nightly.link/qbittorrent/qBittorrent/workflows/ci.yaml/master