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

CMake missing OpenSSL dependency #2446

Closed
alexreinking opened this issue Jul 24, 2024 · 7 comments
Closed

CMake missing OpenSSL dependency #2446

alexreinking opened this issue Jul 24, 2024 · 7 comments

Comments

@alexreinking
Copy link
Contributor

alexreinking commented Jul 24, 2024

When wabt depends on OpenSSL, it fails to find_dependency() the package in its config files. This can be observed when installing wabt via Homebrew; we get the error:

CMake Error at /opt/homebrew/lib/cmake/wabt/wabt-targets.cmake:61 (set_target_properties):
  The link interface of target "wabt::wabt" contains:

    OpenSSL::Crypto

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

This could be fixed by, e.g. adding the following snippet to scripts/wabt-config.cmake.in, just after @PACKAGE_INIT@:

if ("@HAVE_OPENSSL_SHA_H@")
    include(CMakeFindDependencyMacro)
    find_dependency(OpenSSL)
endif ()

If you fix this, pretty please cut a new release 🙏

@keithw
Copy link
Member

keithw commented Jul 24, 2024

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
Copy link
Member

keithw commented Jul 24, 2024

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
Copy link
Contributor Author

alexreinking commented Jul 24, 2024

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
Copy link
Member

keithw commented Jul 24, 2024

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
Copy link
Contributor Author

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
Copy link
Member

keithw commented Jul 25, 2024

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).

@alexreinking
Copy link
Contributor Author

Fixed by #2447

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

No branches or pull requests

2 participants