{"thread":{"id":"60174","subject":"[PATCH 0/4] CMake(Visual C) support for js/doc-unit-tests","startedAt":"2023-08-31T06:15:38Z","lastAt":"2023-09-25T19:11:09Z","messageCount":30,"participants":["Johannes Schindelin via GitGitGadget","Phillip Wood","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"481204","messageId":"pull.1579.git.1693462532.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":null,"subject":"[PATCH 0/4] CMake(Visual C) support for js/doc-unit-tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-31T06:15:28Z","receivedAt":"2023-08-31T06:15:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The recent patch series that adds proper unit testing to Git requires a\ncouple of add-on patches to make it work with the CMake build on Windows\n(Visual C). This patch series aims to provide that support.\n\nThis patch series is based on js/doc-unit-tests.\n\nJohannes Schindelin (4):\n cmake: also build unit tests\n unit-tests: do not mistake `.pdb` files for being executable\n unit-tests: do show relative file paths\n artifacts-tar: when including `.dll` files, don't forget the\n unit-tests\n\n Makefile | 2 +-\n contrib/buildsystems/CMakeLists.txt | 18 +++++++++++\n t/Makefile | 2 +-\n t/unit-tests/test-lib.c | 48 ++++++++++++++++++++++++++---\n 4 files changed, 64 insertions(+), 6 deletions(-)\n\n\nbase-commit: 03f9bc407975bba86d1d1807519d76e1693ff66f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1579%2Fdscho%2Fdoc-unit-tests-and-cmake-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1579/dscho/doc-unit-tests-and-cmake-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1579\n-- \ngitgitgadget\n"},{"id":"481205","messageId":"2cc1c03d85153356edd55fc0747a957db3c94ab0.1693462532.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.git.1693462532.gitgitgadget@gmail.com","subject":"[PATCH 1/4] cmake: also build unit tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-31T06:15:29Z","receivedAt":"2023-08-31T06:15:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA new, better way to run unit tests was just added to Git. This adds\nsupport for building those unit tests via CMake.\n\nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 2f6e0197ffa..45016213358 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main)\n parse_makefile_for_sources(test-reftable_SOURCES \"REFTABLE_TEST_OBJS\")\n list(TRANSFORM test-reftable_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n \n+#unit-tests\n+add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)\n+\n+parse_makefile_for_scripts(unit_test_PROGRAMS \"UNIT_TEST_PROGRAMS\" \"\")\n+foreach(unit_test ${unit_test_PROGRAMS})\n+\tadd_executable(\"${unit_test}\" \"${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c\")\n+\ttarget_link_libraries(\"${unit_test}\" unit-test-lib common-main)\n+\tset_target_properties(\"${unit_test}\"\n+\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\tif(MSVC)\n+\t\tset_target_properties(\"${unit_test}\"\n+\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\t\tset_target_properties(\"${unit_test}\"\n+\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\tendif()\n+\tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n+endforeach()\n+\n #test-tool\n parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n \n-- \ngitgitgadget\n\n"},{"id":"481206","messageId":"2b4e36c05c9e01b1e489100531fd01515b0786ab.1693462532.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.git.1693462532.gitgitgadget@gmail.com","subject":"[PATCH 3/4] unit-tests: do show relative file paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-31T06:15:31Z","receivedAt":"2023-08-31T06:15:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nVisual C interpolates `__FILE__` with the absolute _Windows_ path of\nthe source file. GCC interpolates it with the relative path, and the\ntests even verify that.\n\nSo let's make sure that the unit tests only emit such paths.\n\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/test-lib.c | 48 +++++++++++++++++++++++++++++++++++++----\n 1 file changed, 44 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\nindex 70030d587ff..bd94079af61 100644\n--- a/t/unit-tests/test-lib.c\n+++ b/t/unit-tests/test-lib.c\n@@ -21,6 +21,42 @@ static struct {\n \t.result = RESULT_NONE,\n };\n \n+#ifndef _MSC_VER\n+#define make_relative(location) location\n+#else\n+/*\n+ * Visual C interpolates the absolute Windows path for `__FILE__`,\n+ * but we want to see relative paths, as verified by t0080.\n+ */\n+#include \"strbuf.h\"\n+#include \"dir.h\"\n+\n+static const char *make_relative(const char *location)\n+{\n+\tstatic const char *prefix;\n+\tstatic size_t prefix_len;\n+\tstatic struct strbuf buf = STRBUF_INIT;\n+\n+\tif (!prefix) {\n+\t\tstrbuf_addstr(&buf, __FILE__);\n+\t\tif (!strbuf_strip_suffix(&buf, \"\\\\t\\\\unit-tests\\\\test-lib.c\"))\n+\t\t\tdie(\"unexpected suffix of '%s'\");\n+\t\tstrbuf_complete(&buf, '\\\\');\n+\t\tprefix = strbuf_detach(&buf, &prefix_len);\n+\t}\n+\n+\t/* Does it not start with the expected prefix? */\n+\tif (fspathncmp(location, prefix, prefix_len))\n+\t\treturn location;\n+\n+\tstrbuf_reset(&buf);\n+\tstrbuf_addstr(&buf, location + prefix_len);\n+\tconvert_slashes(buf.buf);\n+\n+\treturn buf.buf;\n+}\n+#endif\n+\n static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n {\n \tfflush(stderr);\n@@ -147,7 +183,8 @@ int test__run_end(int was_run UNUSED, const char *location, const char *format,\n \t\t\tbreak;\n \n \t\tcase RESULT_NONE:\n-\t\t\ttest_msg(\"BUG: test has no checks at %s\", location);\n+\t\t\ttest_msg(\"BUG: test has no checks at %s\",\n+\t\t\t\t make_relative(location));\n \t\t\tprintf(\"not ok %d\", ctx.count);\n \t\t\tprint_description(format, ap);\n \t\t\tctx.result = RESULT_FAILURE;\n@@ -193,13 +230,15 @@ int test_assert(const char *location, const char *check, int ok)\n \tassert(ctx.running);\n \n \tif (ctx.result == RESULT_SKIP) {\n-\t\ttest_msg(\"skipping check '%s' at %s\", check, location);\n+\t\ttest_msg(\"skipping check '%s' at %s\", check,\n+\t\t\t make_relative(location));\n \t\treturn 0;\n \t} else if (!ctx.todo) {\n \t\tif (ok) {\n \t\t\ttest_pass();\n \t\t} else {\n-\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check, location);\n+\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check,\n+\t\t\t\t make_relative(location));\n \t\t\ttest_fail();\n \t\t}\n \t}\n@@ -224,7 +263,8 @@ int test__todo_end(const char *location, const char *check, int res)\n \tif (ctx.result == RESULT_SKIP)\n \t\treturn 0;\n \tif (!res) {\n-\t\ttest_msg(\"todo check '%s' succeeded at %s\", check, location);\n+\t\ttest_msg(\"todo check '%s' succeeded at %s\", check,\n+\t\t\t make_relative(location));\n \t\ttest_fail();\n \t} else {\n \t\ttest_todo();\n-- \ngitgitgadget\n\n"},{"id":"481207","messageId":"90db3d5d41f8042a28cce02f910e7b6d5904308d.1693462532.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.git.1693462532.gitgitgadget@gmail.com","subject":"[PATCH 2/4] unit-tests: do not mistake `.pdb` files for being executable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-31T06:15:30Z","receivedAt":"2023-08-31T06:15:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen building the unit tests via CMake, the `.pdb` files are built.\nThose are, essentially, files containing the debug information\nseparately from the executables.\n\nLet's not confuse them with the executables we actually want to run.\n\nSigned-off-by: Johannes Schindelin \n---\n t/Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 095334bfdec..38fe0ded5bd 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -42,7 +42,7 @@ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))\n TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))\n CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))\n CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl\n-UNIT_TESTS = $(sort $(filter-out %.h %.c %.o unit-tests/t-basic%,$(wildcard unit-tests/t-*)))\n+UNIT_TESTS = $(sort $(filter-out %.h %.c %.o %.pdb unit-tests/t-basic%,$(wildcard unit-tests/t-*)))\n \n # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)\n # checks all tests in all scripts via a single invocation, so tell individual\n-- \ngitgitgadget\n\n"},{"id":"481208","messageId":"fb03f5aa6e59bd191b2c3d9a6446828cc3130fa5.1693462532.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.git.1693462532.gitgitgadget@gmail.com","subject":"[PATCH 4/4] artifacts-tar: when including `.dll` files, don't forget the unit-tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-31T06:15:32Z","receivedAt":"2023-08-31T06:15:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of recent, Git also builds executables in `t/unit-tests/`. For\ntechnical reasons, when building with CMake and Visual C, the\ndependencies (\".dll files\") need to be copied there, too, otherwise\nrunning the executable will fail \"due to missing dependencies\".\n\nThe CMake definition already contains the directives to copy those\n`.dll` files, but we also need to adjust the `artifacts-tar` rule in\nthe `Makefile` accordingly to let the `vs-test` job in the CI runs\npass successfully.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4016da6e39c..d95a7b19b50 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3596,7 +3596,7 @@ rpm::\n .PHONY: rpm\n \n ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n-OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll t/unit-tests/*.dll)\n endif\n \n artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n-- \ngitgitgadget\n"},{"id":"481694","messageId":"69f6f263-06e1-4fef-abd9-d6c03ae0c148@gmail.com","threadId":"60174","inReplyTo":"2b4e36c05c9e01b1e489100531fd01515b0786ab.1693462532.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] unit-tests: do show relative file paths","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-11T13:25:09Z","receivedAt":"2023-09-11T21:39:16Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Visual C interpolates `__FILE__` with the absolute _Windows_ path of\n> the source file. GCC interpolates it with the relative path, and the\n> tests even verify that.\n\nOh, that's a pain\n\n> So let's make sure that the unit tests only emit such paths.\n\nMakes sense\n\n> +#ifndef _MSC_VER\n> +#define make_relative(location) location\n> +#else\n> +/*\n> + * Visual C interpolates the absolute Windows path for `__FILE__`,\n> + * but we want to see relative paths, as verified by t0080.\n> + */\n> +#include \"strbuf.h\"\n> +#include \"dir.h\"\n> +\n> +static const char *make_relative(const char *location)\n> +{\n> +\tstatic const char *prefix;\n> +\tstatic size_t prefix_len;\n> +\tstatic struct strbuf buf = STRBUF_INIT;\n\nSo far test-lib.c avoids using things like struct strbuf that it will be \nused to test. In this instance we're only using it on one particular \ncompiler so it may not matter so much. We could avoid it but I'm not \nsure it is worth the extra complexity. One thing I noted in this patch \nis that prefix is leaked but I'm not sure if you run any leak checkers \non the msvc build.\n\nstatic const char *make_relative(const char *location)\n{\n\tstatic char prefix[] = __FILE__\n\tstatic size_t *prefix_len = (size_t)-1;\n\tstatic char buf[PATH_MAX];\n\n\tif (prefix == (size_t)-1) {\n\t\tconst char *path = \"\\\\t\\unit-tests\\\\test-lib.c\";\n\t\tsize_t path_len = strlen(path);\n\t\t\n\t\tprefix_len = strlen(prefix);\n\t\tif (prefix_len < path_len) ||\n\t\t memcmp(prefix + prefix_len - path_len, path, path_len)\n\t\t\tdie(...);\n\t\tprefix_len -= path_len - 1; /* keep trailing '\\\\' */\n\t\tprefix[prefix_len] = '\\0';\n\t}\n\n\t/* Does it not start with the expected prefix? */\n\tif (fspathncmp(location, prefix, prefix_len))\n\t\treturn location;\n\n\tif (strlen(location) - prefix_len > sizeof(buf) - 1)\n\t\tdie(...)\n\n\t/* +1 to copy NUL terminator */\n\tmemcpy(buf, location + prefix_len, strlen(location) - prefix_len + 1);\n\tconvert_slashes(buf);\n\n\treturn buf;\n}\n\nBest Wishes\n\nPhillip\n"},{"id":"481702","messageId":"d7d1505c-8d65-4b64-8814-3e3b1e46f8ac@gmail.com","threadId":"60174","inReplyTo":"2cc1c03d85153356edd55fc0747a957db3c94ab0.1693462532.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/4] cmake: also build unit tests","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-11T13:23:44Z","receivedAt":"2023-09-11T21:39:21Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> A new, better way to run unit tests was just added to Git. This adds\n> support for building those unit tests via CMake.\n\nThis patch builds the unit tests but does not add them to the list of \ntests run by CTest - how are the tests typically run on the CMake build?\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> contrib/buildsystems/CMakeLists.txt | 18 ++++++++++++++++++\n> 1 file changed, 18 insertions(+)\n> \n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index 2f6e0197ffa..45016213358 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main)\n> parse_makefile_for_sources(test-reftable_SOURCES \"REFTABLE_TEST_OBJS\")\n> list(TRANSFORM test-reftable_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n> \n> +#unit-tests\n> +add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)\n> +\n> +parse_makefile_for_scripts(unit_test_PROGRAMS \"UNIT_TEST_PROGRAMS\" \"\")\n> +foreach(unit_test ${unit_test_PROGRAMS})\n> +\tadd_executable(\"${unit_test}\" \"${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c\")\n> +\ttarget_link_libraries(\"${unit_test}\" unit-test-lib common-main)\n> +\tset_target_properties(\"${unit_test}\"\n> +\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests)\n> +\tif(MSVC)\n> +\t\tset_target_properties(\"${unit_test}\"\n> +\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/unit-tests)\n> +\t\tset_target_properties(\"${unit_test}\"\n> +\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n> +\tendif()\n> +\tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n> +endforeach()\n> +\n> #test-tool\n> parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n> \n"},{"id":"481975","messageId":"2cc1c03d85153356edd55fc0747a957db3c94ab0.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 1/7] cmake: also build unit tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:22Z","receivedAt":"2023-09-18T20:54:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA new, better way to run unit tests was just added to Git. This adds\nsupport for building those unit tests via CMake.\n\nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 2f6e0197ffa..45016213358 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main)\n parse_makefile_for_sources(test-reftable_SOURCES \"REFTABLE_TEST_OBJS\")\n list(TRANSFORM test-reftable_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n \n+#unit-tests\n+add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)\n+\n+parse_makefile_for_scripts(unit_test_PROGRAMS \"UNIT_TEST_PROGRAMS\" \"\")\n+foreach(unit_test ${unit_test_PROGRAMS})\n+\tadd_executable(\"${unit_test}\" \"${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c\")\n+\ttarget_link_libraries(\"${unit_test}\" unit-test-lib common-main)\n+\tset_target_properties(\"${unit_test}\"\n+\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\tif(MSVC)\n+\t\tset_target_properties(\"${unit_test}\"\n+\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\t\tset_target_properties(\"${unit_test}\"\n+\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\tendif()\n+\tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n+endforeach()\n+\n #test-tool\n parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n \n-- \ngitgitgadget\n\n"},{"id":"481977","messageId":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.git.1693462532.gitgitgadget@gmail.com","subject":"[PATCH v2 0/7] CMake(Visual C) support for js/doc-unit-tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:21Z","receivedAt":"2023-09-18T20:54:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The recent patch series that adds proper unit testing to Git requires a\ncouple of add-on patches to make it work with the CMake build on Windows\n(Visual C). This patch series aims to provide that support.\n\nThis patch series is based on js/doc-unit-tests.\n\nChanges since v1:\n\n * The code added to test-lib.c now avoids using a strbuf.\n * The unit tests are now also handled via CTest.\n * While at it, I cleaned up a little in the CTest-related definitions.\n\nJohannes Schindelin (7):\n cmake: also build unit tests\n unit-tests: do not mistake `.pdb` files for being executable\n unit-tests: do show relative file paths\n artifacts-tar: when including `.dll` files, don't forget the\n unit-tests\n cmake: fix typo in variable name\n cmake: use test names instead of full paths\n cmake: handle also unit tests\n\n Makefile | 2 +-\n contrib/buildsystems/CMakeLists.txt | 38 ++++++++++++++++++---\n t/Makefile | 2 +-\n t/unit-tests/test-lib.c | 52 ++++++++++++++++++++++++++---\n 4 files changed, 84 insertions(+), 10 deletions(-)\n\n\nbase-commit: 03f9bc407975bba86d1d1807519d76e1693ff66f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1579%2Fdscho%2Fdoc-unit-tests-and-cmake-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1579/dscho/doc-unit-tests-and-cmake-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1579\n\nRange-diff vs v1:\n\n 1: 2cc1c03d851 = 1: 2cc1c03d851 cmake: also build unit tests\n 2: 90db3d5d41f = 2: 90db3d5d41f unit-tests: do not mistake `.pdb` files for being executable\n 3: 2b4e36c05c9 ! 3: f0b804129e8 unit-tests: do show relative file paths\n @@ t/unit-tests/test-lib.c: static struct {\n + * Visual C interpolates the absolute Windows path for `__FILE__`,\n + * but we want to see relative paths, as verified by t0080.\n + */\n -+#include \"strbuf.h\"\n +#include \"dir.h\"\n +\n +static const char *make_relative(const char *location)\n +{\n -+\tstatic const char *prefix;\n ++\tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n +\tstatic size_t prefix_len;\n -+\tstatic struct strbuf buf = STRBUF_INIT;\n +\n -+\tif (!prefix) {\n -+\t\tstrbuf_addstr(&buf, __FILE__);\n -+\t\tif (!strbuf_strip_suffix(&buf, \"\\\\t\\\\unit-tests\\\\test-lib.c\"))\n -+\t\t\tdie(\"unexpected suffix of '%s'\");\n -+\t\tstrbuf_complete(&buf, '\\\\');\n -+\t\tprefix = strbuf_detach(&buf, &prefix_len);\n ++\tif (!prefix_len) {\n ++\t\tsize_t len = strlen(prefix);\n ++\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n ++\t\tsize_t needle_len = strlen(needle);\n ++\n ++\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n ++\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n ++\n ++\t\t/* let it end in a directory separator */\n ++\t\tprefix_len = len - needle_len + 1;\n +\t}\n +\n +\t/* Does it not start with the expected prefix? */\n +\tif (fspathncmp(location, prefix, prefix_len))\n +\t\treturn location;\n +\n -+\tstrbuf_reset(&buf);\n -+\tstrbuf_addstr(&buf, location + prefix_len);\n -+\tconvert_slashes(buf.buf);\n ++\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n ++\t/* convert backslashes to forward slashes */\n ++\tfor (p = buf; *p; p++)\n ++\t\tif (*p == '\\\\')\n ++\t\t\t*p = '/';\n +\n -+\treturn buf.buf;\n ++\treturn buf;\n +}\n +#endif\n +\n 4: fb03f5aa6e5 = 4: a70339f57a7 artifacts-tar: when including `.dll` files, don't forget the unit-tests\n -: ----------- > 5: 75a74571fbe cmake: fix typo in variable name\n -: ----------- > 6: 41228df1b46 cmake: use test names instead of full paths\n -: ----------- > 7: 003d44e9f0d cmake: handle also unit tests\n\n-- \ngitgitgadget\n"},{"id":"481976","messageId":"90db3d5d41f8042a28cce02f910e7b6d5904308d.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 2/7] unit-tests: do not mistake `.pdb` files for being executable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:23Z","receivedAt":"2023-09-18T20:54:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen building the unit tests via CMake, the `.pdb` files are built.\nThose are, essentially, files containing the debug information\nseparately from the executables.\n\nLet's not confuse them with the executables we actually want to run.\n\nSigned-off-by: Johannes Schindelin \n---\n t/Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 095334bfdec..38fe0ded5bd 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -42,7 +42,7 @@ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))\n TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))\n CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))\n CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl\n-UNIT_TESTS = $(sort $(filter-out %.h %.c %.o unit-tests/t-basic%,$(wildcard unit-tests/t-*)))\n+UNIT_TESTS = $(sort $(filter-out %.h %.c %.o %.pdb unit-tests/t-basic%,$(wildcard unit-tests/t-*)))\n \n # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)\n # checks all tests in all scripts via a single invocation, so tell individual\n-- \ngitgitgadget\n\n"},{"id":"481978","messageId":"f0b804129e8a21449cbb6f346473d3570182ddfa.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 3/7] unit-tests: do show relative file paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:24Z","receivedAt":"2023-09-18T20:54:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nVisual C interpolates `__FILE__` with the absolute _Windows_ path of\nthe source file. GCC interpolates it with the relative path, and the\ntests even verify that.\n\nSo let's make sure that the unit tests only emit such paths.\n\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/test-lib.c | 52 +++++++++++++++++++++++++++++++++++++----\n 1 file changed, 48 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\nindex 70030d587ff..744e223ee98 100644\n--- a/t/unit-tests/test-lib.c\n+++ b/t/unit-tests/test-lib.c\n@@ -21,6 +21,46 @@ static struct {\n \t.result = RESULT_NONE,\n };\n \n+#ifndef _MSC_VER\n+#define make_relative(location) location\n+#else\n+/*\n+ * Visual C interpolates the absolute Windows path for `__FILE__`,\n+ * but we want to see relative paths, as verified by t0080.\n+ */\n+#include \"dir.h\"\n+\n+static const char *make_relative(const char *location)\n+{\n+\tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n+\tstatic size_t prefix_len;\n+\n+\tif (!prefix_len) {\n+\t\tsize_t len = strlen(prefix);\n+\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n+\t\tsize_t needle_len = strlen(needle);\n+\n+\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n+\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n+\n+\t\t/* let it end in a directory separator */\n+\t\tprefix_len = len - needle_len + 1;\n+\t}\n+\n+\t/* Does it not start with the expected prefix? */\n+\tif (fspathncmp(location, prefix, prefix_len))\n+\t\treturn location;\n+\n+\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n+\t/* convert backslashes to forward slashes */\n+\tfor (p = buf; *p; p++)\n+\t\tif (*p == '\\\\')\n+\t\t\t*p = '/';\n+\n+\treturn buf;\n+}\n+#endif\n+\n static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n {\n \tfflush(stderr);\n@@ -147,7 +187,8 @@ int test__run_end(int was_run UNUSED, const char *location, const char *format,\n \t\t\tbreak;\n \n \t\tcase RESULT_NONE:\n-\t\t\ttest_msg(\"BUG: test has no checks at %s\", location);\n+\t\t\ttest_msg(\"BUG: test has no checks at %s\",\n+\t\t\t\t make_relative(location));\n \t\t\tprintf(\"not ok %d\", ctx.count);\n \t\t\tprint_description(format, ap);\n \t\t\tctx.result = RESULT_FAILURE;\n@@ -193,13 +234,15 @@ int test_assert(const char *location, const char *check, int ok)\n \tassert(ctx.running);\n \n \tif (ctx.result == RESULT_SKIP) {\n-\t\ttest_msg(\"skipping check '%s' at %s\", check, location);\n+\t\ttest_msg(\"skipping check '%s' at %s\", check,\n+\t\t\t make_relative(location));\n \t\treturn 0;\n \t} else if (!ctx.todo) {\n \t\tif (ok) {\n \t\t\ttest_pass();\n \t\t} else {\n-\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check, location);\n+\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check,\n+\t\t\t\t make_relative(location));\n \t\t\ttest_fail();\n \t\t}\n \t}\n@@ -224,7 +267,8 @@ int test__todo_end(const char *location, const char *check, int res)\n \tif (ctx.result == RESULT_SKIP)\n \t\treturn 0;\n \tif (!res) {\n-\t\ttest_msg(\"todo check '%s' succeeded at %s\", check, location);\n+\t\ttest_msg(\"todo check '%s' succeeded at %s\", check,\n+\t\t\t make_relative(location));\n \t\ttest_fail();\n \t} else {\n \t\ttest_todo();\n-- \ngitgitgadget\n\n"},{"id":"481979","messageId":"a70339f57a7f49b5db7d7b96131e53601592e8b5.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 4/7] artifacts-tar: when including `.dll` files, don't forget the unit-tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:25Z","receivedAt":"2023-09-18T20:54:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of recent, Git also builds executables in `t/unit-tests/`. For\ntechnical reasons, when building with CMake and Visual C, the\ndependencies (\".dll files\") need to be copied there, too, otherwise\nrunning the executable will fail \"due to missing dependencies\".\n\nThe CMake definition already contains the directives to copy those\n`.dll` files, but we also need to adjust the `artifacts-tar` rule in\nthe `Makefile` accordingly to let the `vs-test` job in the CI runs\npass successfully.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4016da6e39c..d95a7b19b50 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3596,7 +3596,7 @@ rpm::\n .PHONY: rpm\n \n ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n-OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll t/unit-tests/*.dll)\n endif\n \n artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n-- \ngitgitgadget\n\n"},{"id":"481980","messageId":"75a74571fbee7677d0e473a199d99c10da35ba09.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 5/7] cmake: fix typo in variable name","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:26Z","receivedAt":"2023-09-18T20:54:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nSigned-off-by: Johannes Schindelin \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 45016213358..ad197ea433f 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1102,10 +1102,10 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})\n \tfile(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-completion.bash DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/)\n endif()\n \n-file(GLOB test_scipts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n+file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n \n #test\n-foreach(tsh ${test_scipts})\n+foreach(tsh ${test_scripts})\n \tadd_test(NAME ${tsh}\n \t\tCOMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx\n \t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)\n-- \ngitgitgadget\n\n"},{"id":"481981","messageId":"41228df1b469d9a79f3278fe8c1ca37082600669.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 6/7] cmake: use test names instead of full paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:27Z","receivedAt":"2023-09-18T20:54:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe primary purpose of Git's CMake definition is to allow developing Git\nin Visual Studio. As part of that, the CTest feature allows running\nindividual test scripts conveniently in Visual Studio's Test Explorer.\n\nHowever, this Test Explorer's design targets object-oriented languages\nand therefore expects the test names in the form\n`..`. And since we specify the full path\nof the test scripts instead, including the ugly `/.././t/` part, these\ndots confuse the Test Explorer and it uses a large part of the path as\n\"namespace\".\n\nLet's just use `t.` instead. This still adds an ugly \"Empty\nNamespace\" layer by default, but at least the ugly absolute path is now\ngone.\n\nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex ad197ea433f..ff1a8cc348f 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1106,13 +1106,14 @@ file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n \n #test\n foreach(tsh ${test_scripts})\n-\tadd_test(NAME ${tsh}\n+\tstring(REGEX REPLACE \".*/(.*)\\\\.sh\" \"\\\\1\" test_name ${tsh})\n+\tadd_test(NAME \"t.${test_name}\"\n \t\tCOMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx\n \t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)\n endforeach()\n \n # This test script takes an extremely long time and is known to time out even\n # on fast machines because it requires in excess of one hour to run\n-set_tests_properties(\"${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh\" PROPERTIES TIMEOUT 4000)\n+set_tests_properties(\"t.t7112-reset-submodule\" PROPERTIES TIMEOUT 4000)\n \n endif()#BUILD_TESTING\n-- \ngitgitgadget\n\n"},{"id":"481982","messageId":"003d44e9f0d73efae887d6b61ebdffb3c4e0904f.1695070468.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v2 7/7] cmake: handle also unit tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-18T20:54:28Z","receivedAt":"2023-09-18T20:54:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe unit tests should also be available e.g. in Visual Studio's Test\nExplorer when configuring Git's source code via CMake.\n\nSuggested-by: Phillip Wood \nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex ff1a8cc348f..35d451856a0 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -981,6 +981,17 @@ foreach(unit_test ${unit_test_PROGRAMS})\n \t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n \tendif()\n \tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n+\n+\t# t-basic intentionally fails tests, to validate the unit-test infrastructure.\n+\t# Therefore, it should only be run as part of t0080, which verifies that it\n+\t# fails only in the expected ways.\n+\t#\n+\t# All other unit tests should be run.\n+\tif(NOT ${unit_test} STREQUAL \"t-basic\")\n+\t\tadd_test(NAME \"unit-tests.${unit_test}\"\n+\t\t\tCOMMAND \"./${unit_test}\"\n+\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t/unit-tests)\n+\tendif()\n endforeach()\n \n #test-tool\n-- \ngitgitgadget\n"},{"id":"481983","messageId":"4e13e2ce-9cca-eee4-788a-bf47fc29bc07@gmx.de","threadId":"60174","inReplyTo":"d7d1505c-8d65-4b64-8814-3e3b1e46f8ac@gmail.com","subject":"Re: [PATCH 1/4] cmake: also build unit tests","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-18T20:58:28Z","receivedAt":"2023-09-18T20:58:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Mon, 11 Sep 2023, Phillip Wood wrote:\n\n> On 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > A new, better way to run unit tests was just added to Git. This adds\n> > support for building those unit tests via CMake.\n>\n> This patch builds the unit tests but does not add them to the list of tests\n> run by CTest - how are the tests typically run on the CMake build?\n\nYou're right, I missed that the unit tests are run as part not of t0080,\nbut as a separate target in `t/Makefile`.\n\nI've added a couple of patches to clean up the CTest part and then run the\nunit test (t-strbuf, and whatever is added to the `UNIT_TEST_PROGRAMS`\nvariable in `Makefile`).\n\nThank you,\nJohannes\n"},{"id":"481984","messageId":"b14a99f8-46c2-b54d-e24b-919f3ac8f2e0@gmx.de","threadId":"60174","inReplyTo":"69f6f263-06e1-4fef-abd9-d6c03ae0c148@gmail.com","subject":"Re: [PATCH 3/4] unit-tests: do show relative file paths","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-18T21:00:06Z","receivedAt":"2023-09-18T21:00:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Mon, 11 Sep 2023, Phillip Wood wrote:\n\n> On 31/08/2023 07:15, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > Visual C interpolates `__FILE__` with the absolute _Windows_ path of\n> > the source file. GCC interpolates it with the relative path, and the\n> > tests even verify that.\n>\n> Oh, that's a pain\n>\n> > So let's make sure that the unit tests only emit such paths.\n>\n> Makes sense\n>\n> > +#ifndef _MSC_VER\n> > +#define make_relative(location) location\n> > +#else\n> > +/*\n> > + * Visual C interpolates the absolute Windows path for `__FILE__`,\n> > + * but we want to see relative paths, as verified by t0080.\n> > + */\n> > +#include \"strbuf.h\"\n> > +#include \"dir.h\"\n> > +\n> > +static const char *make_relative(const char *location)\n> > +{\n> > +\tstatic const char *prefix;\n> > +\tstatic size_t prefix_len;\n> > +\tstatic struct strbuf buf = STRBUF_INIT;\n>\n> So far test-lib.c avoids using things like struct strbuf that it will be used\n> to test. In this instance we're only using it on one particular compiler so it\n> may not matter so much. We could avoid it but I'm not sure it is worth the\n> extra complexity. One thing I noted in this patch is that prefix is leaked but\n> I'm not sure if you run any leak checkers on the msvc build.\n\nI changed the code not to use a strbuf, and I'm now working exclusively on\nstatic buffers instead of `malloc()`ing anything.\n\nThank you,\nJohannes\n"},{"id":"482142","messageId":"7d373691-450a-4f95-abdd-f5cd7993ee27@gmail.com","threadId":"60174","inReplyTo":"f0b804129e8a21449cbb6f346473d3570182ddfa.1695070468.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/7] unit-tests: do show relative file paths","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-22T14:35:01Z","receivedAt":"2023-09-22T14:35:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 18/09/2023 21:54, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> Visual C interpolates `__FILE__` with the absolute _Windows_ path of\n> the source file. GCC interpolates it with the relative path, and the\n> tests even verify that.\n> \n> So let's make sure that the unit tests only emit such paths.\n\nThis looks good to me. Calling strlcpy() without checking the return \nvalue is a pet peeve of mine but the silent truncation doesn't matter \nhere and the buffer ought to be long enough anyway.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> t/unit-tests/test-lib.c | 52 +++++++++++++++++++++++++++++++++++++----\n> 1 file changed, 48 insertions(+), 4 deletions(-)\n> \n> diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\n> index 70030d587ff..744e223ee98 100644\n> --- a/t/unit-tests/test-lib.c\n> +++ b/t/unit-tests/test-lib.c\n> @@ -21,6 +21,46 @@ static struct {\n> \t.result = RESULT_NONE,\n> };\n> \n> +#ifndef _MSC_VER\n> +#define make_relative(location) location\n> +#else\n> +/*\n> + * Visual C interpolates the absolute Windows path for `__FILE__`,\n> + * but we want to see relative paths, as verified by t0080.\n> + */\n> +#include \"dir.h\"\n> +\n> +static const char *make_relative(const char *location)\n> +{\n> +\tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n> +\tstatic size_t prefix_len;\n> +\n> +\tif (!prefix_len) {\n> +\t\tsize_t len = strlen(prefix);\n> +\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n> +\t\tsize_t needle_len = strlen(needle);\n> +\n> +\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n> +\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n> +\n> +\t\t/* let it end in a directory separator */\n> +\t\tprefix_len = len - needle_len + 1;\n> +\t}\n> +\n> +\t/* Does it not start with the expected prefix? */\n> +\tif (fspathncmp(location, prefix, prefix_len))\n> +\t\treturn location;\n> +\n> +\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n> +\t/* convert backslashes to forward slashes */\n> +\tfor (p = buf; *p; p++)\n> +\t\tif (*p == '\\\\')\n> +\t\t\t*p = '/';\n> +\n> +\treturn buf;\n> +}\n> +#endif\n> +\n> static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n> {\n> \tfflush(stderr);\n> @@ -147,7 +187,8 @@ int test__run_end(int was_run UNUSED, const char *location, const char *format,\n> \t\t\tbreak;\n> \n> \t\tcase RESULT_NONE:\n> -\t\t\ttest_msg(\"BUG: test has no checks at %s\", location);\n> +\t\t\ttest_msg(\"BUG: test has no checks at %s\",\n> +\t\t\t\t make_relative(location));\n> \t\t\tprintf(\"not ok %d\", ctx.count);\n> \t\t\tprint_description(format, ap);\n> \t\t\tctx.result = RESULT_FAILURE;\n> @@ -193,13 +234,15 @@ int test_assert(const char *location, const char *check, int ok)\n> \tassert(ctx.running);\n> \n> \tif (ctx.result == RESULT_SKIP) {\n> -\t\ttest_msg(\"skipping check '%s' at %s\", check, location);\n> +\t\ttest_msg(\"skipping check '%s' at %s\", check,\n> +\t\t\t make_relative(location));\n> \t\treturn 0;\n> \t} else if (!ctx.todo) {\n> \t\tif (ok) {\n> \t\t\ttest_pass();\n> \t\t} else {\n> -\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check, location);\n> +\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check,\n> +\t\t\t\t make_relative(location));\n> \t\t\ttest_fail();\n> \t\t}\n> \t}\n> @@ -224,7 +267,8 @@ int test__todo_end(const char *location, const char *check, int res)\n> \tif (ctx.result == RESULT_SKIP)\n> \t\treturn 0;\n> \tif (!res) {\n> -\t\ttest_msg(\"todo check '%s' succeeded at %s\", check, location);\n> +\t\ttest_msg(\"todo check '%s' succeeded at %s\", check,\n> +\t\t\t make_relative(location));\n> \t\ttest_fail();\n> \t} else {\n> \t\ttest_todo();\n"},{"id":"482143","messageId":"7d1d78c5-5781-4f5e-90f3-c02e74af31b2@gmail.com","threadId":"60174","inReplyTo":"41228df1b469d9a79f3278fe8c1ca37082600669.1695070468.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 6/7] cmake: use test names instead of full paths","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-22T14:37:20Z","receivedAt":"2023-09-22T14:37:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 18/09/2023 21:54, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> The primary purpose of Git's CMake definition is to allow developing Git\n> in Visual Studio. As part of that, the CTest feature allows running\n> individual test scripts conveniently in Visual Studio's Test Explorer.\n> \n> However, this Test Explorer's design targets object-oriented languages\n> and therefore expects the test names in the form\n> `..`. And since we specify the full path\n> of the test scripts instead, including the ugly `/.././t/` part, these\n> dots confuse the Test Explorer and it uses a large part of the path as\n> \"namespace\".\n> \n> Let's just use `t.` instead. This still adds an ugly \"Empty\n> Namespace\" layer by default, but at least the ugly absolute path is now\n> gone.\n\nThat does sound like a worthwhile improvement. If we used `git.t.` \nwould that fix the \"Empty Namespace\" problem? (probably not worth a \nre-roll on its own)\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> contrib/buildsystems/CMakeLists.txt | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index ad197ea433f..ff1a8cc348f 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -1106,13 +1106,14 @@ file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n> \n> #test\n> foreach(tsh ${test_scripts})\n> -\tadd_test(NAME ${tsh}\n> +\tstring(REGEX REPLACE \".*/(.*)\\\\.sh\" \"\\\\1\" test_name ${tsh})\n> +\tadd_test(NAME \"t.${test_name}\"\n> \t\tCOMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx\n> \t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)\n> endforeach()\n> \n> # This test script takes an extremely long time and is known to time out even\n> # on fast machines because it requires in excess of one hour to run\n> -set_tests_properties(\"${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh\" PROPERTIES TIMEOUT 4000)\n> +set_tests_properties(\"t.t7112-reset-submodule\" PROPERTIES TIMEOUT 4000)\n> \n> endif()#BUILD_TESTING\n"},{"id":"482144","messageId":"81b8fda7-fa2e-4ccf-94cb-3fdd3995948b@gmail.com","threadId":"60174","inReplyTo":"003d44e9f0d73efae887d6b61ebdffb3c4e0904f.1695070468.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 7/7] cmake: handle also unit tests","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-09-22T14:39:04Z","receivedAt":"2023-09-22T14:39:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 18/09/2023 21:54, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> The unit tests should also be available e.g. in Visual Studio's Test\n> Explorer when configuring Git's source code via CMake.\n> \n> Suggested-by: Phillip Wood \n> Signed-off-by: Johannes Schindelin \n\nThis looks good to me, it is nice to see it being careful not to run \nt-basic with the other unit tests.\n\nBest Wishes\n\nPhillip\n\n> ---\n> contrib/buildsystems/CMakeLists.txt | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n> \n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index ff1a8cc348f..35d451856a0 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -981,6 +981,17 @@ foreach(unit_test ${unit_test_PROGRAMS})\n> \t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n> \tendif()\n> \tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n> +\n> +\t# t-basic intentionally fails tests, to validate the unit-test infrastructure.\n> +\t# Therefore, it should only be run as part of t0080, which verifies that it\n> +\t# fails only in the expected ways.\n> +\t#\n> +\t# All other unit tests should be run.\n> +\tif(NOT ${unit_test} STREQUAL \"t-basic\")\n> +\t\tadd_test(NAME \"unit-tests.${unit_test}\"\n> +\t\t\tCOMMAND \"./${unit_test}\"\n> +\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t/unit-tests)\n> +\tendif()\n> endforeach()\n> \n> #test-tool\n"},{"id":"482240","messageId":"fabf28b2-7579-c214-34d3-2718b158ac13@gmx.de","threadId":"60174","inReplyTo":"7d1d78c5-5781-4f5e-90f3-c02e74af31b2@gmail.com","subject":"Re: [PATCH v2 6/7] cmake: use test names instead of full paths","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-09-25T09:06:07Z","receivedAt":"2023-09-25T09:06:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Fri, 22 Sep 2023, Phillip Wood wrote:\n\n> On 18/09/2023 21:54, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> >\n> > The primary purpose of Git's CMake definition is to allow developing Git\n> > in Visual Studio. As part of that, the CTest feature allows running\n> > individual test scripts conveniently in Visual Studio's Test Explorer.\n> >\n> > However, this Test Explorer's design targets object-oriented languages\n> > and therefore expects the test names in the form\n> > `..`. And since we specify the full path\n> > of the test scripts instead, including the ugly `/.././t/` part, these\n> > dots confuse the Test Explorer and it uses a large part of the path as\n> > \"namespace\".\n> >\n> > Let's just use `t.` instead. This still adds an ugly \"Empty\n> > Namespace\" layer by default, but at least the ugly absolute path is now\n> > gone.\n>\n> That does sound like a worthwhile improvement. If we used `git.t.` would\n> that fix the \"Empty Namespace\" problem? (probably not worth a re-roll on its\n> own)\n\nTurns out I did not play around with this enough. If I use `t.suite.` as a\nprefix, it makes the empty name space go away.\n\nI'll do that, then.\n\nCiao,\nJohannes\n"},{"id":"482241","messageId":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v2.git.1695070468.gitgitgadget@gmail.com","subject":"[PATCH v3 0/7] CMake(Visual C) support for js/doc-unit-tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:29Z","receivedAt":"2023-09-25T11:20:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The recent patch series that adds proper unit testing to Git requires a\ncouple of add-on patches to make it work with the CMake build on Windows\n(Visual C). This patch series aims to provide that support.\n\nThis patch series is based on js/doc-unit-tests.\n\nChanges since v2:\n\n * Thanks to Phillip Wood's prodding, I managed to avoid the \"Empty\n Namespace\" problem in Visual Studio's Test Explorer.\n\nChanges since v1:\n\n * The code added to test-lib.c now avoids using a strbuf.\n * The unit tests are now also handled via CTest.\n * While at it, I cleaned up a little in the CTest-related definitions.\n\nJohannes Schindelin (7):\n cmake: also build unit tests\n unit-tests: do not mistake `.pdb` files for being executable\n unit-tests: do show relative file paths\n artifacts-tar: when including `.dll` files, don't forget the\n unit-tests\n cmake: fix typo in variable name\n cmake: use test names instead of full paths\n cmake: handle also unit tests\n\n Makefile | 2 +-\n contrib/buildsystems/CMakeLists.txt | 38 ++++++++++++++++++---\n t/Makefile | 2 +-\n t/unit-tests/test-lib.c | 52 ++++++++++++++++++++++++++---\n 4 files changed, 84 insertions(+), 10 deletions(-)\n\n\nbase-commit: 03f9bc407975bba86d1d1807519d76e1693ff66f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1579%2Fdscho%2Fdoc-unit-tests-and-cmake-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1579/dscho/doc-unit-tests-and-cmake-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1579\n\nRange-diff vs v2:\n\n 1: 2cc1c03d851 = 1: 2cc1c03d851 cmake: also build unit tests\n 2: 90db3d5d41f = 2: 90db3d5d41f unit-tests: do not mistake `.pdb` files for being executable\n 3: f0b804129e8 = 3: f0b804129e8 unit-tests: do show relative file paths\n 4: a70339f57a7 = 4: a70339f57a7 artifacts-tar: when including `.dll` files, don't forget the unit-tests\n 5: 75a74571fbe = 5: 75a74571fbe cmake: fix typo in variable name\n 6: 41228df1b46 ! 6: 0a2d08b91e5 cmake: use test names instead of full paths\n @@ Commit message\n dots confuse the Test Explorer and it uses a large part of the path as\n \"namespace\".\n \n - Let's just use `t.` instead. This still adds an ugly \"Empty\n - Namespace\" layer by default, but at least the ugly absolute path is now\n - gone.\n + Let's just use `t.suite.` instead. This presents the tests in\n + Visual Studio's Test Explorer in the following form by default (i.e.\n + unless the user changes the view via the \"Group by\" menu):\n +\n + ◢ ◈ git\n + ◢ ◈ t\n + ◢ ◈ suite\n + ◈ t0000-basic\n + ◈ t0001-init\n + ◈ t0002-gitfile\n + [...]\n \n Signed-off-by: Johannes Schindelin \n \n @@ contrib/buildsystems/CMakeLists.txt: file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}\n foreach(tsh ${test_scripts})\n -\tadd_test(NAME ${tsh}\n +\tstring(REGEX REPLACE \".*/(.*)\\\\.sh\" \"\\\\1\" test_name ${tsh})\n -+\tadd_test(NAME \"t.${test_name}\"\n ++\tadd_test(NAME \"t.suite.${test_name}\"\n \t\tCOMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx\n \t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)\n endforeach()\n @@ contrib/buildsystems/CMakeLists.txt: file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}\n # This test script takes an extremely long time and is known to time out even\n # on fast machines because it requires in excess of one hour to run\n -set_tests_properties(\"${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh\" PROPERTIES TIMEOUT 4000)\n -+set_tests_properties(\"t.t7112-reset-submodule\" PROPERTIES TIMEOUT 4000)\n ++set_tests_properties(\"t.suite.t7112-reset-submodule\" PROPERTIES TIMEOUT 4000)\n \n endif()#BUILD_TESTING\n 7: 003d44e9f0d ! 7: 64707240a4e cmake: handle also unit tests\n @@ contrib/buildsystems/CMakeLists.txt: foreach(unit_test ${unit_test_PROGRAMS})\n +\t#\n +\t# All other unit tests should be run.\n +\tif(NOT ${unit_test} STREQUAL \"t-basic\")\n -+\t\tadd_test(NAME \"unit-tests.${unit_test}\"\n ++\t\tadd_test(NAME \"t.unit-tests.${unit_test}\"\n +\t\t\tCOMMAND \"./${unit_test}\"\n +\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t/unit-tests)\n +\tendif()\n\n-- \ngitgitgadget\n"},{"id":"482242","messageId":"90db3d5d41f8042a28cce02f910e7b6d5904308d.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 2/7] unit-tests: do not mistake `.pdb` files for being executable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:31Z","receivedAt":"2023-09-25T11:20:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen building the unit tests via CMake, the `.pdb` files are built.\nThose are, essentially, files containing the debug information\nseparately from the executables.\n\nLet's not confuse them with the executables we actually want to run.\n\nSigned-off-by: Johannes Schindelin \n---\n t/Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/Makefile b/t/Makefile\nindex 095334bfdec..38fe0ded5bd 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -42,7 +42,7 @@ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))\n TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))\n CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))\n CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl\n-UNIT_TESTS = $(sort $(filter-out %.h %.c %.o unit-tests/t-basic%,$(wildcard unit-tests/t-*)))\n+UNIT_TESTS = $(sort $(filter-out %.h %.c %.o %.pdb unit-tests/t-basic%,$(wildcard unit-tests/t-*)))\n \n # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`)\n # checks all tests in all scripts via a single invocation, so tell individual\n-- \ngitgitgadget\n\n"},{"id":"482243","messageId":"2cc1c03d85153356edd55fc0747a957db3c94ab0.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 1/7] cmake: also build unit tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:30Z","receivedAt":"2023-09-25T11:20:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA new, better way to run unit tests was just added to Git. This adds\nsupport for building those unit tests via CMake.\n\nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 2f6e0197ffa..45016213358 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main)\n parse_makefile_for_sources(test-reftable_SOURCES \"REFTABLE_TEST_OBJS\")\n list(TRANSFORM test-reftable_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n \n+#unit-tests\n+add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c)\n+\n+parse_makefile_for_scripts(unit_test_PROGRAMS \"UNIT_TEST_PROGRAMS\" \"\")\n+foreach(unit_test ${unit_test_PROGRAMS})\n+\tadd_executable(\"${unit_test}\" \"${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c\")\n+\ttarget_link_libraries(\"${unit_test}\" unit-test-lib common-main)\n+\tset_target_properties(\"${unit_test}\"\n+\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\tif(MSVC)\n+\t\tset_target_properties(\"${unit_test}\"\n+\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\t\tset_target_properties(\"${unit_test}\"\n+\t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n+\tendif()\n+\tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n+endforeach()\n+\n #test-tool\n parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n \n-- \ngitgitgadget\n\n"},{"id":"482244","messageId":"f0b804129e8a21449cbb6f346473d3570182ddfa.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 3/7] unit-tests: do show relative file paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:32Z","receivedAt":"2023-09-25T11:20:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nVisual C interpolates `__FILE__` with the absolute _Windows_ path of\nthe source file. GCC interpolates it with the relative path, and the\ntests even verify that.\n\nSo let's make sure that the unit tests only emit such paths.\n\nSigned-off-by: Johannes Schindelin \n---\n t/unit-tests/test-lib.c | 52 +++++++++++++++++++++++++++++++++++++----\n 1 file changed, 48 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c\nindex 70030d587ff..744e223ee98 100644\n--- a/t/unit-tests/test-lib.c\n+++ b/t/unit-tests/test-lib.c\n@@ -21,6 +21,46 @@ static struct {\n \t.result = RESULT_NONE,\n };\n \n+#ifndef _MSC_VER\n+#define make_relative(location) location\n+#else\n+/*\n+ * Visual C interpolates the absolute Windows path for `__FILE__`,\n+ * but we want to see relative paths, as verified by t0080.\n+ */\n+#include \"dir.h\"\n+\n+static const char *make_relative(const char *location)\n+{\n+\tstatic char prefix[] = __FILE__, buf[PATH_MAX], *p;\n+\tstatic size_t prefix_len;\n+\n+\tif (!prefix_len) {\n+\t\tsize_t len = strlen(prefix);\n+\t\tconst char *needle = \"\\\\t\\\\unit-tests\\\\test-lib.c\";\n+\t\tsize_t needle_len = strlen(needle);\n+\n+\t\tif (len < needle_len || strcmp(needle, prefix + len - needle_len))\n+\t\t\tdie(\"unexpected suffix of '%s'\", prefix);\n+\n+\t\t/* let it end in a directory separator */\n+\t\tprefix_len = len - needle_len + 1;\n+\t}\n+\n+\t/* Does it not start with the expected prefix? */\n+\tif (fspathncmp(location, prefix, prefix_len))\n+\t\treturn location;\n+\n+\tstrlcpy(buf, location + prefix_len, sizeof(buf));\n+\t/* convert backslashes to forward slashes */\n+\tfor (p = buf; *p; p++)\n+\t\tif (*p == '\\\\')\n+\t\t\t*p = '/';\n+\n+\treturn buf;\n+}\n+#endif\n+\n static void msg_with_prefix(const char *prefix, const char *format, va_list ap)\n {\n \tfflush(stderr);\n@@ -147,7 +187,8 @@ int test__run_end(int was_run UNUSED, const char *location, const char *format,\n \t\t\tbreak;\n \n \t\tcase RESULT_NONE:\n-\t\t\ttest_msg(\"BUG: test has no checks at %s\", location);\n+\t\t\ttest_msg(\"BUG: test has no checks at %s\",\n+\t\t\t\t make_relative(location));\n \t\t\tprintf(\"not ok %d\", ctx.count);\n \t\t\tprint_description(format, ap);\n \t\t\tctx.result = RESULT_FAILURE;\n@@ -193,13 +234,15 @@ int test_assert(const char *location, const char *check, int ok)\n \tassert(ctx.running);\n \n \tif (ctx.result == RESULT_SKIP) {\n-\t\ttest_msg(\"skipping check '%s' at %s\", check, location);\n+\t\ttest_msg(\"skipping check '%s' at %s\", check,\n+\t\t\t make_relative(location));\n \t\treturn 0;\n \t} else if (!ctx.todo) {\n \t\tif (ok) {\n \t\t\ttest_pass();\n \t\t} else {\n-\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check, location);\n+\t\t\ttest_msg(\"check \\\"%s\\\" failed at %s\", check,\n+\t\t\t\t make_relative(location));\n \t\t\ttest_fail();\n \t\t}\n \t}\n@@ -224,7 +267,8 @@ int test__todo_end(const char *location, const char *check, int res)\n \tif (ctx.result == RESULT_SKIP)\n \t\treturn 0;\n \tif (!res) {\n-\t\ttest_msg(\"todo check '%s' succeeded at %s\", check, location);\n+\t\ttest_msg(\"todo check '%s' succeeded at %s\", check,\n+\t\t\t make_relative(location));\n \t\ttest_fail();\n \t} else {\n \t\ttest_todo();\n-- \ngitgitgadget\n\n"},{"id":"482245","messageId":"a70339f57a7f49b5db7d7b96131e53601592e8b5.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 4/7] artifacts-tar: when including `.dll` files, don't forget the unit-tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:33Z","receivedAt":"2023-09-25T11:21:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of recent, Git also builds executables in `t/unit-tests/`. For\ntechnical reasons, when building with CMake and Visual C, the\ndependencies (\".dll files\") need to be copied there, too, otherwise\nrunning the executable will fail \"due to missing dependencies\".\n\nThe CMake definition already contains the directives to copy those\n`.dll` files, but we also need to adjust the `artifacts-tar` rule in\nthe `Makefile` accordingly to let the `vs-test` job in the CI runs\npass successfully.\n\nSigned-off-by: Johannes Schindelin \n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4016da6e39c..d95a7b19b50 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3596,7 +3596,7 @@ rpm::\n .PHONY: rpm\n \n ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n-OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n+OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll t/unit-tests/*.dll)\n endif\n \n artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n-- \ngitgitgadget\n\n"},{"id":"482246","messageId":"75a74571fbee7677d0e473a199d99c10da35ba09.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 5/7] cmake: fix typo in variable name","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:34Z","receivedAt":"2023-09-25T11:21:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nSigned-off-by: Johannes Schindelin \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 45016213358..ad197ea433f 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1102,10 +1102,10 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH})\n \tfile(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-completion.bash DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/)\n endif()\n \n-file(GLOB test_scipts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n+file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n \n #test\n-foreach(tsh ${test_scipts})\n+foreach(tsh ${test_scripts})\n \tadd_test(NAME ${tsh}\n \t\tCOMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx\n \t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)\n-- \ngitgitgadget\n\n"},{"id":"482247","messageId":"64707240a4eabde1e96446f9314e13248c35ed41.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 7/7] cmake: handle also unit tests","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:36Z","receivedAt":"2023-09-25T11:21:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe unit tests should also be available e.g. in Visual Studio's Test\nExplorer when configuring Git's source code via CMake.\n\nSuggested-by: Phillip Wood \nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 5e0c237dfd4..d21835ca65c 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -981,6 +981,17 @@ foreach(unit_test ${unit_test_PROGRAMS})\n \t\t\tPROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests)\n \tendif()\n \tlist(APPEND PROGRAMS_BUILT \"${unit_test}\")\n+\n+\t# t-basic intentionally fails tests, to validate the unit-test infrastructure.\n+\t# Therefore, it should only be run as part of t0080, which verifies that it\n+\t# fails only in the expected ways.\n+\t#\n+\t# All other unit tests should be run.\n+\tif(NOT ${unit_test} STREQUAL \"t-basic\")\n+\t\tadd_test(NAME \"t.unit-tests.${unit_test}\"\n+\t\t\tCOMMAND \"./${unit_test}\"\n+\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t/unit-tests)\n+\tendif()\n endforeach()\n \n #test-tool\n-- \ngitgitgadget\n"},{"id":"482248","messageId":"0a2d08b91e56aba19e02cd43a55d4c74c40cfe71.1695640837.git.gitgitgadget@gmail.com","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"[PATCH v3 6/7] cmake: use test names instead of full paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-09-25T11:20:35Z","receivedAt":"2023-09-25T11:21:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe primary purpose of Git's CMake definition is to allow developing Git\nin Visual Studio. As part of that, the CTest feature allows running\nindividual test scripts conveniently in Visual Studio's Test Explorer.\n\nHowever, this Test Explorer's design targets object-oriented languages\nand therefore expects the test names in the form\n`..`. And since we specify the full path\nof the test scripts instead, including the ugly `/.././t/` part, these\ndots confuse the Test Explorer and it uses a large part of the path as\n\"namespace\".\n\nLet's just use `t.suite.` instead. This presents the tests in\nVisual Studio's Test Explorer in the following form by default (i.e.\nunless the user changes the view via the \"Group by\" menu):\n\n\t◢ ◈ git\n\t ◢ ◈ t\n\t ◢ ◈ suite\n\t ◈ t0000-basic\n\t ◈ t0001-init\n\t ◈ t0002-gitfile\n\t [...]\n\nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex ad197ea433f..5e0c237dfd4 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1106,13 +1106,14 @@ file(GLOB test_scripts \"${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh\")\n \n #test\n foreach(tsh ${test_scripts})\n-\tadd_test(NAME ${tsh}\n+\tstring(REGEX REPLACE \".*/(.*)\\\\.sh\" \"\\\\1\" test_name ${tsh})\n+\tadd_test(NAME \"t.suite.${test_name}\"\n \t\tCOMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx\n \t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)\n endforeach()\n \n # This test script takes an extremely long time and is known to time out even\n # on fast machines because it requires in excess of one hour to run\n-set_tests_properties(\"${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh\" PROPERTIES TIMEOUT 4000)\n+set_tests_properties(\"t.suite.t7112-reset-submodule\" PROPERTIES TIMEOUT 4000)\n \n endif()#BUILD_TESTING\n-- \ngitgitgadget\n\n"},{"id":"482300","messageId":"xmqq34z2t6gv.fsf@gitster.g","threadId":"60174","inReplyTo":"pull.1579.v3.git.1695640836.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/7] CMake(Visual C) support for js/doc-unit-tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-25T19:09:52Z","receivedAt":"2023-09-25T19:11:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> The recent patch series that adds proper unit testing to Git requires a\n> couple of add-on patches to make it work with the CMake build on Windows\n> (Visual C). This patch series aims to provide that support.\n>\n> This patch series is based on js/doc-unit-tests.\n>\n> Changes since v2:\n>\n> * Thanks to Phillip Wood's prodding, I managed to avoid the \"Empty\n> Namespace\" problem in Visual Studio's Test Explorer.\n\nThanks. Will replace.\n\n> + ◢ ◈ git\n> + ◢ ◈ t\n> + ◢ ◈ suite\n> + ◈ t0000-basic\n> + ◈ t0001-init\n> + ◈ t0002-gitfile\n> + [...]\n\nI somehow liked this part of the log message very much ;-)\n"}]}