build: Use SECP256K1_STATICLIB macro instead of warning suppressions

[hebasto]

The changes from #1209 has been described as:

Fixed declarations of API variables for MSVC (__declspec(dllimport)). This fixes MSVC builds of programs which link against a libsecp256k1 DLL dynamically and use API variables (and not only API functions). Unfortunately, the MSVC linker now will emit warning LNK4217 when trying to link against libsecp256k1 statically. Pass /ignore:4217 to the linker to suppress this warning.

Apparently, this description is not complete. When building Bitcoin Core, the other warning is raised as well:

LINK : warning LNK4217: symbol 'secp256k1_context_static' defined in 'libsecp256k1.lib(secp256k1.obj)' is imported by 'libbitcoin_consensus.lib(pubkey.obj)' in function '"public: static bool __cdecl CPubKey::CheckLowS(class std::vector<unsigned char,class std::allocator<unsigned char> > const &)" (?CheckLowS@CPubKey@@SA_NAEBV?$vector@EV?$allocator@E@std@@@std@@@Z)' [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
LINK : warning LNK4286: symbol 'secp256k1_context_static' defined in 'libsecp256k1.lib(secp256k1.obj)' is imported by 'libbitcoin_common.lib(key.obj)' [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin\test_bitcoin.vcxproj]

This PR provides to the user of a static libsecp256k1 library an option to define the SECP256K1_STATICLIB macro instead of ignoring MSVC linker warnings LNK4217 and LNK4286.

[real-or-random]

Concept ACK

The macro is cheap, and if it helps Core get a cleaner build, then we should just add it. Many hours of researching and experimenting went into that piece of code, so I'll probably follow up with a PR that adds extensive comments.

---

Apparently, this description is not complete. When building Bitcoin Core, the other warning is raised as well:

LINK : warning LNK4217: symbol 'secp256k1_nonce_function_rfc6979' defined in 'libsecp256k1.lib(secp256k1.obj)' is imported by 'libbitcoin_common.lib(key.obj)' in function '"public: bool __cdecl CKey::Sign(class uint256 const &,class std::vector<unsigned char,class std::allocator<unsigned char> > &,bool,unsigned int)const " (?Sign@CKey@@QEBA_NAEBVuint256@@AEAV?$vector@EV?$allocator@E@std@@@std@@_NI@Z)' [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin\test_bitcoin.vcxproj]

Do you mean the LNK4286 is also raised? I see that one in the logs.

[hebasto]

Do you mean the LNK4286 is also raised? I see that one in the logs.

Yes. I put the wrong line into the PR description. Updated.

[hebasto]

When building Bitcoin Core, the other warning is raised as well:

LINK : warning LNK4217: symbol 'secp256k1_context_static' defined in 'libsecp256k1.lib(secp256k1.obj)' is imported by 'libbitcoin_consensus.lib(pubkey.obj)' in function '"public: static bool __cdecl CPubKey::CheckLowS(class std::vector<unsigned char,class std::allocator<unsigned char> > const &)" (?CheckLowS@CPubKey@@SA_NAEBV?$vector@EV?$allocator@E@std@@@std@@@Z)' [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin\test_bitcoin.vcxproj]
LINK : warning LNK4286: symbol 'secp256k1_context_static' defined in 'libsecp256k1.lib(secp256k1.obj)' is imported by 'libbitcoin_common.lib(key.obj)' [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\test_bitcoin\test_bitcoin.vcxproj]

This PR provides to the user of a static libsecp256k1 library an option to define the SECP256K1_STATICLIB macro instead of ignoring MSVC linker warnings LNK4217 and LNK4286.

@sipsorcery Friendly ping :)

As a Windows connoisseur, what do you think?

[sipsorcery]

As a Windows user I'm used to just ignoring crap from the OS so I would have stopped at adding an option to ignore the warnings.

This PR is a step further and makes it even cleaner so looks good to me.

ACK 6177b8a.

[hebasto]

Closing in favor of #1362.