diff mbox series

[v2,5/5] cmake: set up proper dependencies for generated clar headers

Message ID bb005979e7eb335b0178094251b5c37682d7d47b.1729506329.git.ps@pks.im (mailing list archive)
State Accepted
Commit 30bf9f0aaa9acc0712210fe5908a13234f1efd45
Headers show
Series t/unit-tests: improve clar platform compatibility | expand

Commit Message

Patrick Steinhardt Oct. 21, 2024, 10:56 a.m. UTC 
The auto-generated headers used by clar are written at configure time
and thus do not get regenerated automatically. Refactor the build
recipes such that we use custom commands instead, which also has the
benefit that we can reuse the same infrastructure as our Makefile.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
 1 file changed, 12 insertions(+), 38 deletions(-)

Comments

Johannes Schindelin Nov. 5, 2024, 7:55 p.m. UTC | #1 
Hi Patrick,

On Mon, 21 Oct 2024, Patrick Steinhardt wrote:

> The auto-generated headers used by clar are written at configure time
> and thus do not get regenerated automatically. Refactor the build
> recipes such that we use custom commands instead, which also has the
> benefit that we can reuse the same infrastructure as our Makefile.

For the record: I did not use a shell script to generate the header for a
specific reason: Unix shell scripts are not native to Windows. Therefore
they cannot in general be run on Windows, however that was precisely the
idea for the CMake definition: to be run on a vanilla Windows with Visual
Studio installed.

Sadly, even Git's CI definition sets things up in a way that Git for
Windows' Bash can be used in the CMake definition, but in the intended use
case (opening a checkout of git/git in Visual Studio without any further
tools required) won't have a usable Bash.

Therefore I am unsure whether this patch is desirable.

Ciao,
Johannes

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
>  1 file changed, 12 insertions(+), 38 deletions(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 093852ad9d6..9f80ab92656 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})
>  endforeach()
>
>  parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
> -
> -set(clar_decls "")
> -set(clar_cbs "")
> -set(clar_cbs_count 0)
> -set(clar_suites "static struct clar_suite _clar_suites[] = {\n")
> -list(LENGTH clar_test_SUITES clar_suites_count)
> -foreach(suite ${clar_test_SUITES})
> -	file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls
> -		REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$")
> -
> -	list(LENGTH decls decls_count)
> -	string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" "    { \"\\2\", &\\1 },\n" cbs ${decls})
> -	string(JOIN "" cbs ${cbs})
> -	list(TRANSFORM decls PREPEND "extern ")
> -	string(JOIN ";\n" decls ${decls})
> -
> -	string(APPEND clar_decls "${decls};\n")
> -	string(APPEND clar_cbs
> -		"static const struct clar_func _clar_cb_${suite}[] = {\n"
> -		${cbs}
> -		"};\n")
> -	string(APPEND clar_suites
> -		"    {\n"
> -		"        \"${suite}\",\n"
> -		"        { NULL, NULL },\n"
> -		"        { NULL, NULL },\n"
> -		"        _clar_cb_${suite}, ${decls_count}, 1\n"
> -		"    },\n")
> -	math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}")
> -endforeach()
> -string(APPEND clar_suites
> -	"};\n"
> -	"static const size_t _clar_suite_count = ${clar_suites_count};\n"
> -	"static const size_t _clar_callback_count = ${clar_cbs_count};\n")
> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}")
> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}")
> -
>  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")
> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
> +	COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES}
> +	DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})
> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> +	COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> +	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
> +
> +add_library(unit-tests-lib ${clar_test_SUITES}
> +	"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> +)
>  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)
> --
> 2.47.0.72.gef8ce8f3d4.dirty
>
>
Phillip Wood Nov. 6, 2024, 10:59 a.m. UTC | #2 
Hi Johannes

On 05/11/2024 19:55, Johannes Schindelin wrote:
> Hi Patrick,
> 
> On Mon, 21 Oct 2024, Patrick Steinhardt wrote:
> 
>> The auto-generated headers used by clar are written at configure time
>> and thus do not get regenerated automatically. Refactor the build
>> recipes such that we use custom commands instead, which also has the
>> benefit that we can reuse the same infrastructure as our Makefile.
> 
> For the record: I did not use a shell script to generate the header for a
> specific reason: Unix shell scripts are not native to Windows. Therefore
> they cannot in general be run on Windows, however that was precisely the
> idea for the CMake definition: to be run on a vanilla Windows with Visual
> Studio installed.
> 
> Sadly, even Git's CI definition sets things up in a way that Git for
> Windows' Bash can be used in the CMake definition, but in the intended use
> case (opening a checkout of git/git in Visual Studio without any further
> tools required) won't have a usable Bash.
> 
> Therefore I am unsure whether this patch is desirable.

CMakeLists.txt tries to find sh.exe from git-for-windows and errors out 
if it cannot be found. It then uses that shell to run a number of 
scripts. Perhaps we should do the same in this patch? It would certainly 
be a worthwhile improvement to regenerate this file at build time if the 
source has changed.

Best Wishes

Phillip

