[PATCH v2 0/6] ci: improvements to our Rust infrastructure

[PATCH v2 0/6] ci: improvements to our Rust infrastructure

[Patrick Steinhardt]

Hi,

this small patch series introduces some improvements for our Rust
infrastructure. Most importantly, it introduces a couple of static
analysis checks to verify consistent formatting, use Clippy for linting
and to verify our minimum supported Rust version.

Furthermore, this series also introduces support for building with Rust
enabled on Windows.

The series is built on top of 45547b60ac (Merge branch 'master' of
https://github.com/j6t/gitk, 2025-10-05) with ps/rust-balloon at
e425c40aa0 (ci: enable Rust for breaking-changes jobs, 2025-10-02) and
ps/gitlab-ci-windows-improvements at 3c4925c3f5 (t8020: fix test failure
due to indeterministic tag sorting, 2025-10-02) merged into it.

Changes in v2:
  - Adjust comments for `encode_varint()` and `decode_varint()` based on
    brian's feedback.
  - Some small improvements to commit messages.
  - Not changed is the default column limit used by Rust. I think using
    the column limit of 100 used by the Rust ecosystem is sensible, but
    if there is a majority advocating for a limit of 80 I'll adapt this.
  - Link to v1: https://lore.kernel.org/r/20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (6):
      ci: deduplicate calls to `apt-get update`
      ci: check formatting of our Rust code
      rust/varint: add safety comments
      ci: check for common Rust mistakes via Clippy
      ci: verify minimum supported Rust version
      rust: support for Windows

 .github/workflows/main.yml | 15 +++++++++++++++
 .gitlab-ci.yml             | 13 ++++++++++++-
 Cargo.toml                 |  1 +
 Makefile                   | 14 ++++++++++++--
 ci/install-dependencies.sh | 17 +++++++++++++----
 ci/run-rust-checks.sh      | 22 ++++++++++++++++++++++
 meson.build                |  4 ++++
 src/cargo-meson.sh         | 11 +++++++++--
 src/varint.rs              | 15 +++++++++++++++
 9 files changed, 103 insertions(+), 9 deletions(-)

Range-diff versus v1:

1:  26bd2c3713 ! 1:  fa3ba52997 ci: deduplicate calls to `apt-get update`
    @@ Commit message
         only required to perform job-specific tasks.
     
         In both steps we use `apt-get update` to update our repository sources.
    -    This is unecessary though: all platforms that use Aptitude would have
    +    This is unnecessary though: all platforms that use Aptitude would have
         already executed this command in the distro-specific step anyway.
     
         Drop the redundant calls.
2:  5116c20fd1 ! 2:  89e7b67d5c ci: check formatting of our Rust code
    @@ Commit message
         ci: check formatting of our Rust code
     
         Introduce a CI check that verifies that our Rust code is well-formatted.
    -    This check uses rustfmt(1), which is the de-facto standard in the Rust
    -    world.
    +    This check uses `cargo fmt`, which is a wrapper around rustfmt(1) that
    +    executes formatting for all Rust source files. rustfmt(1) itself is the
    +    de-facto standard for formatting code in the Rust ecosystem.
     
         The rustfmt(1) tool allows to tweak the final format in theory. In
         practice though, the Rust ecosystem has aligned on style "editions".
3:  5b4c3303b7 < -:  ---------- rust/varint: add safety comments
-:  ---------- > 3:  3c91ec55dc rust/varint: add safety comments
4:  68925b41c0 = 4:  481500d7b8 ci: check for common Rust mistakes via Clippy
5:  c3f0943066 = 5:  09e73796ec ci: verify minimum supported Rust version
6:  0f968950a8 = 6:  acd460fac5 rust: support for Windows

---
base-commit: 8c8e270f2aba359479c4c2b4ab3c62726e5dac9d
change-id: 20251007-b4-pks-ci-rust-8422e6a8196e

[PATCH v2 1/6] ci: deduplicate calls to `apt-get update`

[Patrick Steinhardt]

When installing dependencies we first check for the distribution that is
in use and then we check for the specific job. In the first step we
already install all dependencies required to build and test Git, whereas
the second step installs a couple of additional dependencies that are
only required to perform job-specific tasks.

In both steps we use `apt-get update` to update our repository sources.
This is unnecessary though: all platforms that use Aptitude would have
already executed this command in the distro-specific step anyway.

Drop the redundant calls.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 0d3aa496fc..645d035250 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -120,21 +120,17 @@ esac
 
 case "$jobname" in
 ClangFormat)
