mbox series

[v2,0/4] Build improvements for clar

Message ID 20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im (mailing list archive)
Headers show
Series Build improvements for clar | expand

Message

Patrick Steinhardt Nov. 11, 2024, 8:24 a.m. UTC 
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

Comments

Phillip Wood Nov. 11, 2024, 10:09 a.m. UTC | #1 
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:
> 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
>