Re: [PATCH v2 0/4] Build improvements for clar
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Nov 11, 2024, 10:09 UTC
- Message-ID
- <2757e5ab-6fce-40db-b11d-af9577a66908@gmail.com>
- In-Reply-To
- <20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im>
Hi Patrick
I've not actually tested them but having read through them these patches look good to me. While it may be a bit ugly to have an awk script lurking inside a shell script it does mean if we ever change the implementation of the shell script to process the files in a different way we wont have to worry about updating the build system(s).
Best Wishes
Phillip
On 11/11/2024 08:24, Patrick Steinhardt wrote:
Show 72 quoted lines
> Hi,
>
> Dscho has reported in [1] that the CMake build instructions for clar do
> not work well on Windows/MSVC because we execute the shell scripts
> directly instead of using the discovered `SH_EXE`. This small patch
> series fixes the issue.
>
> Changes in v2:
>
> - Wrap overly long lines in the CMake build instructions.
> - Add the VERBATIM option.
>
> Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im
>
> Thanks!
>
> Patrick
>
> [1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@gmx.de>
>
> To: git@vger.kernel.org
> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Cc: Phillip Wood <phillip.wood123@gmail.com>
> Cc: Junio C Hamano <gitster@pobox.com>
>
> Patrick Steinhardt (4):
> t/unit-tests: convert "clar-generate.awk" into a shell script
> cmake: use SH_EXE to execute clar scripts
> cmake: use verbatim arguments when invoking clar commands
> Makefile: let clar header targets depend on their scripts
>
> Makefile | 6 ++--
> contrib/buildsystems/CMakeLists.txt | 16 ++++++---
> t/unit-tests/clar-generate.awk | 50 ----------------------------
> t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 78 insertions(+), 57 deletions(-)
>
> Range-diff versus v1:
>
> 1: 23d84e6c50 ! 1: 832222f7f5 t/unit-tests: convert "clar-generate.awk" into a shell script
> @@ Commit message
> may otherwise be a problem with build systems on platforms that use a
> different shell.
>
> + While at it, wrap the overly lines in the CMake build instructions.
> +
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>
> ## Makefile ##
> @@ contrib/buildsystems/CMakeLists.txt: add_custom_command(OUTPUT "${CMAKE_BINARY_D
> 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")
> -+ COMMAND "${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" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h")
> ++ COMMAND "${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"
> ++ "${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"
> 2: a41b1f4746 < -: ---------- cmake: use SH_EXE to execute clar scripts
> -: ---------- > 2: 38601f7bdf cmake: use SH_EXE to execute clar scripts
> -: ---------- > 3: 146ebd3841 cmake: use verbatim arguments when invoking clar commands
> 3: 01c1c51e6a = 4: 341c831192 Makefile: let clar header targets depend on their scripts
>
> ---
> base-commit: facbe4f633e4ad31e641f64617bc88074c659959
> change-id: 20241108-pks-clar-build-improvements-1c3962a9a79f
>