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

[Patrick Steinhardt <ps@pks.im>]

On Mon, Nov 18, 2024 at 12:47:32PM -0600, Justin Tobler wrote:
> On 24/11/15 08:32AM, Patrick Steinhardt wrote:
> > 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?

Yes, it would. I decided to bundle the use of SH_EXE in our CMake build
instructions into a single patch though, which is the third patch, so
that I don't have to explain the change twice.

Patrick