From 4478ad37a7d233b8db4d46dd563ece0bc8b00af4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 6 Mar 2025 10:26:18 +0000 Subject: [PATCH 1/3] ident: stop assuming that `gw_gecos` is writable In 590e081dea7c (ident: add NO_GECOS_IN_PWENT for systems without pw_gecos in struct passwd, 2011-05-19), code was introduced to iterate over the `gw_gecos` field; The loop variable is of type `char *`, which assumes that `gw_gecos` is writable. However, it is not necessarily writable (and it is a bad idea to have it writable in the first place), so let's switch the loop variable type to `const char *`. This is not a new problem, but what is new is the Meson build. While it does not trigger in CI builds, imitating the commands of `ci/run-build-and-tests.sh` in a regular Git for Windows SDK (`meson setup build . --fatal-meson-warnings --warnlevel 2 --werror --wrap-mode nofallback -Dfuzzers=true` followed by `meson compile -C build --` results in this beautiful error: "cc" [...] -o libgit.a.p/ident.c.obj "-c" ../ident.c ../ident.c: In function 'copy_gecos': ../ident.c:68:18: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] 68 | for (src = get_gecos(w); *src && *src != ','; src++) { | ^ cc1.exe: all warnings being treated as errors Now, why does this not trigger in CI? The answer is as simple as it is puzzling: The `win+Meson` job completely side-steps Git for Windows' development environment, opting instead to use the GCC that is on the `PATH` in GitHub-hosted `windows-latest` runners. That GCC is pinned to v12.2.0 and targets the UCRT (unlikely to change any time soon, see https://github.com/actions/runner-images/blob/win25/20250303.1/images/windows/toolsets/toolset-2022.json#L132-L141). That is in stark contrast to Git for Windows, which uses GCC v14.2.0 and targets MSVCRT. Git for Windows' `Makefile`-based build also obviously uses different compiler flags, otherwise this compile error would have had plenty of opportunity in almost 14 years to surface. In other words, contrary to my expectations, the `win+Meson` job is ill-equipped to replace the `win build` job because it exercises a completely different tool version/compiler flags vector than what Git for Windows needs. Nevertheless, there is currently this huge push, including breaking changes after -rc1 and all, for switching to Meson. Therefore, we need to make it work, somehow, even in Git for Windows' SDK, hence this patch, at this point in time. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- ident.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ident.c b/ident.c index caf41fb2a9..967895d885 100644 --- a/ident.c +++ b/ident.c @@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus) static void copy_gecos(const struct passwd *w, struct strbuf *name) { - char *src; + const char *src; /* Traditionally GECOS field had office phone numbers etc, separated * with commas. Also & stands for capitalized form of the login name. From 31761f391192909f6486ecb532e72dfaee756be6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 6 Mar 2025 10:26:19 +0000 Subject: [PATCH 2/3] meson: fix sorting In 904339edbd80 (Introduce support for the Meson build system, 2024-12-06) the `meson.build` file was introduced, adding also a Windows-specific list of source files. This list was obviously meant to be sorted alphabetically, but there is one mistake. Let's fix that. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index e86085b0a4..efe2871c9d 100644 --- a/meson.build +++ b/meson.build @@ -1109,11 +1109,11 @@ elif host_machine.system() == 'windows' libgit_sources += [ 'compat/mingw.c', 'compat/winansi.c', + 'compat/win32/dirent.c', 'compat/win32/flush.c', 'compat/win32/path-utils.c', 'compat/win32/pthread.c', 'compat/win32/syslog.c', - 'compat/win32/dirent.c', 'compat/win32mmap.c', 'compat/nedmalloc/nedmalloc.c', ] From 970916368764347c38d36c727b2e6e0086e784e3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 6 Mar 2025 10:26:20 +0000 Subject: [PATCH 3/3] cmake: generalize the handling of the `CLAR_TEST_OBJS` list A late-comer to the v2.49.0 party, `sk/unit-test-oid`, added yet another array item to `CLAR_TEST_OBJS`, causing the `win+VS build` job to fail with symptoms like this one: unit-tests-lib.lib(u-oid-array.obj) : error LNK2019: unresolved external symbol cl_parse_any_oid referenced in function fill_array This is a similar scenario to the one that forced me to write 8afda42fce60 (cmake: generalize the handling of the `UNIT_TEST_OBJS` list, 2024-09-18): The hard-coded echo of `CLAR_TEST_OBJS` in `CMakeLists.txt` that recapitulates faithfully what was already hard-coded in `Makefile` would either have to be updated whack-a-mole style, or generalized. Just like I chose the latter option for `UNIT_TEST_OBJS`, I now do the same for `CLAR_TEST_OBJS`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index c6fbd57e15..25b495fa73 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1001,10 +1001,14 @@ parse_makefile_for_sources(unit-test_SOURCES ${CMAKE_SOURCE_DIR}/Makefile "UNIT_ list(TRANSFORM unit-test_SOURCES REPLACE "\\$\\(UNIT_TEST_DIR\\)/" "${CMAKE_SOURCE_DIR}/t/unit-tests/") add_library(unit-test-lib STATIC ${unit-test_SOURCES}) +parse_makefile_for_sources(clar-test_SOURCES ${CMAKE_SOURCE_DIR}/Makefile "CLAR_TEST_OBJS") +list(TRANSFORM clar-test_SOURCES REPLACE "\\$\\(UNIT_TEST_DIR\\)/" "${CMAKE_SOURCE_DIR}/t/unit-tests/") +add_library(clar-test-lib STATIC ${clar-test_SOURCES}) + parse_makefile_for_scripts(unit_test_PROGRAMS "UNIT_TEST_PROGRAMS" "") foreach(unit_test ${unit_test_PROGRAMS}) add_executable("${unit_test}" "${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c") - target_link_libraries("${unit_test}" unit-test-lib common-main) + target_link_libraries("${unit_test}" unit-test-lib clar-test-lib common-main) set_target_properties("${unit_test}" PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests/bin) if(MSVC) @@ -1046,13 +1050,13 @@ add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" VERBATIM) add_library(unit-tests-lib ${clar_test_SUITES} - "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" ) +target_include_directories(clar-test-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests") target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests") -add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c") -target_link_libraries(unit-tests unit-tests-lib common-main) +add_executable(unit-tests) +target_link_libraries(unit-tests unit-tests-lib clar-test-lib common-main) set_target_properties(unit-tests PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests/bin) if(MSVC)