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

From: Taylor Blau <[email-masked]>
Date: Mon, 14 Oct 2024 17:46:30 -0400

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?

> 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