Add mingw32-w64/wine CI build

sipa commented at 5:58 pm on April 17, 2021: contributor

in .cirrus.yml:216 in de230aaa12 outdated

211+    ECDH: yes
212+    RECOVERY: yes
213+    EXPERIMENTAL: yes
214+    SCHNORRSIG: yes
215+    CTIMETEST: no
216+    CFLAGS: -O2 -static -static-libgcc

real-or-random commented at 11:26 am on April 18, 2021:

Do we need the static flags? I guess it’s just my lack of experience with mingw but maybe add a comment.

Also -O2 is our default, so we probably shouldn’t set it explicitly here.

real-or-random commented at 11:27 am on April 18, 2021: contributor

Nice!

Concept ACK

sipa force-pushed on Apr 18, 2021

real-or-random commented at 7:07 pm on April 18, 2021: contributor

ok it seems there was a reason why -static was there: https://cirrus-ci.com/task/5119836217933824?logs=test#L185 libtool: warning: undefined symbols not allowed in x86_64-w64-mingw32 shared libraries; building static only (and a few other messages then when linking the benchmarks)

Maybe ./configure --disable-shared does the job then without specifying CFLAGS. In the end it doesn’t matter for CI, I was just curious because we could use this to add build docs for mingw.

real-or-random commented at 7:09 pm on April 18, 2021: contributor

Can you also change file *tests || true to file *tests* || true ? Otherwise it won’t match the “.exe”…

sipa commented at 7:46 pm on April 18, 2021: contributor

@real-or-random That wasn’t the reason actually. I tried this first in the minisketch repository (see https://github.com/sipa/minisketch/pull/33), and there the issue is that C++ builds to building against shared libgcc by default (something related to being able to catch exceptions across libraries), but I couldn’t get that to work with Wine. For C, static libgcc is the default, so this problem doesn’t occur. I’ve just removed it.

real-or-random cross-referenced this on Apr 19, 2021 from issue mingw builds by real-or-random

real-or-random commented at 12:11 pm on April 19, 2021: contributor

Ok makes sense. We can take care of more improvements to mingw later then (#923).

ACK mod the file thing.

sipa force-pushed on Apr 24, 2021

sipa commented at 0:16 am on April 24, 2021: contributor

Can you also change file *tests || true to file tests || true ? Otherwise it won’t match the “.exe”…

Done.

real-or-random approved

real-or-random commented at 2:35 pm on April 29, 2021: contributor

ACK e2750b173527a80444929000b32ccd9b103878f7

jonasnick commented at 3:21 pm on April 29, 2021: contributor

utACK e2750b173527a80444929000b32ccd9b103878f7

jonasnick commented at 3:27 pm on April 29, 2021: contributor

Hm any idea why wine complains so loudly (on cirrus). Is that supposed to be ignored?

0it looks like wine32 is missing, you should install it.
1as root, please execute "apt-get install wine32"

sipa commented at 3:40 pm on April 29, 2021: contributor

It’s only testing 64-bit binaries, so it doesn’t need wine32 I think. We could probably just add it to the dockerfile to silence it.

real-or-random commented at 3:54 pm on April 29, 2021: contributor

AFAIU wine32 would be required to run 32-bit binaries, which we don’t need.

I’m not sure why Wine warns. Maybe because 32bit is still a common thing on Windows and it’s an issue that many people run into?

in .cirrus.yml:207 in e2750b1735 outdated

202+  container:
203+    dockerfile: ci/linux-debian.Dockerfile
204+    cpu: 1
205+    memory: 1G
206+  env:
207+    WINE_CMD: wine

real-or-random commented at 10:02 am on April 30, 2021:

I think this would hide the warning.

0    WINE_CMD: wine64-stable

sipa commented at 5:39 pm on April 30, 2021:

IIRC I tried that, but it didn’t work. I think invoking wine64 directly doesn’t start wineserver.

in ci/linux-debian.Dockerfile:14 in e2750b1735 outdated

 9@@ -10,4 +10,8 @@ RUN apt-get install --no-install-recommends --no-upgrade -y \
10         make automake libtool pkg-config dpkg-dev valgrind qemu-user \
11         gcc clang libc6-dbg \
12         gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \
13-        gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x
14+        gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \
15+        wine gcc-mingw-w64-x86-64

real-or-random commented at 10:03 am on April 30, 2021:

(not strictly necessary but probably cleaner:)

0        wine64 gcc-mingw-w64-x86-64

in ci/linux-debian.Dockerfile:17 in e2750b1735 outdated

 9@@ -10,4 +10,8 @@ RUN apt-get install --no-install-recommends --no-upgrade -y \
10         make automake libtool pkg-config dpkg-dev valgrind qemu-user \
11         gcc clang libc6-dbg \
12         gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \
13-        gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x
14+        gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \
15+        wine gcc-mingw-w64-x86-64
16+
17+# Run a dummy command in wine to make it set up configuration
18+RUN wine true || true

real-or-random commented at 10:03 am on April 30, 2021:

0RUN wine64-stable true || true

real-or-random commented at 9:37 pm on April 30, 2021: contributor

@sipa Ok, I played around with the docker container. If I’m not mistaken:

If you install wine (which pulls in wine64 as dependency), then wine64 <cmd> (or wine64-stable <cmd>) works without the warning
If you install just wine64, then WINESERVER=/usr/lib/wine/wineserver64 wine64-stable <cmd> works without the warning. (Indeed, without the env variable wineserver can’t be found.)

Both are fine but the first is more natural…

By the way, wine xcopy works better as a dummy command. There’s no true on windows.

sipa force-pushed on Apr 30, 2021

sipa commented at 11:33 pm on April 30, 2021: contributor

@real-or-random Ok, used your first variant, and also changed true -> xcopy.

Add mingw32-w64/wine CI build 4dc37bf81b

sipa force-pushed on Apr 30, 2021

real-or-random commented at 11:17 am on May 1, 2021: contributor

Ok, great. Now there seems to be one final hiccup:

0WARNING: could not read 16 bytes from /dev/urandom; falling back to insecure PRNG

( https://cirrus-ci.com/task/4912068852711424?logs=test#L258 )

I can sometimes reproduce this in the Docker image and then the seed always has some random bytes in front followed by zeros. You can cherry-pick this fix: https://github.com/real-or-random/secp256k1/commit/625b1aa071a78dca3c2ca3377a3500d74914290b, see https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-160#generic-text-routine-mappings.

tests: fopen /dev/urandom in binary mode

This makes a difference with mingw builds on Wine, where the subsequent
fread() may abort early in the default text mode.

The Microsoft C docs say:
"In text mode, CTRL+Z is interpreted as an EOF character on input."

ed5a199bed

sipa commented at 0:05 am on May 2, 2021: contributor

@real-or-random Cherry picked.

real-or-random approved

real-or-random commented at 9:29 am on May 2, 2021: contributor

ACK ed5a199bed65bf084f34ce18d35807d31a1c75bb

jonasnick commented at 12:56 pm on May 2, 2021: contributor

utACK ed5a199bed65bf084f34ce18d35807d31a1c75bb

jonasnick merged this on May 2, 2021

jonasnick closed this on May 2, 2021

jonasnick cross-referenced this on Jun 14, 2021 from issue Upstream PRs 831, 907, 903, 889, 918, 906, 928, 922, 933, Merge bitcoin-core/secp256k1#936: Fix gen_context/ASM build on ARM, 925, 937, 926, Merge bitcoin-core/secp256k1#940: contrib: Explain explicit header guards, 850, 930, 941, 846, 947, 662, 950 by jonasnick

Add mingw32-w64/wine CI build #922