> Ciao,
> Johannes
> 
>>
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>> ---
>>   contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------
>>   1 file changed, 12 insertions(+), 38 deletions(-)
>>
>> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
>> index 093852ad9d6..9f80ab92656 100644
>> --- a/contrib/buildsystems/CMakeLists.txt
>> +++ b/contrib/buildsystems/CMakeLists.txt
>> @@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})
>>   endforeach()
>>
>>   parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
>> -
>> -set(clar_decls "")
>> -set(clar_cbs "")
>> -set(clar_cbs_count 0)
>> -set(clar_suites "static struct clar_suite _clar_suites[] = {\n")
>> -list(LENGTH clar_test_SUITES clar_suites_count)
>> -foreach(suite ${clar_test_SUITES})
>> -	file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls
>> -		REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$")
>> -
>> -	list(LENGTH decls decls_count)
>> -	string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" "    { \"\\2\", &\\1 },\n" cbs ${decls})
>> -	string(JOIN "" cbs ${cbs})
>> -	list(TRANSFORM decls PREPEND "extern ")
>> -	string(JOIN ";\n" decls ${decls})
>> -
>> -	string(APPEND clar_decls "${decls};\n")
>> -	string(APPEND clar_cbs
>> -		"static const struct clar_func _clar_cb_${suite}[] = {\n"
>> -		${cbs}
>> -		"};\n")
>> -	string(APPEND clar_suites
>> -		"    {\n"
>> -		"        \"${suite}\",\n"
>> -		"        { NULL, NULL },\n"
>> -		"        { NULL, NULL },\n"
>> -		"        _clar_cb_${suite}, ${decls_count}, 1\n"
>> -		"    },\n")
>> -	math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}")
>> -endforeach()
>> -string(APPEND clar_suites
>> -	"};\n"
>> -	"static const size_t _clar_suite_count = ${clar_suites_count};\n"
>> -	"static const size_t _clar_callback_count = ${clar_cbs_count};\n")
>> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}")
>> -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}")
>> -
>>   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")
>> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>> +	COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES}
>> +	DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})
>> +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> +	COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> +	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
>> +
>> +add_library(unit-tests-lib ${clar_test_SUITES}
>> +	"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
>> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>> +	"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> +)
>>   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)
>> --
>> 2.47.0.72.gef8ce8f3d4.dirty
>>
>>
>
Patrick Steinhardt Nov. 8, 2024, 12:59 p.m. UTC | #3 
On Wed, Nov 06, 2024 at 10:59:08AM +0000, Phillip Wood wrote:
> Hi Johannes
> 
> On 05/11/2024 19:55, Johannes Schindelin wrote:
> > Hi Patrick,
> > 
> > On Mon, 21 Oct 2024, Patrick Steinhardt wrote:
> > 
> > > The auto-generated headers used by clar are written at configure time
> > > and thus do not get regenerated automatically. Refactor the build
> > > recipes such that we use custom commands instead, which also has the
> > > benefit that we can reuse the same infrastructure as our Makefile.
> > 
> > For the record: I did not use a shell script to generate the header for a
> > specific reason: Unix shell scripts are not native to Windows. Therefore
> > they cannot in general be run on Windows, however that was precisely the
> > idea for the CMake definition: to be run on a vanilla Windows with Visual
> > Studio installed.
> > 
> > Sadly, even Git's CI definition sets things up in a way that Git for
> > Windows' Bash can be used in the CMake definition, but in the intended use
> > case (opening a checkout of git/git in Visual Studio without any further
> > tools required) won't have a usable Bash.
> > 
> > Therefore I am unsure whether this patch is desirable.
> 
> CMakeLists.txt tries to find sh.exe from git-for-windows and errors out if
> it cannot be found. It then uses that shell to run a number of scripts.
> Perhaps we should do the same in this patch? It would certainly be a
> worthwhile improvement to regenerate this file at build time if the source
> has changed.

Yeah, I think this solution makes most sense. I'll send a patch in a bit
to address this.

Thanks, both of you!

Patrick
diff mbox series

Patch

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 093852ad9d6..9f80ab92656 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1002,46 +1002,20 @@  foreach(unit_test ${unit_test_PROGRAMS})
 endforeach()
 
 parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
-
-set(clar_decls "")
-set(clar_cbs "")
-set(clar_cbs_count 0)
-set(clar_suites "static struct clar_suite _clar_suites[] = {\n")
-list(LENGTH clar_test_SUITES clar_suites_count)
-foreach(suite ${clar_test_SUITES})
-	file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls
-		REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$")
-
-	list(LENGTH decls decls_count)
-	string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" "    { \"\\2\", &\\1 },\n" cbs ${decls})
-	string(JOIN "" cbs ${cbs})
-	list(TRANSFORM decls PREPEND "extern ")
-	string(JOIN ";\n" decls ${decls})
-
-	string(APPEND clar_decls "${decls};\n")
-	string(APPEND clar_cbs
-		"static const struct clar_func _clar_cb_${suite}[] = {\n"
-		${cbs}
-		"};\n")
-	string(APPEND clar_suites
-		"    {\n"
-		"        \"${suite}\",\n"
-		"        { NULL, NULL },\n"
-		"        { NULL, NULL },\n"
-		"        _clar_cb_${suite}, ${decls_count}, 1\n"
-		"    },\n")
-	math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}")
-endforeach()
-string(APPEND clar_suites
-	"};\n"
-	"static const size_t _clar_suite_count = ${clar_suites_count};\n"
-	"static const size_t _clar_callback_count = ${clar_cbs_count};\n")
-file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}")
-file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}")
-
 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")
+add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
+	COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES}
+	DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})
+add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
+	COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
+	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
+
+add_library(unit-tests-lib ${clar_test_SUITES}
+	"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
+	"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
+	"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
+)
 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)