-	sudo apt-get -q update
 	sudo apt-get -q -y install clang-format
 	;;
 StaticAnalysis)
-	sudo apt-get -q update
 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
 		libexpat-dev gettext make
 	;;
 sparse)
-	sudo apt-get -q update -q
 	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
 		libexpat-dev gettext zlib1g-dev sparse
 	;;
 Documentation)
-	sudo apt-get -q update
 	sudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make
 
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||

-- 
2.51.0.764.g787ff6f08a.dirty

[PATCH v2 2/6] ci: check formatting of our Rust code

[Patrick Steinhardt]

Introduce a CI check that verifies that our Rust code is well-formatted.
This check uses `cargo fmt`, which is a wrapper around rustfmt(1) that
executes formatting for all Rust source files. rustfmt(1) itself is the
de-facto standard for formatting code in the Rust ecosystem.

The rustfmt(1) tool allows to tweak the final format in theory. In
practice though, the Rust ecosystem has aligned on style "editions".
These editions only exist to ensure that any potential changes to the
style don't cause reformats to existing code bases. Other than that,
most Rust projects out there accept this default style of a specific
edition.

Let's do the same and use that default style. It may not be anyone's
favorite, but it is consistent and by making it part of our CI we also
enforce it right from the start.

Note that we don't have to pick a specific style edition here, as the
edition is automatically derived from the edition we have specified in
our "Cargo.toml" file.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 15 +++++++++++++++
 .gitlab-ci.yml             | 11 +++++++++++
 ci/install-dependencies.sh |  5 +++++
 ci/run-rust-checks.sh      | 12 ++++++++++++
 4 files changed, 43 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 393ea4d1cc..9e36b5c5e3 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -458,6 +458,21 @@ jobs:
     - run: ci/install-dependencies.sh
     - run: ci/run-static-analysis.sh
     - run: ci/check-directional-formatting.bash
+  rust-analysis:
+    needs: ci-config
+    if: needs.ci-config.outputs.enabled == 'yes'
+    env:
+      jobname: RustAnalysis
+      CI_JOB_IMAGE: ubuntu:rolling
+    runs-on: ubuntu-latest
+    container: ubuntu:rolling
+    concurrency:
+      group: rust-analysis-${{ github.ref }}
+      cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
+    steps:
+    - uses: actions/checkout@v4
+    - run: ci/install-dependencies.sh
+    - run: ci/run-rust-checks.sh
   sparse:
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index f7d57d1ee9..a47d839e39 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -212,6 +212,17 @@ static-analysis:
     - ./ci/run-static-analysis.sh
     - ./ci/check-directional-formatting.bash
 
+rust-analysis:
+  image: ubuntu:rolling
+  stage: analyze
+  needs: [ ]
+  variables:
+    jobname: RustAnalysis
+  before_script:
+    - ./ci/install-dependencies.sh
+  script:
+    - ./ci/run-rust-checks.sh
+
 check-whitespace:
   image: ubuntu:latest
   stage: analyze
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 645d035250..a24b07edff 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -126,6 +126,11 @@ StaticAnalysis)
 	sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
 		libexpat-dev gettext make
 	;;
+RustAnalysis)
+	sudo apt-get -q -y install rustup
+	rustup default stable
+	rustup component add rustfmt
+	;;
 sparse)
 	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
 		libexpat-dev gettext zlib1g-dev sparse
diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh
new file mode 100755
index 0000000000..082eb52f11
--- /dev/null
+++ b/ci/run-rust-checks.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+. ${0%/*}/lib.sh
+
+set +x
+
+if ! group "Check Rust formatting" cargo fmt --all --check
+then
+	RET=1
+fi
+
+exit $RET

-- 
2.51.0.764.g787ff6f08a.dirty

[PATCH v2 3/6] rust/varint: add safety comments

[Patrick Steinhardt]

The `decode_varint()` and `encode_varint()` functions in our Rust crate
are reimplementations of the respective C functions. As such, we are
naturally forced to use the same interface in both Rust and C, which
makes use of raw pointers. The consequence is that the code needs to be
marked as unsafe in Rust.

It is common practice in Rust to provide safety documentation for every
block that is marked as unsafe. This common practice is also enforced by
Clippy, Rust's static analyser. We don't have Clippy wired up yet, and
we could of course just disable this check. But we're about to wire it
up, and it is reasonable to always enforce documentation for unsafe
blocks.

Add such safety comments to already squelch those warnings now. While at
it, also document the functions' behaviour.

Helped-by: "brian m. carlson" <sandals@crustytoothpaste.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 src/varint.rs | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/varint.rs b/src/varint.rs
index 6e610bdd8e..06492dfc5e 100644
--- a/src/varint.rs
+++ b/src/varint.rs
@@ -1,3 +1,10 @@
+/// Decode the variable-length integer stored in `bufp` and return the decoded value.
+///
+/// Returns 0 in case the decoded integer would overflow u64::MAX.
+///
+/// # Safety
+///
+/// The buffer must be NUL-terminated to ensure safety.
 #[no_mangle]
 pub unsafe extern "C" fn decode_varint(bufp: *mut *const u8) -> u64 {
     let mut buf = *bufp;
@@ -22,6 +29,14 @@ pub unsafe extern "C" fn decode_varint(bufp: *mut *const u8) -> u64 {
     val
 }
 
+/// Encode `value` into `buf` as a variable-length integer unless `buf` is null.
+///
+/// Returns the number of bytes written, or, if `buf` is null, the number of bytes that would be
+/// written to encode the integer.
+///
+/// # Safety
+///
+/// `buf` must either be null or point to at least 16 bytes of memory.
 #[no_mangle]
 pub unsafe extern "C" fn encode_varint(value: u64, buf: *mut u8) -> u8 {
     let mut varint: [u8; 16] = [0; 16];

-- 
2.51.0.764.g787ff6f08a.dirty

[PATCH v2 4/6] ci: check for common Rust mistakes via Clippy

[Patrick Steinhardt]

Introduce a CI check that uses Clippy to perform checks for common
mistakes and suggested code improvements. Clippy is the official static
analyser of the Rust project and thus the de-facto standard.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 2 +-
 ci/run-rust-checks.sh      | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index a24b07edff..dcd22ddd95 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -129,7 +129,7 @@ StaticAnalysis)
 RustAnalysis)
 	sudo apt-get -q -y install rustup
 	rustup default stable
-	rustup component add rustfmt
+	rustup component add clippy rustfmt
 	;;
 sparse)
 	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh
index 082eb52f11..fb5ea8991b 100755
--- a/ci/run-rust-checks.sh
+++ b/ci/run-rust-checks.sh
@@ -9,4 +9,9 @@ then
 	RET=1
 fi
 
+if ! group "Check for common Rust mistakes" cargo clippy --all-targets --all-features -- -Dwarnings
+then
+	RET=1
+fi
+
 exit $RET

-- 
2.51.0.764.g787ff6f08a.dirty

[PATCH v2 5/6] ci: verify minimum supported Rust version

[Patrick Steinhardt]

In the current state of our Rust code base we don't really have any
requirements for the minimum supported Rust version yet, as we don't use
any features introduced by a recent version of Rust. Consequently, we
have decided that we want to aim for a rather old version and edition of
Rust, where the hope is that using an old version will make alternatives
like gccrs viable earlier for compiling Git.

But while we specify the Rust edition, we don't yet specify a Rust
version. And even if we did, the Rust version would only be enforced for
our own code, but not for any of our dependencies.

We don't yet have any dependencies at the current point in time. But
let's add some safeguards by specifying the minimum supported Rust
version and using cargo-msrv(1) to verify that this version can be
satisfied for all of our dependencies.

Note that we fix the version of cargo-msrv(1) at v0.18.1. This is the
latest release supported by Ubuntu's Rust version.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Cargo.toml                 | 1 +
 ci/install-dependencies.sh | 8 ++++++++
 ci/run-rust-checks.sh      | 5 +++++
 3 files changed, 14 insertions(+)

diff --git a/Cargo.toml b/Cargo.toml
index 45c9b34981..2f51bf5d5f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -2,6 +2,7 @@
 name = "gitcore"
 version = "0.1.0"
 edition = "2018"
+rust-version = "1.49.0"
 
 [lib]
 crate-type = ["staticlib"]
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index dcd22ddd95..29e558bb9c 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -10,6 +10,8 @@ begin_group "Install dependencies"
 P4WHENCE=https://cdist2.perforce.com/perforce/r23.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 JGITWHENCE=https://repo1.maven.org/maven2/org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh
+CARGO_MSRV_VERSION=0.18.4
+CARGO_MSRV_WHENCE=https://github.com/foresterre/cargo-msrv/releases/download/v$CARGO_MSRV_VERSION/cargo-msrv-x86_64-unknown-linux-musl-v$CARGO_MSRV_VERSION.tgz
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -130,6 +132,12 @@ RustAnalysis)
 	sudo apt-get -q -y install rustup
 	rustup default stable
 	rustup component add clippy rustfmt
+
+	wget -q "$CARGO_MSRV_WHENCE" -O "cargo-msvc.tgz"
+	sudo mkdir -p "$CUSTOM_PATH"
+	sudo tar -xf "cargo-msvc.tgz" --strip-components=1 \
+		--directory "$CUSTOM_PATH" --wildcards "*/cargo-msrv"
+	sudo chmod a+x "$CUSTOM_PATH/cargo-msrv"
 	;;
 sparse)
 	sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \
diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh
index fb5ea8991b..b5ad9e8dc6 100755
--- a/ci/run-rust-checks.sh
+++ b/ci/run-rust-checks.sh
@@ -14,4 +14,9 @@ then
 	RET=1
 fi
 
+if ! group "Check for minimum required Rust version" cargo msrv verify
+then
+	RET=1
+fi
+
 exit $RET

-- 
2.51.0.764.g787ff6f08a.dirty

[PATCH v2 6/6] rust: support for Windows

[Patrick Steinhardt]

The initial patch series that introduced Rust into the core of Git only
cared about macOS and Linux. This specifically leaves out Windows, which
indeed fails to build right now due to two issues:

  - The Rust runtime requires `GetUserProfileDirectoryW()`, but we don't
    link against "userenv.dll".

  - The path of the Rust library built on Windows is different than on
    most other systems systems.

Fix both of these issues to support Windows.

Note that this commit fixes the Meson-based job in GitHub's CI. Meson
auto-detects the availability of Rust, and as the Windows runner has
Rust installed by default it already enabled Rust support there. But due
to the above issues that job fails consistently.

Install Rust on GitLab CI, as well, to improve test coverage there.

Based-on-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Based-on-patch-by: Ezekiel Newren <ezekielnewren@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .gitlab-ci.yml     |  2 +-
 Makefile           | 14 ++++++++++++--
 meson.build        |  4 ++++
 src/cargo-meson.sh | 11 +++++++++--
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a47d839e39..b419a84e2c 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -161,7 +161,7 @@ test:mingw64:
     - saas-windows-medium-amd64
   before_script:
     - *windows_before_script
-    - choco install -y git meson ninja
+    - choco install -y git meson ninja rust-ms
     - Import-Module $env:ChocolateyInstall\helpers\chocolateyProfile.psm1
     - refreshenv
 
diff --git a/Makefile b/Makefile
index 7ea149598d..366fd173e7 100644
--- a/Makefile
+++ b/Makefile
@@ -929,10 +929,17 @@ TEST_SHELL_PATH = $(SHELL_PATH)
 LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 REFTABLE_LIB = reftable/libreftable.a
+
 ifdef DEBUG
-RUST_LIB = target/debug/libgitcore.a
+RUST_TARGET_DIR = target/debug
 else
-RUST_LIB = target/release/libgitcore.a
+RUST_TARGET_DIR = target/release
+endif
+
+ifeq ($(uname_S),Windows)
+RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib
+else
+RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a
 endif
 
 # xdiff and reftable libs may in turn depend on what is in libgit.a
