{"thread":{"id":"62288","subject":"Bug report","startedAt":"2024-10-09T03:23:26Z","lastAt":"2024-10-21T06:59:32Z","messageCount":20,"participants":["Ed Reel","Johannes Schindelin","Patrick Steinhardt","Taylor Blau","Toon Claes"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"504509","messageId":"CAGjHeYfyH+cOMYYYHnFR+Vu9T+RbmzO1SpB_-kbmBSf1DitJhA@mail.gmail.com","threadId":"62288","inReplyTo":null,"subject":"Bug report","fromName":"Ed Reel","fromEmail":"edreel@gmail.com","sentAt":"2024-10-09T03:23:14Z","receivedAt":"2024-10-09T03:23:26Z","isPatch":false,"sender":{"key":"edreel@gmail.com","avatar":null},"body":"See attachment...\n\n\n\n-- \n\"God gave us two ears and one mouth to remind us we should listen\ntwice as much as we talk.\"\n\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\nAttempt to build with cmake\n\nWhat did you expect to happen? (Expected behavior)\nSuccessful build\n\nWhat happened instead? (Actual behavior)\nUnsuccessful build\n\nWhat's different between what you expected and what actually happened?\n[5*10/162] Building C object CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o\nFAILED: CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o \n/usr/local/bin/cc -DBINDIR=\\\"bin\\\" -DDEFAULT_GIT_TEMPLATE_DIR=\\\"share/git-core/templates\\\" -DDEFAULT_HELP_FORMAT=\\\"html\\\" -DETC_GITATTRIBUTES=\\\"etc/gitattributes\\\" -DETC_GITCONFIG=\\\"etc/gitconfig\\\" -DFALLBACK_RUNTIME_PREFIX=\\\"/home/chronos\\\" -DFREAD_READS_DIRECTORIES -DGIT_BUILT_FROM_COMMIT=\\\"\\\" -DGIT_EXEC_PATH=\\\"libexec/git-core\\\" -DGIT_HOST_CPU=\\\"x86_64\\\" -DGIT_HTML_PATH=\\\"share/doc/git-doc\\\" -DGIT_INFO_PATH=\\\"share/info\\\" -DGIT_LOCALE_PATH=\\\"share/locale\\\" -DGIT_MAN_PATH=\\\"share/man\\\" -DGIT_USER_AGENT=\\\"git/2.47.0.GIT\\\" -DGIT_VERSION=\\\"2.47.0.GIT\\\" -DHAVE_ALLOCA_H -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_DEV_TTY -DHAVE_GETDELIM -DHAVE_PATHS_H -DHAVE_STRINGS_H -DHAVE_SYSINFO -DINTERNAL_QSORT -DNO_OPENSSL -DNO_STRLCPY -DPAGER_ENV=\"\\\"LESS=FRX LV=-c\\\"\" -DPROCFS_EXECUTABLE_PATH=\\\"/proc/self/exe\\\" -DRUNTIME_PREFIX -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\\\"git-compat-util.h\\\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\\\"git-compat-util.h\\\" -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1_DC -DSHA256_BLK -DSUPPORTS_SIMPLE_IPC -DUSE_CURL_FOR_IMAP_SEND -DUSE_LIBPCRE2 -I/usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/../.. -I/usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/builddir -I/usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/../../t/unit-tests -O2 -pipe -ffat-lto-objects -fPIC   -flto=auto -O3 -DNDEBUG -flto=auto -fno-fat-lto-objects -MD -MT CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o -MF CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o.d -o CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o -c /usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c\nIn file included from /usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c:1:\n/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/unit-test.h:3:10: fatal error: clar-decls.h: No such file or directory\n    3 | #include \"clar-decls.h\"\n      |          ^~~~~~~~~~~~~~\ncompilation terminated.\n[14*1/162] Linking C executable git\nIn function ‘strbuf_add’,\n    inlined from ‘strbuf_addstr’ at /usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/../../strbuf.h:310:2,\n    inlined from ‘verify_one_pack’ at /usr/local/tmp/crew/git.20241009021715.dir/builtin/verify-pack.c:40:3,\n    inlined from ‘cmd_verify_pack’ at /usr/local/tmp/crew/git.20241009021715.dir/builtin/verify-pack.c:90:7:\n/usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:312:9: warning: ‘memcpy’ writing 5 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=]\n  312 |         memcpy(sb->buf + sb->len, data, len);\n      |         ^\n/usr/local/tmp/crew/git.20241009021715.dir/strbuf.c: In function ‘cmd_verify_pack’:\n/usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:65:6: note: destination object ‘strbuf_slopbuf’ of size 1\n   65 | char strbuf_slopbuf[1];\n      |      ^\nIn function ‘strbuf_release’,\n    inlined from ‘strbuf_release’ at /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:75:6,\n    inlined from ‘bundle_uri_parse_line’ at /usr/local/tmp/crew/git.20241009021715.dir/bundle-uri.c:939:2,\n    inlined from ‘get_remote_bundle_uri’ at /usr/local/tmp/crew/git.20241009021715.dir/connect.c:527:8,\n    inlined from ‘get_bundle_uri’ at /usr/local/tmp/crew/git.20241009021715.dir/transport.c:406:9:\n/usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:78:17: warning: ‘free’ called on unallocated object ‘strbuf_slopbuf’ [-Wfree-nonheap-object]\n   78 |                 free(sb->buf);\n      |                 ^\n/usr/local/tmp/crew/git.20241009021715.dir/strbuf.c: In function ‘get_bundle_uri’:\n/usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:65:6: note: declared here\n   65 | char strbuf_slopbuf[1];\n      |      ^\nninja: build stopped: subcommand failed.\nThere was a build error.\n\n[System Info]\ngit version:\ngit version 2.46.1.GIT\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nlibcurl: 8.10.0\nzlib: 1.3.1\nuname: Linux 6.4.0-1mx-ahs-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.4.4-1~mx23+1 (2023-07-26) x86_64\ncompiler info: gnuc: 14.2\nlibc info: glibc: 2.27\n$SHELL (typically, interactive shell): /bin/bash\n"},{"id":"504970","messageId":"f7910f55-1a77-dec8-7aa2-36688d59c215@gmx.de","threadId":"62288","inReplyTo":"CAGjHeYfyH+cOMYYYHnFR+Vu9T+RbmzO1SpB_-kbmBSf1DitJhA@mail.gmail.com","subject":"Re: Bug report","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-10-14T06:10:05Z","receivedAt":"2024-10-14T06:10:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ed,\n\nOn Tue, 8 Oct 2024, Ed Reel wrote:\n\n> See attachment...\n\nYou will find that Git mailing list readers typically frown upon having to\ndeal with text that was stuffed into an attachment, because it is much\nharder to quote that plain text in the mail body. Maybe next time just\npaste it into the mail body?\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> Attempt to build with cmake\n\nBut which branch and/or commit?\n\n> What did you expect to happen? (Expected behavior)\n> Successful build\n\n> What happened instead? (Actual behavior)\n> Unsuccessful build\n\n> What's different between what you expected and what actually happened?\n> [5*10/162] Building C object CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o\n> FAILED: CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o\n> /usr/local/bin/cc -DBINDIR=\\\"bin\\\" -DDEFAULT_GIT_TEMPLATE_DIR=\\\"share/git-core/templates\\\" -DDEFAULT_HELP_FORMAT=\\\"html\\\" -DETC_GITATTRIBUTES=\\\"etc/gitattributes\\\" -DETC_GITCONFIG=\\\"etc/gitconfig\\\" -DFALLBACK_RUNTIME_PREFIX=\\\"/home/chronos\\\" -DFREAD_READS_DIRECTORIES -DGIT_BUILT_FROM_COMMIT=\\\"\\\" -DGIT_EXEC_PATH=\\\"libexec/git-core\\\" -DGIT_HOST_CPU=\\\"x86_64\\\" -DGIT_HTML_PATH=\\\"share/doc/git-doc\\\" -DGIT_INFO_PATH=\\\"share/info\\\" -DGIT_LOCALE_PATH=\\\"share/locale\\\" -DGIT_MAN_PATH=\\\"share/man\\\" -DGIT_USER_AGENT=\\\"git/2.47.0.GIT\\\" -DGIT_VERSION=\\\"2.47.0.GIT\\\" -DHAVE_ALLOCA_H -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_DEV_TTY -DHAVE_GETDELIM -DHAVE_PATHS_H -DHAVE_STRINGS_H -DHAVE_SYSINFO -DINTERNAL_QSORT -DNO_OPENSSL -DNO_STRLCPY -DPAGER_ENV=\"\\\"LESS=FRX LV=-c\\\"\" -DPROCFS_EXECUTABLE_PATH=\\\"/proc/self/exe\\\" -DRUNTIME_PREFIX -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\\\"git-compat-util.h\\\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\\\"git-compat-util.h\\\" -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1_DC -DSHA256_BLK -DSUPPORTS_SIMPLE_IPC -DUSE_CURL_FOR_IMAP_SEND -DUSE_LIBPCRE2 -I/usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/../.. -I/usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/builddir -I/usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/../../t/unit-tests -O2 -pipe -ffat-lto-objects -fPIC   -flto=auto -O3 -DNDEBUG -flto=auto -fno-fat-lto-objects -MD -MT CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o -MF CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o.d -o CMakeFiles/unit-tests-lib.dir/usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c.o -c /usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c\n> In file included from /usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/strvec.c:1:\n> /usr/local/tmp/crew/git.20241009021715.dir/t/unit-tests/unit-test.h:3:10: fatal error: clar-decls.h: No such file or directory\n>     3 | #include \"clar-decls.h\"\n>       |          ^~~~~~~~~~~~~~\n> compilation terminated.\n> [14*1/162] Linking C executable git\n> In function ‘strbuf_add’,\n>     inlined from ‘strbuf_addstr’ at /usr/local/tmp/crew/git.20241009021715.dir/contrib/buildsystems/../../strbuf.h:310:2,\n\nThe path starting with `/usr/` suggests that this is not Windows. And\nfurther below, the information from a working Git suggests that you try to\nbuild this on Linux.\n\nAt the moment, the CMake definition very much targets Windows, not any\nother platform, because it provides the only convenient way to build Git\nin Visual Studio without having to jump through substantial hoops.\n\nSince you seem on a Unix-ish platform, why not try the `make`-based build,\nwhich is how the Git maintainer builds the project?\n\nCiao,\nJohannes\n\n>     inlined from ‘verify_one_pack’ at /usr/local/tmp/crew/git.20241009021715.dir/builtin/verify-pack.c:40:3,\n>     inlined from ‘cmd_verify_pack’ at /usr/local/tmp/crew/git.20241009021715.dir/builtin/verify-pack.c:90:7:\n> /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:312:9: warning: ‘memcpy’ writing 5 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=]\n>   312 |         memcpy(sb->buf + sb->len, data, len);\n>       |         ^\n> /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c: In function ‘cmd_verify_pack’:\n> /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:65:6: note: destination object ‘strbuf_slopbuf’ of size 1\n>    65 | char strbuf_slopbuf[1];\n>       |      ^\n> In function ‘strbuf_release’,\n>     inlined from ‘strbuf_release’ at /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:75:6,\n>     inlined from ‘bundle_uri_parse_line’ at /usr/local/tmp/crew/git.20241009021715.dir/bundle-uri.c:939:2,\n>     inlined from ‘get_remote_bundle_uri’ at /usr/local/tmp/crew/git.20241009021715.dir/connect.c:527:8,\n>     inlined from ‘get_bundle_uri’ at /usr/local/tmp/crew/git.20241009021715.dir/transport.c:406:9:\n> /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:78:17: warning: ‘free’ called on unallocated object ‘strbuf_slopbuf’ [-Wfree-nonheap-object]\n>    78 |                 free(sb->buf);\n>       |                 ^\n> /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c: In function ‘get_bundle_uri’:\n> /usr/local/tmp/crew/git.20241009021715.dir/strbuf.c:65:6: note: declared here\n>    65 | char strbuf_slopbuf[1];\n>       |      ^\n> ninja: build stopped: subcommand failed.\n> There was a build error.\n\n> [System Info]\n> git version:\n> git version 2.46.1.GIT\n> cpu: x86_64\n> no commit associated with this build\n> sizeof-long: 8\n> sizeof-size_t: 8\n> shell-path: /bin/sh\n> libcurl: 8.10.0\n> zlib: 1.3.1\n> uname: Linux 6.4.0-1mx-ahs-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.4.4-1~mx23+1 (2023-07-26) x86_64\n> compiler info: gnuc: 14.2\n> libc info: glibc: 2.27\n> $SHELL (typically, interactive shell): /bin/bash\n"},{"id":"505024","messageId":"cover.1728914219.git.ps@pks.im","threadId":"62288","inReplyTo":"CAGjHeYfyH+cOMYYYHnFR+Vu9T+RbmzO1SpB_-kbmBSf1DitJhA@mail.gmail.com","subject":"[PATCH 0/3] cmake: fix autogenerated clar headers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-14T14:06:38Z","receivedAt":"2024-10-14T14:06:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series fixes how we set up autogenerated clar headers\nin CMake such that the clar builds again. The underlying issue is that\nwe accidentally added the source directory, not the build directory, to\nour list of include directories. I guess this went unnoticed because if\nyou also happen to use Makefiles in the same repo, we'd have found the\nMakefile-generated headers by accident.\n\nThe other two patches unify the infra to generate \"clar-decls.h\", which\nhas basically been pulled out of my build system modernization patch\nseries.\n\nTaylor: note that this conflicts with the patch series at [1]. The\nconflict can be solved by accepting the Makefile of this series here and\nadding the following on top of \"t/unit-tests/generate-clar-decls.sh\":\n\ndiff --cc Makefile\nindex 87b86c5be1,0101d349f3..0000000000\n--- a/Makefile\n+++ b/Makefile\ndiff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh\nindex 6646a90f71..49c61d29b3 100755\n--- a/t/unit-tests/generate-clar-decls.sh\n+++ b/t/unit-tests/generate-clar-decls.sh\n@@ -13,6 +13,6 @@ while test \"$#\" -ne 0\n do\n \tsuite=\"$1\"\n \tshift\n-\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n+\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\\)$/extern \\1;/p\" \"$suite\" ||\n \texit 1\n done >\"$OUTPUT\"\n\nThanks!\n\nPatrick\n\n[1]: <cover.1728903464.git.ps@pks.im>\n\nPatrick Steinhardt (3):\n  Makefile: extract script to generate clar declarations\n  cmake: fix compilation of clar-based unit tests\n  cmake: set up proper dependencies for generated clar headers\n\n Makefile                            |  4 +--\n contrib/buildsystems/CMakeLists.txt | 52 ++++++++---------------------\n t/unit-tests/generate-clar-decls.sh | 18 ++++++++++\n 3 files changed, 32 insertions(+), 42 deletions(-)\n create mode 100755 t/unit-tests/generate-clar-decls.sh\n\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505025","messageId":"346aa2f08307fb9c501a4b9ac3322beb44dc9d5d.1728914219.git.ps@pks.im","threadId":"62288","inReplyTo":"cover.1728914219.git.ps@pks.im","subject":"[PATCH 1/3] Makefile: extract script to generate clar declarations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-14T14:06:41Z","receivedAt":"2024-10-14T14:06:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Extract the script to generate function declarations for the clar unit\ntesting framework into a standalone script. This is done such that we\ncan reuse it in other build systems.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                            |  4 +---\n t/unit-tests/generate-clar-decls.sh | 18 ++++++++++++++++++\n 2 files changed, 19 insertions(+), 3 deletions(-)\n create mode 100755 t/unit-tests/generate-clar-decls.sh\n\ndiff --git a/Makefile b/Makefile\nindex feeed6f9321..87b86c5be1a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3904,9 +3904,7 @@ GIT-TEST-SUITES: FORCE\n             fi\n \n $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n-\t$(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \\\n-\t\tsed -ne \"s/^\\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\\)/extern \\1;/p\" $(UNIT_TEST_DIR)/$$suite.c; \\\n-\tdone >$@\n+\t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES))\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 $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\ndiff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh\nnew file mode 100755\nindex 00000000000..6646a90f711\n--- /dev/null\n+++ b/t/unit-tests/generate-clar-decls.sh\n@@ -0,0 +1,18 @@\n+#!/bin/sh\n+\n+if test $# -lt 2\n+then\n+\techo \"USAGE: $0 <OUTPUT> <SUITE>...\" 2>&1\n+\texit 1\n+fi\n+\n+OUTPUT=\"$1\"\n+shift\n+\n+while test \"$#\" -ne 0\n+do\n+\tsuite=\"$1\"\n+\tshift\n+\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n+\texit 1\n+done >\"$OUTPUT\"\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505026","messageId":"b9afeffda292a068e81d05b91f759a5c53a24b15.1728914219.git.ps@pks.im","threadId":"62288","inReplyTo":"cover.1728914219.git.ps@pks.im","subject":"[PATCH 2/3] cmake: fix compilation of clar-based unit tests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-14T14:06:44Z","receivedAt":"2024-10-14T14:06:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The compilation of clar-based unit tests is broken because we do not\nadd the binary directory into which we generate the \"clar-decls.h\" and\n\"clar.suite\" files as include directories. Instead, we accidentally set\nup the source directory as include directory.\n\nFix this and propagate the include directories of \"unit-tests.lib\" to\nthe \"unit-tests\" executable so that the latter uses the same include\ndirectories.\n\nReported-by: Ed Reel <edreel@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 62af7b33d2f..093852ad9d6 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1042,7 +1042,7 @@ file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\" \"${clar_decls}\" \"${clar\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n add_library(unit-tests-lib ${clar_test_SUITES} \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\")\n-target_include_directories(unit-tests-lib PRIVATE \"${CMAKE_SOURCE_DIR}/t/unit-tests\")\n+target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n add_executable(unit-tests \"${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c\")\n target_link_libraries(unit-tests unit-tests-lib common-main)\n set_target_properties(unit-tests\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505027","messageId":"129d28ae48ad984e9fae8bf803ae50a390b15a46.1728914219.git.ps@pks.im","threadId":"62288","inReplyTo":"cover.1728914219.git.ps@pks.im","subject":"[PATCH 3/3] cmake: set up proper dependencies for generated clar headers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-14T14:06:47Z","receivedAt":"2024-10-14T14:06:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The auto-generated headers used by clar are written at configure time\nand thus do not get regenerated automatically. Refactor the build\nrecipes such that we use custom commands instead, which also has the\nbenefit that we can reuse the same infrastructure as our Makefile.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------\n 1 file changed, 12 insertions(+), 38 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 093852ad9d6..9f80ab92656 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})\n endforeach()\n \n parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n-\n-set(clar_decls \"\")\n-set(clar_cbs \"\")\n-set(clar_cbs_count 0)\n-set(clar_suites \"static struct clar_suite _clar_suites[] = {\\n\")\n-list(LENGTH clar_test_SUITES clar_suites_count)\n-foreach(suite ${clar_test_SUITES})\n-\tfile(STRINGS \"${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c\" decls\n-\t\tREGEX \"^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\\\(void\\\\)$\")\n-\n-\tlist(LENGTH decls decls_count)\n-\tstring(REGEX REPLACE \"void (test_${suite}__([a-zA-Z_0-9]*))\\\\(void\\\\)\" \"    { \\\"\\\\2\\\", &\\\\1 },\\n\" cbs ${decls})\n-\tstring(JOIN \"\" cbs ${cbs})\n-\tlist(TRANSFORM decls PREPEND \"extern \")\n-\tstring(JOIN \";\\n\" decls ${decls})\n-\n-\tstring(APPEND clar_decls \"${decls};\\n\")\n-\tstring(APPEND clar_cbs\n-\t\t\"static const struct clar_func _clar_cb_${suite}[] = {\\n\"\n-\t\t${cbs}\n-\t\t\"};\\n\")\n-\tstring(APPEND clar_suites\n-\t\t\"    {\\n\"\n-\t\t\"        \\\"${suite}\\\",\\n\"\n-\t\t\"        { NULL, NULL },\\n\"\n-\t\t\"        { NULL, NULL },\\n\"\n-\t\t\"        _clar_cb_${suite}, ${decls_count}, 1\\n\"\n-\t\t\"    },\\n\")\n-\tmath(EXPR clar_cbs_count \"${clar_cbs_count}+${decls_count}\")\n-endforeach()\n-string(APPEND clar_suites\n-\t\"};\\n\"\n-\t\"static const size_t _clar_suite_count = ${clar_suites_count};\\n\"\n-\t\"static const size_t _clar_callback_count = ${clar_cbs_count};\\n\")\n-file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" \"${clar_decls}\")\n-file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\" \"${clar_decls}\" \"${clar_cbs}\" \"${clar_suites}\")\n-\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n-add_library(unit-tests-lib ${clar_test_SUITES} \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.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+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+\n+add_library(unit-tests-lib ${clar_test_SUITES}\n+\t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n+\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n+)\n target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n add_executable(unit-tests \"${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c\")\n target_link_libraries(unit-tests unit-tests-lib common-main)\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505073","messageId":"Zw2PzoYBvm59jy51@nand.local","threadId":"62288","inReplyTo":"cover.1728914219.git.ps@pks.im","subject":"Re: [PATCH 0/3] cmake: fix autogenerated clar headers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-14T21:40:30Z","receivedAt":"2024-10-14T21:40:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 14, 2024 at 04:06:38PM +0200, Patrick Steinhardt wrote:\n> Hi,\n>\n> this small patch series fixes how we set up autogenerated clar headers\n> in CMake such that the clar builds again. The underlying issue is that\n> we accidentally added the source directory, not the build directory, to\n> our list of include directories. I guess this went unnoticed because if\n> you also happen to use Makefiles in the same repo, we'd have found the\n> Makefile-generated headers by accident.\n>\n> The other two patches unify the infra to generate \"clar-decls.h\", which\n> has basically been pulled out of my build system modernization patch\n> series.\n>\n> Taylor: note that this conflicts with the patch series at [1]. The\n> conflict can be solved by accepting the Makefile of this series here and\n> adding the following on top of \"t/unit-tests/generate-clar-decls.sh\":\n\nThanks. I had initially kicked the latter out of 'seen' when applying\nboth to my tree. But when I went back and read the contents of these\npatches more carefully I saw your note here, which is very much\nappreciated.\n\nThanks,\nTaylor\n"},{"id":"505074","messageId":"Zw2QT/Kx6IoPhJtT@nand.local","threadId":"62288","inReplyTo":"346aa2f08307fb9c501a4b9ac3322beb44dc9d5d.1728914219.git.ps@pks.im","subject":"Re: [PATCH 1/3] Makefile: extract script to generate clar declarations","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-14T21:42:39Z","receivedAt":"2024-10-14T21:42:42Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 14, 2024 at 04:06:41PM +0200, Patrick Steinhardt wrote:\n> Extract the script to generate function declarations for the clar unit\n> testing framework into a standalone script. This is done such that we\n> can reuse it in other build systems.\n\nMakes sense.\n\n> +while test \"$#\" -ne 0\n> +do\n> +\tsuite=\"$1\"\n> +\tshift\n\nI'm perhaps nit-picking here, but I find:\n\n    for suite in \"$@\"\n    do\n        ...\n    done\n\nto be much more readable than 'while test \"$#\" -ne 0' and 'shift'\nabove, since the for-loop variant does not need to mangle its arguments\nwith shift.\n\n> +\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n\nThis part is a faithful translation of the original Makefile. Looking\ngood.\n\nThanks,\nTaylor\n"},{"id":"505075","messageId":"Zw2RNsptlkHCyTgi@nand.local","threadId":"62288","inReplyTo":"b9afeffda292a068e81d05b91f759a5c53a24b15.1728914219.git.ps@pks.im","subject":"Re: [PATCH 2/3] cmake: fix compilation of clar-based unit tests","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-14T21:46:30Z","receivedAt":"2024-10-14T21:46:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 14, 2024 at 04:06:44PM +0200, Patrick Steinhardt wrote:\n> The compilation of clar-based unit tests is broken because we do not\n> add the binary directory into which we generate the \"clar-decls.h\" and\n> \"clar.suite\" files as include directories. Instead, we accidentally set\n> up the source directory as include directory.\n\nI am confused. What is the difference between CMAKE_SOURCE_DIR and\nCMAKE_BINARY_DIR here, and why does the difference between the two\nmatter?\n\n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index 62af7b33d2f..093852ad9d6 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -1042,7 +1042,7 @@ file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\" \"${clar_decls}\" \"${clar\n>  list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n>  list(TRANSFORM clar_test_SUITES APPEND \".c\")\n>  add_library(unit-tests-lib ${clar_test_SUITES} \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\")\n> -target_include_directories(unit-tests-lib PRIVATE \"${CMAKE_SOURCE_DIR}/t/unit-tests\")\n> +target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n\nThis also changes the 'scope' parameter of 'target_include_directories'\nfrom PRIVATE to PUBLIC, but the commit message doesn't mention such a\nchange.\n\nIs it intentional? If so, can the commit message be updated to explain\nwhy this is done? If not, is this a stray change that snuck in?\n\n(If all of this is obvious to you, I apologize for the confusion on my\nend. I'm not at all familiar with our CMake bits, so the extra\nexplanation would help me quite a bit in making sense of this.)\n\nThanks,\nTaylor\n"},{"id":"505076","messageId":"Zw2RaUNoQz+OTcKZ@nand.local","threadId":"62288","inReplyTo":"129d28ae48ad984e9fae8bf803ae50a390b15a46.1728914219.git.ps@pks.im","subject":"Re: [PATCH 3/3] cmake: set up proper dependencies for generated clar headers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-14T21:47:21Z","receivedAt":"2024-10-14T21:47:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 14, 2024 at 04:06:47PM +0200, Patrick Steinhardt wrote:\n> The auto-generated headers used by clar are written at configure time\n> and thus do not get regenerated automatically. Refactor the build\n> recipes such that we use custom commands instead, which also has the\n> benefit that we can reuse the same infrastructure as our Makefile.\n\nOK.\n\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> +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> +\n> +add_library(unit-tests-lib ${clar_test_SUITES}\n> +\t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n> +\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n> +\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n> +)\n>  target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n>  add_executable(unit-tests \"${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c\")\n>  target_link_libraries(unit-tests unit-tests-lib common-main)\n\nAhh. The post-image is a definite improvement ;-).\n\nThanks,\nTaylor\n"},{"id":"505125","messageId":"Zw4w9o_YQy4quegm@pks.im","threadId":"62288","inReplyTo":"Zw2QT/Kx6IoPhJtT@nand.local","subject":"Re: [PATCH 1/3] Makefile: extract script to generate clar declarations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-15T09:08:51Z","receivedAt":"2024-10-15T09:09:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 14, 2024 at 05:42:39PM -0400, Taylor Blau wrote:\n> On Mon, Oct 14, 2024 at 04:06:41PM +0200, Patrick Steinhardt wrote:\n> > +while test \"$#\" -ne 0\n> > +do\n> > +\tsuite=\"$1\"\n> > +\tshift\n> \n> I'm perhaps nit-picking here, but I find:\n> \n>     for suite in \"$@\"\n>     do\n>         ...\n>     done\n> \n> to be much more readable than 'while test \"$#\" -ne 0' and 'shift'\n> above, since the for-loop variant does not need to mangle its arguments\n> with shift.\n\nIt certainly is, yeah. I know that there was a reason to write it in\nthis convoluted way... was it compatibility with non-Bash shells when\nthere were spaces in the paths? I don't know, and I cannot find a case\nright now where it breaks.\n\nWill adapt accordingly.\n\nPatrick\n"},{"id":"505126","messageId":"Zw4xNwnrLicx2nDa@pks.im","threadId":"62288","inReplyTo":"Zw2RNsptlkHCyTgi@nand.local","subject":"Re: [PATCH 2/3] cmake: fix compilation of clar-based unit tests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-15T09:09:11Z","receivedAt":"2024-10-15T09:09:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 14, 2024 at 05:46:30PM -0400, Taylor Blau wrote:\n> On Mon, Oct 14, 2024 at 04:06:44PM +0200, Patrick Steinhardt wrote:\n> > The compilation of clar-based unit tests is broken because we do not\n> > add the binary directory into which we generate the \"clar-decls.h\" and\n> > \"clar.suite\" files as include directories. Instead, we accidentally set\n> > up the source directory as include directory.\n> \n> I am confused. What is the difference between CMAKE_SOURCE_DIR and\n> CMAKE_BINARY_DIR here, and why does the difference between the two\n> matter?\n\nThis is for out-of-tree builds. The outputs generated by CMake are\nwritten into the binary directory, which is not the source directory\nwhere the Git source files are stored.\n\n> > diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> > index 62af7b33d2f..093852ad9d6 100644\n> > --- a/contrib/buildsystems/CMakeLists.txt\n> > +++ b/contrib/buildsystems/CMakeLists.txt\n> > @@ -1042,7 +1042,7 @@ file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\" \"${clar_decls}\" \"${clar\n> >  list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n> >  list(TRANSFORM clar_test_SUITES APPEND \".c\")\n> >  add_library(unit-tests-lib ${clar_test_SUITES} \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\")\n> > -target_include_directories(unit-tests-lib PRIVATE \"${CMAKE_SOURCE_DIR}/t/unit-tests\")\n> > +target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n> \n> This also changes the 'scope' parameter of 'target_include_directories'\n> from PRIVATE to PUBLIC, but the commit message doesn't mention such a\n> change.\n\nIt does mention it, it's the \"propagate the include directories\" part.\n\n> Is it intentional? If so, can the commit message be updated to explain\n> why this is done? If not, is this a stray change that snuck in?\n> \n> (If all of this is obvious to you, I apologize for the confusion on my\n> end. I'm not at all familiar with our CMake bits, so the extra\n> explanation would help me quite a bit in making sense of this.)\n\nThat's fair. I'll clarify the message a bit to provide more context.\n\nPatrick\n"},{"id":"505128","messageId":"7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im","threadId":"62288","inReplyTo":"cover.1728985514.git.ps@pks.im","subject":"[PATCH v2 1/3] Makefile: extract script to generate clar declarations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-15T09:46:11Z","receivedAt":"2024-10-15T09:46:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Extract the script to generate function declarations for the clar unit\ntesting framework into a standalone script. This is done such that we\ncan reuse it in other build systems.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Makefile                            |  4 +---\n t/unit-tests/generate-clar-decls.sh | 16 ++++++++++++++++\n 2 files changed, 17 insertions(+), 3 deletions(-)\n create mode 100755 t/unit-tests/generate-clar-decls.sh\n\ndiff --git a/Makefile b/Makefile\nindex feeed6f9321..87b86c5be1a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3904,9 +3904,7 @@ GIT-TEST-SUITES: FORCE\n             fi\n \n $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n-\t$(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \\\n-\t\tsed -ne \"s/^\\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\\)/extern \\1;/p\" $(UNIT_TEST_DIR)/$$suite.c; \\\n-\tdone >$@\n+\t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES))\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 $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\ndiff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh\nnew file mode 100755\nindex 00000000000..81da732917a\n--- /dev/null\n+++ b/t/unit-tests/generate-clar-decls.sh\n@@ -0,0 +1,16 @@\n+#!/bin/sh\n+\n+if test $# -lt 2\n+then\n+\techo \"USAGE: $0 <OUTPUT> <SUITE>...\" 2>&1\n+\texit 1\n+fi\n+\n+OUTPUT=\"$1\"\n+shift\n+\n+for suite in \"$@\"\n+do\n+\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n+\texit 1\n+done >\"$OUTPUT\"\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505129","messageId":"cover.1728985514.git.ps@pks.im","threadId":"62288","inReplyTo":"CAGjHeYfyH+cOMYYYHnFR+Vu9T+RbmzO1SpB_-kbmBSf1DitJhA@mail.gmail.com","subject":"[PATCH v2 0/3] cmake: fix autogenerated clar headers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-15T09:46:08Z","receivedAt":"2024-10-15T09:46:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis is the second version of this patch series that fixes building clar\nwith the CMake build system.\n\nChanges compared to v1:\n\n  - Use a for loop instead of `while; shift`.\n\n  - Provide a bit more context around the PRIVATE/PUBLIC switch in the\n    commit message.\n\nThanks!\n\nPatrick\n\nPatrick Steinhardt (3):\n  Makefile: extract script to generate clar declarations\n  cmake: fix compilation of clar-based unit tests\n  cmake: set up proper dependencies for generated clar headers\n\n Makefile                            |  4 +--\n contrib/buildsystems/CMakeLists.txt | 52 ++++++++---------------------\n t/unit-tests/generate-clar-decls.sh | 16 +++++++++\n 3 files changed, 30 insertions(+), 42 deletions(-)\n create mode 100755 t/unit-tests/generate-clar-decls.sh\n\nRange-diff against v1:\n1:  346aa2f0830 ! 1:  7a619677c7a Makefile: extract script to generate clar declarations\n    @@ t/unit-tests/generate-clar-decls.sh (new)\n     +OUTPUT=\"$1\"\n     +shift\n     +\n    -+while test \"$#\" -ne 0\n    ++for suite in \"$@\"\n     +do\n    -+\tsuite=\"$1\"\n    -+\tshift\n     +\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n     +\texit 1\n     +done >\"$OUTPUT\"\n2:  b9afeffda29 ! 2:  447afc4a0f3 cmake: fix compilation of clar-based unit tests\n    @@ Commit message\n         \"clar.suite\" files as include directories. Instead, we accidentally set\n         up the source directory as include directory.\n     \n    -    Fix this and propagate the include directories of \"unit-tests.lib\" to\n    -    the \"unit-tests\" executable so that the latter uses the same include\n    -    directories.\n    +    Fix this by including the binary directory instead of the source\n    +    directory. Furthermore, set up the include directories as PUBLIC instead\n    +    of PRIVATE such that they propagate from \"unit-tests.lib\" to the\n    +    \"unit-tests\" executable, which needs to include the same directory.\n     \n         Reported-by: Ed Reel <edreel@gmail.com>\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n3:  129d28ae48a = 3:  cf4955b2ddc cmake: set up proper dependencies for generated clar headers\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505130","messageId":"447afc4a0f37812b6488a51915a86302c20ddb12.1728985514.git.ps@pks.im","threadId":"62288","inReplyTo":"cover.1728985514.git.ps@pks.im","subject":"[PATCH v2 2/3] cmake: fix compilation of clar-based unit tests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-15T09:46:14Z","receivedAt":"2024-10-15T09:46:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The compilation of clar-based unit tests is broken because we do not\nadd the binary directory into which we generate the \"clar-decls.h\" and\n\"clar.suite\" files as include directories. Instead, we accidentally set\nup the source directory as include directory.\n\nFix this by including the binary directory instead of the source\ndirectory. Furthermore, set up the include directories as PUBLIC instead\nof PRIVATE such that they propagate from \"unit-tests.lib\" to the\n\"unit-tests\" executable, which needs to include the same directory.\n\nReported-by: Ed Reel <edreel@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 62af7b33d2f..093852ad9d6 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1042,7 +1042,7 @@ file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\" \"${clar_decls}\" \"${clar\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n add_library(unit-tests-lib ${clar_test_SUITES} \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\")\n-target_include_directories(unit-tests-lib PRIVATE \"${CMAKE_SOURCE_DIR}/t/unit-tests\")\n+target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n add_executable(unit-tests \"${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c\")\n target_link_libraries(unit-tests unit-tests-lib common-main)\n set_target_properties(unit-tests\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505131","messageId":"cf4955b2ddc04786a15a2f59b6a96c9862491854.1728985514.git.ps@pks.im","threadId":"62288","inReplyTo":"cover.1728985514.git.ps@pks.im","subject":"[PATCH v2 3/3] cmake: set up proper dependencies for generated clar headers","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-15T09:46:17Z","receivedAt":"2024-10-15T09:46:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The auto-generated headers used by clar are written at configure time\nand thus do not get regenerated automatically. Refactor the build\nrecipes such that we use custom commands instead, which also has the\nbenefit that we can reuse the same infrastructure as our Makefile.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n contrib/buildsystems/CMakeLists.txt | 50 +++++++----------------------\n 1 file changed, 12 insertions(+), 38 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 093852ad9d6..9f80ab92656 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS})\n endforeach()\n \n parse_makefile_for_scripts(clar_test_SUITES \"CLAR_TEST_SUITES\" \"\")\n-\n-set(clar_decls \"\")\n-set(clar_cbs \"\")\n-set(clar_cbs_count 0)\n-set(clar_suites \"static struct clar_suite _clar_suites[] = {\\n\")\n-list(LENGTH clar_test_SUITES clar_suites_count)\n-foreach(suite ${clar_test_SUITES})\n-\tfile(STRINGS \"${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c\" decls\n-\t\tREGEX \"^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\\\(void\\\\)$\")\n-\n-\tlist(LENGTH decls decls_count)\n-\tstring(REGEX REPLACE \"void (test_${suite}__([a-zA-Z_0-9]*))\\\\(void\\\\)\" \"    { \\\"\\\\2\\\", &\\\\1 },\\n\" cbs ${decls})\n-\tstring(JOIN \"\" cbs ${cbs})\n-\tlist(TRANSFORM decls PREPEND \"extern \")\n-\tstring(JOIN \";\\n\" decls ${decls})\n-\n-\tstring(APPEND clar_decls \"${decls};\\n\")\n-\tstring(APPEND clar_cbs\n-\t\t\"static const struct clar_func _clar_cb_${suite}[] = {\\n\"\n-\t\t${cbs}\n-\t\t\"};\\n\")\n-\tstring(APPEND clar_suites\n-\t\t\"    {\\n\"\n-\t\t\"        \\\"${suite}\\\",\\n\"\n-\t\t\"        { NULL, NULL },\\n\"\n-\t\t\"        { NULL, NULL },\\n\"\n-\t\t\"        _clar_cb_${suite}, ${decls_count}, 1\\n\"\n-\t\t\"    },\\n\")\n-\tmath(EXPR clar_cbs_count \"${clar_cbs_count}+${decls_count}\")\n-endforeach()\n-string(APPEND clar_suites\n-\t\"};\\n\"\n-\t\"static const size_t _clar_suite_count = ${clar_suites_count};\\n\"\n-\t\"static const size_t _clar_callback_count = ${clar_cbs_count};\\n\")\n-file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\" \"${clar_decls}\")\n-file(WRITE \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\" \"${clar_decls}\" \"${clar_cbs}\" \"${clar_suites}\")\n-\n list(TRANSFORM clar_test_SUITES PREPEND \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n list(TRANSFORM clar_test_SUITES APPEND \".c\")\n-add_library(unit-tests-lib ${clar_test_SUITES} \"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.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+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+\n+add_library(unit-tests-lib ${clar_test_SUITES}\n+\t\"${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c\"\n+\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h\"\n+\t\"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n+)\n target_include_directories(unit-tests-lib PUBLIC \"${CMAKE_BINARY_DIR}/t/unit-tests\")\n add_executable(unit-tests \"${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c\")\n target_link_libraries(unit-tests unit-tests-lib common-main)\n-- \n2.47.0.72.gef8ce8f3d4.dirty\n\n"},{"id":"505199","messageId":"Zw7BX0BEzC07DRby@nand.local","threadId":"62288","inReplyTo":"7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im","subject":"Re: [PATCH v2 1/3] Makefile: extract script to generate clar declarations","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-15T19:24:15Z","receivedAt":"2024-10-15T19:24:18Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 15, 2024 at 11:46:11AM +0200, Patrick Steinhardt wrote:\n> +for suite in \"$@\"\n> +do\n> +\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n> +\texit 1\n> +done >\"$OUTPUT\"\n\nMuch cleaner, thanks.\n\n> 2.47.0.72.gef8ce8f3d4.dirty\n\nThanks,\nTaylor\n"},{"id":"505200","messageId":"Zw7BpycbbZDjX6Gt@nand.local","threadId":"62288","inReplyTo":"cover.1728985514.git.ps@pks.im","subject":"Re: [PATCH v2 0/3] cmake: fix autogenerated clar headers","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-15T19:25:27Z","receivedAt":"2024-10-15T19:25:29Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 15, 2024 at 11:46:08AM +0200, Patrick Steinhardt wrote:\n> Patrick Steinhardt (3):\n>   Makefile: extract script to generate clar declarations\n>   cmake: fix compilation of clar-based unit tests\n>   cmake: set up proper dependencies for generated clar headers\n\nThanks; will queue.\n\nThanks,\nTaylor\n"},{"id":"505445","messageId":"871q0dcu8i.fsf@iotcl.com","threadId":"62288","inReplyTo":"7a619677c7af6ba8213a36208e20ab75c4318e38.1728985514.git.ps@pks.im","subject":"Re: [PATCH v2 1/3] Makefile: extract script to generate clar declarations","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-10-18T15:21:17Z","receivedAt":"2024-10-18T15:21:31Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Extract the script to generate function declarations for the clar unit\n> testing framework into a standalone script. This is done such that we\n> can reuse it in other build systems.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Makefile                            |  4 +---\n>  t/unit-tests/generate-clar-decls.sh | 16 ++++++++++++++++\n>  2 files changed, 17 insertions(+), 3 deletions(-)\n>  create mode 100755 t/unit-tests/generate-clar-decls.sh\n>\n> diff --git a/Makefile b/Makefile\n> index feeed6f9321..87b86c5be1a 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -3904,9 +3904,7 @@ GIT-TEST-SUITES: FORCE\n>              fi\n>  \n>  $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES\n> -\t$(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \\\n> -\t\tsed -ne \"s/^\\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\\)/extern \\1;/p\" $(UNIT_TEST_DIR)/$$suite.c; \\\n> -\tdone >$@\n> +\t$(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh \"$@\" $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES))\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>  $(CLAR_TEST_OBJS): $(UNIT_TEST_DIR)/clar-decls.h\n> diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh\n> new file mode 100755\n> index 00000000000..81da732917a\n> --- /dev/null\n> +++ b/t/unit-tests/generate-clar-decls.sh\n> @@ -0,0 +1,16 @@\n> +#!/bin/sh\n> +\n> +if test $# -lt 2\n> +then\n> +\techo \"USAGE: $0 <OUTPUT> <SUITE>...\" 2>&1\n> +\texit 1\n> +fi\n> +\n> +OUTPUT=\"$1\"\n> +shift\n> +\n> +for suite in \"$@\"\n> +do\n> +\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n\nIn the Makefile the first `suite` was wrapped in curly braces. And I\nthink we need to keep them in this script as well. I noticed because I\nwas reviewing this code in my editor I've noticed it highlights\n\"source__\" as the variable name. You can see what happens if you add\n`set -x` to the top of the script:\n\n    $ make t/unit-tests/clar-decls.h V=1\n    /bin/sh t/unit-tests/generate-clar-decls.sh \"t/unit-tests/clar-decls.h\" t/unit-tests/ctype.c t/unit-tests/strvec.c\n    + test 3 -lt 2\n    + OUTPUT=t/unit-tests/clar-decls.h\n    + shift\n    + for suite in \"$@\"\n    + sed -ne 's/^\\(void test_[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p' t/unit-tests/ctype.c\n    + for suite in \"$@\"\n    + sed -ne 's/^\\(void test_[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p' t/unit-tests/strvec.c\n\nSo it seems the script currently works \"by accident\".\n\nYou should replace the first $suite with something like:\n\n    $(basename $suite .c)\n\nOne other suggestion, and feel free to disagree. What do you think about\nreplacing the `$(patsubst ...)` in the recipe to `$(filter %.c,$^)`?\n\n--\nToon\n\n"},{"id":"505589","messageId":"ZxX7w7dGdalOyDo4@pks.im","threadId":"62288","inReplyTo":"871q0dcu8i.fsf@iotcl.com","subject":"Re: [PATCH v2 1/3] Makefile: extract script to generate clar declarations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-21T06:59:26Z","receivedAt":"2024-10-21T06:59:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 18, 2024 at 05:21:17PM +0200, Toon Claes wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh\n> > new file mode 100755\n> > index 00000000000..81da732917a\n> > --- /dev/null\n> > +++ b/t/unit-tests/generate-clar-decls.sh\n> > @@ -0,0 +1,16 @@\n> > +#!/bin/sh\n> > +\n> > +if test $# -lt 2\n> > +then\n> > +\techo \"USAGE: $0 <OUTPUT> <SUITE>...\" 2>&1\n> > +\texit 1\n> > +fi\n> > +\n> > +OUTPUT=\"$1\"\n> > +shift\n> > +\n> > +for suite in \"$@\"\n> > +do\n> > +\tsed -ne \"s/^\\(void test_$suite__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p\" \"$suite\" ||\n> \n> In the Makefile the first `suite` was wrapped in curly braces. And I\n> think we need to keep them in this script as well. I noticed because I\n> was reviewing this code in my editor I've noticed it highlights\n> \"source__\" as the variable name. You can see what happens if you add\n> `set -x` to the top of the script:\n> \n>     $ make t/unit-tests/clar-decls.h V=1\n>     /bin/sh t/unit-tests/generate-clar-decls.sh \"t/unit-tests/clar-decls.h\" t/unit-tests/ctype.c t/unit-tests/strvec.c\n>     + test 3 -lt 2\n>     + OUTPUT=t/unit-tests/clar-decls.h\n>     + shift\n>     + for suite in \"$@\"\n>     + sed -ne 's/^\\(void test_[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p' t/unit-tests/ctype.c\n>     + for suite in \"$@\"\n>     + sed -ne 's/^\\(void test_[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$\\)/extern \\1;/p' t/unit-tests/strvec.c\n> \n> So it seems the script currently works \"by accident\".\n\nOh, indeed.\n\n> You should replace the first $suite with something like:\n> \n>     $(basename $suite .c)\n> \n> One other suggestion, and feel free to disagree. What do you think about\n> replacing the `$(patsubst ...)` in the recipe to `$(filter %.c,$^)`?\n\nYup, both of these are good suggestions.\n\nThanks!\n\nPatrick\n"}]}