Conversation
|
Before looking at it further, this seems to include #23640, right? Does it really need it or could you remove all the other commits except for the last one here? |
I wasn't sure, but it seems like it does, at least using Autoconf 2.69. When attempting to regenerate the configure script with If based on master directly. |
|
Sorry but I simply don't see how is this possible as the last commit doesn't do anything that materially changes anything. You must have some local changes/cached files breaking this. You should create a PR with just this commit (after fixing the problems in it which result in the CI failures). |
apteryks
force-pushed
the
support-system-catch2
branch
from
b55e838 to
2dbd68d
Compare
vadz
added
the
work needed
Maybe it's my version of Autoconf (2.69); there shouldn't be anything cached in my environment: I'll try cherry picking the single commit I expect to fix this (stop versioning Autoconf-provided M4 files). |
apteryks
force-pushed
the
support-system-catch2
branch
2 times, most recently
from
2974386 to
3f20bb6
Compare
* configure.in (--with-catch2): New option. [PKG_CONFIG]: Search catch2 via pkg-config, fall back to use bundled copy. Add a new WX_TEST_CXXFLAGS Autoconf variable to configure the include directory. * build/bakefiles/config.bkl (WX_TEST_CXXFLAGS): Add option. * tests/test.bkl [autoconf]: Add WX_TEST_CXXFLAGS option to cxxflags. * configure: Regenerate via 'make -f build/autogen.mk'. * tests/Makefile.in: Likewise.
apteryks
force-pushed
the
support-system-catch2
branch
from
3f20bb6 to
7c2019f
Compare
vadz
left a comment
vadz
left a comment
There was a problem hiding this comment.
Thanks for the update but unfortunately there are still some things that could be improved.
I also think that it would make sense to default to built-in version: unlike the other libraries, this is not used for the binaries being distributed, so I don't see any drawbacks to using the built-in version by default and it would avoid warnings about not finding the system one, which would be annoying to see.
| <option name="WX_CFLAGS"/> | ||
| <option name="WX_CXXFLAGS"/> | ||
| <option name="WX_LDFLAGS"/> | ||
| <option name="WX_TEST_CXXFLAGS"/> |
There was a problem hiding this comment.
I think it's a bit misleading to call it like this, it should probably be called CATCH2_CXXFLAGS or something like this instead.
| $srcdir/3rdparty/catch/single_include/catch2/catch.hpp couldn't be found. | ||
| dnl Check for catch (C++ Automated Test Cases in Headers) availability. | ||
| if test "$wxUSE_CATCH2" != "builtin"; then | ||
| if test -n "$PKG_CONFIG"; then |
There was a problem hiding this comment.
We don't test for PKG_CONFIG anywhere else, so either we shouldn't do it here neither or we should do it in all the other places too... But is it even possible for it to be empty? I.e. won't configure stop if PKG_PROG_PKG_CONFIG doesn't find it?
| if test -n "$PKG_CONFIG"; then | ||
| dnl TODO: Support catch2 version 3, which does not | ||
| dnl ship a catch2/catch.hpp header anymore. | ||
| PKG_CHECK_MODULES([CATCH2], [catch2 < 3],, [ |
There was a problem hiding this comment.
We should use CATCH2_CFLAGS somewhere, otherwise compilation will fail if it's not installed under /usr/include.
|
This PR has been in "work needed" state for more than a year and will be closed soon. Please update it, e.g. by commenting here, if anybody intends to resume working on it. |
|
Just FYI, I'll fix and apply it, finally. |
Add --with-catch2 configure option which can be used with "sys" value to use the system version of CATCH2. By default, still use the built-in version, however, as it doesn't have any real drawbacks. Closes wxWidgets#23654. Co-authored-by: Vadim Zeitlin <[email protected]>
Thank you. Sorry for dropping the ball. |
This was broken in 01ca081 (Allow using system CATCH2 library in configure, 2023-06-15) where a wrong include path was used in this case. See wxWidgets#23654.
This was broken in 01ca081 (Allow using system CATCH2 library in configure, 2023-06-15) where a wrong include path was used in this case. See wxWidgets#23654.
This was broken in 01ca081 (Allow using system CATCH2 library in configure, 2023-06-15) where a wrong include path was used in this case. See wxWidgets#23654.
2 participants
Teach the Autoconf/Bakefile build system to use catch2 from the system.