@@ -1538,6 +1545,9 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)
 ifdef WITH_RUST
 BASIC_CFLAGS += -DWITH_RUST
 GITLIBS += $(RUST_LIB)
+ifeq ($(uname_S),Windows)
+EXTLIBS += -luserenv
+endif
 endif
 
 ifdef SANITIZE
diff --git a/meson.build b/meson.build
index ec55d6a5fd..a9c865b2af 100644
--- a/meson.build
+++ b/meson.build
@@ -1707,6 +1707,10 @@ rust_option = get_option('rust').disable_auto_if(not cargo.found())
 if rust_option.allowed()
   subdir('src')
   libgit_c_args += '-DWITH_RUST'
+
+  if host_machine.system() == 'windows'
+    libgit_dependencies += compiler.find_library('userenv')
+  endif
 else
   libgit_sources += [
     'varint.c',
diff --git a/src/cargo-meson.sh b/src/cargo-meson.sh
index 99400986d9..3998db0435 100755
--- a/src/cargo-meson.sh
+++ b/src/cargo-meson.sh
@@ -26,7 +26,14 @@ then
 	exit $RET
 fi
 
-if ! cmp "$BUILD_DIR/$BUILD_TYPE/libgitcore.a" "$BUILD_DIR/libgitcore.a" >/dev/null 2>&1
+case "$(cargo -vV | sed -s 's/^host: \(.*\)$/\1/')" in
+	*-windows-*)
+		LIBNAME=gitcore.lib;;
+	*)
+		LIBNAME=libgitcore.a;;
+esac
+
+if ! cmp "$BUILD_DIR/$BUILD_TYPE/$LIBNAME" "$BUILD_DIR/libgitcore.a" >/dev/null 2>&1
 then
-	cp "$BUILD_DIR/$BUILD_TYPE/libgitcore.a" "$BUILD_DIR/libgitcore.a"
+	cp "$BUILD_DIR/$BUILD_TYPE/$LIBNAME" "$BUILD_DIR/libgitcore.a"
 fi

-- 
2.51.0.764.g787ff6f08a.dirty

Re: [PATCH v2 0/6] ci: improvements to our Rust infrastructure

[Patrick Steinhardt]

On Wed, Oct 08, 2025 at 08:27:11AM +0200, Patrick Steinhardt wrote:
> Hi,
> 
> this small patch series introduces some improvements for our Rust
> infrastructure. Most importantly, it introduces a couple of static
> analysis checks to verify consistent formatting, use Clippy for linting
> and to verify our minimum supported Rust version.
> 
> Furthermore, this series also introduces support for building with Rust
> enabled on Windows.
> 
> The series is built on top of 45547b60ac (Merge branch 'master' of
> https://github.com/j6t/gitk, 2025-10-05) with ps/rust-balloon at
> e425c40aa0 (ci: enable Rust for breaking-changes jobs, 2025-10-02) and
> ps/gitlab-ci-windows-improvements at 3c4925c3f5 (t8020: fix test failure
> due to indeterministic tag sorting, 2025-10-02) merged into it.
> 
> Changes in v2:
>   - Adjust comments for `encode_varint()` and `decode_varint()` based on
>     brian's feedback.
>   - Some small improvements to commit messages.
>   - Not changed is the default column limit used by Rust. I think using
>     the column limit of 100 used by the Rust ecosystem is sensible, but
>     if there is a majority advocating for a limit of 80 I'll adapt this.
>   - Link to v1: https://lore.kernel.org/r/20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im

Sorry, I noticed that threading is broken here. An update of b4 made me
lose my custom patches that implement shallow threading by accident. The
next version will connect back to the original thread again.

Patrick

Re: [PATCH v2 0/6] ci: improvements to our Rust infrastructure

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Changes in v2:
>   - Adjust comments for `encode_varint()` and `decode_varint()` based on
>     brian's feedback.
>   - Some small improvements to commit messages.
>   - Not changed is the default column limit used by Rust. I think using
>     the column limit of 100 used by the Rust ecosystem is sensible, but
>     if there is a majority advocating for a limit of 80 I'll adapt this.
>   - Link to v1: https://lore.kernel.org/r/20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im

OK.  I am ambivalent on the column limit one, but otherwise, this
topic looks quite well done already.

Thanks.