ci: Add MSVC builds #1084

pull real-or-random wants to merge 12 commits into bitcoin-core:master from real-or-random:202203-msvc changing 8 files +165 −47
  1. real-or-random commented at 7:12 PM on March 10, 2022: contributor

    This adds MSVC builds built on Linux using wine. This requires some settings of tools and flags because the autotools support for MSVC is naturally somewhat limited.

    The advantage of this approach is that it is compatible with our existing CI scripts, so there's no need to write a Windows CI script (in PowerShell or similar). If we want to test building and running on Windows native (e.g., as supported by Cirrus CI) we could still do this in the future.

    Another advantage of this approach is that contributors can simply use the docker image if they need a MSVC installation in a non-Windows environment.

  2. real-or-random commented at 7:37 PM on March 10, 2022: contributor

    by the way, rather for fun:

    Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)

32-bit MSVC -O2 (over wine):
ecdsa_verify                  ,   226.0       ,   227.0       ,   230.0
ecdsa_sign                    ,   133.0       ,   134.0       ,   134.0
ecdh                          ,   226.0       ,   227.0       ,   229.0
ecdsa_recover                 ,   230.0       ,   231.0       ,   233.0
schnorrsig_sign               ,   120.0       ,   120.0       ,   123.0
schnorrsig_verify             ,   227.0       ,   228.0       ,   232.0

32-bit MinGw -O2 (over wine):
ecdsa_verify                  ,   195.0       ,   196.0       ,   198.0
ecdsa_sign                    ,   115.0       ,   116.0       ,   117.0
ecdh                          ,   199.0       ,   200.0       ,   204.0
ecdsa_recover                 ,   199.0       ,   200.0       ,   204.0
schnorrsig_sign               ,   101.0       ,   102.0       ,   103.0
schnorrsig_verify             ,   196.0       ,   197.0       ,   200.0

32-bit Linux GCC -O2:
ecdsa_verify                  ,   195.0       ,   197.0       ,   200.0    
ecdsa_sign                    ,   115.0       ,   116.0       ,   122.0    
ecdh                          ,   199.0       ,   200.0       ,   202.0    
ecdsa_recover                 ,   199.0       ,   201.0       ,   202.0    
schnorrsig_sign               ,   101.0       ,   102.0       ,   104.0    
schnorrsig_verify             ,   196.0       ,   197.0       ,   199.0

    edit: Fixed Linux figures, looks reasonable now.

  3. real-or-random force-pushed on Mar 10, 2022
  4. real-or-random force-pushed on Mar 10, 2022
  5. real-or-random force-pushed on Mar 12, 2022
  6. real-or-random force-pushed on Mar 12, 2022
  7. real-or-random force-pushed on Mar 12, 2022
  8. real-or-random commented at 10:55 AM on March 12, 2022: contributor

    Ready for review (if CI passes...)

  9. real-or-random marked this as ready for review on Mar 12, 2022
  10. real-or-random force-pushed on Mar 12, 2022
  11. real-or-random commented at 10:09 AM on March 13, 2022: contributor

    This will probably conflict with #1088. Let's first try to merge the fix there.

    edit: rebased on top of #1088.

  12. real-or-random force-pushed on Mar 14, 2022
  13. real-or-random force-pushed on Mar 14, 2022
  14. real-or-random force-pushed on Mar 15, 2022
  15. real-or-random force-pushed on Mar 15, 2022
  16. real-or-random force-pushed on Mar 16, 2022
  17. real-or-random force-pushed on Mar 16, 2022
  18. real-or-random force-pushed on Mar 16, 2022
  19. bench: Make benchmarks compile on MSVC 1a6be5745f
  20. configure: Output message when checking for valgrind cca8cbbac8
  21. real-or-random force-pushed on Mar 16, 2022
  22. real-or-random force-pushed on Mar 16, 2022
  23. configure: Don't abort if the compiler does not define __STDC__
    This removes a check for $ac_cv_prog_cc_c89 which is set by AC_PROG_CC
if defined(__STDC__) in the preprocessor. (Standard compliant compilers
are supposed to define __STDC__ to 1 but the value is actually not
checked here.)

Unfortunately, MSVC doesn't define it, so configure fails for MSVC.

This check is not very useful in practice. Over 30 years after C89 has
been released, there are no C compilers out there that are not
sufficiently compliant with C89 for the project. The only practically
relevant case was that the check rejected C++ compilers. A different
method to reject C++ compilers will be introduced in a later commit.
    1cc0941414
  24. build: Reject C++ compilers in the preprocessor 3b4f3d0d46
  25. configure: Add a few CFLAGS for MSVC 09f3d71c51
  26. real-or-random force-pushed on Mar 17, 2022
  27. real-or-random force-pushed on Mar 18, 2022
  28. sipa commented at 6:18 PM on June 8, 2022: contributor

    utACK c16afd001ac441c7729475fa18bcea60ef2bad2f

  29. in .cirrus.yml:260 in c16afd001a outdated 
     259 |      EXPERIMENTAL: yes
 260 |      SCHNORRSIG: yes
 261 |      CTIMETEST: no
 262 | +  matrix:
 263 | +    - name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
 264 | +      env:
    jonasnick commented at 9:24 AM on June 23, 2022:

    There are a few trailing whitespaces in cirrus.yml and cirrus.sh.

    real-or-random commented at 8:48 AM on June 29, 2022:

    fixed

  30. in src/modules/schnorrsig/bench_impl.h:99 in c16afd001a outdated 
      96 | -    free(data.msgs);
  97 | -    free(data.sigs);
  98 | +    free((void *)data.keypairs);
  99 | +    free((void *)data.pk);
 100 | +    free((void *)data.msgs);
 101 | +    free((void *)data.sigs);
    jonasnick commented at 9:25 AM on June 23, 2022:

    Perhaps add comment so this isn't changed back at the next best opportunity?

    real-or-random commented at 9:12 AM on June 29, 2022:

    done

  31. in src/secp256k1.c:16 in c16afd001a outdated 
      11 | + * because it is still good to know if this will lead to real trouble, so
  12 | + * there is a possibility to override the check. But be warned that
  13 | + * compiling with a C++ compiler is not supported. */
  14 | +#if defined(__cplusplus) && !defined(SECP256K1_CPLUSPLUS_TEST_OVERRIDE)
  15 | +#error Trying to compile a C project with a C++ compiler.
  16 | +#endif
    jonasnick commented at 9:28 AM on June 23, 2022:

    Is it intended that I don't see this error with g++ unless I also compile with -fpermissive? This is the case because g++ aborts earlier due to other errors. So if someone would accidentally use g++ (without -fpermissive) then they may just be confused vs. having a rather clear error message as before. Not saying that this is terrible, but seems like a change from the previous behavior.

    real-or-random commented at 4:35 PM on June 28, 2022:

    Hm, I see the error with g++ though it's buried within hundreds of others errors. I think it's fine as long it won't compile. And this check is better because it's independent of autotools (and will work when compiling manually, etc.)

    jonasnick commented at 5:42 PM on June 28, 2022:

    is better because it's independent of autotools

    Right, ok.

  32. in configure.ac:119 in c16afd001a outdated 
     131 | +
 132 | +    # MSVC
 133 | +    # Assume MSVC if we're building for Windows but not with GCC or compatible;
 134 | +    # libtool makes the same assumption internally.
 135 | +    # Note that "/opt" and "-opt" are equivalent for MSVC; we use "-opt" because "/opt" looks like a path.
 136 | +    if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then
    jonasnick commented at 9:31 AM on June 23, 2022:

    Instead of

    if test "x$GCC" = "xyes"; then ...
if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then ...

    why not set up the conditions like this

    if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then ...
else ...

    or

    if test "x$GCC" = "xyes"; then ...
if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then
else error("something is wrong")
    real-or-random commented at 4:49 PM on June 28, 2022:

    why not set up the conditions like this

    if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then ...
