* [PATCH 0/3] Build improvements for clar
@ 2024-11-08 13:16 Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 1/3] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
` (5 more replies)
0 siblings, 6 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-08 13:16 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood
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.
[1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@gmx.de>
Thanks!
Patrick
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Patrick Steinhardt (3):
t/unit-tests: convert "clar-generate.awk" into a shell script
cmake: use SH_EXE to execute clar scripts
Makefile: let clar header targets depend on their scripts
Makefile | 6 ++--
contrib/buildsystems/CMakeLists.txt | 6 ++--
t/unit-tests/clar-generate.awk | 50 ----------------------------
t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++
4 files changed, 69 insertions(+), 56 deletions(-)
---
base-commit: facbe4f633e4ad31e641f64617bc88074c659959
change-id: 20241108-pks-clar-build-improvements-1c3962a9a79f
Best regards,
--
Patrick Steinhardt <ps@pks.im>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/3] t/unit-tests: convert "clar-generate.awk" into a shell script
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
@ 2024-11-08 13:16 ` Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 2/3] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
` (4 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-08 13:16 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood
Convert "clar-generate.awk" into a shell script that invokes awk(1).
This allows us to avoid the shell redirect in the build system, which
may otherwise be a problem with build systems on platforms that use a
different shell.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 2 +-
contrib/buildsystems/CMakeLists.txt | 4 +--
t/unit-tests/clar-generate.awk | 50 ----------------------------
t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++
4 files changed, 66 insertions(+), 53 deletions(-)
diff --git a/Makefile b/Makefile
index d06c9a8ffa7b637050c9619a367fbe61e7243a74..5232b913fd20f01a7e5f41d46178e93d52c9f534 100644
--- a/Makefile
+++ b/Makefile
@@ -3907,7 +3907,7 @@ GIT-TEST-SUITES: FORCE
$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
- $(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
+ $(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite
$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
$(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 8974bb9fa202a0556fd9b16d105836d8cb66f543..f31936e5c8dea76a4bc1eba75d87468c809f59ee 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1008,8 +1008,8 @@ 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})
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")
add_library(unit-tests-lib ${clar_test_SUITES}
"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
diff --git a/t/unit-tests/clar-generate.awk b/t/unit-tests/clar-generate.awk
deleted file mode 100644
index ab71ce6c9fc2c3d49d826f3550a95be893114508..0000000000000000000000000000000000000000
--- a/t/unit-tests/clar-generate.awk
+++ /dev/null
@@ -1,50 +0,0 @@
-function add_suite(suite, initialize, cleanup, count) {
- if (!suite) return
- suite_count++
- callback_count += count
- suites = suites " {\n"
- suites = suites " \"" suite "\",\n"
- suites = suites " " initialize ",\n"
- suites = suites " " cleanup ",\n"
- suites = suites " _clar_cb_" suite ", " count ", 1\n"
- suites = suites " },\n"
-}
-
-BEGIN {
- suites = "static struct clar_suite _clar_suites[] = {\n"
-}
-
-{
- print
- name = $3; sub(/\(.*$/, "", name)
- suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
- short_name = name; sub(/^.*__/, "", short_name)
- cb = "{ \"" short_name "\", &" name " }"
- if (suite != prev_suite) {
- add_suite(prev_suite, initialize, cleanup, count)
- if (callbacks) callbacks = callbacks "};\n"
- callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
- initialize = "{ NULL, NULL }"
- cleanup = "{ NULL, NULL }"
- count = 0
- prev_suite = suite
- }
- if (short_name == "initialize") {
- initialize = cb
- } else if (short_name == "cleanup") {
- cleanup = cb
- } else {
- callbacks = callbacks " " cb ",\n"
- count++
- }
-}
-
-END {
- add_suite(suite, initialize, cleanup, count)
- suites = suites "};"
- if (callbacks) callbacks = callbacks "};"
- print callbacks
- print suites
- print "static const size_t _clar_suite_count = " suite_count ";"
- print "static const size_t _clar_callback_count = " callback_count ";"
-}
diff --git a/t/unit-tests/generate-clar-suites.sh b/t/unit-tests/generate-clar-suites.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d5c712221e46a2eaa288fff5262438e5f04d6f72
--- /dev/null
+++ b/t/unit-tests/generate-clar-suites.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+if test $# -lt 2
+then
+ echo "USAGE: $0 <CLAR_DECLS_H> <OUTPUT>" 2>&1
+ exit 1
+fi
+
+CLAR_DECLS_H="$1"
+OUTPUT="$2"
+
+awk '
+ function add_suite(suite, initialize, cleanup, count) {
+ if (!suite) return
+ suite_count++
+ callback_count += count
+ suites = suites " {\n"
+ suites = suites " \"" suite "\",\n"
+ suites = suites " " initialize ",\n"
+ suites = suites " " cleanup ",\n"
+ suites = suites " _clar_cb_" suite ", " count ", 1\n"
+ suites = suites " },\n"
+ }
+
+ BEGIN {
+ suites = "static struct clar_suite _clar_suites[] = {\n"
+ }
+
+ {
+ print
+ name = $3; sub(/\(.*$/, "", name)
+ suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
+ short_name = name; sub(/^.*__/, "", short_name)
+ cb = "{ \"" short_name "\", &" name " }"
+ if (suite != prev_suite) {
+ add_suite(prev_suite, initialize, cleanup, count)
+ if (callbacks) callbacks = callbacks "};\n"
+ callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
+ initialize = "{ NULL, NULL }"
+ cleanup = "{ NULL, NULL }"
+ count = 0
+ prev_suite = suite
+ }
+ if (short_name == "initialize") {
+ initialize = cb
+ } else if (short_name == "cleanup") {
+ cleanup = cb
+ } else {
+ callbacks = callbacks " " cb ",\n"
+ count++
+ }
+ }
+
+ END {
+ add_suite(suite, initialize, cleanup, count)
+ suites = suites "};"
+ if (callbacks) callbacks = callbacks "};"
+ print callbacks
+ print suites
+ print "static const size_t _clar_suite_count = " suite_count ";"
+ print "static const size_t _clar_callback_count = " callback_count ";"
+ }
+' "$CLAR_DECLS_H" >"$OUTPUT"
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/3] cmake: use SH_EXE to execute clar scripts
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 1/3] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
@ 2024-11-08 13:16 ` Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 3/3] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
` (3 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-08 13:16 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood
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.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
contrib/buildsystems/CMakeLists.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index f31936e5c8dea76a4bc1eba75d87468c809f59ee..041967e2e187476cb9d26b329deb388765c48420 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1005,10 +1005,10 @@ 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}
+ COMMAND ${SH_EXE} ${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})
add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
- 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"
+ COMMAND ${SH_EXE} "${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}
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/3] Makefile: let clar header targets depend on their scripts
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 1/3] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 2/3] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
@ 2024-11-08 13:16 ` Patrick Steinhardt
2024-11-10 14:30 ` [PATCH 0/3] Build improvements for clar Phillip Wood
` (2 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-08 13:16 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood
The targets that generate clar headers depend on their source files, but
not on the script that is actually generating the output. Fix the issue
by adding the missing dependencies.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 5232b913fd20f01a7e5f41d46178e93d52c9f534..549b24e7fdbbdc173dfec79cdaddf67ccba52e14 100644
--- a/Makefile
+++ b/Makefile
@@ -3904,9 +3904,9 @@ GIT-TEST-SUITES: FORCE
echo "$$FLAGS" >GIT-TEST-SUITES; \
fi
-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) $(UNIT_TEST_DIR)/generate-clar-decls.sh GIT-TEST-SUITES
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
-$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
+$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(UNIT_TEST_DIR)/generate-clar-suites.sh
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite
$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] Build improvements for clar
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
` (2 preceding siblings ...)
2024-11-08 13:16 ` [PATCH 3/3] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
@ 2024-11-10 14:30 ` Phillip Wood
2024-11-11 1:34 ` Junio C Hamano
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
5 siblings, 1 reply; 27+ messages in thread
From: Phillip Wood @ 2024-11-10 14:30 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Johannes Schindelin
Hi Patrick
On 08/11/2024 13:16, 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.
I've been using the CMake build in Visual Studio the last couple of days
as my hard drive with linux on it died. I ended up with a slightly
different fix using "sh -c" rather than putting the awk script inside
a shell script. See the diff below. I don't have a strong preference
either way but it would be nice to fix the line wrapping and add
VERBATIM so that paths containing special characters are quoted correctly
Best Wishes
Phillip
---- >8 ----
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index f0a1a75382a..b8a37b3870d 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -989,11 +989,21 @@ 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"
+ "${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}
+ VERBATIM)
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 ${SH_EXE} "-c" [[awk -f "$1" "$2" >"$3"]] awk
+ "${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"
+ VERBATIM)
add_library(unit-tests-lib ${clar_test_SUITES}
"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] Build improvements for clar
2024-11-10 14:30 ` [PATCH 0/3] Build improvements for clar Phillip Wood
@ 2024-11-11 1:34 ` Junio C Hamano
2024-11-11 8:29 ` Patrick Steinhardt
0 siblings, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2024-11-11 1:34 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, git, Johannes Schindelin
Phillip Wood <phillip.wood123@gmail.com> writes:
> I've been using the CMake build in Visual Studio the last couple of days
> as my hard drive with linux on it died. I ended up with a slightly
> different fix using "sh -c" rather than putting the awk script inside
> a shell script. See the diff below. I don't have a strong preference
> either way but it would be nice to fix the line wrapping and add
> VERBATIM so that paths containing special characters are quoted correctly
Thanks for comments.
I've committed the same sin number of times, but a scriptlet written
in a third language as a string literal in a shell script is
somewhat awkward to maintain, so I may have slight preference for
your variant. Either way, we are now letting the shell, and not
CMake, to spawn "awk", so if that was the reason why the file needs
to be changed (i.e. CMake perhaps failed to or found a wrong awk),
either of your two approaches would solve that by delegating the
task to the shell.
>
> Best Wishes
>
> Phillip
>
> ---- >8 ----
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index f0a1a75382a..b8a37b3870d 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -989,11 +989,21 @@ 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"
> + "${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}
> + VERBATIM)
> 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 ${SH_EXE} "-c" [[awk -f "$1" "$2" >"$3"]] awk
> + "${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"
> + VERBATIM)
> add_library(unit-tests-lib ${clar_test_SUITES}
> "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 0/4] Build improvements for clar
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
` (3 preceding siblings ...)
2024-11-10 14:30 ` [PATCH 0/3] Build improvements for clar Phillip Wood
@ 2024-11-11 8:24 ` Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
` (4 more replies)
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
5 siblings, 5 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-11 8:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
@ 2024-11-11 8:24 ` Patrick Steinhardt
2024-11-11 22:58 ` Junio C Hamano
2024-11-11 8:24 ` [PATCH v2 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
` (3 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-11 8:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
Convert "clar-generate.awk" into a shell script that invokes awk(1).
This allows us to avoid the shell redirect in the build system, which
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 | 2 +-
contrib/buildsystems/CMakeLists.txt | 7 ++--
t/unit-tests/clar-generate.awk | 50 ----------------------------
t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++
4 files changed, 69 insertions(+), 53 deletions(-)
diff --git a/Makefile b/Makefile
index d06c9a8ffa7b637050c9619a367fbe61e7243a74..5232b913fd20f01a7e5f41d46178e93d52c9f534 100644
--- a/Makefile
+++ b/Makefile
@@ -3907,7 +3907,7 @@ GIT-TEST-SUITES: FORCE
$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
- $(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
+ $(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite
$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
$(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 8974bb9fa202a0556fd9b16d105836d8cb66f543..da99dc3087a218d30e0fd1044567d7148d0d80a9 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1008,8 +1008,11 @@ 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})
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")
add_library(unit-tests-lib ${clar_test_SUITES}
"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
diff --git a/t/unit-tests/clar-generate.awk b/t/unit-tests/clar-generate.awk
deleted file mode 100644
index ab71ce6c9fc2c3d49d826f3550a95be893114508..0000000000000000000000000000000000000000
--- a/t/unit-tests/clar-generate.awk
+++ /dev/null
@@ -1,50 +0,0 @@
-function add_suite(suite, initialize, cleanup, count) {
- if (!suite) return
- suite_count++
- callback_count += count
- suites = suites " {\n"
- suites = suites " \"" suite "\",\n"
- suites = suites " " initialize ",\n"
- suites = suites " " cleanup ",\n"
- suites = suites " _clar_cb_" suite ", " count ", 1\n"
- suites = suites " },\n"
-}
-
-BEGIN {
- suites = "static struct clar_suite _clar_suites[] = {\n"
-}
-
-{
- print
- name = $3; sub(/\(.*$/, "", name)
- suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
- short_name = name; sub(/^.*__/, "", short_name)
- cb = "{ \"" short_name "\", &" name " }"
- if (suite != prev_suite) {
- add_suite(prev_suite, initialize, cleanup, count)
- if (callbacks) callbacks = callbacks "};\n"
- callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
- initialize = "{ NULL, NULL }"
- cleanup = "{ NULL, NULL }"
- count = 0
- prev_suite = suite
- }
- if (short_name == "initialize") {
- initialize = cb
- } else if (short_name == "cleanup") {
- cleanup = cb
- } else {
- callbacks = callbacks " " cb ",\n"
- count++
- }
-}
-
-END {
- add_suite(suite, initialize, cleanup, count)
- suites = suites "};"
- if (callbacks) callbacks = callbacks "};"
- print callbacks
- print suites
- print "static const size_t _clar_suite_count = " suite_count ";"
- print "static const size_t _clar_callback_count = " callback_count ";"
-}
diff --git a/t/unit-tests/generate-clar-suites.sh b/t/unit-tests/generate-clar-suites.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d5c712221e46a2eaa288fff5262438e5f04d6f72
--- /dev/null
+++ b/t/unit-tests/generate-clar-suites.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+if test $# -lt 2
+then
+ echo "USAGE: $0 <CLAR_DECLS_H> <OUTPUT>" 2>&1
+ exit 1
+fi
+
+CLAR_DECLS_H="$1"
+OUTPUT="$2"
+
+awk '
+ function add_suite(suite, initialize, cleanup, count) {
+ if (!suite) return
+ suite_count++
+ callback_count += count
+ suites = suites " {\n"
+ suites = suites " \"" suite "\",\n"
+ suites = suites " " initialize ",\n"
+ suites = suites " " cleanup ",\n"
+ suites = suites " _clar_cb_" suite ", " count ", 1\n"
+ suites = suites " },\n"
+ }
+
+ BEGIN {
+ suites = "static struct clar_suite _clar_suites[] = {\n"
+ }
+
+ {
+ print
+ name = $3; sub(/\(.*$/, "", name)
+ suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
+ short_name = name; sub(/^.*__/, "", short_name)
+ cb = "{ \"" short_name "\", &" name " }"
+ if (suite != prev_suite) {
+ add_suite(prev_suite, initialize, cleanup, count)
+ if (callbacks) callbacks = callbacks "};\n"
+ callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
+ initialize = "{ NULL, NULL }"
+ cleanup = "{ NULL, NULL }"
+ count = 0
+ prev_suite = suite
+ }
+ if (short_name == "initialize") {
+ initialize = cb
+ } else if (short_name == "cleanup") {
+ cleanup = cb
+ } else {
+ callbacks = callbacks " " cb ",\n"
+ count++
+ }
+ }
+
+ END {
+ add_suite(suite, initialize, cleanup, count)
+ suites = suites "};"
+ if (callbacks) callbacks = callbacks "};"
+ print callbacks
+ print suites
+ print "static const size_t _clar_suite_count = " suite_count ";"
+ print "static const size_t _clar_callback_count = " callback_count ";"
+ }
+' "$CLAR_DECLS_H" >"$OUTPUT"
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 2/4] cmake: use SH_EXE to execute clar scripts
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
@ 2024-11-11 8:24 ` Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
` (2 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-11 8:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
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
+ "${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})
add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
- COMMAND "${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh"
+ COMMAND ${SH_EXE} "${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"
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
@ 2024-11-11 8:24 ` Patrick Steinhardt
2024-11-13 15:41 ` Johannes Schindelin
2024-11-11 8:25 ` [PATCH v2 4/4] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
2024-11-11 10:09 ` [PATCH v2 0/4] Build improvements for clar Phillip Wood
4 siblings, 1 reply; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-11 8:24 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
Pass the VERBATIM option to `add_custom_command()`. Like this, all
arguments to the commands will be escaped properly for the build tool so
that the invoked command receives each argument unchanged.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
contrib/buildsystems/CMakeLists.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
"${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})
+ ${clar_test_SUITES}
+ VERBATIM)
add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
COMMAND ${SH_EXE} "${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")
+ "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
+ VERBATIM)
add_library(unit-tests-lib ${clar_test_SUITES}
"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v2 4/4] Makefile: let clar header targets depend on their scripts
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
` (2 preceding siblings ...)
2024-11-11 8:24 ` [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
@ 2024-11-11 8:25 ` Patrick Steinhardt
2024-11-11 10:09 ` [PATCH v2 0/4] Build improvements for clar Phillip Wood
4 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-11 8:25 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
The targets that generate clar headers depend on their source files, but
not on the script that is actually generating the output. Fix the issue
by adding the missing dependencies.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 5232b913fd20f01a7e5f41d46178e93d52c9f534..549b24e7fdbbdc173dfec79cdaddf67ccba52e14 100644
--- a/Makefile
+++ b/Makefile
@@ -3904,9 +3904,9 @@ GIT-TEST-SUITES: FORCE
echo "$$FLAGS" >GIT-TEST-SUITES; \
fi
-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) $(UNIT_TEST_DIR)/generate-clar-decls.sh GIT-TEST-SUITES
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
-$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
+$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(UNIT_TEST_DIR)/generate-clar-suites.sh
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite
$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
--
2.47.0.229.g8f8d6eee53.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] Build improvements for clar
2024-11-11 1:34 ` Junio C Hamano
@ 2024-11-11 8:29 ` Patrick Steinhardt
2024-11-11 22:52 ` Junio C Hamano
0 siblings, 1 reply; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-11 8:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phillip Wood, git, Johannes Schindelin
On Mon, Nov 11, 2024 at 10:34:27AM +0900, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> > I've been using the CMake build in Visual Studio the last couple of days
> > as my hard drive with linux on it died. I ended up with a slightly
> > different fix using "sh -c" rather than putting the awk script inside
> > a shell script. See the diff below. I don't have a strong preference
> > either way but it would be nice to fix the line wrapping and add
> > VERBATIM so that paths containing special characters are quoted correctly
>
> Thanks for comments.
>
> I've committed the same sin number of times, but a scriptlet written
> in a third language as a string literal in a shell script is
> somewhat awkward to maintain, so I may have slight preference for
> your variant. Either way, we are now letting the shell, and not
> CMake, to spawn "awk", so if that was the reason why the file needs
> to be changed (i.e. CMake perhaps failed to or found a wrong awk),
> either of your two approaches would solve that by delegating the
> task to the shell.
Yeah, I don't think it's particularly beautiful either. Personally, I'd
still lean towards my solution, mostly because it allows us to iterate
more readily in the future in case we ever need more logic in this
context, and it avoids having to handle the redirect in the build
system.
So I'll take the VERBATIM and line wrapping suggestions, but for now
keep the shell script. I'll adapt though if you think that this is too
much of an abomination.
Patrick
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/4] Build improvements for clar
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
` (3 preceding siblings ...)
2024-11-11 8:25 ` [PATCH v2 4/4] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
@ 2024-11-11 10:09 ` Phillip Wood
4 siblings, 0 replies; 27+ messages in thread
From: Phillip Wood @ 2024-11-11 10:09 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Johannes Schindelin, Junio C Hamano
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
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/3] Build improvements for clar
2024-11-11 8:29 ` Patrick Steinhardt
@ 2024-11-11 22:52 ` Junio C Hamano
0 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2024-11-11 22:52 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Phillip Wood, git, Johannes Schindelin
Patrick Steinhardt <ps@pks.im> writes:
> So I'll take the VERBATIM and line wrapping suggestions, but for now
> keep the shell script. I'll adapt though if you think that this is too
> much of an abomination.
I do not expect me to be editing the awk script buried inside a
shell string literal myself, so it is more or less a "meh" to me,
but once people start iterating, they may find it irritating that
their favourite editor do not help them with its "awk" mode while
editing the script. Once that happens, they may split the script
out themselves. Before that, I do not deeply care ;-)
Thanks.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script
2024-11-11 8:24 ` [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
@ 2024-11-11 22:58 ` Junio C Hamano
2024-11-12 5:56 ` Patrick Steinhardt
0 siblings, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2024-11-11 22:58 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Johannes Schindelin, Phillip Wood
Patrick Steinhardt <ps@pks.im> writes:
> While at it, wrap the overly lines in the CMake build instructions.
overly "long" lines?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script
2024-11-11 22:58 ` Junio C Hamano
@ 2024-11-12 5:56 ` Patrick Steinhardt
0 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-12 5:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Phillip Wood
On Tue, Nov 12, 2024 at 07:58:54AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > While at it, wrap the overly lines in the CMake build instructions.
>
> overly "long" lines?
Oops, yes. Queued locally, will send it out when receiving more feedback
on this series.
Patrick
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands
2024-11-11 8:24 ` [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
@ 2024-11-13 15:41 ` Johannes Schindelin
2024-11-14 10:28 ` Phillip Wood
0 siblings, 1 reply; 27+ messages in thread
From: Johannes Schindelin @ 2024-11-13 15:41 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Phillip Wood, Junio C Hamano
Hi Patrick,
On Mon, 11 Nov 2024, Patrick Steinhardt wrote:
> Pass the VERBATIM option to `add_custom_command()`. Like this, all
> arguments to the commands will be escaped properly for the build tool so
> that the invoked command receives each argument unchanged.
I would not be surprised if this `VERBATIM` was unaware of the quirky
escaping that the MSYS2 runtime (and therefore the Bash) requires. See the
commit message of ad1559252945 (tests: add a helper to stress test
argument quoting, 2019-09-18) for details.
Having said that, this patch certainly does not make things _worse_, even
if it probably does not have the intended effect on Windows. But then, it
does not matter because most git/git source checkouts live at absolute
paths that do not need to be quoted.
Ciao,
Johannes
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> contrib/buildsystems/CMakeLists.txt | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
> "${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})
> + ${clar_test_SUITES}
> + VERBATIM)
> add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> COMMAND ${SH_EXE} "${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")
> + "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
> + VERBATIM)
>
> add_library(unit-tests-lib ${clar_test_SUITES}
> "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
>
> --
> 2.47.0.229.g8f8d6eee53.dirty
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands
2024-11-13 15:41 ` Johannes Schindelin
@ 2024-11-14 10:28 ` Phillip Wood
0 siblings, 0 replies; 27+ messages in thread
From: Phillip Wood @ 2024-11-14 10:28 UTC (permalink / raw)
To: Johannes Schindelin, Patrick Steinhardt; +Cc: git, Junio C Hamano
Hi Johannes
On 13/11/2024 15:41, Johannes Schindelin wrote:
>
> I would not be surprised if this `VERBATIM` was unaware of the quirky
> escaping that the MSYS2 runtime (and therefore the Bash) requires. See the
> commit message of ad1559252945 (tests: add a helper to stress test
> argument quoting, 2019-09-18) for details.
>
> Having said that, this patch certainly does not make things _worse_, even
> if it probably does not have the intended effect on Windows. But then, it
> does not matter because most git/git source checkouts live at absolute
> paths that do not need to be quoted.
It may not be perfect but it is an improvement on Windows. When I was
playing around with
COMMAND ${SH_EXE} -c [[awk -f "$1" "$2" >"$3"]] ...
to build clar.suite on Windows it failed to work without VERBATIM.
Best Wishes
Phillip
> Ciao,
> Johannes
>
>>
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>> ---
>> contrib/buildsystems/CMakeLists.txt | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
>> index 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644
>> --- a/contrib/buildsystems/CMakeLists.txt
>> +++ b/contrib/buildsystems/CMakeLists.txt
>> @@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>> "${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})
>> + ${clar_test_SUITES}
>> + VERBATIM)
>> add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
>> COMMAND ${SH_EXE} "${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")
>> + "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
>> + VERBATIM)
>>
>> add_library(unit-tests-lib ${clar_test_SUITES}
>> "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
>>
>> --
>> 2.47.0.229.g8f8d6eee53.dirty
>>
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 0/4] Build improvements for clar
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
` (4 preceding siblings ...)
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
@ 2024-11-15 7:32 ` Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
` (4 more replies)
5 siblings, 5 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-15 7:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
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.
Changes in v3:
- Fix missing word.
Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im
Link to v2: https://lore.kernel.org/r/20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@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 v2:
1: 90d2402c5a ! 1: dc713c236b 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.
+ While at it, wrap the overly long lines in the CMake build instructions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
2: cf298664d8 = 2: 19d292cc5c cmake: use SH_EXE to execute clar scripts
3: ff557f8985 = 3: 52f5090a76 cmake: use verbatim arguments when invoking clar commands
4: c634e2a6d4 = 4: 35117454d4 Makefile: let clar header targets depend on their scripts
---
base-commit: facbe4f633e4ad31e641f64617bc88074c659959
change-id: 20241108-pks-clar-build-improvements-1c3962a9a79f
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
@ 2024-11-15 7:32 ` Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
` (3 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-15 7:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
Convert "clar-generate.awk" into a shell script that invokes awk(1).
This allows us to avoid the shell redirect in the build system, which
may otherwise be a problem with build systems on platforms that use a
different shell.
While at it, wrap the overly long lines in the CMake build instructions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 2 +-
contrib/buildsystems/CMakeLists.txt | 7 ++--
t/unit-tests/clar-generate.awk | 50 ----------------------------
t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++
4 files changed, 69 insertions(+), 53 deletions(-)
diff --git a/Makefile b/Makefile
index d06c9a8ffa7b637050c9619a367fbe61e7243a74..5232b913fd20f01a7e5f41d46178e93d52c9f534 100644
--- a/Makefile
+++ b/Makefile
@@ -3907,7 +3907,7 @@ GIT-TEST-SUITES: FORCE
$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
- $(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite
+ $(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite
$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
$(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 8974bb9fa202a0556fd9b16d105836d8cb66f543..da99dc3087a218d30e0fd1044567d7148d0d80a9 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1008,8 +1008,11 @@ 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})
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")
add_library(unit-tests-lib ${clar_test_SUITES}
"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
diff --git a/t/unit-tests/clar-generate.awk b/t/unit-tests/clar-generate.awk
deleted file mode 100644
index ab71ce6c9fc2c3d49d826f3550a95be893114508..0000000000000000000000000000000000000000
--- a/t/unit-tests/clar-generate.awk
+++ /dev/null
@@ -1,50 +0,0 @@
-function add_suite(suite, initialize, cleanup, count) {
- if (!suite) return
- suite_count++
- callback_count += count
- suites = suites " {\n"
- suites = suites " \"" suite "\",\n"
- suites = suites " " initialize ",\n"
- suites = suites " " cleanup ",\n"
- suites = suites " _clar_cb_" suite ", " count ", 1\n"
- suites = suites " },\n"
-}
-
-BEGIN {
- suites = "static struct clar_suite _clar_suites[] = {\n"
-}
-
-{
- print
- name = $3; sub(/\(.*$/, "", name)
- suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
- short_name = name; sub(/^.*__/, "", short_name)
- cb = "{ \"" short_name "\", &" name " }"
- if (suite != prev_suite) {
- add_suite(prev_suite, initialize, cleanup, count)
- if (callbacks) callbacks = callbacks "};\n"
- callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
- initialize = "{ NULL, NULL }"
- cleanup = "{ NULL, NULL }"
- count = 0
- prev_suite = suite
- }
- if (short_name == "initialize") {
- initialize = cb
- } else if (short_name == "cleanup") {
- cleanup = cb
- } else {
- callbacks = callbacks " " cb ",\n"
- count++
- }
-}
-
-END {
- add_suite(suite, initialize, cleanup, count)
- suites = suites "};"
- if (callbacks) callbacks = callbacks "};"
- print callbacks
- print suites
- print "static const size_t _clar_suite_count = " suite_count ";"
- print "static const size_t _clar_callback_count = " callback_count ";"
-}
diff --git a/t/unit-tests/generate-clar-suites.sh b/t/unit-tests/generate-clar-suites.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d5c712221e46a2eaa288fff5262438e5f04d6f72
--- /dev/null
+++ b/t/unit-tests/generate-clar-suites.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+if test $# -lt 2
+then
+ echo "USAGE: $0 <CLAR_DECLS_H> <OUTPUT>" 2>&1
+ exit 1
+fi
+
+CLAR_DECLS_H="$1"
+OUTPUT="$2"
+
+awk '
+ function add_suite(suite, initialize, cleanup, count) {
+ if (!suite) return
+ suite_count++
+ callback_count += count
+ suites = suites " {\n"
+ suites = suites " \"" suite "\",\n"
+ suites = suites " " initialize ",\n"
+ suites = suites " " cleanup ",\n"
+ suites = suites " _clar_cb_" suite ", " count ", 1\n"
+ suites = suites " },\n"
+ }
+
+ BEGIN {
+ suites = "static struct clar_suite _clar_suites[] = {\n"
+ }
+
+ {
+ print
+ name = $3; sub(/\(.*$/, "", name)
+ suite = name; sub(/^test_/, "", suite); sub(/__.*$/, "", suite)
+ short_name = name; sub(/^.*__/, "", short_name)
+ cb = "{ \"" short_name "\", &" name " }"
+ if (suite != prev_suite) {
+ add_suite(prev_suite, initialize, cleanup, count)
+ if (callbacks) callbacks = callbacks "};\n"
+ callbacks = callbacks "static const struct clar_func _clar_cb_" suite "[] = {\n"
+ initialize = "{ NULL, NULL }"
+ cleanup = "{ NULL, NULL }"
+ count = 0
+ prev_suite = suite
+ }
+ if (short_name == "initialize") {
+ initialize = cb
+ } else if (short_name == "cleanup") {
+ cleanup = cb
+ } else {
+ callbacks = callbacks " " cb ",\n"
+ count++
+ }
+ }
+
+ END {
+ add_suite(suite, initialize, cleanup, count)
+ suites = suites "};"
+ if (callbacks) callbacks = callbacks "};"
+ print callbacks
+ print suites
+ print "static const size_t _clar_suite_count = " suite_count ";"
+ print "static const size_t _clar_callback_count = " callback_count ";"
+ }
+' "$CLAR_DECLS_H" >"$OUTPUT"
--
2.47.0.251.gb31fb630c0.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
@ 2024-11-15 7:32 ` Patrick Steinhardt
2024-11-18 18:47 ` Justin Tobler
2024-11-15 7:32 ` [PATCH v3 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
` (2 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-15 7:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
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
+ "${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})
add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
- COMMAND "${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh"
+ COMMAND ${SH_EXE} "${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"
--
2.47.0.251.gb31fb630c0.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 3/4] cmake: use verbatim arguments when invoking clar commands
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
@ 2024-11-15 7:32 ` Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 4/4] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
2024-11-19 16:51 ` [PATCH v3 0/4] Build improvements for clar Justin Tobler
4 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-15 7:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
Pass the VERBATIM option to `add_custom_command()`. Like this, all
arguments to the commands will be escaped properly for the build tool so
that the invoked command receives each argument unchanged.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
contrib/buildsystems/CMakeLists.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
"${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})
+ ${clar_test_SUITES}
+ VERBATIM)
add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
COMMAND ${SH_EXE} "${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")
+ "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h"
+ VERBATIM)
add_library(unit-tests-lib ${clar_test_SUITES}
"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c"
--
2.47.0.251.gb31fb630c0.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v3 4/4] Makefile: let clar header targets depend on their scripts
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
` (2 preceding siblings ...)
2024-11-15 7:32 ` [PATCH v3 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
@ 2024-11-15 7:32 ` Patrick Steinhardt
2024-11-19 16:51 ` [PATCH v3 0/4] Build improvements for clar Justin Tobler
4 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-15 7:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Phillip Wood, Junio C Hamano
The targets that generate clar headers depend on their source files, but
not on the script that is actually generating the output. Fix the issue
by adding the missing dependencies.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 5232b913fd20f01a7e5f41d46178e93d52c9f534..549b24e7fdbbdc173dfec79cdaddf67ccba52e14 100644
--- a/Makefile
+++ b/Makefile
@@ -3904,9 +3904,9 @@ GIT-TEST-SUITES: FORCE
echo "$$FLAGS" >GIT-TEST-SUITES; \
fi
-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES
+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) $(UNIT_TEST_DIR)/generate-clar-decls.sh GIT-TEST-SUITES
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^)
-$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h
+$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(UNIT_TEST_DIR)/generate-clar-suites.sh
$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite
$(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite
$(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h
--
2.47.0.251.gb31fb630c0.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts
2024-11-15 7:32 ` [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
@ 2024-11-18 18:47 ` Justin Tobler
2024-11-19 6:13 ` Patrick Steinhardt
0 siblings, 1 reply; 27+ messages in thread
From: Justin Tobler @ 2024-11-18 18:47 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Johannes Schindelin, Phillip Wood, Junio C Hamano
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?
-Justin
> + "${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})
> add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite"
> - COMMAND "${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh"
> + COMMAND ${SH_EXE} "${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"
>
> --
> 2.47.0.251.gb31fb630c0.dirty
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts
2024-11-18 18:47 ` Justin Tobler
@ 2024-11-19 6:13 ` Patrick Steinhardt
0 siblings, 0 replies; 27+ messages in thread
From: Patrick Steinhardt @ 2024-11-19 6:13 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, Johannes Schindelin, Phillip Wood, Junio C Hamano
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 0/4] Build improvements for clar
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
` (3 preceding siblings ...)
2024-11-15 7:32 ` [PATCH v3 4/4] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
@ 2024-11-19 16:51 ` Justin Tobler
2024-11-20 1:24 ` Junio C Hamano
4 siblings, 1 reply; 27+ messages in thread
From: Justin Tobler @ 2024-11-19 16:51 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Johannes Schindelin, Phillip Wood, Junio C Hamano
On 24/11/15 08:32AM, 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.
>
> Changes in v3:
>
> - Fix missing word.
>
> Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im
> Link to v2: https://lore.kernel.org/r/20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im
>
> Thanks!
>
> Patrick
I've reviewed this series and overall this version looks good to me.
Embedding the awk script in the shell script and changing how the
scripts get executed to address platform related build issues seems
sensable to me.
-Justin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3 0/4] Build improvements for clar
2024-11-19 16:51 ` [PATCH v3 0/4] Build improvements for clar Justin Tobler
@ 2024-11-20 1:24 ` Junio C Hamano
0 siblings, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2024-11-20 1:24 UTC (permalink / raw)
To: Justin Tobler; +Cc: Patrick Steinhardt, git, Johannes Schindelin, Phillip Wood
Justin Tobler <jltobler@gmail.com> writes:
> On 24/11/15 08:32AM, 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.
>>
>> Changes in v3:
>>
>> - Fix missing word.
>>
>> Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im
>> Link to v2: https://lore.kernel.org/r/20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im
>>
>> Thanks!
>>
>> Patrick
>
> I've reviewed this series and overall this version looks good to me.
> Embedding the awk script in the shell script and changing how the
> scripts get executed to address platform related build issues seems
> sensable to me.
Thanks, both. Let's merge it down to 'next'.
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2024-11-20 1:24 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-08 13:16 [PATCH 0/3] Build improvements for clar Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 1/3] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 2/3] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
2024-11-08 13:16 ` [PATCH 3/3] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
2024-11-10 14:30 ` [PATCH 0/3] Build improvements for clar Phillip Wood
2024-11-11 1:34 ` Junio C Hamano
2024-11-11 8:29 ` Patrick Steinhardt
2024-11-11 22:52 ` Junio C Hamano
2024-11-11 8:24 ` [PATCH v2 0/4] " Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
2024-11-11 22:58 ` Junio C Hamano
2024-11-12 5:56 ` Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
2024-11-11 8:24 ` [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
2024-11-13 15:41 ` Johannes Schindelin
2024-11-14 10:28 ` Phillip Wood
2024-11-11 8:25 ` [PATCH v2 4/4] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
2024-11-11 10:09 ` [PATCH v2 0/4] Build improvements for clar Phillip Wood
2024-11-15 7:32 ` [PATCH v3 " Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 1/4] t/unit-tests: convert "clar-generate.awk" into a shell script Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts Patrick Steinhardt
2024-11-18 18:47 ` Justin Tobler
2024-11-19 6:13 ` Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 3/4] cmake: use verbatim arguments when invoking clar commands Patrick Steinhardt
2024-11-15 7:32 ` [PATCH v3 4/4] Makefile: let clar header targets depend on their scripts Patrick Steinhardt
2024-11-19 16:51 ` [PATCH v3 0/4] Build improvements for clar Justin Tobler
2024-11-20 1:24 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).