Re: [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Nov 19, 2024, 06:13 UTC
- Message-ID
- <ZzwsZfNgJck3V45F@pks.im>
- In-Reply-To
- <j5dohg2u5vguiffj3dkqimach725ca2yhyzu4cookd6gvjoav2@qizyg7mazhyz>
On Mon, Nov 18, 2024 at 12:47:32PM -0600, Justin Tobler wrote:
Show 39 quoted lines
> 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