Re: [PATCH 0/7] RFC: Accelerate xdiff and begin its rustification

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 7271 bytes --]

Hi Ezekiel,                                                                                                                                                                                                           
pleasure to make your acquaintance!

On Thu, 17 Jul 2025, Ezekiel Newren via GitGitGadget wrote:

> 1. Windows fails to build. I don’t know which rust toolchain is even
>    correct for this or if multiple are needed.  Example failed build:
>    https://github.com/git/git/actions/runs/16353209191

There are a couple of problems, not just one. Here are the patches that I
would like to ask you to take custody of (for your convenience, I have
pushed them to https://github.com/dscho/git as the `xdiff_rust_speedup`
branch). Please find them below. They _just_ fix the build, but the tests
with win+Meson still fail (and as "win+Meson test" jobs keep the logs of
the failed tests a well-guarded secret, due to time constraints I have to
stop looking into this for now).

Thank you for working on this,
Johannes

-- snipsnap --
From 72c50ee3f9df5ccfe48bf6f44b2c6bba05a680bf Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 19 Jul 2025 21:24:07 +0200
Subject: [PATCH 1/3] Do support Windows again after requiring Rust

By default, Rust wants to build MS Visual C-compatible libraries on
Windows, because that is _the_ native C compiler.

Git is historically lacking in its MSVC support, and the official Git
for Windows versions are built using GCC instead. As a consequence, a
(subset of a) GCC toolchain is installed as part of the `windows-build`
job of every CI build.

Naturally, this requires adjustments in how Rust is called, most
importantly it requires installing support for a GCC-compatible build
target.

Let's make the necessary adjustment both in the CI-specific code that
installs Rust as well as in the Windows-specific configuration in
`config.mak.uname`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 ci/install-rust.sh | 3 +++
 config.mak.uname   | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/ci/install-rust.sh b/ci/install-rust.sh
index 141ceddb17cfe..c22baa629ceb7 100644
--- a/ci/install-rust.sh
+++ b/ci/install-rust.sh
@@ -28,6 +28,9 @@ if [ "$BITNESS" = "32" ]; then
   $CARGO_HOME/bin/rustup default --force-non-host $RUST_VERSION || exit $?
 else
   $CARGO_HOME/bin/rustup default $RUST_VERSION || exit $?
+  if [ "$CI_OS_NAME" = "windows" ]; then
+    $CARGO_HOME/bin/rustup target add x86_64-pc-windows-gnu || exit $?
+  fi
 fi
 
 . $CARGO_HOME/env
diff --git a/config.mak.uname b/config.mak.uname
index 3e26bb074a4b5..fbe7cebf40edd 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -727,19 +727,28 @@ ifeq ($(uname_S),MINGW)
 		prefix = /mingw32
 		HOST_CPU = i686
 		BASIC_LDFLAGS += -Wl,--pic-executable,-e,_mainCRTStartup
+		CARGO_BUILD_TARGET = i686-pc-windows-gnu
         endif
         ifeq (MINGW64,$(MSYSTEM))
 		prefix = /mingw64
 		HOST_CPU = x86_64
 		BASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup
+		CARGO_BUILD_TARGET = x86_64-pc-windows-gnu
         else ifeq (CLANGARM64,$(MSYSTEM))
 		prefix = /clangarm64
 		HOST_CPU = aarch64
 		BASIC_LDFLAGS += -Wl,--pic-executable,-e,mainCRTStartup
+		CARGO_BUILD_TARGET = aarch64-pc-windows-gnu
         else
 		COMPAT_CFLAGS += -D_USE_32BIT_TIME_T
 		BASIC_LDFLAGS += -Wl,--large-address-aware
         endif
+
+	export CARGO_BUILD_TARGET
+	RUST_TARGET_DIR = rust/target/$(CARGO_BUILD_TARGET)/$(RUST_BUILD_MODE)
+	# Unfortunately now needed because of Rust
+	EXTLIBS += -luserenv
+
 	CC = gcc
 	COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -DDETECT_MSYS_TTY \
 		-fstack-protector-strong
-- 
2.50.1.windows.1


From ef6e4394ae26d8f28cb0d9e456810ce0818e623b Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 19 Jul 2025 23:08:11 +0200
Subject: [PATCH 2/3] win+Meson: allow for xdiff to be compiled with MSVC

The `build_rust.sh` script is quite opinionated about the naming scheme
of the C compiler: It assumes that the xdiff library file will be named
`libxdiff.a`.

However, MS Visual C generates `xdiff.lib` files instead; This naming
scheme has been in use in a very, very long time.

Let's allow for that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 build_rust.sh |  7 ++++++-
 meson.build   | 12 +++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/build_rust.sh b/build_rust.sh
index 4c12135cd2050..694d48d857a58 100755
--- a/build_rust.sh
+++ b/build_rust.sh
@@ -44,7 +44,12 @@ fi
 
 cd $dir_rust && cargo clean && pwd && cargo build -p $crate $rust_args; cd ..
 
-libfile="lib${crate}.a"
+if grep x86_64-pc-windows-msvc rust/target/.rustc_info.json
+then
+  libfile="${crate}.lib"
+else
+  libfile="lib${crate}.a"
+fi
 dst=$dir_build/$libfile
 
 if [ "$dir_git_root" != "$dir_build" ]; then
diff --git a/meson.build b/meson.build
index 047d7e5b66306..5e89a5dd0e00f 100644
--- a/meson.build
+++ b/meson.build
@@ -277,8 +277,16 @@ else
   rustflags = '-Aunused_imports -Adead_code -C debuginfo=2 -C opt-level=1 -C force-frame-pointers=yes'
 endif
 
+compiler = meson.get_compiler('c')
+
+if compiler.get_id() == 'msvc'
+  xdiff_lib_filename = 'xdiff.lib'
+else
+  xdiff_lib_filename = 'libxdiff.a'
+endif
+
 rust_build_xdiff = custom_target('rust_build_xdiff',
-  output: 'libxdiff.a',
+  output: xdiff_lib_filename,
   build_by_default: true,
   build_always_stale: true,
   command: [
@@ -288,8 +296,6 @@ rust_build_xdiff = custom_target('rust_build_xdiff',
   install: false,
 )
 
-compiler = meson.get_compiler('c')
-
 libgit_sources = [
   'abspath.c',
   'add-interactive.c',
-- 
2.50.1.windows.1


From 9c3b017cfa069211027fbb1f6d3b97c8e7edda81 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 19 Jul 2025 23:22:57 +0200
Subject: [PATCH 3/3] win+Meson: do allow linking with the Rust-built xdiff

When linking against the Rust-built `xdiff`, there is now a new required
dependency: Without _also_ linking to the system library `userenv`, the
compile would fail with this error message:

  xdiff.lib(std-c85e9beb7923f636.std.df32d1bc89881d89-cgu.0.rcgu.o) :
  error LNK2019: unresolved external symbol __imp_GetUserProfileDirectoryW
  referenced in function _ZN3std3env8home_dir17hfd1c3b6676cd78f6E

Therefore, just like we do in case of Makefile-based builds on Windows,
we now also link to that library when building with Meson.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index 5e89a5dd0e00f..af015f04763fd 100644
--- a/meson.build
+++ b/meson.build
@@ -1260,6 +1260,7 @@ elif host_machine.system() == 'windows'
   ]
 
   libgit_dependencies += compiler.find_library('ntdll')
+  libgit_dependencies += compiler.find_library('userenv')
   libgit_include_directories += 'compat/win32'
   if compiler.get_id() == 'msvc'
     libgit_include_directories += 'compat/vcbuild/include'
-- 
2.50.1.windows.1