Re: [PATCH 2/3] cmake: fix compilation of clar-based unit tests

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 14, 2024, 21:46 UTC
Message-ID: <Zw2RNsptlkHCyTgi@nand.local>
In-Reply-To: <b9afeffda292a068e81d05b91f759a5c53a24b15.1728914219.git.ps@pks.im>

On Mon, Oct 14, 2024 at 04:06:44PM +0200, Patrick Steinhardt wrote:

> The compilation of clar-based unit tests is broken because we do not
> add the binary directory into which we generate the "clar-decls.h" and
> "clar.suite" files as include directories. Instead, we accidentally set
> up the source directory as include directory.

I am confused. What is the difference between CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR here, and why does the difference between the two matter?

Show 10 quoted lines

> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 62af7b33d2f..093852ad9d6 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1042,7 +1042,7 @@ file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar
>  list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/")
>  list(TRANSFORM clar_test_SUITES APPEND ".c")
>  add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c")
> -target_include_directories(unit-tests-lib PRIVATE "${CMAKE_SOURCE_DIR}/t/unit-tests")
> +target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests")

This also changes the 'scope' parameter of 'target_include_directories' from PRIVATE to PUBLIC, but the commit message doesn't mention such a change.

Is it intentional? If so, can the commit message be updated to explain why this is done? If not, is this a stray change that snuck in?

(If all of this is obvious to you, I apologize for the confusion on my end. I'm not at all familiar with our CMake bits, so the extra explanation would help me quite a bit in making sense of this.)

Thanks, Taylor