Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link bcrypt in CMake tests on MSVC #147

Closed
wants to merge 1 commit into from

Conversation

13steinj
Copy link

See #68, #78, #122, related boostorg/boost_install#54.


@pdimov I think you unintentionally broke the cmake tests for 1.85.0 on msvc; given you're involved in the above linked issues ;)

Needed for orgs/individuals that run boost tests in CI, e.g., sanity check to verify that internal / to-be-upstreamed patches haven't broken other behavior.

@pdimov
Copy link
Member

pdimov commented Apr 19, 2024

But the Windows CMake tests are passing, see https://github.com/boostorg/uuid/actions/runs/8150026167.

@13steinj
Copy link
Author

I can't confirm that the following is the cause for at least a few days unfortunately, but I suspect we end up disabling the autolink feature somehow.

I don't know if the cmake tests here should be relying on it, but I would guess not.

@pdimov
Copy link
Member

pdimov commented Apr 19, 2024

You are probablu defining BOOST_ALL_NO_LIB somewhere, which disables UUID's autolinking to bcrypt.lib.

That UUID uses the Boost autolink mechanism to autolink to bcrypt.lib is highly questionable in itself, because it's only meant for autolinking to Boost libraries, not to system ones. So I'll probably change that to use #pragma comment directly and to not look at BOOST_ALL_NO_LIB, although I'm sure this will invite its share of objections from people who do use BOOST_ALL_NO_LIB to disable all autolink.

In either case, this isn't quite the right fix. The Boost:uuid target should link to bcrypt, and not when the compiler ID is MSVC, but when the platform is Windows.

Although that's not entirely correct either because UUID can use either Bcrypt or Wincrypt and that's determined at compile time.

@13steinj
Copy link
Author

In either case, this isn't quite the right fix. The Boost:uuid target should link to bcrypt, and not when the compiler ID is MSVC, but when the platform is Windows.

Happy to make that change. It's been so busy that I didn't notice a top level CMakeLists.txt and target was added in ~1.77

Although that's not entirely correct either because UUID can use either Bcrypt or Wincrypt and that's determined at compile time.

I was under the impression that wincrypt / MS CryptoAPI was deprecated, and that the alternative (CNG) was introduced with Vista aka 2007 (and possibly, is the same time the CryptoAPI was deprecated). I would suggest that support of use of such an old lib not be maintained; AFAIK even tools on the larger side of things like FFmpeg made the switch years ago.

pdimov added a commit that referenced this pull request Apr 20, 2024
pdimov added a commit that referenced this pull request Apr 20, 2024
@pdimov
Copy link
Member

pdimov commented Apr 20, 2024

I've enabled autolinking to bcrypt.lib even when BOOST_ALL_NO_LIB is defined, which should take care of the issue for now, until a more permanent solution emerges.

@pdimov pdimov closed this Apr 20, 2024
@13steinj 13steinj deleted the cmake-tests-bcrypt branch April 30, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants