CMake missing OpenSSL dependency

[alexreinking]

CMake missing OpenSSL dependency

[keithw]

Hmm, I'm confused by this report and couldn't immediately replicate it. We don't see to use find_dependency() anywhere in all of wabt. We do have find_package(OpenSSL QUIET) in CMakeLists.txt, but it does seem to work for me on a GNU/Linux machine with OpenSSL installed:

$ cmake -S . -B build
-- The C compiler identification is GNU 13.2.0
-- The CXX compiler identification is GNU 13.2.0

[...]
-- Looking for openssl/sha.h
-- Looking for openssl/sha.h - found
-- Using OpenSSL libcrypto for SHA-256
[...]
-- Configuring done (2.5s)
-- Generating done (0.1s)
-- Build files have been written to: /home/keithw/stanford/wabt/build

It also seems to work fine in CI on Mac (and finds the OpenSSL there and uses it).

We might want to just move from the current SHA-256 setup (which uses either external OpenSSL or vendored PicoSHA2) to a world where it's always vendored BLAKE3. Because that will probably lead to fewer questions about external dependencies. But I can't immediately replicate this report... :-/

[keithw]

What is the failure mode you see here -- does it fail to build? Or does it successfully fall back to the vendored/internal PicoSHA2?

[alexreinking]

This happens in downstreams (e.g. Halide, where I observed the issue) when going through:

find_package(wabt REQUIRED)
target_link_libraries(mylib PRIVATE wabt::wabt)

This doesn't affect the main build, which uses find_package instead.

There's no test for this in CI, unfortunately. When testing out of the build directory, the OpenSSL libraries are already linked into the test apps.

[keithw]

Are you able to contribute a CI test that exercises this configuration so we can make sure not to break it? Honestly I think we should probably just eliminate the OpenSSL dependency entirely, but whatever we do should not break users like you.

[alexreinking]

I will be able to... eventually. I'm waiting for my company to approve contributing under the W3 CLA that the README indicates is required.

[keithw]

AFAIK we don't actually require or verify CG membership before merging PRs on this repo (and certainly not for something like a CI config that helps somebody).