{"thread":{"id":"62468","subject":"[PATCH 0/3] Build improvements for clar","startedAt":"2024-11-08T13:16:47Z","lastAt":"2024-11-20T01:24:13Z","messageCount":27,"participants":["Patrick Steinhardt","Phillip Wood","Junio C Hamano","Johannes Schindelin","Justin Tobler"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"506856","messageId":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","threadId":"62468","inReplyTo":null,"subject":"[PATCH 0/3] Build improvements for clar","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-08T13:16:22Z","receivedAt":"2024-11-08T13:16:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nDscho has reported in [1] that the CMake build instructions for clar do\nnot work well on Windows/MSVC because we execute the shell scripts\ndirectly instead of using the discovered `SH_EXE`. This small patch\nseries fixes the issue.\n\n[1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@gmx.de>\n\nThanks!\n\nPatrick\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\nPatrick Steinhardt (3):\n      t/unit-tests: convert \"clar-generate.awk\" into a shell script\n      cmake: use SH_EXE to execute clar scripts\n      Makefile: let clar header targets depend on their scripts\n\n Makefile                             |  6 ++--\n contrib/buildsystems/CMakeLists.txt  |  6 ++--\n t/unit-tests/clar-generate.awk       | 50 ----------------------------\n t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 69 insertions(+), 56 deletions(-)\n---\nbase-commit: facbe4f633e4ad31e641f64617bc88074c659959\nchange-id: 20241108-pks-clar-build-improvements-1c3962a9a79f\n\nBest regards,\n-- \nPatrick Steinhardt <ps@pks.im>\n\n"},{"id":"506857","messageId":"20241108-pks-clar-build-improvements-v1-1-25c1fe65ce37@pks.im","threadId":"62468","inReplyTo":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","subject":"[PATCH 1/3] t/unit-tests: convert \"clar-generate.awk\" into a shell script","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-08T13:16:23Z","receivedAt":"2024-11-08T13:16:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Convert \"clar-generate.awk\" into a shell script that invokes awk(1).\nThis allows us to avoid the shell redirect in the build system, which\nmay otherwise be a problem with build systems on platforms that use a\ndifferent shell.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                             |  2 +-\n contrib/buildsystems/CMakeLists.txt  |  4 +--\n t/unit-tests/clar-generate.awk       | 50 ----------------------------\n t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 66 insertions(+), 53 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d06c9a8ffa7b637050c9619a367fbe61e7243a74..5232b913fd20f01a7e5f41d46178e93d52c9f534 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3907,7 +3907,7 @@ GIT-TEST-SUITES: FORCE\n $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(filter %.c,$^)\n $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n-\t$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite\n+\t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite\n $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite\n $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n $(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 8974bb9fa202a0556fd9b16d105836d8cb66f543..f31936e5c8dea76a4bc1eba75d87468c809f59ee 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1008,8 +1008,8 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-\tCOMMAND 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\"\n-\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n+\tCOMMAND \"${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\"\n+\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n \n add_library(unit-tests-lib ${clar_test_SUITES}\n \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\ndiff --git a/t/unit-tests/clar-generate.awk b/t/unit-tests/clar-generate.awk\ndeleted file mode 100644\nindex ab71ce6c9fc2c3d49d826f3550a95be893114508..0000000000000000000000000000000000000000\n--- a/t/unit-tests/clar-generate.awk\n+++ /dev/null\n@@ -1,50 +0,0 @@\n-function add_suite(suite, initialize, cleanup, count) {\n-       if (!suite) return\n-       suite_count++\n-       callback_count += count\n-       suites = suites \"    {\\n\"\n-       suites = suites \"        \\\"\" suite \"\\\",\\n\"\n-       suites = suites \"        \" initialize \",\\n\"\n-       suites = suites \"        \" cleanup \",\\n\"\n-       suites = suites \"        _clar_cb_\" suite \", \" count \", 1\\n\"\n-       suites = suites \"    },\\n\"\n-}\n-\n-BEGIN {\n-       suites = \"static struct clar_suite _clar_suites[] = {\\n\"\n-}\n-\n-{\n-       print\n-       name = $3; sub(/\\(.*$/, \"\", name)\n-       suite = name; sub(/^test_/, \"\", suite); sub(/__.*$/, \"\", suite)\n-       short_name = name; sub(/^.*__/, \"\", short_name)\n-       cb = \"{ \\\"\" short_name \"\\\", &\" name \" }\"\n-       if (suite != prev_suite) {\n-               add_suite(prev_suite, initialize, cleanup, count)\n-               if (callbacks) callbacks = callbacks \"};\\n\"\n-               callbacks = callbacks \"static const struct clar_func _clar_cb_\" suite \"[] = {\\n\"\n-               initialize = \"{ NULL, NULL }\"\n-               cleanup = \"{ NULL, NULL }\"\n-               count = 0\n-               prev_suite = suite\n-       }\n-       if (short_name == \"initialize\") {\n-               initialize = cb\n-       } else if (short_name == \"cleanup\") {\n-               cleanup = cb\n-       } else {\n-               callbacks = callbacks \"    \" cb \",\\n\"\n-               count++\n-       }\n-}\n-\n-END {\n-       add_suite(suite, initialize, cleanup, count)\n-       suites = suites \"};\"\n-       if (callbacks) callbacks = callbacks \"};\"\n-       print callbacks\n-       print suites\n-       print \"static const size_t _clar_suite_count = \" suite_count \";\"\n-       print \"static const size_t _clar_callback_count = \" callback_count \";\"\n-}\ndiff --git a/t/unit-tests/generate-clar-suites.sh b/t/unit-tests/generate-clar-suites.sh\nnew file mode 100755\nindex 0000000000000000000000000000000000000000..d5c712221e46a2eaa288fff5262438e5f04d6f72\n--- /dev/null\n+++ b/t/unit-tests/generate-clar-suites.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+if test $# -lt 2\n+then\n+\techo \"USAGE: $0 <CLAR_DECLS_H> <OUTPUT>\" 2>&1\n+\texit 1\n+fi\n+\n+CLAR_DECLS_H=\"$1\"\n+OUTPUT=\"$2\"\n+\n+awk '\n+\tfunction add_suite(suite, initialize, cleanup, count) {\n+\t\tif (!suite) return\n+\t\tsuite_count++\n+\t\tcallback_count += count\n+\t\tsuites = suites \"    {\\n\"\n+\t\tsuites = suites \"        \\\"\" suite \"\\\",\\n\"\n+\t\tsuites = suites \"        \" initialize \",\\n\"\n+\t\tsuites = suites \"        \" cleanup \",\\n\"\n+\t\tsuites = suites \"        _clar_cb_\" suite \", \" count \", 1\\n\"\n+\t\tsuites = suites \"    },\\n\"\n+\t}\n+\n+\tBEGIN {\n+\t\tsuites = \"static struct clar_suite _clar_suites[] = {\\n\"\n+\t}\n+\n+\t{\n+\t\tprint\n+\t\tname = $3; sub(/\\(.*$/, \"\", name)\n+\t\tsuite = name; sub(/^test_/, \"\", suite); sub(/__.*$/, \"\", suite)\n+\t\tshort_name = name; sub(/^.*__/, \"\", short_name)\n+\t\tcb = \"{ \\\"\" short_name \"\\\", &\" name \" }\"\n+\t\tif (suite != prev_suite) {\n+\t\t\tadd_suite(prev_suite, initialize, cleanup, count)\n+\t\t\tif (callbacks) callbacks = callbacks \"};\\n\"\n+\t\t\tcallbacks = callbacks \"static const struct clar_func _clar_cb_\" suite \"[] = {\\n\"\n+\t\t\tinitialize = \"{ NULL, NULL }\"\n+\t\t\tcleanup = \"{ NULL, NULL }\"\n+\t\t\tcount = 0\n+\t\t\tprev_suite = suite\n+\t\t}\n+\t\tif (short_name == \"initialize\") {\n+\t\t\tinitialize = cb\n+\t\t} else if (short_name == \"cleanup\") {\n+\t\t\tcleanup = cb\n+\t\t} else {\n+\t\t\tcallbacks = callbacks \"    \" cb \",\\n\"\n+\t\t\tcount++\n+\t\t}\n+\t}\n+\n+\tEND {\n+\t\tadd_suite(suite, initialize, cleanup, count)\n+\t\tsuites = suites \"};\"\n+\t\tif (callbacks) callbacks = callbacks \"};\"\n+\t\tprint callbacks\n+\t\tprint suites\n+\t\tprint \"static const size_t _clar_suite_count = \" suite_count \";\"\n+\t\tprint \"static const size_t _clar_callback_count = \" callback_count \";\"\n+\t}\n+' \"$CLAR_DECLS_H\" >\"$OUTPUT\"\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506858","messageId":"20241108-pks-clar-build-improvements-v1-2-25c1fe65ce37@pks.im","threadId":"62468","inReplyTo":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","subject":"[PATCH 2/3] cmake: use SH_EXE to execute clar scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-08T13:16:24Z","receivedAt":"2024-11-08T13:16:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In 30bf9f0aaa (cmake: set up proper dependencies for generated clar\nheaders, 2024-10-21), we have deduplicated the logic to generate our\nclar headers by reusing the same scripts that our Makefile does. Despite\nthe deduplication, this refactoring also made us rebuild the headers in\ncase the source files change, which didn't happen previously.\n\nThe commit also introduced an issue though: we execute the scripts\ndirectly, so when the host does not have \"/bin/sh\" available they will\nfail. This is for example the case on Windows when importing the CMake\nproject into Microsoft Visual Studio.\n\nAddress the issue by invoking the scripts with `SH_EXE`, which contains\nthe discovered path of the shell interpreter.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex f31936e5c8dea76a4bc1eba75d87468c809f59ee..041967e2e187476cb9d26b329deb388765c48420 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1005,10 +1005,10 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n-\tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n+\tCOMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-\tCOMMAND \"${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\"\n+\tCOMMAND ${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\"\n \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n \n add_library(unit-tests-lib ${clar_test_SUITES}\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506859","messageId":"20241108-pks-clar-build-improvements-v1-3-25c1fe65ce37@pks.im","threadId":"62468","inReplyTo":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","subject":"[PATCH 3/3] Makefile: let clar header targets depend on their scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-08T13:16:25Z","receivedAt":"2024-11-08T13:16:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The targets that generate clar headers depend on their source files, but\nnot on the script that is actually generating the output. Fix the issue\nby adding the missing dependencies.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5232b913fd20f01a7e5f41d46178e93d52c9f534..549b24e7fdbbdc173dfec79cdaddf67ccba52e14 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3904,9 +3904,9 @@ GIT-TEST-SUITES: FORCE\n \t\techo \"$$FLAGS\" >GIT-TEST-SUITES; \\\n             fi\n \n-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) $(UNIT_TEST_DIR)/generate-clar-decls.sh GIT-TEST-SUITES\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(filter %.c,$^)\n-$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n+$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(UNIT_TEST_DIR)/generate-clar-suites.sh\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite\n $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite\n $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506938","messageId":"890b423c-9756-4e1b-a6d0-3990f1091f5b@gmail.com","threadId":"62468","inReplyTo":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","subject":"Re: [PATCH 0/3] Build improvements for clar","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-11-10T14:30:59Z","receivedAt":"2024-11-10T14:31:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 08/11/2024 13:16, Patrick Steinhardt wrote:\n> Hi,\n> \n> Dscho has reported in [1] that the CMake build instructions for clar do\n> not work well on Windows/MSVC because we execute the shell scripts\n> directly instead of using the discovered `SH_EXE`. This small patch\n> series fixes the issue.\n\nI've been using the CMake build in Visual Studio the last couple of days\nas my hard drive with linux on it died. I ended up with a slightly\ndifferent fix using \"sh -c\" rather than putting the awk script inside\na shell script. See the diff below. I don't have a strong preference\neither way but it would be nice to fix the line wrapping and add\nVERBATIM so that paths containing special characters are quoted correctly\n\nBest Wishes\n\nPhillip\n\n---- >8 ----\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex f0a1a75382a..b8a37b3870d 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -989,11 +989,21 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n  list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n  list(TRANSFORM clar_test_SUITES APPEND \".c\")\n  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n-        COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n-        DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n+        COMMAND ${SH_EXE}\n+                \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\"\n+                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+                ${clar_test_SUITES}\n+        DEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\"\n+                ${clar_test_SUITES}\n+        VERBATIM)\n  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-        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\"\n-        DEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n+        COMMAND ${SH_EXE} \"-c\" [[awk -f \"$1\" \"$2\" >\"$3\"]] awk\n+                \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\"\n+                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n+        DEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\"\n+                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+        VERBATIM)\n  \n  add_library(unit-tests-lib ${clar_test_SUITES}\n          \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n\n\n"},{"id":"506955","messageId":"xmqqiksuk0yk.fsf@gitster.g","threadId":"62468","inReplyTo":"890b423c-9756-4e1b-a6d0-3990f1091f5b@gmail.com","subject":"Re: [PATCH 0/3] Build improvements for clar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-11T01:34:27Z","receivedAt":"2024-11-11T01:34:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> I've been using the CMake build in Visual Studio the last couple of days\n> as my hard drive with linux on it died. I ended up with a slightly\n> different fix using \"sh -c\" rather than putting the awk script inside\n> a shell script. See the diff below. I don't have a strong preference\n> either way but it would be nice to fix the line wrapping and add\n> VERBATIM so that paths containing special characters are quoted correctly\n\nThanks for comments.\n\nI've committed the same sin number of times, but a scriptlet written\nin a third language as a string literal in a shell script is\nsomewhat awkward to maintain, so I may have slight preference for\nyour variant.  Either way, we are now letting the shell, and not\nCMake, to spawn \"awk\", so if that was the reason why the file needs\nto be changed (i.e. CMake perhaps failed to or found a wrong awk),\neither of your two approaches would solve that by delegating the\ntask to the shell.\n\n\n\n\n>\n> Best Wishes\n>\n> Phillip\n>\n> ---- >8 ----\n>\n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index f0a1a75382a..b8a37b3870d 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -989,11 +989,21 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n>  list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n>  list(TRANSFORM clar_test_SUITES APPEND \".c\")\n>  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> -        COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n> -        DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n> +        COMMAND ${SH_EXE}\n> +                \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\"\n> +                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> +                ${clar_test_SUITES}\n> +        DEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\"\n> +                ${clar_test_SUITES}\n> +        VERBATIM)\n>  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n> -        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\"\n> -        DEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n> +        COMMAND ${SH_EXE} \"-c\" [[awk -f \"$1\" \"$2\" >\"$3\"]] awk\n> +                \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\"\n> +                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> +                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n> +        DEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\"\n> +                \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> +        VERBATIM)\n>    add_library(unit-tests-lib ${clar_test_SUITES}\n>          \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n"},{"id":"506968","messageId":"20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im","threadId":"62468","inReplyTo":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","subject":"[PATCH v2 0/4] Build improvements for clar","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-11T08:24:56Z","receivedAt":"2024-11-11T08:25:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nDscho has reported in [1] that the CMake build instructions for clar do\nnot work well on Windows/MSVC because we execute the shell scripts\ndirectly instead of using the discovered `SH_EXE`. This small patch\nseries fixes the issue.\n\nChanges in v2:\n\n    - Wrap overly long lines in the CMake build instructions.\n    - Add the VERBATIM option.\n\nLink to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@gmx.de>\n\nTo: git@vger.kernel.org\nCc: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nCc: Phillip Wood <phillip.wood123@gmail.com>\nCc: Junio C Hamano <gitster@pobox.com>\n\nPatrick Steinhardt (4):\n      t/unit-tests: convert \"clar-generate.awk\" into a shell script\n      cmake: use SH_EXE to execute clar scripts\n      cmake: use verbatim arguments when invoking clar commands\n      Makefile: let clar header targets depend on their scripts\n\n Makefile                             |  6 ++--\n contrib/buildsystems/CMakeLists.txt  | 16 ++++++---\n t/unit-tests/clar-generate.awk       | 50 ----------------------------\n t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 78 insertions(+), 57 deletions(-)\n\nRange-diff versus v1:\n\n1:  23d84e6c50 ! 1:  832222f7f5 t/unit-tests: convert \"clar-generate.awk\" into a shell script\n    @@ Commit message\n         may otherwise be a problem with build systems on platforms that use a\n         different shell.\n     \n    +    While at it, wrap the overly lines in the CMake build instructions.\n    +\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n      ## Makefile ##\n    @@ contrib/buildsystems/CMakeLists.txt: add_custom_command(OUTPUT \"${CMAKE_BINARY_D\n      add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n     -\tCOMMAND 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\"\n     -\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n    -+\tCOMMAND \"${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\"\n    -+\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n    ++\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n    ++\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n    ++\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n    ++\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n    ++\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n      \n      add_library(unit-tests-lib ${clar_test_SUITES}\n      \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n2:  a41b1f4746 < -:  ---------- cmake: use SH_EXE to execute clar scripts\n-:  ---------- > 2:  38601f7bdf cmake: use SH_EXE to execute clar scripts\n-:  ---------- > 3:  146ebd3841 cmake: use verbatim arguments when invoking clar commands\n3:  01c1c51e6a = 4:  341c831192 Makefile: let clar header targets depend on their scripts\n\n---\nbase-commit: facbe4f633e4ad31e641f64617bc88074c659959\nchange-id: 20241108-pks-clar-build-improvements-1c3962a9a79f\n\n"},{"id":"506972","messageId":"20241111-pks-clar-build-improvements-v2-1-d4794d8d1b30@pks.im","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im","subject":"[PATCH v2 1/4] t/unit-tests: convert \"clar-generate.awk\" into a shell script","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-11T08:24:57Z","receivedAt":"2024-11-11T08:25:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Convert \"clar-generate.awk\" into a shell script that invokes awk(1).\nThis allows us to avoid the shell redirect in the build system, which\nmay otherwise be a problem with build systems on platforms that use a\ndifferent shell.\n\nWhile at it, wrap the overly lines in the CMake build instructions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                             |  2 +-\n contrib/buildsystems/CMakeLists.txt  |  7 ++--\n t/unit-tests/clar-generate.awk       | 50 ----------------------------\n t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 69 insertions(+), 53 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d06c9a8ffa7b637050c9619a367fbe61e7243a74..5232b913fd20f01a7e5f41d46178e93d52c9f534 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3907,7 +3907,7 @@ GIT-TEST-SUITES: FORCE\n $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(filter %.c,$^)\n $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n-\t$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite\n+\t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite\n $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite\n $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n $(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 8974bb9fa202a0556fd9b16d105836d8cb66f543..da99dc3087a218d30e0fd1044567d7148d0d80a9 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1008,8 +1008,11 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-\tCOMMAND 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\"\n-\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n+\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n+\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n \n add_library(unit-tests-lib ${clar_test_SUITES}\n \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\ndiff --git a/t/unit-tests/clar-generate.awk b/t/unit-tests/clar-generate.awk\ndeleted file mode 100644\nindex ab71ce6c9fc2c3d49d826f3550a95be893114508..0000000000000000000000000000000000000000\n--- a/t/unit-tests/clar-generate.awk\n+++ /dev/null\n@@ -1,50 +0,0 @@\n-function add_suite(suite, initialize, cleanup, count) {\n-       if (!suite) return\n-       suite_count++\n-       callback_count += count\n-       suites = suites \"    {\\n\"\n-       suites = suites \"        \\\"\" suite \"\\\",\\n\"\n-       suites = suites \"        \" initialize \",\\n\"\n-       suites = suites \"        \" cleanup \",\\n\"\n-       suites = suites \"        _clar_cb_\" suite \", \" count \", 1\\n\"\n-       suites = suites \"    },\\n\"\n-}\n-\n-BEGIN {\n-       suites = \"static struct clar_suite _clar_suites[] = {\\n\"\n-}\n-\n-{\n-       print\n-       name = $3; sub(/\\(.*$/, \"\", name)\n-       suite = name; sub(/^test_/, \"\", suite); sub(/__.*$/, \"\", suite)\n-       short_name = name; sub(/^.*__/, \"\", short_name)\n-       cb = \"{ \\\"\" short_name \"\\\", &\" name \" }\"\n-       if (suite != prev_suite) {\n-               add_suite(prev_suite, initialize, cleanup, count)\n-               if (callbacks) callbacks = callbacks \"};\\n\"\n-               callbacks = callbacks \"static const struct clar_func _clar_cb_\" suite \"[] = {\\n\"\n-               initialize = \"{ NULL, NULL }\"\n-               cleanup = \"{ NULL, NULL }\"\n-               count = 0\n-               prev_suite = suite\n-       }\n-       if (short_name == \"initialize\") {\n-               initialize = cb\n-       } else if (short_name == \"cleanup\") {\n-               cleanup = cb\n-       } else {\n-               callbacks = callbacks \"    \" cb \",\\n\"\n-               count++\n-       }\n-}\n-\n-END {\n-       add_suite(suite, initialize, cleanup, count)\n-       suites = suites \"};\"\n-       if (callbacks) callbacks = callbacks \"};\"\n-       print callbacks\n-       print suites\n-       print \"static const size_t _clar_suite_count = \" suite_count \";\"\n-       print \"static const size_t _clar_callback_count = \" callback_count \";\"\n-}\ndiff --git a/t/unit-tests/generate-clar-suites.sh b/t/unit-tests/generate-clar-suites.sh\nnew file mode 100755\nindex 0000000000000000000000000000000000000000..d5c712221e46a2eaa288fff5262438e5f04d6f72\n--- /dev/null\n+++ b/t/unit-tests/generate-clar-suites.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+if test $# -lt 2\n+then\n+\techo \"USAGE: $0 <CLAR_DECLS_H> <OUTPUT>\" 2>&1\n+\texit 1\n+fi\n+\n+CLAR_DECLS_H=\"$1\"\n+OUTPUT=\"$2\"\n+\n+awk '\n+\tfunction add_suite(suite, initialize, cleanup, count) {\n+\t\tif (!suite) return\n+\t\tsuite_count++\n+\t\tcallback_count += count\n+\t\tsuites = suites \"    {\\n\"\n+\t\tsuites = suites \"        \\\"\" suite \"\\\",\\n\"\n+\t\tsuites = suites \"        \" initialize \",\\n\"\n+\t\tsuites = suites \"        \" cleanup \",\\n\"\n+\t\tsuites = suites \"        _clar_cb_\" suite \", \" count \", 1\\n\"\n+\t\tsuites = suites \"    },\\n\"\n+\t}\n+\n+\tBEGIN {\n+\t\tsuites = \"static struct clar_suite _clar_suites[] = {\\n\"\n+\t}\n+\n+\t{\n+\t\tprint\n+\t\tname = $3; sub(/\\(.*$/, \"\", name)\n+\t\tsuite = name; sub(/^test_/, \"\", suite); sub(/__.*$/, \"\", suite)\n+\t\tshort_name = name; sub(/^.*__/, \"\", short_name)\n+\t\tcb = \"{ \\\"\" short_name \"\\\", &\" name \" }\"\n+\t\tif (suite != prev_suite) {\n+\t\t\tadd_suite(prev_suite, initialize, cleanup, count)\n+\t\t\tif (callbacks) callbacks = callbacks \"};\\n\"\n+\t\t\tcallbacks = callbacks \"static const struct clar_func _clar_cb_\" suite \"[] = {\\n\"\n+\t\t\tinitialize = \"{ NULL, NULL }\"\n+\t\t\tcleanup = \"{ NULL, NULL }\"\n+\t\t\tcount = 0\n+\t\t\tprev_suite = suite\n+\t\t}\n+\t\tif (short_name == \"initialize\") {\n+\t\t\tinitialize = cb\n+\t\t} else if (short_name == \"cleanup\") {\n+\t\t\tcleanup = cb\n+\t\t} else {\n+\t\t\tcallbacks = callbacks \"    \" cb \",\\n\"\n+\t\t\tcount++\n+\t\t}\n+\t}\n+\n+\tEND {\n+\t\tadd_suite(suite, initialize, cleanup, count)\n+\t\tsuites = suites \"};\"\n+\t\tif (callbacks) callbacks = callbacks \"};\"\n+\t\tprint callbacks\n+\t\tprint suites\n+\t\tprint \"static const size_t _clar_suite_count = \" suite_count \";\"\n+\t\tprint \"static const size_t _clar_callback_count = \" callback_count \";\"\n+\t}\n+' \"$CLAR_DECLS_H\" >\"$OUTPUT\"\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506969","messageId":"20241111-pks-clar-build-improvements-v2-2-d4794d8d1b30@pks.im","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im","subject":"[PATCH v2 2/4] cmake: use SH_EXE to execute clar scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-11T08:24:58Z","receivedAt":"2024-11-11T08:25:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In 30bf9f0aaa (cmake: set up proper dependencies for generated clar\nheaders, 2024-10-21), we have deduplicated the logic to generate our\nclar headers by reusing the same scripts that our Makefile does. Despite\nthe deduplication, this refactoring also made us rebuild the headers in\ncase the source files change, which didn't happen previously.\n\nThe commit also introduced an issue though: we execute the scripts\ndirectly, so when the host does not have \"/bin/sh\" available they will\nfail. This is for example the case on Windows when importing the CMake\nproject into Microsoft Visual Studio.\n\nAddress the issue by invoking the scripts with `SH_EXE`, which contains\nthe discovered path of the shell interpreter.\n\nWhile at it, wrap the overly long lines in the CMake build instructions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex da99dc3087a218d30e0fd1044567d7148d0d80a9..2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1005,10 +1005,13 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n-\tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n-\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n+\tCOMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\t\t${clar_test_SUITES}\n+\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n+\t\t${clar_test_SUITES})\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n+\tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506970","messageId":"20241111-pks-clar-build-improvements-v2-3-d4794d8d1b30@pks.im","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im","subject":"[PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-11T08:24:59Z","receivedAt":"2024-11-11T08:25:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Pass the VERBATIM option to `add_custom_command()`. Like this, all\narguments to the commands will be escaped properly for the build tool so\nthat the invoked command receives each argument unchanged.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t${clar_test_SUITES}\n \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n-\t\t${clar_test_SUITES})\n+\t\t${clar_test_SUITES}\n+\tVERBATIM)\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n-\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\tVERBATIM)\n \n add_library(unit-tests-lib ${clar_test_SUITES}\n \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506971","messageId":"20241111-pks-clar-build-improvements-v2-4-d4794d8d1b30@pks.im","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im","subject":"[PATCH v2 4/4] Makefile: let clar header targets depend on their scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-11T08:25:00Z","receivedAt":"2024-11-11T08:25:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The targets that generate clar headers depend on their source files, but\nnot on the script that is actually generating the output. Fix the issue\nby adding the missing dependencies.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5232b913fd20f01a7e5f41d46178e93d52c9f534..549b24e7fdbbdc173dfec79cdaddf67ccba52e14 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3904,9 +3904,9 @@ GIT-TEST-SUITES: FORCE\n \t\techo \"$$FLAGS\" >GIT-TEST-SUITES; \\\n             fi\n \n-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) $(UNIT_TEST_DIR)/generate-clar-decls.sh GIT-TEST-SUITES\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(filter %.c,$^)\n-$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n+$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(UNIT_TEST_DIR)/generate-clar-suites.sh\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite\n $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite\n $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n\n-- \n2.47.0.229.g8f8d6eee53.dirty\n\n"},{"id":"506973","messageId":"ZzHAfgR_NpBfTkpV@pks.im","threadId":"62468","inReplyTo":"xmqqiksuk0yk.fsf@gitster.g","subject":"Re: [PATCH 0/3] Build improvements for clar","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-11T08:29:50Z","receivedAt":"2024-11-11T08:30:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 11, 2024 at 10:34:27AM +0900, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n> > I've been using the CMake build in Visual Studio the last couple of days\n> > as my hard drive with linux on it died. I ended up with a slightly\n> > different fix using \"sh -c\" rather than putting the awk script inside\n> > a shell script. See the diff below. I don't have a strong preference\n> > either way but it would be nice to fix the line wrapping and add\n> > VERBATIM so that paths containing special characters are quoted correctly\n> \n> Thanks for comments.\n> \n> I've committed the same sin number of times, but a scriptlet written\n> in a third language as a string literal in a shell script is\n> somewhat awkward to maintain, so I may have slight preference for\n> your variant.  Either way, we are now letting the shell, and not\n> CMake, to spawn \"awk\", so if that was the reason why the file needs\n> to be changed (i.e. CMake perhaps failed to or found a wrong awk),\n> either of your two approaches would solve that by delegating the\n> task to the shell.\n\nYeah, I don't think it's particularly beautiful either. Personally, I'd\nstill lean towards my solution, mostly because it allows us to iterate\nmore readily in the future in case we ever need more logic in this\ncontext, and it avoids having to handle the redirect in the build\nsystem.\n\nSo I'll take the VERBATIM and line wrapping suggestions, but for now\nkeep the shell script. I'll adapt though if you think that this is too\nmuch of an abomination.\n\nPatrick\n"},{"id":"506980","messageId":"2757e5ab-6fce-40db-b11d-af9577a66908@gmail.com","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im","subject":"Re: [PATCH v2 0/4] Build improvements for clar","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-11-11T10:09:41Z","receivedAt":"2024-11-11T10:09:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nI've not actually tested them but having read through them these patches \nlook good to me. While it may be a bit ugly to have an awk script \nlurking inside a shell script it does mean if we ever change the \nimplementation of the shell script to process the files in a different \nway we wont have to worry about updating the build system(s).\n\nBest Wishes\n\nPhillip\n\nOn 11/11/2024 08:24, Patrick Steinhardt wrote:\n> Hi,\n> \n> Dscho has reported in [1] that the CMake build instructions for clar do\n> not work well on Windows/MSVC because we execute the shell scripts\n> directly instead of using the discovered `SH_EXE`. This small patch\n> series fixes the issue.\n> \n> Changes in v2:\n> \n>      - Wrap overly long lines in the CMake build instructions.\n>      - Add the VERBATIM option.\n> \n> Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im\n> \n> Thanks!\n> \n> Patrick\n> \n> [1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@gmx.de>\n> \n> To: git@vger.kernel.org\n> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Cc: Phillip Wood <phillip.wood123@gmail.com>\n> Cc: Junio C Hamano <gitster@pobox.com>\n> \n> Patrick Steinhardt (4):\n>        t/unit-tests: convert \"clar-generate.awk\" into a shell script\n>        cmake: use SH_EXE to execute clar scripts\n>        cmake: use verbatim arguments when invoking clar commands\n>        Makefile: let clar header targets depend on their scripts\n> \n>   Makefile                             |  6 ++--\n>   contrib/buildsystems/CMakeLists.txt  | 16 ++++++---\n>   t/unit-tests/clar-generate.awk       | 50 ----------------------------\n>   t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n>   4 files changed, 78 insertions(+), 57 deletions(-)\n> \n> Range-diff versus v1:\n> \n> 1:  23d84e6c50 ! 1:  832222f7f5 t/unit-tests: convert \"clar-generate.awk\" into a shell script\n>      @@ Commit message\n>           may otherwise be a problem with build systems on platforms that use a\n>           different shell.\n>       \n>      +    While at it, wrap the overly lines in the CMake build instructions.\n>      +\n>           Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>       \n>        ## Makefile ##\n>      @@ contrib/buildsystems/CMakeLists.txt: add_custom_command(OUTPUT \"${CMAKE_BINARY_D\n>        add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>       -\tCOMMAND 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\"\n>       -\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n>      -+\tCOMMAND \"${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\"\n>      -+\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n>      ++\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n>      ++\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>      ++\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>      ++\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n>      ++\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n>        \n>        add_library(unit-tests-lib ${clar_test_SUITES}\n>        \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n> 2:  a41b1f4746 < -:  ---------- cmake: use SH_EXE to execute clar scripts\n> -:  ---------- > 2:  38601f7bdf cmake: use SH_EXE to execute clar scripts\n> -:  ---------- > 3:  146ebd3841 cmake: use verbatim arguments when invoking clar commands\n> 3:  01c1c51e6a = 4:  341c831192 Makefile: let clar header targets depend on their scripts\n> \n> ---\n> base-commit: facbe4f633e4ad31e641f64617bc88074c659959\n> change-id: 20241108-pks-clar-build-improvements-1c3962a9a79f\n> \n\n"},{"id":"507085","messageId":"xmqqv7wt9ydf.fsf@gitster.g","threadId":"62468","inReplyTo":"ZzHAfgR_NpBfTkpV@pks.im","subject":"Re: [PATCH 0/3] Build improvements for clar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-11T22:52:44Z","receivedAt":"2024-11-11T22:52:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> So I'll take the VERBATIM and line wrapping suggestions, but for now\n> keep the shell script. I'll adapt though if you think that this is too\n> much of an abomination.\n\nI do not expect me to be editing the awk script buried inside a\nshell string literal myself, so it is more or less a \"meh\" to me,\nbut once people start iterating, they may find it irritating that\ntheir favourite editor do not help them with its \"awk\" mode while\nediting the script.  Once that happens, they may split the script\nout themselves.  Before that, I do not deeply care ;-)\n\nThanks.\n\n\n"},{"id":"507086","messageId":"xmqqr07h9y35.fsf@gitster.g","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-1-d4794d8d1b30@pks.im","subject":"Re: [PATCH v2 1/4] t/unit-tests: convert \"clar-generate.awk\" into a shell script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-11T22:58:54Z","receivedAt":"2024-11-11T22:58:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> While at it, wrap the overly lines in the CMake build instructions.\n\noverly \"long\" lines?\n"},{"id":"507105","messageId":"ZzLuD4p_O7yU8MT1@pks.im","threadId":"62468","inReplyTo":"xmqqr07h9y35.fsf@gitster.g","subject":"Re: [PATCH v2 1/4] t/unit-tests: convert \"clar-generate.awk\" into a shell script","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-12T05:56:31Z","receivedAt":"2024-11-12T05:56:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Nov 12, 2024 at 07:58:54AM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > While at it, wrap the overly lines in the CMake build instructions.\n> \n> overly \"long\" lines?\n\nOops, yes. Queued locally, will send it out when receiving more feedback\non this series.\n\nPatrick\n"},{"id":"507223","messageId":"82d3f3aa-b11c-318f-9d47-35e0ebf3ebce@gmx.de","threadId":"62468","inReplyTo":"20241111-pks-clar-build-improvements-v2-3-d4794d8d1b30@pks.im","subject":"Re: [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-11-13T15:41:25Z","receivedAt":"2024-11-13T15:41:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Patrick,\n\nOn Mon, 11 Nov 2024, Patrick Steinhardt wrote:\n\n> Pass the VERBATIM option to `add_custom_command()`. Like this, all\n> arguments to the commands will be escaped properly for the build tool so\n> that the invoked command receives each argument unchanged.\n\nI would not be surprised if this `VERBATIM` was unaware of the quirky\nescaping that the MSYS2 runtime (and therefore the Bash) requires. See the\ncommit message of ad1559252945 (tests: add a helper to stress test\nargument quoting, 2019-09-18) for details.\n\nHaving said that, this patch certainly does not make things _worse_, even\nif it probably does not have the intended effect on Windows. But then, it\ndoes not matter because most git/git source checkouts live at absolute\npaths that do not need to be quoted.\n\nCiao,\nJohannes\n\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  contrib/buildsystems/CMakeLists.txt | 6 ++++--\n>  1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>  \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>  \t\t${clar_test_SUITES}\n>  \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n> -\t\t${clar_test_SUITES})\n> +\t\t${clar_test_SUITES}\n> +\tVERBATIM)\n>  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>  \tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n>  \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>  \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>  \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n> -\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n> +\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> +\tVERBATIM)\n>\n>  add_library(unit-tests-lib ${clar_test_SUITES}\n>  \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n>\n> --\n> 2.47.0.229.g8f8d6eee53.dirty\n>\n>\n"},{"id":"507264","messageId":"53d40c4a-0f88-428a-8fae-cd8cecbe1768@gmail.com","threadId":"62468","inReplyTo":"82d3f3aa-b11c-318f-9d47-35e0ebf3ebce@gmx.de","subject":"Re: [PATCH v2 3/4] cmake: use verbatim arguments when invoking clar commands","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-11-14T10:28:34Z","receivedAt":"2024-11-14T10:28:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 13/11/2024 15:41, Johannes Schindelin wrote:\n> \n> I would not be surprised if this `VERBATIM` was unaware of the quirky\n> escaping that the MSYS2 runtime (and therefore the Bash) requires. See the\n> commit message of ad1559252945 (tests: add a helper to stress test\n> argument quoting, 2019-09-18) for details.\n> \n> Having said that, this patch certainly does not make things _worse_, even\n> if it probably does not have the intended effect on Windows. But then, it\n> does not matter because most git/git source checkouts live at absolute\n> paths that do not need to be quoted.\n\nIt may not be perfect but it is an improvement on Windows. When I was \nplaying around with\n\n\tCOMMAND ${SH_EXE} -c [[awk -f \"$1\" \"$2\" >\"$3\"]] ...\n\nto build clar.suite on Windows it failed to work without VERBATIM.\n\nBest Wishes\n\nPhillip\n\n> Ciao,\n> Johannes\n> \n>>\n>> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>> ---\n>>   contrib/buildsystems/CMakeLists.txt | 6 ++++--\n>>   1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n>> index 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644\n>> --- a/contrib/buildsystems/CMakeLists.txt\n>> +++ b/contrib/buildsystems/CMakeLists.txt\n>> @@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>>   \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>>   \t\t${clar_test_SUITES}\n>>   \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n>> -\t\t${clar_test_SUITES})\n>> +\t\t${clar_test_SUITES}\n>> +\tVERBATIM)\n>>   add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>>   \tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n>>   \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>>   \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>>   \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n>> -\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n>> +\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>> +\tVERBATIM)\n>>\n>>   add_library(unit-tests-lib ${clar_test_SUITES}\n>>   \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n>>\n>> --\n>> 2.47.0.229.g8f8d6eee53.dirty\n>>\n>>\n\n"},{"id":"507344","messageId":"20241115-pks-clar-build-improvements-v3-0-29672bf65ec6@pks.im","threadId":"62468","inReplyTo":"20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im","subject":"[PATCH v3 0/4] Build improvements for clar","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-15T07:32:40Z","receivedAt":"2024-11-15T07:32:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nDscho has reported in [1] that the CMake build instructions for clar do\nnot work well on Windows/MSVC because we execute the shell scripts\ndirectly instead of using the discovered `SH_EXE`. This small patch\nseries fixes the issue.\n\nChanges in v2:\n\n    - Wrap overly long lines in the CMake build instructions.\n    - Add the VERBATIM option.\n\nChanges in v3:\n\n    - Fix missing word.\n\nLink to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im\nLink to v2: https://lore.kernel.org/r/20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <3b2cb360-297a-915c-ae27-c45f38fa49b9@gmx.de>\n\nTo: git@vger.kernel.org\nCc: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nCc: Phillip Wood <phillip.wood123@gmail.com>\nCc: Junio C Hamano <gitster@pobox.com>\n\nPatrick Steinhardt (4):\n      t/unit-tests: convert \"clar-generate.awk\" into a shell script\n      cmake: use SH_EXE to execute clar scripts\n      cmake: use verbatim arguments when invoking clar commands\n      Makefile: let clar header targets depend on their scripts\n\n Makefile                             |  6 ++--\n contrib/buildsystems/CMakeLists.txt  | 16 ++++++---\n t/unit-tests/clar-generate.awk       | 50 ----------------------------\n t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 78 insertions(+), 57 deletions(-)\n\nRange-diff versus v2:\n\n1:  90d2402c5a ! 1:  dc713c236b t/unit-tests: convert \"clar-generate.awk\" into a shell script\n    @@ Commit message\n         may otherwise be a problem with build systems on platforms that use a\n         different shell.\n     \n    -    While at it, wrap the overly lines in the CMake build instructions.\n    +    While at it, wrap the overly long lines in the CMake build instructions.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n2:  cf298664d8 = 2:  19d292cc5c cmake: use SH_EXE to execute clar scripts\n3:  ff557f8985 = 3:  52f5090a76 cmake: use verbatim arguments when invoking clar commands\n4:  c634e2a6d4 = 4:  35117454d4 Makefile: let clar header targets depend on their scripts\n\n---\nbase-commit: facbe4f633e4ad31e641f64617bc88074c659959\nchange-id: 20241108-pks-clar-build-improvements-1c3962a9a79f\n\n"},{"id":"507345","messageId":"20241115-pks-clar-build-improvements-v3-2-29672bf65ec6@pks.im","threadId":"62468","inReplyTo":"20241115-pks-clar-build-improvements-v3-0-29672bf65ec6@pks.im","subject":"[PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-15T07:32:42Z","receivedAt":"2024-11-15T07:33:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In 30bf9f0aaa (cmake: set up proper dependencies for generated clar\nheaders, 2024-10-21), we have deduplicated the logic to generate our\nclar headers by reusing the same scripts that our Makefile does. Despite\nthe deduplication, this refactoring also made us rebuild the headers in\ncase the source files change, which didn't happen previously.\n\nThe commit also introduced an issue though: we execute the scripts\ndirectly, so when the host does not have \"/bin/sh\" available they will\nfail. This is for example the case on Windows when importing the CMake\nproject into Microsoft Visual Studio.\n\nAddress the issue by invoking the scripts with `SH_EXE`, which contains\nthe discovered path of the shell interpreter.\n\nWhile at it, wrap the overly long lines in the CMake build instructions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex da99dc3087a218d30e0fd1044567d7148d0d80a9..2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1005,10 +1005,13 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n-\tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n-\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n+\tCOMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\t\t${clar_test_SUITES}\n+\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n+\t\t${clar_test_SUITES})\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n+\tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n\n-- \n2.47.0.251.gb31fb630c0.dirty\n\n"},{"id":"507346","messageId":"20241115-pks-clar-build-improvements-v3-1-29672bf65ec6@pks.im","threadId":"62468","inReplyTo":"20241115-pks-clar-build-improvements-v3-0-29672bf65ec6@pks.im","subject":"[PATCH v3 1/4] t/unit-tests: convert \"clar-generate.awk\" into a shell script","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-15T07:32:41Z","receivedAt":"2024-11-15T07:33:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Convert \"clar-generate.awk\" into a shell script that invokes awk(1).\nThis allows us to avoid the shell redirect in the build system, which\nmay otherwise be a problem with build systems on platforms that use a\ndifferent shell.\n\nWhile at it, wrap the overly long lines in the CMake build instructions.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                             |  2 +-\n contrib/buildsystems/CMakeLists.txt  |  7 ++--\n t/unit-tests/clar-generate.awk       | 50 ----------------------------\n t/unit-tests/generate-clar-suites.sh | 63 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 69 insertions(+), 53 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex d06c9a8ffa7b637050c9619a367fbe61e7243a74..5232b913fd20f01a7e5f41d46178e93d52c9f534 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3907,7 +3907,7 @@ GIT-TEST-SUITES: FORCE\n $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(filter %.c,$^)\n $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n-\t$(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite\n+\t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite\n $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite\n $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n $(CLAR_TEST_OBJS): EXTRA_CPPFLAGS = -I$(UNIT_TEST_DIR)\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 8974bb9fa202a0556fd9b16d105836d8cb66f543..da99dc3087a218d30e0fd1044567d7148d0d80a9 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1008,8 +1008,11 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n-\tCOMMAND 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\"\n-\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk\" \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n+\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n+\tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n \n add_library(unit-tests-lib ${clar_test_SUITES}\n \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\ndiff --git a/t/unit-tests/clar-generate.awk b/t/unit-tests/clar-generate.awk\ndeleted file mode 100644\nindex ab71ce6c9fc2c3d49d826f3550a95be893114508..0000000000000000000000000000000000000000\n--- a/t/unit-tests/clar-generate.awk\n+++ /dev/null\n@@ -1,50 +0,0 @@\n-function add_suite(suite, initialize, cleanup, count) {\n-       if (!suite) return\n-       suite_count++\n-       callback_count += count\n-       suites = suites \"    {\\n\"\n-       suites = suites \"        \\\"\" suite \"\\\",\\n\"\n-       suites = suites \"        \" initialize \",\\n\"\n-       suites = suites \"        \" cleanup \",\\n\"\n-       suites = suites \"        _clar_cb_\" suite \", \" count \", 1\\n\"\n-       suites = suites \"    },\\n\"\n-}\n-\n-BEGIN {\n-       suites = \"static struct clar_suite _clar_suites[] = {\\n\"\n-}\n-\n-{\n-       print\n-       name = $3; sub(/\\(.*$/, \"\", name)\n-       suite = name; sub(/^test_/, \"\", suite); sub(/__.*$/, \"\", suite)\n-       short_name = name; sub(/^.*__/, \"\", short_name)\n-       cb = \"{ \\\"\" short_name \"\\\", &\" name \" }\"\n-       if (suite != prev_suite) {\n-               add_suite(prev_suite, initialize, cleanup, count)\n-               if (callbacks) callbacks = callbacks \"};\\n\"\n-               callbacks = callbacks \"static const struct clar_func _clar_cb_\" suite \"[] = {\\n\"\n-               initialize = \"{ NULL, NULL }\"\n-               cleanup = \"{ NULL, NULL }\"\n-               count = 0\n-               prev_suite = suite\n-       }\n-       if (short_name == \"initialize\") {\n-               initialize = cb\n-       } else if (short_name == \"cleanup\") {\n-               cleanup = cb\n-       } else {\n-               callbacks = callbacks \"    \" cb \",\\n\"\n-               count++\n-       }\n-}\n-\n-END {\n-       add_suite(suite, initialize, cleanup, count)\n-       suites = suites \"};\"\n-       if (callbacks) callbacks = callbacks \"};\"\n-       print callbacks\n-       print suites\n-       print \"static const size_t _clar_suite_count = \" suite_count \";\"\n-       print \"static const size_t _clar_callback_count = \" callback_count \";\"\n-}\ndiff --git a/t/unit-tests/generate-clar-suites.sh b/t/unit-tests/generate-clar-suites.sh\nnew file mode 100755\nindex 0000000000000000000000000000000000000000..d5c712221e46a2eaa288fff5262438e5f04d6f72\n--- /dev/null\n+++ b/t/unit-tests/generate-clar-suites.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+if test $# -lt 2\n+then\n+\techo \"USAGE: $0 <CLAR_DECLS_H> <OUTPUT>\" 2>&1\n+\texit 1\n+fi\n+\n+CLAR_DECLS_H=\"$1\"\n+OUTPUT=\"$2\"\n+\n+awk '\n+\tfunction add_suite(suite, initialize, cleanup, count) {\n+\t\tif (!suite) return\n+\t\tsuite_count++\n+\t\tcallback_count += count\n+\t\tsuites = suites \"    {\\n\"\n+\t\tsuites = suites \"        \\\"\" suite \"\\\",\\n\"\n+\t\tsuites = suites \"        \" initialize \",\\n\"\n+\t\tsuites = suites \"        \" cleanup \",\\n\"\n+\t\tsuites = suites \"        _clar_cb_\" suite \", \" count \", 1\\n\"\n+\t\tsuites = suites \"    },\\n\"\n+\t}\n+\n+\tBEGIN {\n+\t\tsuites = \"static struct clar_suite _clar_suites[] = {\\n\"\n+\t}\n+\n+\t{\n+\t\tprint\n+\t\tname = $3; sub(/\\(.*$/, \"\", name)\n+\t\tsuite = name; sub(/^test_/, \"\", suite); sub(/__.*$/, \"\", suite)\n+\t\tshort_name = name; sub(/^.*__/, \"\", short_name)\n+\t\tcb = \"{ \\\"\" short_name \"\\\", &\" name \" }\"\n+\t\tif (suite != prev_suite) {\n+\t\t\tadd_suite(prev_suite, initialize, cleanup, count)\n+\t\t\tif (callbacks) callbacks = callbacks \"};\\n\"\n+\t\t\tcallbacks = callbacks \"static const struct clar_func _clar_cb_\" suite \"[] = {\\n\"\n+\t\t\tinitialize = \"{ NULL, NULL }\"\n+\t\t\tcleanup = \"{ NULL, NULL }\"\n+\t\t\tcount = 0\n+\t\t\tprev_suite = suite\n+\t\t}\n+\t\tif (short_name == \"initialize\") {\n+\t\t\tinitialize = cb\n+\t\t} else if (short_name == \"cleanup\") {\n+\t\t\tcleanup = cb\n+\t\t} else {\n+\t\t\tcallbacks = callbacks \"    \" cb \",\\n\"\n+\t\t\tcount++\n+\t\t}\n+\t}\n+\n+\tEND {\n+\t\tadd_suite(suite, initialize, cleanup, count)\n+\t\tsuites = suites \"};\"\n+\t\tif (callbacks) callbacks = callbacks \"};\"\n+\t\tprint callbacks\n+\t\tprint suites\n+\t\tprint \"static const size_t _clar_suite_count = \" suite_count \";\"\n+\t\tprint \"static const size_t _clar_callback_count = \" callback_count \";\"\n+\t}\n+' \"$CLAR_DECLS_H\" >\"$OUTPUT\"\n\n-- \n2.47.0.251.gb31fb630c0.dirty\n\n"},{"id":"507347","messageId":"20241115-pks-clar-build-improvements-v3-3-29672bf65ec6@pks.im","threadId":"62468","inReplyTo":"20241115-pks-clar-build-improvements-v3-0-29672bf65ec6@pks.im","subject":"[PATCH v3 3/4] cmake: use verbatim arguments when invoking clar commands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-15T07:32:43Z","receivedAt":"2024-11-15T07:33:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Pass the VERBATIM option to `add_custom_command()`. Like this, all\narguments to the commands will be escaped properly for the build tool so\nthat the invoked command receives each argument unchanged.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd..8c71f5a1d0290c9204e094fb266f10c7b70af9fb 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1009,13 +1009,15 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t${clar_test_SUITES}\n \tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n-\t\t${clar_test_SUITES})\n+\t\t${clar_test_SUITES}\n+\tVERBATIM)\n add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n-\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\")\n+\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\tVERBATIM)\n \n add_library(unit-tests-lib ${clar_test_SUITES}\n \t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n\n-- \n2.47.0.251.gb31fb630c0.dirty\n\n"},{"id":"507348","messageId":"20241115-pks-clar-build-improvements-v3-4-29672bf65ec6@pks.im","threadId":"62468","inReplyTo":"20241115-pks-clar-build-improvements-v3-0-29672bf65ec6@pks.im","subject":"[PATCH v3 4/4] Makefile: let clar header targets depend on their scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-15T07:32:44Z","receivedAt":"2024-11-15T07:33:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The targets that generate clar headers depend on their source files, but\nnot on the script that is actually generating the output. Fix the issue\nby adding the missing dependencies.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5232b913fd20f01a7e5f41d46178e93d52c9f534..549b24e7fdbbdc173dfec79cdaddf67ccba52e14 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3904,9 +3904,9 @@ GIT-TEST-SUITES: FORCE\n \t\techo \"$$FLAGS\" >GIT-TEST-SUITES; \\\n             fi\n \n-$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n+$(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) $(UNIT_TEST_DIR)/generate-clar-decls.sh GIT-TEST-SUITES\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(filter %.c,$^)\n-$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h\n+$(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(UNIT_TEST_DIR)/generate-clar-suites.sh\n \t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-suites.sh $< $(UNIT_TEST_DIR)/clar.suite\n $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite\n $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n\n-- \n2.47.0.251.gb31fb630c0.dirty\n\n"},{"id":"507529","messageId":"j5dohg2u5vguiffj3dkqimach725ca2yhyzu4cookd6gvjoav2@qizyg7mazhyz","threadId":"62468","inReplyTo":"20241115-pks-clar-build-improvements-v3-2-29672bf65ec6@pks.im","subject":"Re: [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-18T18:47:32Z","receivedAt":"2024-11-18T18:49:22Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/11/15 08:32AM, Patrick Steinhardt wrote:\n> In 30bf9f0aaa (cmake: set up proper dependencies for generated clar\n> headers, 2024-10-21), we have deduplicated the logic to generate our\n> clar headers by reusing the same scripts that our Makefile does. Despite\n> the deduplication, this refactoring also made us rebuild the headers in\n> case the source files change, which didn't happen previously.\n> \n> The commit also introduced an issue though: we execute the scripts\n> directly, so when the host does not have \"/bin/sh\" available they will\n> fail. This is for example the case on Windows when importing the CMake\n> project into Microsoft Visual Studio.\n> \n> Address the issue by invoking the scripts with `SH_EXE`, which contains\n> the discovered path of the shell interpreter.\n> \n> While at it, wrap the overly long lines in the CMake build instructions.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  contrib/buildsystems/CMakeLists.txt | 9 ++++++---\n>  1 file changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index da99dc3087a218d30e0fd1044567d7148d0d80a9..2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -1005,10 +1005,13 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n>  list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n>  list(TRANSFORM clar_test_SUITES APPEND \".c\")\n>  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> -\tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n> -\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n> +\tCOMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n\nIn the previous patch we used `${SHELL_PATH}` to execute the\n\"generate-clar-suites.sh\". Here we use `${SH_EXE}` to execute\n\"generate-clar-decls.sh\". From my understanding this is done to help\ndiscover the shell on different platforms. Naive question, would this\nalso be useful in the former patch?\n\n-Justin\n\n> +\t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> +\t\t${clar_test_SUITES}\n> +\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n> +\t\t${clar_test_SUITES})\n>  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n> -\tCOMMAND \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n> +\tCOMMAND ${SH_EXE} \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n>  \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n>  \t\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n>  \tDEPENDS \"${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-suites.sh\"\n> \n> -- \n> 2.47.0.251.gb31fb630c0.dirty\n> \n> \n"},{"id":"507567","messageId":"ZzwsZfNgJck3V45F@pks.im","threadId":"62468","inReplyTo":"j5dohg2u5vguiffj3dkqimach725ca2yhyzu4cookd6gvjoav2@qizyg7mazhyz","subject":"Re: [PATCH v3 2/4] cmake: use SH_EXE to execute clar scripts","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-11-19T06:13:01Z","receivedAt":"2024-11-19T06:13:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 18, 2024 at 12:47:32PM -0600, Justin Tobler wrote:\n> On 24/11/15 08:32AM, Patrick Steinhardt wrote:\n> > In 30bf9f0aaa (cmake: set up proper dependencies for generated clar\n> > headers, 2024-10-21), we have deduplicated the logic to generate our\n> > clar headers by reusing the same scripts that our Makefile does. Despite\n> > the deduplication, this refactoring also made us rebuild the headers in\n> > case the source files change, which didn't happen previously.\n> > \n> > The commit also introduced an issue though: we execute the scripts\n> > directly, so when the host does not have \"/bin/sh\" available they will\n> > fail. This is for example the case on Windows when importing the CMake\n> > project into Microsoft Visual Studio.\n> > \n> > Address the issue by invoking the scripts with `SH_EXE`, which contains\n> > the discovered path of the shell interpreter.\n> > \n> > While at it, wrap the overly long lines in the CMake build instructions.\n> > \n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  contrib/buildsystems/CMakeLists.txt | 9 ++++++---\n> >  1 file changed, 6 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> > index da99dc3087a218d30e0fd1044567d7148d0d80a9..2db80b7cc3c6aba840f18ffdc78d2cda1877d8cd 100644\n> > --- a/contrib/buildsystems/CMakeLists.txt\n> > +++ b/contrib/buildsystems/CMakeLists.txt\n> > @@ -1005,10 +1005,13 @@ parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n> >  list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n> >  list(TRANSFORM clar_test_SUITES APPEND \".c\")\n> >  add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> > -\tCOMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" ${clar_test_SUITES}\n> > -\tDEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES})\n> > +\tCOMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh\n> \n> In the previous patch we used `${SHELL_PATH}` to execute the\n> \"generate-clar-suites.sh\". Here we use `${SH_EXE}` to execute\n> \"generate-clar-decls.sh\". From my understanding this is done to help\n> discover the shell on different platforms. Naive question, would this\n> also be useful in the former patch?\n\nYes, it would. I decided to bundle the use of SH_EXE in our CMake build\ninstructions into a single patch though, which is the third patch, so\nthat I don't have to explain the change twice.\n\nPatrick\n"},{"id":"507632","messageId":"7jdpmcwlxtyzqrqmcftcdmrw3tqopawtuuzqf3uaff2gklaxba@covcdisniubk","threadId":"62468","inReplyTo":"20241115-pks-clar-build-improvements-v3-0-29672bf65ec6@pks.im","subject":"Re: [PATCH v3 0/4] Build improvements for clar","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2024-11-19T16:51:03Z","receivedAt":"2024-11-19T16:52:54Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 24/11/15 08:32AM, Patrick Steinhardt wrote:\n> Hi,\n> \n> Dscho has reported in [1] that the CMake build instructions for clar do\n> not work well on Windows/MSVC because we execute the shell scripts\n> directly instead of using the discovered `SH_EXE`. This small patch\n> series fixes the issue.\n> \n> Changes in v2:\n> \n>     - Wrap overly long lines in the CMake build instructions.\n>     - Add the VERBATIM option.\n> \n> Changes in v3:\n> \n>     - Fix missing word.\n> \n> Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im\n> Link to v2: https://lore.kernel.org/r/20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im\n> \n> Thanks!\n> \n> Patrick\n\nI've reviewed this series and overall this version looks good to me.\nEmbedding the awk script in the shell script and changing how the\nscripts get executed to address platform related build issues seems\nsensable to me.\n\n-Justin\n"},{"id":"507672","messageId":"xmqqiksiae9w.fsf@gitster.g","threadId":"62468","inReplyTo":"7jdpmcwlxtyzqrqmcftcdmrw3tqopawtuuzqf3uaff2gklaxba@covcdisniubk","subject":"Re: [PATCH v3 0/4] Build improvements for clar","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-11-20T01:24:11Z","receivedAt":"2024-11-20T01:24:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 24/11/15 08:32AM, Patrick Steinhardt wrote:\n>> Hi,\n>> \n>> Dscho has reported in [1] that the CMake build instructions for clar do\n>> not work well on Windows/MSVC because we execute the shell scripts\n>> directly instead of using the discovered `SH_EXE`. This small patch\n>> series fixes the issue.\n>> \n>> Changes in v2:\n>> \n>>     - Wrap overly long lines in the CMake build instructions.\n>>     - Add the VERBATIM option.\n>> \n>> Changes in v3:\n>> \n>>     - Fix missing word.\n>> \n>> Link to v1: https://lore.kernel.org/r/20241108-pks-clar-build-improvements-v1-0-25c1fe65ce37@pks.im\n>> Link to v2: https://lore.kernel.org/r/20241111-pks-clar-build-improvements-v2-0-d4794d8d1b30@pks.im\n>> \n>> Thanks!\n>> \n>> Patrick\n>\n> I've reviewed this series and overall this version looks good to me.\n> Embedding the awk script in the shell script and changing how the\n> scripts get executed to address platform related build issues seems\n> sensable to me.\n\nThanks, both.  Let's merge it down to 'next'.\n\n"}]}