From 5c2ebf604249ef522a2fc4db70216ed0c9322f7e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Oct 2025 13:04:36 +0200 Subject: [PATCH 01/14] gitlab-ci: dedup instructions to disable realtime monitoring The instruction to disable realtime monitoring are shared across all of our Windows-based jobs. Deduplicate it so that we can more readily iterate on it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cf122e706f2..552c033fb0b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -112,6 +112,9 @@ test:osx: - t/failed-test-artifacts when: on_failure +.windows_before_script: &windows_before_script + - Set-MpPreference -DisableRealtimeMonitoring $true + build:mingw64: stage: build tags: @@ -119,7 +122,7 @@ build:mingw64: variables: NO_PERL: 1 before_script: - - Set-MpPreference -DisableRealtimeMonitoring $true + - *windows_before_script - ./ci/install-sdk.ps1 -directory "git-sdk" script: - git-sdk/usr/bin/bash.exe -l -c 'ci/make-test-artifacts.sh artifacts' @@ -136,7 +139,7 @@ test:mingw64: - job: "build:mingw64" artifacts: true before_script: - - Set-MpPreference -DisableRealtimeMonitoring $true + - *windows_before_script - git-sdk/usr/bin/bash.exe -l -c 'tar xf artifacts/artifacts.tar.gz' - New-Item -Path .git/info -ItemType Directory - New-Item .git/info/exclude -ItemType File -Value "/git-sdk" @@ -150,7 +153,7 @@ test:mingw64: tags: - saas-windows-medium-amd64 before_script: - - Set-MpPreference -DisableRealtimeMonitoring $true + - *windows_before_script - choco install -y git meson ninja openssl - Import-Module $env:ChocolateyInstall\helpers\chocolateyProfile.psm1 - refreshenv -- GitLab From e90f6b2b008ceba56a3a83941e50d57c3cb7b4ea Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Oct 2025 13:04:37 +0200 Subject: [PATCH 02/14] gitlab-ci: ignore failures to disable realtime monitoring We have recently introduced a change to disable realtime monitoring for Windows job in GitLab CI. This change led (and still leads) to a quite significant speedup. But there's a catch: seemingly, some of the runners we use already have realtime monitoring disabled. On such a machine, trying to disable the feature again leads to an error that causes the whole job to fail. Safeguard against such failures by explicitly ignoring them. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 552c033fb0b..ed4dc9db94c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -113,7 +113,10 @@ test:osx: when: on_failure .windows_before_script: &windows_before_script - - Set-MpPreference -DisableRealtimeMonitoring $true + # Disabling realtime monitoring fails on some of the runners, but it + # significantly speeds up test execution in the case where it works. We thus + # try our luck, but ignore any failures. + - Set-MpPreference -DisableRealtimeMonitoring $true; $true build:mingw64: stage: build -- GitLab From 82ad27ebcd7f832227a2669ee002cc5e9dffb245 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Oct 2025 13:04:38 +0200 Subject: [PATCH 03/14] gitlab-ci: drop workaround for Python certificate store on Windows On Windows, we have been running into some issues in the past where the certificate store for Python is broken on the GitLab CI runners using Windows. The consequence was that we weren't able to establish any SSL connections via Python, but we need that feature so that we can download the Meson wraps. The workaround we employed was to import certificates from the cURL project into the certificate store via OpenSSL. This is obviously an ugly workaround. But even more importantly, this workaround fails every time Chocolatey updates its OpenSSL packages. The problem here is that the old OpenSSL package installer will be removed immediately once the newer version was published, But the Chocolatey community repository may not yet have propagated the new version of this package to all of its caches. The result is that for a couple hours (or sometimes even one or two days) we always fail to install OpenSSL until the new version was propagated. Luckily though, it turns out that the workaround doesn't seem to be required anymore. Drop it to work around the intermittent failures and to clean up some now-unneeded legacy cruft. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ed4dc9db94c..b388154078d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -157,17 +157,9 @@ test:mingw64: - saas-windows-medium-amd64 before_script: - *windows_before_script - - choco install -y git meson ninja openssl + - choco install -y git meson ninja - Import-Module $env:ChocolateyInstall\helpers\chocolateyProfile.psm1 - refreshenv - # The certificate store for Python on Windows is broken and fails to fetch - # certificates, see https://bugs.python.org/issue36011. This seems to - # mostly be an issue with how the GitLab image is set up as it is a - # non-issue on GitHub Actions. Work around the issue by importing - # cetrificates manually. - - Invoke-WebRequest https://curl.haxx.se/ca/cacert.pem -OutFile cacert.pem - - openssl pkcs12 -export -nokeys -in cacert.pem -out certs.pfx -passout "pass:" - - Import-PfxCertificate -CertStoreLocation Cert:\LocalMachine\Root -FilePath certs.pfx build:msvc-meson: extends: .msvc-meson -- GitLab From 0e98965234df00fbd4a3ab4864edf323ccb8ef17 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Oct 2025 13:04:39 +0200 Subject: [PATCH 04/14] gitlab-ci: upload Meson test logs as JUnit reports When running tests, Meson knows to output both a test log as well as a JUnit test report that collates results. We don't currently upload these results in our GitLab CI at all, which makes it hard to see which tests ran, but also which of our tests may have failed. Upload these JUnit reports as artifacts to make this information more accessible. Note that we also do this for some jobs that don't use Meson and thus don't generate these reports in the first place. GitLab CI handles missing reports gracefully though, so there is no reason to special-case those jobs that don't use Meson. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b388154078d..85401b34a58 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -70,6 +70,8 @@ test:linux: artifacts: paths: - t/failed-test-artifacts + reports: + junit: build/meson-logs/testlog.junit.xml when: on_failure test:osx: @@ -110,6 +112,8 @@ test:osx: artifacts: paths: - t/failed-test-artifacts + reports: + junit: build/meson-logs/testlog.junit.xml when: on_failure .windows_before_script: &windows_before_script @@ -181,6 +185,9 @@ test:msvc-meson: script: - meson test -C build --no-rebuild --print-errorlogs --slice $Env:CI_NODE_INDEX/$Env:CI_NODE_TOTAL parallel: 10 + artifacts: + reports: + junit: build/meson-logs/testlog.junit.xml test:fuzz-smoke-tests: image: ubuntu:latest -- GitLab From 3c4925c3f5dbafec2c0c9b3b7ac7d9086618a18f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Oct 2025 13:04:40 +0200 Subject: [PATCH 05/14] t8020: fix test failure due to indeterministic tag sorting In e6c06e87a2 (last-modified: fix bug when some paths remain unhandled, 2025-09-18), we have fixed a bug where under certain circumstances, git-last-modified(1) would BUG because there's still some unhandled paths. The fix claims that the root cause here is criss-cross merges, and it adds a test case that seemingly exercises this. Curiously, this test case fails on some systems because the actual output does not match our expectations: diff --git a/expect b/actual index 5271607..bdc620e 100644 --- a/expect --- b/actual @@ -1,3 +1,3 @@ km3 a -k2 k +km2 k 1 file error: last command exited with $?=1 not ok 15 - last-modified with subdir and criss-cross merge The output we see is git-name-rev(1) with `--annotate-stdin`. What it does is to take the output of git-last-modified(1), which contains object IDs of the blamed commits, and convert those object IDs into the names of the corresponding tags. Interestingly, we indeed have both "k2" and "km2" as tags, and even more interestingly both of these tags point to the same commit. So the output we get isn't _wrong_, as the tags are ambiguous. But why do both of these tags point to the same commit? "km2" really is supposed to be a merge, but due to the way the test is constructed the merge turns into a fast-forward merge. Which means that the resulting commit-graph does not even contain a criss-cross merge in the first place! A quick test though shows that the test indeed triggers the bug, so the initial analysis that the behaviour is triggered by such merges must be wrong. And it is: seemingly, the issue isn't with criss-cross merges, but rather with a graph where different files in the same directory were modified on both sides of a merge. Refactor the test so that we explicitly test for this specific situation instead of mentioning the "criss-cross merge" red herring. As the test is very specific to the actual layout of the repository we also adapt it to use its own standalone repository. Note that this requires us to drop the `test_when_finished` call in `check_last_modified` because it's not supported to execute that function in a subshell. This refactoring also fixes the original tag ambiguity that caused us to fail on some platforms. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t8020-last-modified.sh | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index e13aad14398..61f00bc15c3 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -33,7 +33,6 @@ check_last_modified() { done && cat >expect && - test_when_finished "rm -f tmp.*" && git ${indir:+-C "$indir"} last-modified "$@" >tmp.1 && git name-rev --annotate-stdin --name-only --tags \ tmp.2 && @@ -128,20 +127,25 @@ test_expect_success 'only last-modified files in the current tree' ' EOF ' -test_expect_success 'last-modified with subdir and criss-cross merge' ' - git checkout -b branch-k1 1 && - mkdir -p a k && - test_commit k1 a/file2 && - git checkout -b branch-k2 && - test_commit k2 k/file2 && - git checkout branch-k1 && - test_merge km2 branch-k2 && - test_merge km3 3 && - check_last_modified <<-\EOF - km3 a - k2 k - 1 file - EOF +test_expect_success 'subdirectory modified via merge' ' + test_when_finished rm -rf repo && + git init repo && + ( + cd repo && + test_commit base && + git switch --create left && + mkdir subdir && + test_commit left subdir/left && + git switch --create right base && + mkdir subdir && + test_commit right subdir/right && + git switch - && + test_merge merge right && + check_last_modified <<-\EOF + merge subdir + base base.t + EOF + ) ' test_expect_success 'cross merge boundaries in blaming' ' -- GitLab From bbb9efe7c1f3c7cd10492d1469df29f847d40629 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 14:36:29 +0200 Subject: [PATCH 06/14] ci: deduplicate calls to `apt-get update` 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 unecessary 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 Signed-off-by: Junio C Hamano --- ci/install-dependencies.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 0d3aa496fc3..645d0352504 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" || -- GitLab From 052e679ac3bd97de033e9bb555fcae4b1f197df0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 14:36:30 +0200 Subject: [PATCH 07/14] 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. 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 Signed-off-by: Junio C Hamano --- .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(+) create mode 100755 ci/run-rust-checks.sh diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 393ea4d1ccf..9e36b5c5e3e 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 f7d57d1ee96..a47d839e39a 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 645d0352504..a24b07edff8 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 00000000000..082eb52f111 --- /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 -- GitLab From 6358a08ff56a9826558d05e45b19c2a1ba57f243 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 14:36:31 +0200 Subject: [PATCH 08/14] rust/varint: add safety comments 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. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- src/varint.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/varint.rs b/src/varint.rs index 6e610bdd8e0..43b48debb54 100644 --- a/src/varint.rs +++ b/src/varint.rs @@ -1,3 +1,6 @@ +/// # Safety +/// +/// Callers must provide a NUL-terminated array to ensure safety. #[no_mangle] pub unsafe extern "C" fn decode_varint(bufp: *mut *const u8) -> u64 { let mut buf = *bufp; @@ -22,6 +25,11 @@ pub unsafe extern "C" fn decode_varint(bufp: *mut *const u8) -> u64 { val } +/// # Safety +/// +/// The provided buffer must be large enough to store the encoded varint. Callers may either provide +/// a `[u8; 16]` here, which is guaranteed to satisfy all encodable numbers. Or they can call this +/// function with a `NULL` pointer first to figure out array size. #[no_mangle] pub unsafe extern "C" fn encode_varint(value: u64, buf: *mut u8) -> u8 { let mut varint: [u8; 16] = [0; 16]; -- GitLab From a94fe9a31233a4e36c6a2b4a7a757db27588a9ea Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 14:36:32 +0200 Subject: [PATCH 09/14] ci: check for common Rust mistakes via Clippy 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 Signed-off-by: Junio C Hamano --- 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 a24b07edff8..dcd22ddd95c 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 082eb52f111..fb5ea8991b8 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 -- GitLab From 11ed16bbfb1d680fa2cbe5887e6d60dc3e385c8a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 14:36:33 +0200 Subject: [PATCH 10/14] ci: verify minimum supported Rust version 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 Signed-off-by: Junio C Hamano --- 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 45c9b34981a..2f51bf5d5ff 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 dcd22ddd95c..29e558bb9cc 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 fb5ea8991b8..b5ad9e8dc6f 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 -- GitLab From 847a13cdfab3833a7d5c650ae55e0947e35331bb Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 14:36:34 +0200 Subject: [PATCH 11/14] rust: support for Windows 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 Based-on-patch-by: Ezekiel Newren Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .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 a47d839e39a..b419a84e2cc 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 7ea149598d8..366fd173e70 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 ec55d6a5fdf..a9c865b2afe 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 99400986d93..3998db04354 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 -- GitLab From a4e913cbec467d1557dcad99b30c57b3a8d8a196 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 11:55:44 +0200 Subject: [PATCH 12/14] rust: generate bindings via cbindgen To: git@vger.kernel.org --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 1, "change-id": "20251009-b4-pks-rust-cbindgen-d80779ed2269", "prefixes": [] } } -- GitLab From 6e860798f8af8fa0da7e9a33c02de24f6996cc2c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:40:50 +0200 Subject: [PATCH 13/14] rust: generate bindings via cbindgen When compiling Git with Rust enabled we replace our C implementation of the varint encoding with a Rust implementation. A prerequisite for doing so is of course that the interfaces for both implementations are exactly the same. If they aren't, then we risk subtle runtime errors. We don't really have a way to detect such interface mismatches though: the code will happily compile if we change either of the implementations without adjusting the other implementation in the same spirit. The risk of divergence is low right now as we only replace a single subsystem. But it is expected that we'll grow more reimplementations over time, so it is bound to increase. A related issue is that we don't have an easy way to implement features exclusively in Rust and make them available to our C library. Again, we don't have such features yet, but there are work-in-progress patch series that will eventually add them. Both of these issues can be addressed by generating C bindings via the cbindgen(1) tool: given a Rust crate, it extracts all functions marked with `extern "C"` and creates a C declaration for them. These are then written into a header file that we can include. Set up this infrastructure in both our Makefile and in Meson. To demonstrate its use, the generated "c-bindings.h" header is included in "varint.h". If we now adapt "varint.rs" to have a different function signature than the C code we'll now get a compiler error: In file included from ../read-cache.c:39: ../varint.h:6:9: error: conflicting types for 'encode_varint' 6 | uint8_t encode_varint(uint64_t, unsigned char *); | ^ ./c-bindings-generated.h:18:9: note: previous declaration is here 18 | uint8_t encode_varint(uint32_t value, uint8_t *buf); Note that the headers are split into two: - "c-bindings-generated.h" is the header generated by cbindgen(1). This header should never be excluded directly, as it may be missing in non-Rust builds. - "c-bindings.h" is the header that should be included by code. The header uses an `#ifdef WITH_RUST` include guard so that it can be included even when building without Rust. Adapt our CI to install cbindgen(1). Signed-off-by: Patrick Steinhardt --- .gitignore | 1 + Makefile | 14 ++++++++++++-- c-bindings.h | 8 ++++++++ cbindgen.toml | 7 +++++++ ci/install-dependencies.sh | 4 ++-- meson.build | 38 +++++++++++++++++++++++++++++++++++++- shared.mak | 1 + varint.h | 2 ++ 8 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 c-bindings.h create mode 100644 cbindgen.toml diff --git a/.gitignore b/.gitignore index 78a45cb5bec..6b565dbddea 100644 --- a/.gitignore +++ b/.gitignore @@ -197,6 +197,7 @@ /gitweb/gitweb.cgi /gitweb/static/gitweb.js /gitweb/static/gitweb.min.* +/c-bindings-generated.h /config-list.h /command-list.h /hook-list.h diff --git a/Makefile b/Makefile index 366fd173e70..c3a5be8ed48 100644 --- a/Makefile +++ b/Makefile @@ -1545,6 +1545,16 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND) ifdef WITH_RUST BASIC_CFLAGS += -DWITH_RUST GITLIBS += $(RUST_LIB) + +# Note: the .depend file emitted by cbindgen uses absolute paths, so we have to +# use those here, as well. +C_BINDINGS = $(shell pwd)/c-bindings-generated.h + +GENERATED_H += $(C_BINDINGS) + +$(C_BINDINGS): cbindgen.toml + $(QUIET_CBINDGEN)cbindgen --output $@ + ifeq ($(uname_S),Windows) EXTLIBS += -luserenv endif @@ -2859,7 +2869,7 @@ missing_compdb_dir = compdb_args = endif -$(OBJECTS): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir) +$(OBJECTS): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir) $(C_BINDINGS) $(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $< %.s: %.c GIT-CFLAGS FORCE @@ -3820,7 +3830,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) $(FUZZ_PROGRAMS) $(RM) $(SP_OBJ) $(RM) $(HCC) - $(RM) -r Cargo.lock target/ + $(RM) -r Cargo.lock target/ $(C_BINDINGS) $(RM) version-def.h $(RM) -r $(dep_dirs) $(compdb_dir) compile_commands.json $(RM) $(test_bindir_programs) diff --git a/c-bindings.h b/c-bindings.h new file mode 100644 index 00000000000..064fc73e2d5 --- /dev/null +++ b/c-bindings.h @@ -0,0 +1,8 @@ +#ifndef C_BINDINGS_H +#define C_BINDINGS_H + +#ifdef WITH_RUST +# include "c-bindings-generated.h" +#endif + +#endif diff --git a/cbindgen.toml b/cbindgen.toml new file mode 100644 index 00000000000..ba4b2d63672 --- /dev/null +++ b/cbindgen.toml @@ -0,0 +1,7 @@ +language = "C" + +# Don't include standard C headers. These are managed by "git-compat-util.h". +no_includes = true + +# Use plain structs instead of using typedefs. +style = "tag" diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 29e558bb9cc..8af142fa93b 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -37,7 +37,7 @@ fedora-*|almalinux-*) MESON_DEPS="meson ninja";; esac dnf -yq update >/dev/null && - dnf -yq install shadow-utils sudo make pkg-config gcc findutils diffutils perl python3 gawk gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel $MESON_DEPS cargo >/dev/null + dnf -yq install shadow-utils sudo make pkg-config gcc findutils diffutils perl python3 gawk gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel $MESON_DEPS cargo cbindgen >/dev/null ;; ubuntu-*|i386/ubuntu-*|debian-*) # Required so that apt doesn't wait for user input on certain packages. @@ -64,7 +64,7 @@ ubuntu-*|i386/ubuntu-*|debian-*) make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \ tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \ libemail-valid-perl libio-pty-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \ - libsecret-1-dev libpcre2-dev meson ninja-build pkg-config cargo \ + libsecret-1-dev libpcre2-dev meson ninja-build pkg-config cargo cbindgen \ ${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE case "$distro" in diff --git a/meson.build b/meson.build index a9c865b2afe..b1cd145ac90 100644 --- a/meson.build +++ b/meson.build @@ -1703,7 +1703,9 @@ version_def_h = custom_target( libgit_sources += version_def_h cargo = find_program('cargo', dirs: program_path, native: true, required: get_option('rust')) -rust_option = get_option('rust').disable_auto_if(not cargo.found()) +cbindgen = find_program('cbindgen', dirs: program_path, native: true, required: get_option('rust')) + +rust_option = get_option('rust').disable_auto_if(not cargo.found() or not cbindgen.found()) if rust_option.allowed() subdir('src') libgit_c_args += '-DWITH_RUST' @@ -1711,6 +1713,40 @@ if rust_option.allowed() if host_machine.system() == 'windows' libgit_dependencies += compiler.find_library('userenv') endif + + cbindgen_kwargs = { + 'build_always_stale': true, + } + + cbindgen_args = [ + cbindgen, + '--output', + '@OUTPUT@', + meson.current_source_dir(), + ] + + # cbindgen only learned about `--depfile` in v0.25.0. In earlier versions we + # thus have to execute it unconditionally. This isn't too bad though, as the + # target file is only updated in case the file actually changes. + if meson.version().version_compare('>=0.62.0') + if cbindgen.version().version_compare('>=0.25.0') + cbindgen_kwargs = { + 'depfile': 'c-bindings-generated.d', + } + + cbindgen_args += [ + '--depfile', + meson.current_build_dir() / 'c-bindings-generated.d', + ] + endif + endif + + libgit_sources += custom_target('cbindgen', + input: 'cbindgen.toml', + output: 'c-bindings-generated.h', + command: cbindgen_args, + kwargs: cbindgen_kwargs, + ) else libgit_sources += [ 'varint.c', diff --git a/shared.mak b/shared.mak index 0e7492076eb..598e58e069c 100644 --- a/shared.mak +++ b/shared.mak @@ -57,6 +57,7 @@ ifndef V ## Used in "Makefile" QUIET_CARGO = @echo ' ' CARGO $@; + QUIET_CBINDGEN = @echo ' ' CBINDGEN $@; QUIET_CC = @echo ' ' CC $@; QUIET_AR = @echo ' ' AR $@; QUIET_LINK = @echo ' ' LINK $@; diff --git a/varint.h b/varint.h index eb401935bd2..e20c1513712 100644 --- a/varint.h +++ b/varint.h @@ -1,6 +1,8 @@ #ifndef VARINT_H #define VARINT_H +#include "c-bindings.h" + uint8_t encode_varint(uint64_t, unsigned char *); uint64_t decode_varint(const unsigned char **); -- GitLab From 5b1070cbc5d97e2164e3bbadf32f277cf5c055a5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 13:16:06 +0200 Subject: [PATCH 14/14] switch to debian --- .github/workflows/main.yml | 3 +-- .gitlab-ci.yml | 2 +- ci/install-dependencies.sh | 6 +++--- ci/lib.sh | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9e36b5c5e3e..135eb5afe92 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -395,9 +395,8 @@ jobs: cc: gcc - jobname: linux-musl-meson image: alpine:latest - # Supported until 2025-04-02. - jobname: linux32 - image: i386/ubuntu:focal + image: i386/debian:latest # A RHEL 8 compatible distro. Supported until 2029-05-31. - jobname: almalinux-8 image: almalinux:8 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b419a84e2cc..85feebf72d6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -63,7 +63,7 @@ test:linux: - jobname: linux-musl-meson image: alpine:latest - jobname: linux32 - image: i386/ubuntu:20.04 + image: i386/debian:latest - jobname: linux-meson image: ubuntu:rolling CC: gcc diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 8af142fa93b..a7b5a38c63f 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -39,7 +39,7 @@ fedora-*|almalinux-*) dnf -yq update >/dev/null && dnf -yq install shadow-utils sudo make pkg-config gcc findutils diffutils perl python3 gawk gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel $MESON_DEPS cargo cbindgen >/dev/null ;; -ubuntu-*|i386/ubuntu-*|debian-*) +ubuntu-*|i386/debian-*|debian-*) # Required so that apt doesn't wait for user input on certain packages. export DEBIAN_FRONTEND=noninteractive @@ -48,9 +48,9 @@ ubuntu-*|i386/ubuntu-*|debian-*) SVN='libsvn-perl subversion' LANGUAGES='language-pack-is' ;; - i386/ubuntu-*) + i386/debian-*) SVN= - LANGUAGES='language-pack-is' + LANGUAGES='locales-all' ;; *) SVN='libsvn-perl subversion' diff --git a/ci/lib.sh b/ci/lib.sh index f561884d401..865a0675371 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -250,7 +250,7 @@ then CI_OS_NAME=osx JOBS=$(nproc) ;; - *,alpine:*|*,fedora:*|*,ubuntu:*|*,i386/ubuntu:*) + *,alpine:*|*,fedora:*|*,ubuntu:*|*,i386/debian:*) CI_OS_NAME=linux JOBS=$(nproc) ;; -- GitLab