else ...

    That's certainly closer to what we have now. When I wrote this, I thought it's a good idea not to fiddle with the CFLAGS at all if this not GCC or compatible (for example, your random compiler X may not support any of these flags but also not error out and just spit out warning, so we'll add all of them wrongly).

    or

    if test "x$GCC" = "xyes"; then ...
if test x"$GCC" != x"yes" && test x"$build_windows" = x"yes"; then
else error("something is wrong")

    That would always error out on platforms other than Windows (if I'm not mistaken).

    jonasnick commented at 5:42 PM on June 28, 2022:

    thought it's a good idea not to fiddle with the CFLAGS at all if this not GCC or compatible

    Yeah that makes sense. Just wanted to make sure this wasn't by accident.

    That would always error out on platforms other than Windows

    I believe I intended the second if to be an else if. Anyway, erroring out is unnecessary.

  33. in .cirrus.yml:284 in c16afd001a outdated 
     283 | +    SCHNORRSIG: yes
 284 | +    CTIMETEST: no
 285 | +    # Set non-essential options that affect the CLI messages here.
 286 | +    # (They depend on the user's taste, so we don't want to set them automatically in configure.ac.)
 287 | +    CFLAGS: -nologo -diagnostics:caret
 288 | +    LDFLAGS: -XCClinker -nologo -XCClinker -diagnostics:caret
    jonasnick commented at 9:32 AM on June 23, 2022:

    Do you think -WX (treat warnings as errors) would be too fragile?

    real-or-random commented at 3:49 PM on June 28, 2022:

    This is done a few lines above in WERROR_CFLAGS.

    IIRC, the reason for a separate variable is that we don't want to set it in CFLAGS, at least for gcc and compatible, because that would override the default CFLAGS of -O2 -g. This will make testing the defaults difficult and then we'd need to pass -g manually. Not this PR but I can add a comment that clarifies this.

    jonasnick commented at 5:42 PM on June 28, 2022:

    Ah sorry I missed this. There's a comment in configure.ac which is sufficient imo.

    real-or-random commented at 9:15 AM on June 29, 2022:

    I added a commit that improves the comment. It took me a while to reconstruct the rationale when answering your question here, though I wrote that code.

  34. in configure.ac:40 in c16afd001a outdated 
      31 | @@ -32,17 +32,19 @@ AM_INIT_AUTOMAKE([1.11.2 foreign subdir-objects])
  32 |  # Make the compilation flags quiet unless V=1 is used.
  33 |  m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
  34 |  
  35 | -PKG_PROG_PKG_CONFIG
  36 | -
  37 |  AC_PROG_CC
  38 | -if test x"$ac_cv_prog_cc_c89" = x"no"; then
  39 | -  AC_MSG_ERROR([c89 compiler support required])
  40 | -fi
    jonasnick commented at 9:35 AM on June 23, 2022:

    Did you try the MSVC -Za flag which according to the docs should set __STDC__? (https://docs.microsoft.com/en-us/cpp/build/reference/za-ze-disable-language-extensions?view=msvc-170)

    real-or-random commented at 4:33 PM on June 28, 2022:

    Yeah, -Za is stupid because even windows headers don't adhere it to it and then the compilation of the examples fail...

    <details>

    \opt\msvc\kits\10\include\10.0.19041.0\um\winnt.h(12657,14): error C2467: illegal declaration of anonymous 'union'
            };
             ^
\opt\msvc\kits\10\include\10.0.19041.0\um\winnt.h(12874,10): error C2467: illegal declaration of anonymous 'struct'
        };
         ^
\opt\msvc\kits\10\include\10.0.19041.0\um\winnt.h(12875,6): error C2467: illegal declaration of anonymous 'union'
    };
     ^
\opt\msvc\kits\10\include\10.0.19041.0\um\winnt.h(18837,6): error C2467: illegal declaration of anonymous 'struct'
    };
     ^
\opt\msvc\kits\10\include\10.0.19041.0\um\winnt.h(21055,24): error C2133: 'Policies': unknown size
    IMAGE_POLICY_ENTRY Policies[];
                       ^
\opt\msvc\kits\10\include\10.0.19041.0\um\winioctl.h(4337,10): error C2467: illegal declaration of anonymous 'struct'
        };
         ^
\opt\msvc\kits\10\include\10.0.19041.0\um\winioctl.h(4338,6): error C2467: illegal declaration of anonymous 'union'
    };

    </details>

    We could use different flags for the examples but this will be more complex, I think.

    jonasnick commented at 5:43 PM on June 28, 2022:

    Ok, let's not make it more complex.

  35. real-or-random force-pushed on Jun 29, 2022
  36. schnorrsig bench: Suppress a stupid warning in MSVC bd81f4140a
  37. configure: Convince autotools to work with MSVC's archiver lib.exe 2be6ba0fed
  38. ci: Add MSVC builds
    This adds MSVC builds built on Linux using wine. This requires some
settings of tools and flags because the autotools support for MSVC is
naturally somewhat limited.

The advantage of this approach is that it is compatible with our
existing CI scripts, so there's no need to write a Windows CI script
(in PowerShell or similar). If we want to test building and running on
Windows native (e.g., as supported by Cirrus CI) we could still do this
in the future.

Another advantage of this approach is that contributors can simply use
the docker image if they need a MSVC installation in a non-Windows
environment.

This commit also improves the Dockerfile by grouping RUN commands
according to Docker docs:
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run
    9efc2e5221
  39. ci: Add 32-bit MinGW64 build
    This commit also raises the TEST_ITERS for wine tasks to the default.
