Re: [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts

From: Justin Tobler <jltobler@gmail.com>
Date: Nov 18, 2024, 18:47 UTC
Message-ID: <j5dohg2u5vguiffj3dkqimach725ca2yhyzu4cookd6gvjoav2@qizyg7mazhyz>
In-Reply-To: <20241115-pks-clar-build-improvements-v3-2-29672bf65ec6@pks.im>

On 24/11/15 08:32AM, Patrick Steinhardt wrote:

Show 32 quoted lines

> In 30bf9f0aaa (cmake: set up proper dependencies for generated clar
> headers, 2024-10-21), we have deduplicated the logic to generate our
> clar headers by reusing the same scripts that our Makefile does. Despite
> the deduplication, this refactoring also made us rebuild the headers in
> case the source files change, which didn't happen previously.
> 
> The commit also introduced an issue though: we execute the scripts
> directly, so when the host does not have "/bin/sh" available they will
> fail. This is for example the case on Windows when importing the CMake
> project into Microsoft Visual Studio.
> 
> Address the issue by invoking the scripts with `SH_EXE`, which contains
> the discovered path of the shell interpreter.
> 
> While at it, wrap the overly long lines in the CMake build instructions.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  contrib/buildsystems/CMakeLists.txt | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index da99dc3087a218d30e0fd1044567d7148d0d80a9..2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1005,10 +1005,13 @@ parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "")
>  list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/")
>  list(TRANSFORM clar_test_SUITES APPEND ".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})
> +	COMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh

In the previous patch we used `${SHELL_PATH}` to execute the "generate-clar-suites.sh". Here we use `${SH_EXE}` to execute "generate-clar-decls.sh". From my understanding this is done to help discover the shell on different platforms. Naive question, would this also be useful in the former patch?

-Justin

Show 15 quoted lines

> +		"${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 "${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh"
> +	COMMAND ${SH_EXE} "${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh"
>  		"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>  		"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>  	DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh"
> 
> -- 
> 2.47.0.251.gb31fb630c0.dirty
> 
>