{"thread":{"id":"62139","subject":"[PATCH] cmake: generalize the handling of the `UNIT_TEST_OBJS` list","startedAt":"2024-09-18T19:29:33Z","lastAt":"2024-09-30T18:06:33Z","messageCount":4,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"503015","messageId":"pull.1797.git.1726687769585.gitgitgadget@gmail.com","threadId":"62139","inReplyTo":null,"subject":"[PATCH] cmake: generalize the handling of the `UNIT_TEST_OBJS` list","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-18T19:29:29Z","receivedAt":"2024-09-18T19:29:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn a15d4465a991 (cmake: also build unit tests, 2023-09-25), I\naccommodated the CMake definition. Seeing that a `UNIT_TEST_OBJS` list\nwas introduced that was built by transforming the `UNIT_TEST_PROGRAMS`\nlist and then adding a single, hard-coded file\n(\"t/unit-tests/test-lib.c\"), I decided to hard-code that in the CMake\ndefinition, too.\n\nThe reason why I hard-coded it instead of imitating the\n`parse_makefile_for_sources()` paradigm that was used elsewhere when\nusing the `Makefile` as source of truth for given lists of files: This\nfunction expects _only_ hard-coded values, and that transformed\n`UNIT_TEST_PROGRAMS` list complicated everything.\n\nIn 872721538c26 (cmake: fix build of `t-oidtree`, 2024-07-12), I\naccommodated the CMake definition again, after seeing that the\n`UNIT_TEST_OBJS` was still defined via that transformed list but now\nappending _two_ hard-coded files (\"t/unit-tests/lib-oid.c\" joined the\nfray).\n\nIn 428672a3b16 (Makefile: stop listing test library objects twice,\n2024-09-16), the `Makefile` was changed so that `UNIT_TEST_OBJS` is\nfinally only constructed using hard-coded file names just like the other\n`*_OBJS` variables. I missed that and therefore did not adjust the CMake\ndefinition. Besides, the code was working, so there was no real need to\nadjust it.\n\nWith a4f50bb1e9b (t/unit-tests: introduce reftable library, 2024-09-16),\nhowever, the `UNIT_TEST_OBJS` list became a trio, and the CMake\ndefinition has to be adjusted again. Now that we can use the\n`parse_makefile_for_sources()` function without many complications,\nlet's do that.\n\nSigned-off-by: Johannes Schindelin \n---\n cmake: generalize the handling of the UNIT_TEST_OBJS list\n \n This is an add-on for ps/reftable-exclude to let it build with CMake and\n Visual C.\n \n This patch on its own does not actually fix vs-build; The patch \"cmake:\n stop looking for REFTABLE_TEST_OBJS in the Makefile\" which I submitted\n in\n https://lore.kernel.org/git/pull.1796.git.1726687400286.gitgitgadget@gmail.com\n is also needed for a successful vs-build.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1797%2Fdscho%2Freftable-exclude%2Bcmake-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1797/dscho/reftable-exclude+cmake-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1797\n\n contrib/buildsystems/CMakeLists.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 832f46b316b..249c72075bb 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -975,13 +975,14 @@ 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-add_library(unit-test-lib-oid OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/lib-oid.c)\n+parse_makefile_for_sources(unit-test_SOURCES \"UNIT_TEST_OBJS\")\n+list(TRANSFORM unit-test_SOURCES REPLACE \"\\\\$\\\\(UNIT_TEST_DIR\\\\)/\" \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n+add_library(unit-test-lib STATIC ${unit-test_SOURCES})\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 unit-test-lib-oid common-main)\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/bin)\n \tif(MSVC)\n\nbase-commit: 18695250667912d8278e76dce453706c3d488173\n-- \ngitgitgadget\n"},{"id":"503295","messageId":"xmqqsetpx16k.fsf@gitster.g","threadId":"62139","inReplyTo":"pull.1797.git.1726687769585.gitgitgadget@gmail.com","subject":"Re: [PATCH] cmake: generalize the handling of the `UNIT_TEST_OBJS` list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-24T01:58:27Z","receivedAt":"2024-09-24T01:58:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> With a4f50bb1e9b (t/unit-tests: introduce reftable library, 2024-09-16),\n> however, the `UNIT_TEST_OBJS` list became a trio, and the CMake\n> definition has to be adjusted again. Now that we can use the\n> `parse_makefile_for_sources()` function without many complications,\n> let's do that.\n\nAm I correct to understand that it is not \"trio\"-ness (has three\nthings) that makes the approach feasible, but the fact that all\nthree things are concrete values?\n\nThe longer-term aspiration is to migrate everything to clar-based\nunit tests, so the list will hopefully keep getting shorter and then\nbecome empty (https://lore.kernel.org/git/Zt_lLsnylKJ9uoqj@pks.im/).\n\nWill queue. Thanks.\n\n"},{"id":"503659","messageId":"0121a34f-6a32-aef7-92d6-e11521fab2d8@gmx.de","threadId":"62139","inReplyTo":"xmqqsetpx16k.fsf@gitster.g","subject":"Re: [PATCH] cmake: generalize the handling of the `UNIT_TEST_OBJS` list","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-09-29T17:48:32Z","receivedAt":"2024-09-29T17:48:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nHi,\n\nOn Mon, 23 Sep 2024, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n>\n> > With a4f50bb1e9b (t/unit-tests: introduce reftable library, 2024-09-16),\n> > however, the `UNIT_TEST_OBJS` list became a trio, and the CMake\n> > definition has to be adjusted again. Now that we can use the\n> > `parse_makefile_for_sources()` function without many complications,\n> > let's do that.\n>\n> Am I correct to understand that it is not \"trio\"-ness (has three\n> things) that makes the approach feasible, but the fact that all\n> three things are concrete values?\n\nNo, the fact that a third entry was added made it obvious that adding\nhard-coded equivalents to the CMake definition is too much of a\nwhac-a-mole thing and that the `Makefile` parsing approach would be\npreferable. With just two items, it was still kind of okay to duplicate\nthe list, but three is a trend.\n\n> The longer-term aspiration is to migrate everything to clar-based\n> unit tests, so the list will hopefully keep getting shorter and then\n> become empty (https://lore.kernel.org/git/Zt_lLsnylKJ9uoqj@pks.im/).\n\nOnce it becomes empty, I will have to send another CMake-related patch\nsimilar to when the `REFTABLE_TEST_OBJS` list went away, as the CMake\ndefinition cannot handle such empty lists.\n\nCiao,\nJohannes\n"},{"id":"503753","messageId":"xmqq8qv98195.fsf@gitster.g","threadId":"62139","inReplyTo":"0121a34f-6a32-aef7-92d6-e11521fab2d8@gmx.de","subject":"Re: [PATCH] cmake: generalize the handling of the `UNIT_TEST_OBJS` list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-30T18:06:30Z","receivedAt":"2024-09-30T18:06:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> The longer-term aspiration is to migrate everything to clar-based\n>> unit tests, so the list will hopefully keep getting shorter and then\n>> become empty (https://lore.kernel.org/git/Zt_lLsnylKJ9uoqj@pks.im/).\n>\n> Once it becomes empty, I will have to send another CMake-related patch\n> similar to when the `REFTABLE_TEST_OBJS` list went away, as the CMake\n> definition cannot handle such empty lists.\n\nIt may be ideal if all contributors learned and updated inputs to\nall the build process options like cmake, make, and autoconf at the\nsame time as they futz with the code they build (in other words, a\nchange that makes such a list to empty includes cmake update), but\ndifferent people chipping in with help in their strong areas and\npolishing the system as a whole as a collaborative effort is\nprobably the best practical alternative.\n\nThanks.\n"}]}