[Spice-devel] [spice-streaming-agent PATCH V2] catch: check for any of catch and catch2

[Frediano Ziglio]

> Catch2 is now in upstream and Fedora (since Fedora 27)
> 
> Signed-off-by: Uri Lublin <uril at redhat.com>
> ---
> 
> Since v1:
>   - check both catch2 and catch (in v1 only catch2 was checked)
> 
> Poeple may find the indentation in configure.ac a bit confusing
> (the second AC_CHECK_HEADER, is in the else block of the first,
>  and not in the else block of AS_IF). I think it's easy to see
> that way that the only difference between catch2 and catch.
> Let me know if it's important and I'll change the indentation.
> 
> ---
>  configure.ac                          | 9 +++++++--
>  src/unittests/Makefile.am             | 1 +
>  src/unittests/test-mjpeg-fallback.cpp | 2 +-
>  src/unittests/test-stream-port.cpp    | 2 +-
>  4 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index c259f7e..9f4fd22 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -119,9 +119,14 @@ case "$enable_tests" in
>    *) AC_MSG_ERROR([bad value ${enable_tests} for enable-tests option]) ;;
>  esac
>  AS_IF([test "x$enable_tests" != "xno"],
> -      [AC_CHECK_HEADER([catch/catch.hpp],have_check="yes",)])
> +      [AC_CHECK_HEADER([catch2/catch.hpp],
> CATCH_CFLAGS=-I/usr/include/catch2,
> +      [AC_CHECK_HEADER([catch/catch.hpp],
> CATCH_CFLAGS=-I/usr/include/catch)])])

These checks assume that if the header is found it must be under /usr/include
which is not the case. User can install "manually" catch and pass CPPFLAGS
options for instance.
It's a pity that AC_CHECK_HEADER does not define where it found that header
but probably it just pass some flags to the compiler and let the compiler
find the header.

> +
> +AC_SUBST([CATCH_CFLAGS])
> +AS_IF([test "x$CATCH_CFLAGS" != "x"],
> +      [have_check="yes"])
>  AS_IF([test "x$enable_tests" = "xyes" && test "x$have_check" != "xyes"],
> -      [AC_MSG_ERROR([Could not find Catch dependency header
> (catch/catch.hpp)])])
> +      [AC_MSG_ERROR([Could not find Catch dependency header (catch.hpp)])])
>  AM_CONDITIONAL([ENABLE_TESTS], [test "x$have_check" = "xyes"])
>  
>  AC_DEFINE_DIR([BINDIR], [bindir], [Where binaries are installed.])
> diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> index 8ce1f7a..cce546d 100644
> --- a/src/unittests/Makefile.am
> +++ b/src/unittests/Makefile.am
> @@ -7,6 +7,7 @@ AM_CPPFLAGS = \
>  	-I$(top_srcdir)/src/unittests \
>  	$(DRM_CFLAGS) \
>  	$(SPICE_PROTOCOL_CFLAGS) \
> +	$(CATCH_CFLAGS) \
>  	$(NULL)
>  
>  AM_CFLAGS = \
> diff --git a/src/unittests/test-mjpeg-fallback.cpp
> b/src/unittests/test-mjpeg-fallback.cpp
> index e39dc49..a8d2cef 100644
> --- a/src/unittests/test-mjpeg-fallback.cpp
> +++ b/src/unittests/test-mjpeg-fallback.cpp
> @@ -1,5 +1,5 @@
>  #define CATCH_CONFIG_MAIN
> -#include "catch/catch.hpp"
> +#include <catch.hpp>
>  
>  #include "mjpeg-fallback.hpp"
>  
> diff --git a/src/unittests/test-stream-port.cpp
> b/src/unittests/test-stream-port.cpp
> index e7b7b89..01e3820 100644
> --- a/src/unittests/test-stream-port.cpp
> +++ b/src/unittests/test-stream-port.cpp
> @@ -5,7 +5,7 @@
>   */
>  
>  #define CATCH_CONFIG_MAIN
> -#include <catch/catch.hpp>
> +#include <catch.hpp>
>  #include <sys/socket.h>
>  #include <signal.h>
>  

Beside that patch seems fine to me.
What about something like this: https://paste.fedoraproject.org/paste/cPE3WoGBXKSuRLk1XYriIw ?

Frediano