RFC: add mingw to appveyor matrix

[tkelman]

As discussed here #2414 (comment)

This is just a start, and is only using the 32-bit mingw.org that is already installed on the AppVeyor VM's. I can keep working on this and add 32 and 64 bit MinGW-w64 distributions. Just need to pick which distribution to download.

It looks like there might be a build issue with mingw.org at the moment. On my fork I got

[ 16%] Building C object CMakeFiles/git2.dir/src/trace.c.obj
In file included from c:/projects/libgit2/src/common.h:66:0,
                 from c:/projects/libgit2/src/buffer.h:10,
                 from c:/projects/libgit2/src/trace.c:8:
c:/projects/libgit2/src/trace.c: In function 'git_trace_set':

c:/projects/libgit2/src/thread-utils.h:279:43: error: expected expression before ')' token
 # define GIT_MEMORY_BARRIER MemoryBarrier()
                                           ^
c:/projects/libgit2/src/trace.c:27:2: note: in expansion of macro 'GIT_MEMORY_BARRIER'
  GIT_MEMORY_BARRIER;
  ^

make[2]: 
*** [CMakeFiles/git2.dir/src/trace.c.obj] Error 1

make[1]: 
*** [CMakeFiles/git2.dir/all] Error 2

Which is interesting, since the cross-compile on Travis appears to be working.

[tkelman]

Sorry for eating up a lot of queue time experimenting with things here. If I knew of a way to only run these on AppVeyor and not Travis, I would. Please cancel any redundant pending builds for this so other things can go through. I have some code in here to fail immediately for obsolete builds on AppVeyor when a newer build is pending for the same PR, this could probably be I went and translated it to bash for Travis too but I don't know what to query from their API.

The 32 and 64 bit MinGW-w64 builds both looked successful, but powershell may have been treating all the compiler warnings as errors or something and it wasn't running the tests. I'm trying to run the build steps from cmd instead of powershell, though that is being pickier about multiline commands.

[tkelman]

I think I see why the cross-compile was working with i586-mingw32msvc-gcc but building with mingw.org on Windows isn't: the former is not setting -DTHREADSAFE=ON -DENABLE_TRACE=ON.

Anyway, this works and runs the tests now, at least for mingw-w64. There are a whole bunch of compiler warnings and 12 test failures due to building without openssl (I could go find a mingw-w64 openssl download), 4 "Unsupported URL protocol," and something else wrong with online::clone::can_cancel and online::clone::certificate_invalid. Could put these in allow_failures: to start with.

[ethomson]

Wow!!! Very cool.

With regards to the failing tests, we either need to build against openssl, or we need to simply not run the online tests. (It should be enough to drop the -ionline argument from clar.)

Long-term, I would like to see us building against openssl and using it, but that's not super critical for me for now. I think that there's a lot of value having the mingw builds even if they're not exercising the online tests.

[tkelman]

I can get a cross-compiled binary of openssl from the opensuse build service. It requires some messy rpm repodata xml parsing, I have a version written in bash (filed prominently under things you shouldn't write in bash) that relies on xmllint which isn't installed on appveyor. I also have a partially complete version written in powershell, I got stuck at needing sort -V though (rpm-style version number sorting). There's also a slightly-outdated Python version available, but its license is not GPL-compatible. under MPL 2.0 license.

(It should be enough to drop the -ionline argument from clar.)

Is that easily doable through ctest too? What would be preferred to start with, running fewer tests with mingw, or allowing failures?

[tkelman]

I found an OpenSSL binary and got it installed on appveyor, it built okay but gave error -1 - failed to create ssl object on i686 and segfaulted on x86_64. The binary was probably built with MSVC, but I likely need one built with the same MinGW variant I'm using. @akikoskinen where were you getting OpenSSL from in your builds?

[carlosmn]

I don't think we should bother too much with OpenSSL on Windows but instead try to get the PR which makes us use WinHTTP from the mingw environments in working order.

[tkelman]

Looks like mingw-w64 has _chsize_s, but that doesn't fix the ftruncate test failure.

Silly me, I had a typo in my ifdef. Fixed now, ftruncate test passing on mingw-w64.

Travis has a big backlog of all the times I rebased and force-pushed this branch. ba6c53b is intended to work through that backlog a little quicker, but could be taken out since I'll admit I haven't tested that code outside of this PR.

[tkelman]

🍰 I fixed the mingw.org build problem too. It was due to empty/missing defines in some of the trace code, and a separate problem with VOLUME_NAME_DOS in a test - neither the trace code nor the tests are built by the Travis mingw cross-compile. These could be fixed by changing _WIN32_WINNT to at least 0x0600, or adding a couple of defines (only under the conditions where they'd otherwise be missing) to mingw-compat.h. I went with the latter for now, felt less invasive.

3 test failures on mingw.org, but could be much worse given the toolchain.

[ethomson]

Thanks for all the hard work here @tkelman - I'll dig into this when I have a moment.

Re cross-compiling - I agree that mingw on Windows is a challenge. However WINE introduces a handful of other issues - we don't work under WINE at the moment (or we didn't the last time that anybody I know tried it) and I don't want to end up testing WINE instead of us, as I'm sure you can sympathize with.

Thanks again, I know you've put a ton of effort into this.

[ethomson]

A recent PR suggests that MINGW_HAS_SECURE_API might be defined when this is supported. This might be more idiomatic and future-proof.

[ethomson]

This looks really nice to me. Not sure if that MINGW_HAS_SECURE_API definition is helpful or not, I'll let you decide what you want to do with that information.

I know that @phkelley has played in the mingw space, I don't know if he has any additional thoughts.

[tkelman]

Re cross-compiling - I agree that mingw on Windows is a challenge. However WINE introduces a handful of other issues - we don't work under WINE at the moment (or we didn't the last time that anybody I know tried it) and I don't want to end up testing WINE instead of us, as I'm sure you can sympathize with.

I agree there. I should've been a little more specific. I like cross-compiling for building binaries, since it's more easily automatable and repeatable via Docker, OBS, Koji, what have you - native dependency management on Windows is still an absolute mess, letting a Linux package manager do the work for you actually works really well for getting binaries built. Testing those cross-compiled binaries via Wine is good to do when things are simple, but also fraught with Wine quirks when things aren't simple. As an anecdote, Julia has a rewritten garbage collector as of a few months ago, which mysteriously broke the step in cross-compilation that depended on Wine - probably due to a bug in Wine.

Not sure if that MINGW_HAS_SECURE_API definition is helpful or not, I'll let you decide what you want to do with that information.

I'll push a test commit and see whether that does the job as well as my initial version. If so, I can rearrange things a little.

[tkelman]

I'll push a test commit and see whether that does the job as well as my initial version. If so, I can rearrange things a little.

Looks like it worked fine. I took out my versions of those commits and did it with a cherry-pick instead, so this vs #2959 could hopefully be merged in either order.

[ethomson]

Thanks for the hard work here, @tkelman . Really happy to see this done up so nicely.

[tkelman]

Cool, you're very welcome. Happy to help make mingw(-w64) easier to support for you guys. Especially if https support without needing openssl can be tidied up and finalized by someone who knows what they're doing.

Don't hesitate to revert either tkelman@8008ab6 or tkelman@ba6c53b if they cause any trouble. They were good to have within the context of this PR, but could also be useful in doing the job of cutting down on queue time elsewhere.