build: 45839th attempt to fix symbol visibility on Windows #1595

pull real-or-random wants to merge 1 commits into bitcoin-core:master from real-or-random:202408-win-visibility changing 1 files +12 −2
  1. real-or-random commented at 11:49 PM on August 24, 2024: contributor

    Fixes #1421. See code comments for rationale.

    Related meta-bug: #1181. This reminds me that we should move forward with #1359.

  2. real-or-random added the label build on Aug 24, 2024
  3. real-or-random force-pushed on Aug 25, 2024
  4. real-or-random force-pushed on Aug 25, 2024
  5. fanquake commented at 10:13 AM on September 6, 2024: member

    I can at least confirm that this "fixes" the output I was seeing in #1421. The most recent comment on the upstream GCC issue is not 100% clear to me.

  6. real-or-random commented at 7:16 PM on September 9, 2024: contributor

    The most recent comment on the upstream GCC issue is not 100% clear to me.

    Can you elaborate on which part is unclear?

    In any case, I could rephrase this such that it says it's a GCC bug. When I wrote it, I wasn't sure yet, but the upstream response seems to confirm that this is a bug.

  7. fanquake commented at 8:45 AM on September 10, 2024: member

    I've re-read, and yes, it seems like the conclusion is that it is a GCC issue. I guess what is actually unclear is if/when anyone might fix it upstream.

  8. include: Avoid visibility("default") on Windows
    Fixes #1421.
    447334cb06
  9. real-or-random force-pushed on Sep 17, 2024
  10. real-or-random commented at 2:23 PM on September 17, 2024: contributor

    Okay. I've nevertheless rephrased the comment to make it (hopefully) clearer.

  11. fanquake commented at 12:21 PM on October 15, 2024: member

    ACK 447334cb06de229024161021cc79b3af32ce8b5c

  12. real-or-random commented at 2:01 PM on October 15, 2024: contributor

    @hebasto Wanna review this? :)

  13. fanquake commented at 8:37 AM on October 16, 2024: member

    also cc @theuni

  14. in include/secp256k1.h:151 in 447334cb06 
     146 | @@ -147,6 +147,15 @@ typedef int (*secp256k1_nonce_function)(
 147 |       * 1. If using Libtool, it defines DLL_EXPORT automatically.
 148 |       * 2. In other cases, SECP256K1_DLL_EXPORT must be defined. */
 149 |  #   define SECP256K1_API extern __declspec (dllexport)
 150 | +#  else
 151 | +    /* Building libsecp256k1 as a static library on Windows.
    hebasto commented at 2:33 PM on October 16, 2024:

    nit: Referencing the use of a static library does not apply to our cases with the tests and noverify_tests targets using CMake, where object files are linked directly.

    real-or-random commented at 2:14 PM on October 17, 2024:

    You mean https://github.com/bitcoin-core/secp256k1/blob/master/src/CMakeLists.txt#L89, right? True, and it's also true for bench_internal and bench_ecmult.

    Would you prefer "Building libsecp256k1 not as a DLL."? (I also dropped the Windows part because it's anyway only Windows in the outer #ifdef.)

    hebasto commented at 4:20 PM on October 17, 2024:

    You mean https://github.com/bitcoin-core/secp256k1/blob/master/src/CMakeLists.txt#L89, right? True, and it's also true for bench_internal and bench_ecmult.

    Would you prefer "Building libsecp256k1 not as a DLL."? (I also dropped the Windows part because it's anyway only Windows in the outer #ifdef.)

    Looks good. However, I'm not sure if it worth invalidating all ACKs.

    real-or-random commented at 4:02 PM on October 21, 2024:

    Looks good. However, I'm not sure if it worth invalidating all ACKs.

    Ok, let's just get it merged.

  15. hebasto approved
  16. hebasto commented at 2:33 PM on October 16, 2024: member

    ACK 447334cb06de229024161021cc79b3af32ce8b5c, tested on Ubuntu 24.04 using the following commands:

    $ env CFLAGS="-flto" cmake -B build --preset dev-mode --toolchain cmake/x86_64-w64-mingw32.toolchain.cmake
$ cmake --build build -j 16 -t tests noverify_tests
  17. theuni approved
  18. theuni commented at 3:59 PM on October 16, 2024: contributor

    ACK 447334cb06de229024161021cc79b3af32ce8b5c

  19. real-or-random merged this on Oct 21, 2024
  20. real-or-random closed this on Oct 21, 2024
Contributors
real-or-randomfanquakehebastotheuni
Labels
build
Linked (view graph)
#1181 Symbol visibility#1359 Fix symbol visibility issues, add test for it#1421 mingw-w64: "visibility attribute not supported in this configuration; ignored" under `-flto`
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-04-22 18:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me