Re: [PATCH 0/7] RFC: Accelerate xdiff and begin its rustification
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 19, 2025, 21:53 UTC
- Message-ID
- <ac871bc4-df93-31f4-55f2-d6fc538a422d@gmx.de>
- In-Reply-To
- <pull.1980.git.git.1752784344.gitgitgadget@gmail.com>
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