The overhead of wine is negligible, so we can certainly afford the same
number of iterations as for native Linux tests.
    3fb3269c22
  40. ci: Run persistent wineserver to speed up wine 51f296a46c
  41. ci: Add a C++ job that compiles the public headers without -fpermissive 8dc4b03341
  42. real-or-random force-pushed on Jun 29, 2022
  43. configure: Improve rationale for WERROR_CFLAGS 49e2acd927
  44. real-or-random commented at 9:16 AM on June 29, 2022: contributor

    addressed all comments

  45. jonasnick commented at 9:30 AM on June 29, 2022: contributor

    ACK 49e2acd927ce9eb806cc10f3a1fd89a9ddd081e2

  46. jonasnick merged this on Jun 29, 2022
  47. jonasnick closed this on Jun 29, 2022
  48. dhruv referenced this in commit 2dfd7005d9 on Jul 19, 2022
  49. dhruv referenced this in commit 21e2acb595 on Jul 20, 2022
  50. dhruv referenced this in commit f70e7d8108 on Jul 20, 2022
  51. dhruv referenced this in commit 49704ff497 on Jul 20, 2022
  52. dhruv referenced this in commit d564c6266f on Jul 20, 2022
  53. dhruv referenced this in commit 908c171261 on Jul 20, 2022
  54. dhruv referenced this in commit a1ac8e1b7f on Jul 20, 2022
  55. dhruv referenced this in commit b02a78d8df on Jul 20, 2022
  56. hebasto referenced this in commit 07695f6c2d on Jul 21, 2022
  57. real-or-random referenced this in commit 7597a3d5e7 on Jul 21, 2022
  58. real-or-random referenced this in commit cabe085bb4 on Jul 21, 2022
  59. real-or-random referenced this in commit 9f8a13dc8e on Jul 21, 2022
  60. dhruv referenced this in commit e5166959a4 on Jul 21, 2022
  61. dhruv referenced this in commit 726cbfe06c on Jul 21, 2022
  62. dhruv referenced this in commit c354ccd3e6 on Jul 21, 2022
  63. dhruv referenced this in commit 296cb3807d on Jul 21, 2022
  64. dhruv referenced this in commit a7efff1c21 on Jul 22, 2022
  65. dhruv referenced this in commit 5667aa958a on Aug 12, 2022
  66. dhruv referenced this in commit 06823cfe29 on Aug 24, 2022
  67. dhruv referenced this in commit 6eca30d4bd on Sep 2, 2022
  68. dhruv referenced this in commit c3ed192dda on Sep 2, 2022
  69. dhruv referenced this in commit 89ebab0601 on Oct 1, 2022
  70. dhruv referenced this in commit d6bcb105c3 on Oct 20, 2022
  71. dhruv referenced this in commit c27eb1e66a on Oct 20, 2022
  72. dhruv referenced this in commit a2e91d2816 on Oct 20, 2022
  73. dhruv referenced this in commit 0b21533c10 on Oct 21, 2022
  74. dhruv referenced this in commit 01dddb4cf6 on Oct 21, 2022
  75. dhruv referenced this in commit a0bb5b6946 on Nov 17, 2022
  76. dhruv referenced this in commit 2e4c03dd67 on Nov 17, 2022
  77. dhruv referenced this in commit 388c9b1b55 on Nov 21, 2022
  78. dhruv referenced this in commit 244eb87643 on Dec 7, 2022
  79. dhruv referenced this in commit 92cddabc43 on Dec 8, 2022
  80. sipa referenced this in commit 9d47e7b71b on Dec 13, 2022
  81. dhruv referenced this in commit 55ffd47cc6 on Dec 14, 2022
  82. dhruv referenced this in commit 967c65b158 on Dec 14, 2022
  83. dhruv referenced this in commit 78b5ddf28b on Jan 11, 2023
  84. dhruv referenced this in commit 215394a1d5 on Jan 11, 2023
  85. div72 referenced this in commit 945b094575 on Mar 14, 2023
  86. str4d referenced this in commit 0df7b459f6 on Apr 21, 2023
  87. dderjoel referenced this in commit 3768744290 on May 23, 2023
  88. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  89. vmta referenced this in commit 8f03457eed on Jul 1, 2023
  90. delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
  91. delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
  92. backpacker69 referenced this in commit 8a5f7d0637 on Mar 5, 2024
  93. Fabcien referenced this in commit 990c94cffd on Jan 27, 2026
  94. Fabcien referenced this in commit e7021cbade on Jan 27, 2026
  95. Fabcien referenced this in commit 768c89bff6 on Feb 3, 2026
  96. Fabcien referenced this in commit b656775128 on Feb 3, 2026
  97. Fabcien referenced this in commit 53f368bff5 on Feb 5, 2026
  98. Fabcien referenced this in commit f4c3fb1147 on Feb 5, 2026
  99. Fabcien referenced this in commit 7dc1a63a14 on Mar 3, 2026
  100. Fabcien referenced this in commit 8fbbc9174c on Mar 3, 2026
Contributors
real-or-randomsipajonasnick
Linked (view graph)
#1127 Bitcoin Core `configure` fails after #1084 was merged#1128 configure: Remove pkgconfig macros again (reintroduced by mismerge)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC