Re: [PATCH v2 0/4] Build improvements for clar

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx

Thanks!

Patrick

[1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@xxxxxx>

To: git@xxxxxxxxxxxxxxx
Cc: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
Cc: Phillip Wood <phillip.wood123@xxxxxxxxx>
Cc: Junio C Hamano <gitster@xxxxxxxxx>

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@xxxxxx>
## 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






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux