-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CI build for Windows #862
base: master
Are you sure you want to change the base?
Conversation
d8a7585
to
70450b4
Compare
70450b4
to
e4405ad
Compare
7afee3a
to
0da2a86
Compare
2fe442e
to
db793e9
Compare
I have a few concerns about Windows CI builds related to performance, overall resource usage, and the impact of longer CI builds on CI build usefulness. A Linux CI build on ubuntu-latest including "make all" (45 seconds), check, install, installcheck, distcheck takes about 4 minutes, so effectively one build takes around 2 minutes. On a Windows build, the MSYS2 setup part alone takes about 3 minutes, and "make all" alone takes more than 17 minutes (22 times as slow) for a total build time (without distcheck) of more than 26 minutes. So this turns CI builds from a 4 minute job on ubuntu-latest to essentially a half hour job on windows-latest, and with two to four (realistically two to three) msys2 matrix So we used to wait about 7 minutes until there is a red cross or a green tick mark next to a commit... and Windows CI builds turn that into about 4 times that. As the Windows CI builds take THAT long, I wonder whether we want Windows CI builds at all. However, IF we do Windows/MSYS2 CI builds however, I would suggest to only start the Windows builds after the Linux (and later macOS) builds have succeeded in order to not take any more resources from the already very strained resource of Windows CPU cycles. |
b347d16
to
e0e8fa8
Compare
.github/workflows/ccpp.yml
Outdated
msys2-w32api-headers | ||
msys2-w32api-runtime | ||
pacboy: | ||
gcc:p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be "cc" - this will give you default gcc in mingw64/ucrt64 envs, but default clang in clang64 env
.github/workflows/ccpp.yml
Outdated
autoconf | ||
automake | ||
gettext-devel | ||
libtool | ||
make | ||
pkgconf | ||
msys2-w32api-headers | ||
msys2-w32api-runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If building w/ autotools, better install just env specific autotools:p
through pacman below, it'll pull in whatever needed.
You don't need msys2-w32api-* packages here, as nothing is being built for the msys2 env...
Here's the list of deps for the MSYS2 repo package.
So in this section, you basically just need git
, all the other ones belong to the env specific list in pacboy section below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We link against ws2_32.dll
and msys2-w32api-runtime
contains a ws2_32
... .a
. Oh. .a
vs .dll
.
msys2-w32api-headers
is a leftover from trying to fix the mingw32 build which does not find the WSAstartup()
function in ws2_32.dll
. However, it did not help, so I can indeed remove it.
The autotools:p
looks like a lovely idea.
Thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ws2_32.dll
is a Windows system library provided by MS, no need to install it.
Basically, msys2 is a POSIX emulation environment (a fork of Cygwin), while mingw64 etc. are native Windows environments one wants to build for. Apart from a couple of choice msys2 tools, one really doesn't need any more development headers and libraries there, everything must come from the native Windows environments.
Install the msys2's libgphoto2 requirement list, use cc instead of gcc to allow for clang, rely on the autotools:p package to pull in all the required packages.
e0e8fa8
to
7563009
Compare
I understand this was blocked primarily because CI (or, well, MSYS2 in general) is unbearably slow to run on each commit. But I do wonder... could it at least be used for libgphoto2 releases? Right now getting libgphoto2 DLLs on Windows is a large common pain, unless you're willing to ask all your users to install MSYS2 which has it prepackaged and available via pacman. Being able to just download latest binary release from Github releases page would make it much easier to integrate it into apps in a cross-platform way. |
The problem is not that CI is slow in general. The CI jobs running on Linux are running OK, and the OSX jobs are bearable. The problem is that especially running CI jobs on Windows is very slow. That makes CI less useful for checking that commits still build. As an aside, the computation minute argument is not all that relevant here. While GitHub does donate our open source project's minutes, we still want to remain a good citizen here, though, and not waste huge amounts of cycles for little gain. GitHub explicitly says that 1 Windows minute is worth 2 Linux minutes, and 1 OSX minute is worth 10 Linux minutes. So with about 4 Linux minutes, plus 8 OSX minutes (equal to 80 Linux minutes), 26 Windows minutes without distcheck (equal to 56 Linux minutes), one CI build run costs about 84 Linux minutes right now with linux+osx, and about 140 Linux minutes with windows builds added. You are arguing for Windows builds not only for CI, but to provide finished binary builds for Windows, which means we need to take a few more steps to comply with the LGPL license. We could accomplish that by creating build artefacts for upload to a well known place (a tarball or zip file) which contains both the binaries and the sources, so that anybody downloading the binaries also gets the sources at the same time, and we do not need establish a separate method to provide everybody who has such binaries with the source code. |
This tries to add a CI build for Windows
Closes #858