| Message ID | b9afeffda292a068e81d05b91f759a5c53a24b15.1728914219.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a4f8a59ddc2718cc2d87f076289fbbef4485f65f |
| Headers | show |
| Series | [1/3] Makefile: extract script to generate clar declarations | expand |
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
On Mon, Oct 14, 2024 at 05:46:30PM -0400, Taylor Blau wrote: > 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? This is for out-of-tree builds. The outputs generated by CMake are written into the binary directory, which is not the source directory where the Git source files are stored. > > 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. It does mention it, it's the "propagate the include directories" part. > 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.) That's fair. I'll clarify the message a bit to provide more context. Patrick
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") add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c") target_link_libraries(unit-tests unit-tests-lib common-main) set_target_properties(unit-tests
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. Fix this and propagate the include directories of "unit-tests.lib" to the "unit-tests" executable so that the latter uses the same include directories. Reported-by: Ed Reel <edreel@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> --- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)