fpagliughi / sockpp

Modern C++ socket library.
BSD 3-Clause "New" or "Revised" License
769 stars 126 forks source link

Build failed with -DSOCKPP_BUILD_TESTS=ON #90

Closed Peter2121 closed 7 months ago

Peter2121 commented 7 months ago

Building sockpp on FreeBSD 13.2 with catch2 installed from package. The tests build is failed: /tests/unit/unit_tests.cpp:59:10: fatal error: 'catch2/catch.hpp' file not found

Indeed, this file is not present in system includes directory:

# ls /usr/local/include/catch2/*.hpp
/usr/local/include/catch2/catch_all.hpp
/usr/local/include/catch2/catch_approx.hpp
/usr/local/include/catch2/catch_assertion_info.hpp
/usr/local/include/catch2/catch_assertion_result.hpp
/usr/local/include/catch2/catch_config.hpp
/usr/local/include/catch2/catch_get_random_seed.hpp
/usr/local/include/catch2/catch_message.hpp
/usr/local/include/catch2/catch_section_info.hpp
/usr/local/include/catch2/catch_session.hpp
/usr/local/include/catch2/catch_tag_alias_autoregistrar.hpp
/usr/local/include/catch2/catch_tag_alias.hpp
/usr/local/include/catch2/catch_template_test_macros.hpp
/usr/local/include/catch2/catch_test_case_info.hpp
/usr/local/include/catch2/catch_test_macros.hpp
/usr/local/include/catch2/catch_test_spec.hpp
/usr/local/include/catch2/catch_timer.hpp
/usr/local/include/catch2/catch_tostring.hpp
/usr/local/include/catch2/catch_totals.hpp
/usr/local/include/catch2/catch_translate_exception.hpp
/usr/local/include/catch2/catch_user_config.hpp
/usr/local/include/catch2/catch_version_macros.hpp
/usr/local/include/catch2/catch_version.hpp

According to port's plist, this file is not installed by the catch2 port

According to this article, this header is removed in catch v3, and the version installed by FreeBSD port/package is 3.4.0.

Do you plan to adjust sockpp to reflect the modifications in catch?

fpagliughi commented 7 months ago

This was fixed by PR #84, and merged into the 'develop' branch, but not yet promoted to 'master' nor released. Development is still a bit unstable.

But with this fix, the build system will determine if Catch2 is v2.x or v3.x and include the proper header file.

Unfortunately, I have a few other open-source projects queued for release before I get to this one, so it will likely be a few months before this is released. In the meantime, if you really need to run the tests, you can build against v2.x or perhaps try to cherry-pick the PR into your local build.

Peter2121 commented 7 months ago

If I understand this patch correctly, in case of catch2 v3 we need just to replace #include "catch2/catch.hpp" by #include "catch2/catch_all.hpp" in all test/unit/*.cpp files?

In such case I would just add such patches into the ports tree waiting for the new release of sockpp. We don't need a support for building with <v3 in ports as the version of catch2 is fixed by the version of catch2 port. So, the patch can be relatively simple.

fpagliughi commented 7 months ago

Yes. The patch (PR) just uses CMake to discover if v2.x or >=v3.x is installed and sets a flag for which of those two headers to include in all the test source files.

The tests themselves just use a minimal set of features that are compatible with either version.

fpagliughi commented 7 months ago

I assume this is resolved? If not, feel free to re-open.

Peter2121 commented 7 months ago

It seems that the patch is not included in the last release. Please, drop some lines here when you release a version with this patch included, so we remove the patch from port.

fpagliughi commented 7 months ago

I just added a few small bug fixes to the v0.8.x line, holding off on new features until v0.9 which should be out soon. (I would consider adding support for Catch v3 as a new "feature").

But maybe the PR would cherry pick cleanly. I'll give it a try, and if it works, I'll push it out.

fpagliughi commented 7 months ago

Done.

v0.8.3 supports Catch2 versions 2.x and 3.x