{"thread":{"id":"63015","subject":"[PATCH 0/2] Hot fixes from Git for Windows v2.49.0-rc0","startedAt":"2025-02-27T15:44:12Z","lastAt":"2025-03-07T18:21:00Z","messageCount":14,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Patrick Steinhardt","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"513150","messageId":"pull.1867.git.1740671049.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":null,"subject":"[PATCH 0/2] Hot fixes from Git for Windows v2.49.0-rc0","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-02-27T15:44:07Z","receivedAt":"2025-02-27T15:44:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I needed many patches to make Git for Windows v2.49.0-rc0 compile and run\nthe many, many CI jobs successfully. These here patches even apply to\nupstream Git. (Technically, the Meson sorting patch is not required to\ncompile, but it was the fall-out from many required adjustments to make the\nMeson jobs happy.)\n\nJohannes Schindelin (2):\n ident: stop assuming that `gw_gecos` is writable\n meson: fix sorting\n\n ident.c | 2 +-\n meson.build | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\n\nbase-commit: b838bf1938926a7a900166136d995d86f8a00e24\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1867%2Fdscho%2Fg4w-hot-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1867/dscho/g4w-hot-fixes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1867\n-- \ngitgitgadget\n"},{"id":"513152","messageId":"045c11dc1d51690eed4dc07da26dcace612f786c.1740671049.git.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":"pull.1867.git.1740671049.gitgitgadget@gmail.com","subject":"[PATCH 1/2] ident: stop assuming that `gw_gecos` is writable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-02-27T15:44:08Z","receivedAt":"2025-02-27T15:44:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 590e081dea7c (ident: add NO_GECOS_IN_PWENT for systems without\npw_gecos in struct passwd, 2011-05-19), code was introduced to iterate\nover the `gw_gecos` field; The loop variable is of type `char *`, which\nassumes that `gw_gecos` is writable.\n\nHowever, it is not necessarily writable (and it is a bad idea to have it\nwritable in the first place), so let's switch the loop variable type to\n`const char *`.\n\nSigned-off-by: Johannes Schindelin \n---\n ident.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ident.c b/ident.c\nindex caf41fb2a98..967895d8850 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)\n \n static void copy_gecos(const struct passwd *w, struct strbuf *name)\n {\n-\tchar *src;\n+\tconst char *src;\n \n \t/* Traditionally GECOS field had office phone numbers etc, separated\n \t * with commas. Also & stands for capitalized form of the login name.\n-- \ngitgitgadget\n\n"},{"id":"513151","messageId":"9d1faeae8a47326fbaef9a7a670920899ab0aa81.1740671049.git.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":"pull.1867.git.1740671049.gitgitgadget@gmail.com","subject":"[PATCH 2/2] meson: fix sorting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-02-27T15:44:09Z","receivedAt":"2025-02-27T15:44:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 904339edbd80 (Introduce support for the Meson build system,\n2024-12-06) the `meson.build` file was introduced, adding also a\nWindows-specific list of source files. This list was obviously meant to\nbe sorted alphabetically, but there is one mistake. Let's fix that.\n\nSigned-off-by: Johannes Schindelin \n---\n meson.build | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex bf95576f839..16e451365e9 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1092,11 +1092,11 @@ elif host_machine.system() == 'windows'\n libgit_sources += [\n 'compat/mingw.c',\n 'compat/winansi.c',\n+ 'compat/win32/dirent.c',\n 'compat/win32/flush.c',\n 'compat/win32/path-utils.c',\n 'compat/win32/pthread.c',\n 'compat/win32/syslog.c',\n- 'compat/win32/dirent.c',\n 'compat/win32mmap.c',\n 'compat/nedmalloc/nedmalloc.c',\n ]\n-- \ngitgitgadget\n"},{"id":"513187","messageId":"xmqqwmdb5bt8.fsf@gitster.g","threadId":"63015","inReplyTo":"045c11dc1d51690eed4dc07da26dcace612f786c.1740671049.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] ident: stop assuming that `gw_gecos` is writable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-27T21:15:31Z","receivedAt":"2025-02-27T21:15:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> In 590e081dea7c (ident: add NO_GECOS_IN_PWENT for systems without\n> pw_gecos in struct passwd, 2011-05-19), code was introduced to iterate\n> over the `gw_gecos` field; The loop variable is of type `char *`, which\n> assumes that `gw_gecos` is writable.\n>\n> However, it is not necessarily writable (and it is a bad idea to have it\n> writable in the first place), so let's switch the loop variable type to\n> `const char *`.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> ident.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nCan we have a bit more details here (not in the commit but after the\nthree-dash line, meant for context) why the change from 2011 needs a\n\"hotfix\" today? Has something in the build environment change? An\nupdated header file has changed definition for \"struct passwd\", or\nsomething silly like that? Even though POSIX.1 does not define\ngecos in its struct passwd in , other string members like\npw_name and pw_dir are writable, which I found funny, and makes me\npuzzled why this code from 2011 needs a hotfix all of the sudden.\n\n> diff --git a/ident.c b/ident.c\n> index caf41fb2a98..967895d8850 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)\n> \n> static void copy_gecos(const struct passwd *w, struct strbuf *name)\n> {\n> -\tchar *src;\n> +\tconst char *src;\n> \n> \t/* Traditionally GECOS field had office phone numbers etc, separated\n> \t * with commas. Also & stands for capitalized form of the login name.\n\nThe patch text itself looks perfectly fine, so I am not opposed to\nits eventual application. Even though we do declare \"src\" as\nnon-const, we only use it to read from it, so declaring it as const\npointer is perfectly fine and more appropriate.\n\nBut I do not see any urgency relative to Git 2.48.0 (or Git 2.48.1\nfor that matter) to mark this as \"hotfix\" implying that it should be\nincluded in 2.49-rc1 in either the patch or the proposed commit log\nmessage.\n\nThanks.\n"},{"id":"513236","messageId":"Z8Fsv-mcJlDbogtd@pks.im","threadId":"63015","inReplyTo":"9d1faeae8a47326fbaef9a7a670920899ab0aa81.1740671049.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] meson: fix sorting","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-02-28T07:58:55Z","receivedAt":"2025-02-28T07:59:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 27, 2025 at 03:44:09PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> In 904339edbd80 (Introduce support for the Meson build system,\n> 2024-12-06) the `meson.build` file was introduced, adding also a\n> Windows-specific list of source files. This list was obviously meant to\n> be sorted alphabetically, but there is one mistake. Let's fix that.\n\nThis looks obviously good to me, thanks!\n\nPatrick\n"},{"id":"513638","messageId":"pull.1867.v2.git.1741256780.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":"pull.1867.git.1740671049.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Hot fixes from Git for Windows v2.49.0-rc0","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-06T10:26:17Z","receivedAt":"2025-03-06T10:26:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I needed many patches to make Git for Windows v2.49.0-rc0 compile and run\nthe many, many CI jobs successfully. These here patches even apply to\nupstream Git. (Technically, the Meson sorting patch is not required to\ncompile, but it was the fall-out from many required adjustments to make the\nMeson jobs happy.)\n\nChanges since v1:\n\n * I spent two hours investigating under which circumstances the (correct)\n compiler error about a non-writable pw_gecos field triggers, and\n augmented the commit message accordingly.\n * Since -rc1, another breaking change necessitated yet another hot fix,\n which I invites to this patch fest (and the patch series hence had to be\n rebased to the current tip of Git's main branch).\n\nJohannes Schindelin (3):\n ident: stop assuming that `gw_gecos` is writable\n meson: fix sorting\n cmake: generalize the handling of the `CLAR_TEST_OBJS` list\n\n contrib/buildsystems/CMakeLists.txt | 12 ++++++++----\n ident.c | 2 +-\n meson.build | 2 +-\n 3 files changed, 10 insertions(+), 6 deletions(-)\n\n\nbase-commit: e969bc875963a10890d61ba84eab3a460bd9e535\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1867%2Fdscho%2Fg4w-hot-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1867/dscho/g4w-hot-fixes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1867\n\nRange-diff vs v1:\n\n 1: 045c11dc1d5 ! 1: 3e9ccffc747 ident: stop assuming that `gw_gecos` is writable\n @@ Commit message\n writable in the first place), so let's switch the loop variable type to\n `const char *`.\n \n + This is not a new problem, but what is new is the Meson build. While it\n + does not trigger in CI builds, imitating the commands of\n + `ci/run-build-and-tests.sh` in a regular Git for Windows SDK (`meson\n + setup build . --fatal-meson-warnings --warnlevel 2 --werror --wrap-mode\n + nofallback -Dfuzzers=true` followed by `meson compile -C build --`\n + results in this beautiful error:\n +\n + \"cc\" [...] -o libgit.a.p/ident.c.obj \"-c\" ../ident.c\n + ../ident.c: In function 'copy_gecos':\n + ../ident.c:68:18: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]\n + 68 | for (src = get_gecos(w); *src && *src != ','; src++) {\n + | ^\n + cc1.exe: all warnings being treated as errors\n +\n + Now, why does this not trigger in CI? The answer is as simple as it is\n + puzzling: The `win+Meson` job completely side-steps Git for Windows'\n + development environment, opting instead to use the GCC that is on the\n + `PATH` in GitHub-hosted `windows-latest` runners. That GCC is pinned to\n + v12.2.0 and targets the UCRT (unlikely to change any time soon, see\n + https://github.com/actions/runner-images/blob/win25/20250303.1/images/windows/toolsets/toolset-2022.json#L132-L141).\n + That is in stark contrast to Git for Windows, which uses GCC v14.2.0 and\n + targets MSVCRT. Git for Windows' `Makefile`-based build also obviously\n + uses different compiler flags, otherwise this compile error would have\n + had plenty of opportunity in almost 14 years to surface.\n +\n + In other words, contrary to my expectations, the `win+Meson` job is\n + ill-equipped to replace the `win build` job because it exercises a\n + completely different tool version/compiler flags vector than what Git\n + for Windows needs.\n +\n + Nevertheless, there is currently this huge push, including breaking\n + changes after -rc1 and all, for switching to Meson. Therefore, we need\n + to make it work, somehow, even in Git for Windows' SDK, hence this\n + patch, at this point in time.\n +\n Signed-off-by: Johannes Schindelin \n \n ## ident.c ##\n 2: 9d1faeae8a4 = 2: 4e9ab3e011f meson: fix sorting\n -: ----------- > 3: 59a2e586e1a cmake: generalize the handling of the `CLAR_TEST_OBJS` list\n\n-- \ngitgitgadget\n"},{"id":"513639","messageId":"3e9ccffc7474698947bdcb6d49b5d0728deadd08.1741256780.git.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":"pull.1867.v2.git.1741256780.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-06T10:26:18Z","receivedAt":"2025-03-06T10:26:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 590e081dea7c (ident: add NO_GECOS_IN_PWENT for systems without\npw_gecos in struct passwd, 2011-05-19), code was introduced to iterate\nover the `gw_gecos` field; The loop variable is of type `char *`, which\nassumes that `gw_gecos` is writable.\n\nHowever, it is not necessarily writable (and it is a bad idea to have it\nwritable in the first place), so let's switch the loop variable type to\n`const char *`.\n\nThis is not a new problem, but what is new is the Meson build. While it\ndoes not trigger in CI builds, imitating the commands of\n`ci/run-build-and-tests.sh` in a regular Git for Windows SDK (`meson\nsetup build . --fatal-meson-warnings --warnlevel 2 --werror --wrap-mode\nnofallback -Dfuzzers=true` followed by `meson compile -C build --`\nresults in this beautiful error:\n\n \"cc\" [...] -o libgit.a.p/ident.c.obj \"-c\" ../ident.c\n ../ident.c: In function 'copy_gecos':\n ../ident.c:68:18: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]\n 68 | for (src = get_gecos(w); *src && *src != ','; src++) {\n | ^\n cc1.exe: all warnings being treated as errors\n\nNow, why does this not trigger in CI? The answer is as simple as it is\npuzzling: The `win+Meson` job completely side-steps Git for Windows'\ndevelopment environment, opting instead to use the GCC that is on the\n`PATH` in GitHub-hosted `windows-latest` runners. That GCC is pinned to\nv12.2.0 and targets the UCRT (unlikely to change any time soon, see\nhttps://github.com/actions/runner-images/blob/win25/20250303.1/images/windows/toolsets/toolset-2022.json#L132-L141).\nThat is in stark contrast to Git for Windows, which uses GCC v14.2.0 and\ntargets MSVCRT. Git for Windows' `Makefile`-based build also obviously\nuses different compiler flags, otherwise this compile error would have\nhad plenty of opportunity in almost 14 years to surface.\n\nIn other words, contrary to my expectations, the `win+Meson` job is\nill-equipped to replace the `win build` job because it exercises a\ncompletely different tool version/compiler flags vector than what Git\nfor Windows needs.\n\nNevertheless, there is currently this huge push, including breaking\nchanges after -rc1 and all, for switching to Meson. Therefore, we need\nto make it work, somehow, even in Git for Windows' SDK, hence this\npatch, at this point in time.\n\nSigned-off-by: Johannes Schindelin \n---\n ident.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/ident.c b/ident.c\nindex caf41fb2a98..967895d8850 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)\n \n static void copy_gecos(const struct passwd *w, struct strbuf *name)\n {\n-\tchar *src;\n+\tconst char *src;\n \n \t/* Traditionally GECOS field had office phone numbers etc, separated\n \t * with commas. Also & stands for capitalized form of the login name.\n-- \ngitgitgadget\n\n"},{"id":"513640","messageId":"4e9ab3e011f0d3c619f58ec88c8bbe8f5cd56ac1.1741256780.git.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":"pull.1867.v2.git.1741256780.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] meson: fix sorting","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-06T10:26:19Z","receivedAt":"2025-03-06T10:26:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 904339edbd80 (Introduce support for the Meson build system,\n2024-12-06) the `meson.build` file was introduced, adding also a\nWindows-specific list of source files. This list was obviously meant to\nbe sorted alphabetically, but there is one mistake. Let's fix that.\n\nSigned-off-by: Johannes Schindelin \n---\n meson.build | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex e86085b0a47..efe2871c9db 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1109,11 +1109,11 @@ elif host_machine.system() == 'windows'\n libgit_sources += [\n 'compat/mingw.c',\n 'compat/winansi.c',\n+ 'compat/win32/dirent.c',\n 'compat/win32/flush.c',\n 'compat/win32/path-utils.c',\n 'compat/win32/pthread.c',\n 'compat/win32/syslog.c',\n- 'compat/win32/dirent.c',\n 'compat/win32mmap.c',\n 'compat/nedmalloc/nedmalloc.c',\n ]\n-- \ngitgitgadget\n\n"},{"id":"513641","messageId":"59a2e586e1ae705471c7003efc007df5447692f9.1741256780.git.gitgitgadget@gmail.com","threadId":"63015","inReplyTo":"pull.1867.v2.git.1741256780.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] cmake: generalize the handling of the `CLAR_TEST_OBJS` list","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-06T10:26:20Z","receivedAt":"2025-03-06T10:26:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nA late-comer to the v2.49.0 party, `sk/unit-test-oid`, added yet another\narray item to `CLAR_TEST_OBJS`, causing the `win+VS build` job to fail\nwith symptoms like this one:\n\n unit-tests-lib.lib(u-oid-array.obj) : error LNK2019: unresolved\n external symbol cl_parse_any_oid referenced in function fill_array\n\nThis is a similar scenario to the one that forced me to write\n8afda42fce60 (cmake: generalize the handling of the `UNIT_TEST_OBJS`\nlist, 2024-09-18): The hard-coded echo of `CLAR_TEST_OBJS` in\n`CMakeLists.txt` that recapitulates faithfully what was already\nhard-coded in `Makefile` would either have to be updated whack-a-mole\nstyle, or generalized.\n\nJust like I chose the latter option for `UNIT_TEST_OBJS`, I now do the\nsame for `CLAR_TEST_OBJS`.\n\nSigned-off-by: Johannes Schindelin \n---\n contrib/buildsystems/CMakeLists.txt | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex c6fbd57e158..25b495fa737 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -1001,10 +1001,14 @@ parse_makefile_for_sources(unit-test_SOURCES ${CMAKE_SOURCE_DIR}/Makefile \"UNIT_\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_sources(clar-test_SOURCES ${CMAKE_SOURCE_DIR}/Makefile \"CLAR_TEST_OBJS\")\n+list(TRANSFORM clar-test_SOURCES REPLACE \"\\\\$\\\\(UNIT_TEST_DIR\\\\)/\" \"${CMAKE_SOURCE_DIR}/t/unit-tests/\")\n+add_library(clar-test-lib STATIC ${clar-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 common-main)\n+\ttarget_link_libraries(\"${unit_test}\" unit-test-lib clar-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@@ -1046,13 +1050,13 @@ add_custom_command(OUTPUT \"${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite\"\n \tVERBATIM)\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(clar-test-lib PUBLIC \"${CMAKE_BINARY_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+add_executable(unit-tests)\n+target_link_libraries(unit-tests unit-tests-lib clar-test-lib common-main)\n set_target_properties(unit-tests\n \tPROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests/bin)\n if(MSVC)\n-- \ngitgitgadget\n"},{"id":"513646","messageId":"Z8l967w0rjrnWGpO@pks.im","threadId":"63015","inReplyTo":"3e9ccffc7474698947bdcb6d49b5d0728deadd08.1741256780.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-06T10:50:19Z","receivedAt":"2025-03-06T10:50:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Mar 06, 2025 at 10:26:18AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> Now, why does this not trigger in CI? The answer is as simple as it is\n> puzzling: The `win+Meson` job completely side-steps Git for Windows'\n> development environment, opting instead to use the GCC that is on the\n> `PATH` in GitHub-hosted `windows-latest` runners. That GCC is pinned to\n> v12.2.0 and targets the UCRT (unlikely to change any time soon, see\n> https://github.com/actions/runner-images/blob/win25/20250303.1/images/windows/toolsets/toolset-2022.json#L132-L141).\n> That is in stark contrast to Git for Windows, which uses GCC v14.2.0 and\n> targets MSVCRT. Git for Windows' `Makefile`-based build also obviously\n> uses different compiler flags, otherwise this compile error would have\n> had plenty of opportunity in almost 14 years to surface.\n\nOh, interesting. I didn't even know that the Windows runners had GCC in\ntheir PATH, and thus I didn't expect it to use that compiler at all. On\nGitLab for example we can see that it uses the MSVC compiler as I did\nexpect [1]:\n\n Activating VS 17.10.2\n C compiler for the host machine: cl (msvc 19.40.33811 \"Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33811 for x64\")\n C linker for the host machine: link link 14.40.33811.0\n\nBut you're right, on GitHub that's not the case:\n\n C compiler for the host machine: gcc (gcc 12.2.0 \"gcc (x86_64-posix-seh-rev2, Built by MinGW-W64 project) 12.2.0\")\n C linker for the host machine: gcc ld.bfd 2.39\n\nWe can easily fix that by passing the `--vsenv` flag to Meson. I'll send\na patch soonish.\n\nPatrick\n\n[1]: https://gitlab.com/gitlab-org/git/-/jobs/9324989037#L95\n[2]: https://github.com/git/git/actions/runs/13686408338/job/38270746786#step:5:15\n"},{"id":"513653","messageId":"ba3bb865-d6fc-3125-e32d-935fa501c0c5@gmx.de","threadId":"63015","inReplyTo":"Z8l967w0rjrnWGpO@pks.im","subject":"Re: [PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-03-06T12:26:44Z","receivedAt":"2025-03-06T12:26:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Patrick,\n\nOn Thu, 6 Mar 2025, Patrick Steinhardt wrote:\n\n> On Thu, Mar 06, 2025 at 10:26:18AM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin \n> > Now, why does this not trigger in CI? The answer is as simple as it is\n> > puzzling: The `win+Meson` job completely side-steps Git for Windows'\n> > development environment, opting instead to use the GCC that is on the\n> > `PATH` in GitHub-hosted `windows-latest` runners. That GCC is pinned to\n> > v12.2.0 and targets the UCRT (unlikely to change any time soon, see\n> > https://github.com/actions/runner-images/blob/win25/20250303.1/images/windows/toolsets/toolset-2022.json#L132-L141).\n> > That is in stark contrast to Git for Windows, which uses GCC v14.2.0 and\n> > targets MSVCRT. Git for Windows' `Makefile`-based build also obviously\n> > uses different compiler flags, otherwise this compile error would have\n> > had plenty of opportunity in almost 14 years to surface.\n>\n> Oh, interesting. I didn't even know that the Windows runners had GCC in\n> their PATH, and thus I didn't expect it to use that compiler at all. On\n> GitLab for example we can see that it uses the MSVC compiler as I did\n> expect [1]:\n>\n> Activating VS 17.10.2\n> C compiler for the host machine: cl (msvc 19.40.33811 \"Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33811 for x64\")\n> C linker for the host machine: link link 14.40.33811.0\n>\n> But you're right, on GitHub that's not the case:\n>\n> C compiler for the host machine: gcc (gcc 12.2.0 \"gcc (x86_64-posix-seh-rev2, Built by MinGW-W64 project) 12.2.0\")\n> C linker for the host machine: gcc ld.bfd 2.39\n>\n> We can easily fix that by passing the `--vsenv` flag to Meson. I'll send\n> a patch soonish.\n>\n> Patrick\n>\n> [1]: https://gitlab.com/gitlab-org/git/-/jobs/9324989037#L95\n> [2]: https://github.com/git/git/actions/runs/13686408338/job/38270746786#step:5:15\n\nPlease do not invest more time on the Visual Studio support via Meson. No\ncontributor will use this, and I want to stop spending my time on this.\nThe user experience of configuring a Visual Studio build via Meson is just\ntoo weak compared to the ease of CMake-based builds, and while not many\nVisual Studio users are familiar with CMake, even dramatically less will\neven so much as know about Meson.\n\nI plan on dropping all pretense that Git supports Visual Studio-based\ncontributions soon after v2.49.0 comes out, e.g. by deleting the CMake\ndefinition and also deleting whatever Meson-specific stuff I can get away\ndeleting in Git for Windows. It was not worth the time I invested.\n\nCiao,\nJohannes\n"},{"id":"513716","messageId":"xmqq8qpijezc.fsf@gitster.g","threadId":"63015","inReplyTo":"3e9ccffc7474698947bdcb6d49b5d0728deadd08.1741256780.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-06T16:33:43Z","receivedAt":"2025-03-06T16:33:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> In other words, contrary to my expectations, the `win+Meson` job is\n> ill-equipped to replace the `win build` job because it exercises a\n> completely different tool version/compiler flags vector than what Git\n> for Windows needs.\n\nIt is apparent that meson support is a new procedure to build our\ncodebase that is untested and unproven on Windows at all, given that\namong all people who may have stake in Windows you are discovering\nproblems in it this late in the cycle. Nobody knows what other\nbreakages, other than something obvious and easy to catch like \"ah,\ncompiler refuses to go further\", are lurking under the radar.\n\nI would be reluctant to trust the build artifact out of meson-based\nbuild on Windows after seeing your report, especially the above\npart.\n\nA reasonable alternative may be to declare that meson-based build is\nnot ready yet at this point, and possibly disable win+Meson jobs to\npunt and divert our engineering resources elsewhere in the meantime.\nFor a new thing, having an uneven support depending on the platform\nearly in the evolution is not unusual or to be ashamed of.\n\n> Nevertheless, there is currently this huge push, including breaking\n> changes after -rc1 and all, for switching to Meson. Therefore, we need\n> to make it work, somehow, even in Git for Windows' SDK, hence this\n> patch, at this point in time.\n\nAs I said earlier already, I do not mind turning the type of this\npointer, which is only used to read from a struct member, like this\npatch does. It is the right thing to do, so I'll apply.\n\nBut I personally would not be comfortable with the product built\nwith \"completely different tool version/compiler flags vector than\nwhat G4W needs\", even the compilation passes with just this small\nchange. If I were using Windows, that is.\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> ident.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nThanks, will apply.\n\n> diff --git a/ident.c b/ident.c\n> index caf41fb2a98..967895d8850 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)\n> \n> static void copy_gecos(const struct passwd *w, struct strbuf *name)\n> {\n> -\tchar *src;\n> +\tconst char *src;\n> \n> \t/* Traditionally GECOS field had office phone numbers etc, separated\n> \t * with commas. Also & stands for capitalized form of the login name.\n"},{"id":"513741","messageId":"Z8rEIffQeVCjd_U8@pks.im","threadId":"63015","inReplyTo":"xmqq8qpijezc.fsf@gitster.g","subject":"Re: [PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-07T10:02:09Z","receivedAt":"2025-03-07T10:02:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Mar 06, 2025 at 08:33:43AM -0800, Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > In other words, contrary to my expectations, the `win+Meson` job is\n> > ill-equipped to replace the `win build` job because it exercises a\n> > completely different tool version/compiler flags vector than what Git\n> > for Windows needs.\n> \n> It is apparent that meson support is a new procedure to build our\n> codebase that is untested and unproven on Windows at all, given that\n> among all people who may have stake in Windows you are discovering\n> problems in it this late in the cycle. Nobody knows what other\n> breakages, other than something obvious and easy to catch like \"ah,\n> compiler refuses to go further\", are lurking under the radar.\n> \n> I would be reluctant to trust the build artifact out of meson-based\n> build on Windows after seeing your report, especially the above\n> part.\n> \n> A reasonable alternative may be to declare that meson-based build is\n> not ready yet at this point, and possibly disable win+Meson jobs to\n> punt and divert our engineering resources elsewhere in the meantime.\n> For a new thing, having an uneven support depending on the platform\n> early in the evolution is not unusual or to be ashamed of.\n\nI think it would be a bit sad to disable those jobs. They build and pass\nthe test suite alright in Git itself, even though they fail downstream\nin Git for Windows. They help me quite a bit to ensure that I don't\nregress anything that already is working while I'm iterating on the\ncurrent set of features. So in the end, I view them more as testing more\nvariants of Windows than replacing what we currently have, similar to\nhow we test Git on different Linux distributions.\n\nI have said before that I'm very willing to help to figure out any\nissues, regardless of which platform, and I stand by that statement --\nif you see anything that is broken in this context and report the issue\nto me I'll jump on it immediately.\n\n> > Nevertheless, there is currently this huge push, including breaking\n> > changes after -rc1 and all, for switching to Meson. Therefore, we need\n> > to make it work, somehow, even in Git for Windows' SDK, hence this\n> > patch, at this point in time.\n> \n> As I said earlier already, I do not mind turning the type of this\n> pointer, which is only used to read from a struct member, like this\n> patch does. It is the right thing to do, so I'll apply.\n> \n> But I personally would not be comfortable with the product built\n> with \"completely different tool version/compiler flags vector than\n> what G4W needs\", even the compilation passes with just this small\n> change. If I were using Windows, that is.\n\nThat's completely fair. The CI job we have isn't meant to verify that we\nhave a G4W-compatible distribution falling out of it, it merely verifies\nthat we can build and pass tests in such a \"standalone\" (that is,\nwithout the SDK) configuration. We might eventually want to introduce\nanother job that _does_ use the SDK with Meson, as well, but I didn't\nyet see a need for that until now.\n\nPatrick\n"},{"id":"513784","messageId":"xmqqjz90em7q.fsf@gitster.g","threadId":"63015","inReplyTo":"Z8rEIffQeVCjd_U8@pks.im","subject":"Re: [PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-07T18:20:57Z","receivedAt":"2025-03-07T18:21:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> I think it would be a bit sad to disable those jobs. They build and pass\n> the test suite alright in Git itself, even though they fail downstream\n> in Git for Windows. They help me quite a bit to ensure that I don't\n> regress anything that already is working while I'm iterating on the\n> current set of features. So in the end, I view them more as testing more\n> variants of Windows than replacing what we currently have, similar to\n> how we test Git on different Linux distributions.\n\nHmph, but compared to Linux or macOS platforms, do developers on\nWindows (and users of Git on Windows, including but not limited to\nusers of Git-for-windows) benefit from having the code base to be\ntested on \"more variants of Windows\", or would it be more noise that\nthey need to go visit the failing CI and spend time to triage if the\nbreakage is something they should worry about?\n\nThe above is more or less a rhetorical question. I think by now\neverybody knows I do not like monoculture, and if we had infinite\nengineering resources, I would think it would be healthy to have\nmore than one prominent and competing Windows port of Git (no, I\nknow about Cygwin, but I hear that the platform is POSIXy enough, so\nI do not exactly consider Git running on Cygwin qualifies as \"a\nWindows port\"). But we do not seem to live in such a world.\n\n> I have said before that I'm very willing to help to figure out any\n> issues, regardless of which platform, and I stand by that\n> statement -- if you see anything that is broken in this context\n> and report the issue to me I'll jump on it immediately.\n\nIt's ultimately up to Windows folks to take you up on the offer.\n\n>> > Nevertheless, there is currently this huge push, including breaking\n>> > changes after -rc1 and all, for switching to Meson. Therefore, we need\n>> > to make it work, somehow, even in Git for Windows' SDK, hence this\n>> > patch, at this point in time.\n>> ...\n> That's completely fair. The CI job we have isn't meant to verify that we\n> have a G4W-compatible distribution falling out of it, it merely verifies\n> that we can build and pass tests in such a \"standalone\" (that is,\n> without the SDK) configuration. We might eventually want to introduce\n> another job that _does_ use the SDK with Meson, as well, but I didn't\n> yet see a need for that until now.\n\nKnowing that it is the most widely used platform, I naturally and\nnaïvely was expecting and hoping that there are folks other than\nDscho who have enough bandwidth and inclination to help in this\narea, but so far, having a set of jobs in CI that use a toolchain\nthat is different from what G4W uses (as expected) did not quite\nhelp X-<. Nobody noticed it until the last minute.\n\nWhich made me say that we do not seem to live in such a world, which\nin turn makes me accept that putting all Windows eggs in a single\nbasket and watch it closely may be a reasonable alternative when it\ncomes to Windows [*], than hoping that diverse set of different\nbuilds eventually help flourishing Windows developer community.\n\nI dunno.\n\n\n\n[Footnote]\n\n * Yes, I admit that it may be another way to say that I do not care\n the particular platform deeply enough.\n\n"}]}