{"thread":{"id":"64262","subject":"[PATCH 0/6] ci: improvements to our Rust infrastructure","startedAt":"2025-10-07T12:36:42Z","lastAt":"2025-11-21T21:39:04Z","messageCount":37,"participants":["Patrick Steinhardt","Karthik Nayak","Eric Sunshine","Junio C Hamano","brian m. carlson","Chris Torek","SZEDER Gábor","Justin Tobler","Ezekiel Newren","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"528097","messageId":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","threadId":"64262","inReplyTo":null,"subject":"[PATCH 0/6] ci: improvements to our Rust infrastructure","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:28Z","receivedAt":"2025-10-07T12:36:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series introduces some improvements for our Rust\ninfrastructure. Most importantly, it introduces a couple of static\nanalysis checks to verify consistent formatting, use Clippy for linting\nand to verify our minimum supported Rust version.\n\nFurthermore, this series also introduces support for building with Rust\nenabled on Windows.\n\nThe series is built on top of 45547b60ac (Merge branch 'master' of\nhttps://github.com/j6t/gitk, 2025-10-05) with ps/rust-balloon at\ne425c40aa0 (ci: enable Rust for breaking-changes jobs, 2025-10-02) and\nps/gitlab-ci-windows-improvements at 3c4925c3f5 (t8020: fix test failure\ndue to indeterministic tag sorting, 2025-10-02) merged into it.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (6):\n      ci: deduplicate calls to `apt-get update`\n      ci: check formatting of our Rust code\n      rust/varint: add safety comments\n      ci: check for common Rust mistakes via Clippy\n      ci: verify minimum supported Rust version\n      rust: support for Windows\n\n .github/workflows/main.yml | 15 +++++++++++++++\n .gitlab-ci.yml             | 13 ++++++++++++-\n Cargo.toml                 |  1 +\n Makefile                   | 14 ++++++++++++--\n ci/install-dependencies.sh | 17 +++++++++++++----\n ci/run-rust-checks.sh      | 22 ++++++++++++++++++++++\n meson.build                |  4 ++++\n src/cargo-meson.sh         | 11 +++++++++--\n src/varint.rs              |  8 ++++++++\n 9 files changed, 96 insertions(+), 9 deletions(-)\n\n\n---\nbase-commit: 8c8e270f2aba359479c4c2b4ab3c62726e5dac9d\nchange-id: 20251007-b4-pks-ci-rust-8422e6a8196e\n\n"},{"id":"528098","messageId":"20251007-b4-pks-ci-rust-v1-1-394502abe7ea@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH 1/6] ci: deduplicate calls to `apt-get update`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:29Z","receivedAt":"2025-10-07T12:36:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When installing dependencies we first check for the distribution that is\nin use and then we check for the specific job. In the first step we\nalready install all dependencies required to build and test Git, whereas\nthe second step installs a couple of additional dependencies that are\nonly required to perform job-specific tasks.\n\nIn both steps we use `apt-get update` to update our repository sources.\nThis is unecessary though: all platforms that use Aptitude would have\nalready executed this command in the distro-specific step anyway.\n\nDrop the redundant calls.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/install-dependencies.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 0d3aa496fc..645d035250 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -120,21 +120,17 @@ esac\n \n case \"$jobname\" in\n ClangFormat)\n-\tsudo apt-get -q update\n \tsudo apt-get -q -y install clang-format\n \t;;\n StaticAnalysis)\n-\tsudo apt-get -q update\n \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n \t\tlibexpat-dev gettext make\n \t;;\n sparse)\n-\tsudo apt-get -q update -q\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n \t\tlibexpat-dev gettext zlib1g-dev sparse\n \t;;\n Documentation)\n-\tsudo apt-get -q update\n \tsudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make\n \n \ttest -n \"$ALREADY_HAVE_ASCIIDOCTOR\" ||\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528099","messageId":"20251007-b4-pks-ci-rust-v1-2-394502abe7ea@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH 2/6] ci: check formatting of our Rust code","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:30Z","receivedAt":"2025-10-07T12:36:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a CI check that verifies that our Rust code is well-formatted.\nThis check uses rustfmt(1), which is the de-facto standard in the Rust\nworld.\n\nThe rustfmt(1) tool allows to tweak the final format in theory. In\npractice though, the Rust ecosystem has aligned on style \"editions\".\nThese editions only exist to ensure that any potential changes to the\nstyle don't cause reformats to existing code bases. Other than that,\nmost Rust projects out there accept this default style of a specific\nedition.\n\nLet's do the same and use that default style. It may not be anyone's\nfavorite, but it is consistent and by making it part of our CI we also\nenforce it right from the start.\n\nNote that we don't have to pick a specific style edition here, as the\nedition is automatically derived from the edition we have specified in\nour \"Cargo.toml\" file.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .github/workflows/main.yml | 15 +++++++++++++++\n .gitlab-ci.yml             | 11 +++++++++++\n ci/install-dependencies.sh |  5 +++++\n ci/run-rust-checks.sh      | 12 ++++++++++++\n 4 files changed, 43 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 393ea4d1cc..9e36b5c5e3 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -458,6 +458,21 @@ jobs:\n     - run: ci/install-dependencies.sh\n     - run: ci/run-static-analysis.sh\n     - run: ci/check-directional-formatting.bash\n+  rust-analysis:\n+    needs: ci-config\n+    if: needs.ci-config.outputs.enabled == 'yes'\n+    env:\n+      jobname: RustAnalysis\n+      CI_JOB_IMAGE: ubuntu:rolling\n+    runs-on: ubuntu-latest\n+    container: ubuntu:rolling\n+    concurrency:\n+      group: rust-analysis-${{ github.ref }}\n+      cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}\n+    steps:\n+    - uses: actions/checkout@v4\n+    - run: ci/install-dependencies.sh\n+    - run: ci/run-rust-checks.sh\n   sparse:\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex f7d57d1ee9..a47d839e39 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -212,6 +212,17 @@ static-analysis:\n     - ./ci/run-static-analysis.sh\n     - ./ci/check-directional-formatting.bash\n \n+rust-analysis:\n+  image: ubuntu:rolling\n+  stage: analyze\n+  needs: [ ]\n+  variables:\n+    jobname: RustAnalysis\n+  before_script:\n+    - ./ci/install-dependencies.sh\n+  script:\n+    - ./ci/run-rust-checks.sh\n+\n check-whitespace:\n   image: ubuntu:latest\n   stage: analyze\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 645d035250..a24b07edff 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -126,6 +126,11 @@ StaticAnalysis)\n \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n \t\tlibexpat-dev gettext make\n \t;;\n+RustAnalysis)\n+\tsudo apt-get -q -y install rustup\n+\trustup default stable\n+\trustup component add rustfmt\n+\t;;\n sparse)\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n \t\tlibexpat-dev gettext zlib1g-dev sparse\ndiff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\nnew file mode 100755\nindex 0000000000..082eb52f11\n--- /dev/null\n+++ b/ci/run-rust-checks.sh\n@@ -0,0 +1,12 @@\n+#!/bin/sh\n+\n+. ${0%/*}/lib.sh\n+\n+set +x\n+\n+if ! group \"Check Rust formatting\" cargo fmt --all --check\n+then\n+\tRET=1\n+fi\n+\n+exit $RET\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528100","messageId":"20251007-b4-pks-ci-rust-v1-3-394502abe7ea@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH 3/6] rust/varint: add safety comments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:31Z","receivedAt":"2025-10-07T12:36:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `decode_varint()` and `encode_varint()` functions in our Rust crate\nare reimplementations of the respective C functions. As such, we are\nnaturally forced to use the same interface in both Rust and C, which\nmakes use of raw pointers. The consequence is that the code needs to be\nmarked as unsafe in Rust.\n\nIt is common practice in Rust to provide safety documentation for every\nblock that is marked as unsafe. This common practice is also enforced by\nClippy, Rust's static analyser. We don't have Clippy wired up yet, and\nwe could of course just disable this check. But we're about to wire it\nup, and it is reasonable to always enforce documentation for unsafe\nblocks.\n\nAdd such safety comments to already squelch those warnings now.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n src/varint.rs | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/src/varint.rs b/src/varint.rs\nindex 6e610bdd8e..43b48debb5 100644\n--- a/src/varint.rs\n+++ b/src/varint.rs\n@@ -1,3 +1,6 @@\n+/// # Safety\n+///\n+/// Callers must provide a NUL-terminated array to ensure safety.\n #[no_mangle]\n pub unsafe extern \"C\" fn decode_varint(bufp: *mut *const u8) -> u64 {\n     let mut buf = *bufp;\n@@ -22,6 +25,11 @@ pub unsafe extern \"C\" fn decode_varint(bufp: *mut *const u8) -> u64 {\n     val\n }\n \n+/// # Safety\n+///\n+/// The provided buffer must be large enough to store the encoded varint. Callers may either provide\n+/// a `[u8; 16]` here, which is guaranteed to satisfy all encodable numbers. Or they can call this\n+/// function with a `NULL` pointer first to figure out array size.\n #[no_mangle]\n pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n     let mut varint: [u8; 16] = [0; 16];\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528101","messageId":"20251007-b4-pks-ci-rust-v1-4-394502abe7ea@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH 4/6] ci: check for common Rust mistakes via Clippy","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:32Z","receivedAt":"2025-10-07T12:36:52Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a CI check that uses Clippy to perform checks for common\nmistakes and suggested code improvements. Clippy is the official static\nanalyser of the Rust project and thus the de-facto standard.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/install-dependencies.sh | 2 +-\n ci/run-rust-checks.sh      | 5 +++++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex a24b07edff..dcd22ddd95 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -129,7 +129,7 @@ StaticAnalysis)\n RustAnalysis)\n \tsudo apt-get -q -y install rustup\n \trustup default stable\n-\trustup component add rustfmt\n+\trustup component add clippy rustfmt\n \t;;\n sparse)\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\ndiff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\nindex 082eb52f11..fb5ea8991b 100755\n--- a/ci/run-rust-checks.sh\n+++ b/ci/run-rust-checks.sh\n@@ -9,4 +9,9 @@ then\n \tRET=1\n fi\n \n+if ! group \"Check for common Rust mistakes\" cargo clippy --all-targets --all-features -- -Dwarnings\n+then\n+\tRET=1\n+fi\n+\n exit $RET\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528102","messageId":"20251007-b4-pks-ci-rust-v1-5-394502abe7ea@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH 5/6] ci: verify minimum supported Rust version","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:33Z","receivedAt":"2025-10-07T12:36:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In the current state of our Rust code base we don't really have any\nrequirements for the minimum supported Rust version yet, as we don't use\nany features introduced by a recent version of Rust. Consequently, we\nhave decided that we want to aim for a rather old version and edition of\nRust, where the hope is that using an old version will make alternatives\nlike gccrs viable earlier for compiling Git.\n\nBut while we specify the Rust edition, we don't yet specify a Rust\nversion. And even if we did, the Rust version would only be enforced for\nour own code, but not for any of our dependencies.\n\nWe don't yet have any dependencies at the current point in time. But\nlet's add some safeguards by specifying the minimum supported Rust\nversion and using cargo-msrv(1) to verify that this version can be\nsatisfied for all of our dependencies.\n\nNote that we fix the version of cargo-msrv(1) at v0.18.1. This is the\nlatest release supported by Ubuntu's Rust version.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Cargo.toml                 | 1 +\n ci/install-dependencies.sh | 8 ++++++++\n ci/run-rust-checks.sh      | 5 +++++\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Cargo.toml b/Cargo.toml\nindex 45c9b34981..2f51bf5d5f 100644\n--- a/Cargo.toml\n+++ b/Cargo.toml\n@@ -2,6 +2,7 @@\n name = \"gitcore\"\n version = \"0.1.0\"\n edition = \"2018\"\n+rust-version = \"1.49.0\"\n \n [lib]\n crate-type = [\"staticlib\"]\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dcd22ddd95..29e558bb9c 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -10,6 +10,8 @@ begin_group \"Install dependencies\"\n P4WHENCE=https://cdist2.perforce.com/perforce/r23.2\n LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION\n 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\n+CARGO_MSRV_VERSION=0.18.4\n+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\n \n # Make sudo a no-op and execute the command directly when running as root.\n # While using sudo would be fine on most platforms when we are root already,\n@@ -130,6 +132,12 @@ RustAnalysis)\n \tsudo apt-get -q -y install rustup\n \trustup default stable\n \trustup component add clippy rustfmt\n+\n+\twget -q \"$CARGO_MSRV_WHENCE\" -O \"cargo-msvc.tgz\"\n+\tsudo mkdir -p \"$CUSTOM_PATH\"\n+\tsudo tar -xf \"cargo-msvc.tgz\" --strip-components=1 \\\n+\t\t--directory \"$CUSTOM_PATH\" --wildcards \"*/cargo-msrv\"\n+\tsudo chmod a+x \"$CUSTOM_PATH/cargo-msrv\"\n \t;;\n sparse)\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\ndiff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\nindex fb5ea8991b..b5ad9e8dc6 100755\n--- a/ci/run-rust-checks.sh\n+++ b/ci/run-rust-checks.sh\n@@ -14,4 +14,9 @@ then\n \tRET=1\n fi\n \n+if ! group \"Check for minimum required Rust version\" cargo msrv verify\n+then\n+\tRET=1\n+fi\n+\n exit $RET\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528103","messageId":"20251007-b4-pks-ci-rust-v1-6-394502abe7ea@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH 6/6] rust: support for Windows","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T12:36:34Z","receivedAt":"2025-10-07T12:36:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The initial patch series that introduced Rust into the core of Git only\ncared about macOS and Linux. This specifically leaves out Windows, which\nindeed fails to build right now due to two issues:\n\n  - The Rust runtime requires `GetUserProfileDirectoryW()`, but we don't\n    link against \"userenv.dll\".\n\n  - The path of the Rust library built on Windows is different than on\n    most other systems systems.\n\nFix both of these issues to support Windows.\n\nNote that this commit fixes the Meson-based job in GitHub's CI. Meson\nauto-detects the availability of Rust, and as the Windows runner has\nRust installed by default it already enabled Rust support there. But due\nto the above issues that job fails consistently.\n\nInstall Rust on GitLab CI, as well, to improve test coverage there.\n\nBased-on-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nBased-on-patch-by: Ezekiel Newren <ezekielnewren@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml     |  2 +-\n Makefile           | 14 ++++++++++++--\n meson.build        |  4 ++++\n src/cargo-meson.sh | 11 +++++++++--\n 4 files changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex a47d839e39..b419a84e2c 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -161,7 +161,7 @@ test:mingw64:\n     - saas-windows-medium-amd64\n   before_script:\n     - *windows_before_script\n-    - choco install -y git meson ninja\n+    - choco install -y git meson ninja rust-ms\n     - Import-Module $env:ChocolateyInstall\\helpers\\chocolateyProfile.psm1\n     - refreshenv\n \ndiff --git a/Makefile b/Makefile\nindex 7ea149598d..366fd173e7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -929,10 +929,17 @@ TEST_SHELL_PATH = $(SHELL_PATH)\n LIB_FILE = libgit.a\n XDIFF_LIB = xdiff/lib.a\n REFTABLE_LIB = reftable/libreftable.a\n+\n ifdef DEBUG\n-RUST_LIB = target/debug/libgitcore.a\n+RUST_TARGET_DIR = target/debug\n else\n-RUST_LIB = target/release/libgitcore.a\n+RUST_TARGET_DIR = target/release\n+endif\n+\n+ifeq ($(uname_S),Windows)\n+RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib\n+else\n+RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a\n endif\n \n # xdiff and reftable libs may in turn depend on what is in libgit.a\n@@ -1538,6 +1545,9 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)\n ifdef WITH_RUST\n BASIC_CFLAGS += -DWITH_RUST\n GITLIBS += $(RUST_LIB)\n+ifeq ($(uname_S),Windows)\n+EXTLIBS += -luserenv\n+endif\n endif\n \n ifdef SANITIZE\ndiff --git a/meson.build b/meson.build\nindex ec55d6a5fd..a9c865b2af 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1707,6 +1707,10 @@ rust_option = get_option('rust').disable_auto_if(not cargo.found())\n if rust_option.allowed()\n   subdir('src')\n   libgit_c_args += '-DWITH_RUST'\n+\n+  if host_machine.system() == 'windows'\n+    libgit_dependencies += compiler.find_library('userenv')\n+  endif\n else\n   libgit_sources += [\n     'varint.c',\ndiff --git a/src/cargo-meson.sh b/src/cargo-meson.sh\nindex 99400986d9..3998db0435 100755\n--- a/src/cargo-meson.sh\n+++ b/src/cargo-meson.sh\n@@ -26,7 +26,14 @@ then\n \texit $RET\n fi\n \n-if ! cmp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n+case \"$(cargo -vV | sed -s 's/^host: \\(.*\\)$/\\1/')\" in\n+\t*-windows-*)\n+\t\tLIBNAME=gitcore.lib;;\n+\t*)\n+\t\tLIBNAME=libgitcore.a;;\n+esac\n+\n+if ! cmp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n then\n-\tcp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\"\n+\tcp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\"\n fi\n\n-- \n2.51.0.764.g787ff6f08a.dirty\n\n"},{"id":"528112","messageId":"CAOLa=ZSatdP84CmW0TLzP00CrV02P3ahmXBCp_HhxRYQXzry6A@mail.gmail.com","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-1-394502abe7ea@pks.im","subject":"Re: [PATCH 1/6] ci: deduplicate calls to `apt-get update`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-07T12:54:00Z","receivedAt":"2025-10-07T12:54:02Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When installing dependencies we first check for the distribution that is\n> in use and then we check for the specific job. In the first step we\n> already install all dependencies required to build and test Git, whereas\n> the second step installs a couple of additional dependencies that are\n> only required to perform job-specific tasks.\n>\n> In both steps we use `apt-get update` to update our repository sources.\n> This is unecessary though: all platforms that use Aptitude would have\n> already executed this command in the distro-specific step anyway.\n>\n\nNit: s/unecessary/unnecessary\n\nThe patch looks good.\n"},{"id":"528113","messageId":"CAOLa=ZT8TDiA=1cAsnS6RkHL-5J2+3YBorBjKsKWm38oaXt0Fg@mail.gmail.com","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-2-394502abe7ea@pks.im","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-07T13:04:41Z","receivedAt":"2025-10-07T13:04:45Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Introduce a CI check that verifies that our Rust code is well-formatted.\n> This check uses rustfmt(1), which is the de-facto standard in the Rust\n> world.\n>\n> The rustfmt(1) tool allows to tweak the final format in theory. In\n> practice though, the Rust ecosystem has aligned on style \"editions\".\n> These editions only exist to ensure that any potential changes to the\n> style don't cause reformats to existing code bases. Other than that,\n> most Rust projects out there accept this default style of a specific\n> edition.\n>\n> Let's do the same and use that default style. It may not be anyone's\n> favorite, but it is consistent and by making it part of our CI we also\n> enforce it right from the start.\n>\n> Note that we don't have to pick a specific style edition here, as the\n> edition is automatically derived from the edition we have specified in\n> our \"Cargo.toml\" file.\n\nOne small nit: We should mention that `cargo fmt` is simply a wrapper\naround `rustfmt`, which also handles file discovery.\n"},{"id":"528118","messageId":"aOUavBJ6kipuYcr5@pks.im","threadId":"64262","inReplyTo":"CAOLa=ZT8TDiA=1cAsnS6RkHL-5J2+3YBorBjKsKWm38oaXt0Fg@mail.gmail.com","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-07T13:50:52Z","receivedAt":"2025-10-07T13:50:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Oct 07, 2025 at 06:04:41AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > Introduce a CI check that verifies that our Rust code is well-formatted.\n> > This check uses rustfmt(1), which is the de-facto standard in the Rust\n> > world.\n> >\n> > The rustfmt(1) tool allows to tweak the final format in theory. In\n> > practice though, the Rust ecosystem has aligned on style \"editions\".\n> > These editions only exist to ensure that any potential changes to the\n> > style don't cause reformats to existing code bases. Other than that,\n> > most Rust projects out there accept this default style of a specific\n> > edition.\n> >\n> > Let's do the same and use that default style. It may not be anyone's\n> > favorite, but it is consistent and by making it part of our CI we also\n> > enforce it right from the start.\n> >\n> > Note that we don't have to pick a specific style edition here, as the\n> > edition is automatically derived from the edition we have specified in\n> > our \"Cargo.toml\" file.\n> \n> One small nit: We should mention that `cargo fmt` is simply a wrapper\n> around `rustfmt`, which also handles file discovery.\n\nGood idea, I'll include this in the next version. Thanks!\n\nPatrick\n"},{"id":"528138","messageId":"CAPig+cQ7xJky+F=g=NMrN6BQfP+ZV2KF4RF2eLqtULKgMTR5_g@mail.gmail.com","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-2-394502abe7ea@pks.im","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Eric Sunshine","fromEmail":"ericsunshine@gmail.com","sentAt":"2025-10-07T17:13:18Z","receivedAt":"2025-10-07T17:13:30Z","isPatch":true,"sender":{"key":"ericsunshine@gmail.com","avatar":null},"body":"On Tue, Oct 7, 2025 at 8:37 AM Patrick Steinhardt <ps@pks.im> wrote:\n> Introduce a CI check that verifies that our Rust code is well-formatted.\n> This check uses rustfmt(1), which is the de-facto standard in the Rust\n> world.\n>\n> The rustfmt(1) tool allows to tweak the final format in theory. In\n> practice though, the Rust ecosystem has aligned on style \"editions\".\n> These editions only exist to ensure that any potential changes to the\n> style don't cause reformats to existing code bases. Other than that,\n> most Rust projects out there accept this default style of a specific\n> edition.\n>\n> Let's do the same and use that default style. It may not be anyone's\n> favorite, but it is consistent and by making it part of our CI we also\n> enforce it right from the start.\n\nIn a different thread, I wrote[1]:\n\n    There are more than a few developers on this project (including\n    myself) who still use 80-column editors and terminals. As a\n    general style guideline, this project does recommend wrapping code\n    to fit within 80 columns (except in cases when doing so would\n    severely hurt readability). I imagine that the same sort of\n    guideline would be appreciated in Rust code, as well, by those who\n    still stick with 80 columns.\n\n    I bring this up because, although it hasn't been such a big deal\n    with the existing C code, assuming that developers run `rustfmt`\n    on the code before sending a patch series, then this may become an\n    issue if different developers have `rustfmt` configured to enforce\n    different maximum column width, especially since `rustfmt` is\n    likely to reformat the entire file rather than just the region\n    that has just been edited.  So, if this code gets checked in as-is\n    with these very wide lines, and then someone else, who has\n    `rustfmt` configured for 80-columns edits the file, then it\n    becomes a problem.\n\n    As such, can we also add a project-wide `rustfmt.toml` which, at\n    minimum, sets the maximum line width to 80? For instance:\n\n        max_width = 80\n\nLater in the same thread, I wrote[2]:\n\n    Project guidelines have long suggested 80 columns as a desirable\n    maximum not only for C code, but for pretty much all other\n    resources, including shell code, Perl code, and documentation\n    files. This suggested maximum works well for adherents of\n    80-columns and (presumably) hasn't been too onerous for developers\n    who use wider windows; at least we haven't heard people clamoring\n    to increase the suggested maximum column limit. As such, it does\n    not seem far-fetched to expect that the project guidelines\n    should/could/would also apply to Rust code.\n\nUnfortunately, what little discussion there was petered out quickly\nwithout resolution, but it seems that it would be a good idea to make\nsome sort of decision earlier (while there is still very little Rust\ncode committed to the project) rather than later.\n\n[1]: https://lore.kernel.org/git/CAPig+cTZch_pvfurtjBTNphMeRQL6jSBSjNY-4mffjoXZ4eqcw@mail.gmail.com/\n[2]: https://lore.kernel.org/git/CAPig+cTdJAjuekz6YXDkxTjTRxsPEzSUxhoD8nK9k7uA4s=rHQ@mail.gmail.com/\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n"},{"id":"528145","messageId":"xmqqbjmik3y9.fsf@gitster.g","threadId":"64262","inReplyTo":"CAPig+cQ7xJky+F=g=NMrN6BQfP+ZV2KF4RF2eLqtULKgMTR5_g@mail.gmail.com","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-07T17:38:06Z","receivedAt":"2025-10-07T17:38:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <ericsunshine@gmail.com> writes:\n\n>     I bring this up because, although it hasn't been such a big deal\n>     with the existing C code, assuming that developers run `rustfmt`\n>     on the code before sending a patch series, then this may become an\n>     issue if different developers have `rustfmt` configured to enforce\n>     different maximum column width, especially since `rustfmt` is\n>     likely to reformat the entire file rather than just the region\n>     that has just been edited.  So, if this code gets checked in as-is\n>     with these very wide lines, and then someone else, who has\n>     `rustfmt` configured for 80-columns edits the file, then it\n>     becomes a problem.\n>\n>     As such, can we also add a project-wide `rustfmt.toml` which, at\n>     minimum, sets the maximum line width to 80? For instance:\n>\n>         max_width = 80\n>\n> Later in the same thread, I wrote[2]:\n>\n>     Project guidelines have long suggested 80 columns as a desirable\n>     maximum not only for C code, but for pretty much all other\n>     resources, including shell code, Perl code, and documentation\n>     files. This suggested maximum works well for adherents of\n>     80-columns and (presumably) hasn't been too onerous for developers\n>     who use wider windows; at least we haven't heard people clamoring\n>     to increase the suggested maximum column limit. As such, it does\n>     not seem far-fetched to expect that the project guidelines\n>     should/could/would also apply to Rust code.\n>\n> Unfortunately, what little discussion there was petered out quickly\n> without resolution, but it seems that it would be a good idea to make\n> some sort of decision earlier (while there is still very little Rust\n> code committed to the project) rather than later.\n\nI do not see a particular reason to lift the 80-column limit for a\nspecific language, whether it is Rust or AsciiDoc.  I myself use my\nterminal set to slightly wider than 80 columns these days, but that\nis primarily to accomodate the fact that code in a patch that are\nquoted a few times in the discussion would grow from their original\nline length, not to write pieces of code that are wider than 80\ncolumns myself.\n\nWill it inconvenience wider Rust ecosystem when we get big (meaning,\nthey have to work with our code) and we as the project norm use\ndifferent line-length setting from others, perhaps by looking too\ndifferent from everybody else, or something?\n\n"},{"id":"528147","messageId":"CAPig+cRvugLP63CYUXw7pf-7obErQYenrVvNeSYhegQ57PQ8KA@mail.gmail.com","threadId":"64262","inReplyTo":"xmqqbjmik3y9.fsf@gitster.g","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Eric Sunshine","fromEmail":"ericsunshine@gmail.com","sentAt":"2025-10-07T18:03:21Z","receivedAt":"2025-10-07T18:03:35Z","isPatch":true,"sender":{"key":"ericsunshine@gmail.com","avatar":null},"body":"On Tue, Oct 7, 2025 at 1:38 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <ericsunshine@gmail.com> writes:\n> > Later in the same thread, I wrote[2]:\n> >\n> >     Project guidelines have long suggested 80 columns as a desirable\n> >     maximum not only for C code, but for pretty much all other\n> >     resources, including shell code, Perl code, and documentation\n> >     files. This suggested maximum works well for adherents of\n> >     80-columns and (presumably) hasn't been too onerous for developers\n> >     who use wider windows; at least we haven't heard people clamoring\n> >     to increase the suggested maximum column limit. As such, it does\n> >     not seem far-fetched to expect that the project guidelines\n> >     should/could/would also apply to Rust code.\n>\n> I do not see a particular reason to lift the 80-column limit for a\n> specific language, whether it is Rust or AsciiDoc. [...]\n>\n> Will it inconvenience wider Rust ecosystem when we get big (meaning,\n> they have to work with our code) and we as the project norm use\n> different line-length setting from others, perhaps by looking too\n> different from everybody else, or something?\n\nAs a general answer, I would assume that third-party projects wanting\nto use Rust code from the Git project would do so by importing one or\nmore \"crates\" that the Git project publishes rather than importing raw\ncode directly from the Git project. In this case they never deal\ndirectly with Git's Rust code itself, but instead interact via the Git\ncrate's public API.\n\nIf a third-party project does want/need to import some raw Git Rust\ncode directly but has no plans to actually edit the code, then there\nshould be no problem. If the project does plan to edit the imported\ncode and periodically update it from upstream Git, then it's a bit\nmore onerous, though perhaps not so much so; running the Git upstream\ncode through `rustfmt` before import into the project is one simple\nstep which can easily be automated.\n"},{"id":"528175","messageId":"aOWPBZg5MXzGcNmU@fruit.crustytoothpaste.net","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-2-394502abe7ea@pks.im","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-10-07T22:07:01Z","receivedAt":"2025-10-07T22:07:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-10-07 at 12:36:30, Patrick Steinhardt wrote:\n> Introduce a CI check that verifies that our Rust code is well-formatted.\n> This check uses rustfmt(1), which is the de-facto standard in the Rust\n> world.\n> \n> The rustfmt(1) tool allows to tweak the final format in theory. In\n> practice though, the Rust ecosystem has aligned on style \"editions\".\n> These editions only exist to ensure that any potential changes to the\n> style don't cause reformats to existing code bases. Other than that,\n> most Rust projects out there accept this default style of a specific\n> edition.\n> \n> Let's do the same and use that default style. It may not be anyone's\n> favorite, but it is consistent and by making it part of our CI we also\n> enforce it right from the start.\n\nYes, I think this is the right decision.  We _can_ customize it if we\nwant, but I never do, even though I don't love some of the policies\n(like the cuddled elses), since having a fixed standard avoids all of\nthe argument.  The Rust code I'll be sending in within the next few\nweeks already uses rustfmt.\n\nEven if we did decide to customize it, I think there's enormous value in\njust being able to run the tool and accept what it outputs, saving us\nenormous amounts of time not having to discuss style nits.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"528178","messageId":"aOWXSO5GInJI8-NZ@fruit.crustytoothpaste.net","threadId":"64262","inReplyTo":"CAPig+cQ7xJky+F=g=NMrN6BQfP+ZV2KF4RF2eLqtULKgMTR5_g@mail.gmail.com","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-10-07T22:42:16Z","receivedAt":"2025-10-07T22:42:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-10-07 at 17:13:18, Eric Sunshine wrote:\n> Later in the same thread, I wrote[2]:\n> \n>     Project guidelines have long suggested 80 columns as a desirable\n>     maximum not only for C code, but for pretty much all other\n>     resources, including shell code, Perl code, and documentation\n>     files. This suggested maximum works well for adherents of\n>     80-columns and (presumably) hasn't been too onerous for developers\n>     who use wider windows; at least we haven't heard people clamoring\n>     to increase the suggested maximum column limit. As such, it does\n>     not seem far-fetched to expect that the project guidelines\n>     should/could/would also apply to Rust code.\n\nMy preference is actually that we stick with the default.  I use (and\nfor a long time have used) a 132-character editor window and I find it\nquite useful to have the extra space.  The DEC VT100 did 132 columns\n(available on your local Linux system as `vt100-w`), so I think there's\nplenty of precedent for that being an acceptable width[0].\n\nI did previously use 80-column terminals when I had a tiny laptop\nscreen, but modern display resolutions over the past decade, even on\nsmaller laptops, have made it entirely possible to get several wider\nterminal windows (or in my case, tmux panes) on one screen.  One of my\ncurrent tmux panes is now 213×54 and I really enjoy the extra space.\n\nThe default Rust behaviour is 100 characters[1], which I think is a fine\ndefault.  I won't be enormously angsty if we say we still absolutely\nmust stick to 80-character lines, but I also think we should take this\nopportunity to choose the Rust defaults for Rust.  C, Perl, and text\nformats like AsciiDoc do not have rigid defaults about indentation\nstyle, tabs vs. spaces, and line length; Rust does.  We wouldn't use\ntabs in Rust (the default is four spaces) because we use it everywhere\nelse, so I think we should take the opportunity to use the Rust defaults\nhere as well.\n\nWhatever we ultimately decide, I plan to send an update to our\n`.editorconfig` file.  I think that's less useful in general for Rust,\nwhere we have an automatic tidy tool and CI to check for it, but there\nare some people for whom it will be useful and we might as well keep it\ncorrect.\n\n[0] For what it's worth, Linus also thinks longer lines and 132-column\nterminals are useful.  I don't agree with him about everything,\ncertainly, but I think we see eye to eye here:\nhttps://lkml.org/lkml/2020/5/29/1038.\n[1] % rustfmt --print-config=default /dev/stdout | grep '^max_width'\nmax_width = 100\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"528181","messageId":"CAPx1GveBE5mi7R3kOwYM2ER7rmVyS3Hwbe4o-m7UzdtFutouZw@mail.gmail.com","threadId":"64262","inReplyTo":"aOWXSO5GInJI8-NZ@fruit.crustytoothpaste.net","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2025-10-07T22:58:06Z","receivedAt":"2025-10-07T22:58:20Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Tue, Oct 7, 2025 at 3:42 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> My preference is actually that we stick with the default.  I use (and\n> for a long time have used) a 132-character editor window and I find it\n> quite useful to have the extra space. [mass snip]\n\nSince I started doing some Go programming (which I still do far more\nthan Rust) I do the same. I actually let it go to 140 or more sometimes,\nwith vim settings to put shadow marks at 80 and 132.\n\nI still use my trusty 80x50 windows for holding a lot of individual\nwindows at a time though. :-)\n\nChris\n"},{"id":"528188","messageId":"aOWwcqyithDKQzVs@fruit.crustytoothpaste.net","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-3-394502abe7ea@pks.im","subject":"Re: [PATCH 3/6] rust/varint: add safety comments","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-10-08T00:29:38Z","receivedAt":"2025-10-08T00:29:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-10-07 at 12:36:31, Patrick Steinhardt wrote:\n> +/// # Safety\n> +///\n> +/// The provided buffer must be large enough to store the encoded varint. Callers may either provide\n> +/// a `[u8; 16]` here, which is guaranteed to satisfy all encodable numbers. Or they can call this\n> +/// function with a `NULL` pointer first to figure out array size.\n>  #[no_mangle]\n>  pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n>      let mut varint: [u8; 16] = [0; 16];\n\nI'm planning to do something a little different with this code by\nrefactoring it out into a Rust function, so at that point it will no\nlonger be possible to provide a buffer smaller than 16 bytes.  Note that\nall callers of this function pass a 16-byte buffer, so that should be\nsafe.\n\nThat doesn't mean that you can't send this patch (and I think your patch\nis good), just that we shouldn't tell people we can use a buffer smaller\nthan 16 bytes, since that will at some point no longer be true.\n\nHere's the current version of the patch I'm planning on sending for\nreference.  I can rebase onto your series once Junio picks it up.\n\n-- >% --\nFrom 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001\nFrom: \"brian m. carlson\" <sandals@crustytoothpaste.net>\nDate: Wed, 8 Oct 2025 00:27:56 +0000\nSubject: [PATCH] varint: write a safe Rust version of encode_varint\n\nOur original version of encode_varint in Rust used pointers much like\nthe C version did.  However, if we end up using this function elsewhere\nin Rust, it would be better to have a safe version that we can use.\n\nIn addition, writing our unsafe C-compatible version in terms of a safe\nRust version makes it obvious what our requirements are.  For instance,\nwe do not need buf to actually point anywhere and can accept a null\npointer if we just want the length, and we can clearly indicate that we\nrequire 16 bytes worth of memory to encode data by creating an\nappropriate slice.  All of our existing callers always pass a 16-byte\nbuffer, so we can safely assume that.\n\nWe can then improve our Rust version by performing normal bounds\nchecking to make sure that we don't exceed the buffer size and use the\nstandard usize return for lengths, converting as necessary in the\nC-compatible caller.\n\nMove the C-compatible code to a mod c to keep things tidy and allow us\nto have a different Rust version.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n src/varint.rs | 34 ++++++++++++++++++++++++++++------\n 1 file changed, 28 insertions(+), 6 deletions(-)\n\ndiff --git a/src/varint.rs b/src/varint.rs\nindex 6e610bdd8e..83990afe7a 100644\n--- a/src/varint.rs\n+++ b/src/varint.rs\n@@ -22,8 +22,7 @@ pub unsafe extern \"C\" fn decode_varint(bufp: *mut *const u8) -> u64 {\n     val\n }\n \n-#[no_mangle]\n-pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n+pub fn encode_varint(value: u64, buf: Option<&mut [u8]>) -> usize {\n     let mut varint: [u8; 16] = [0; 16];\n     let mut pos = varint.len() - 1;\n \n@@ -37,16 +36,19 @@ pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n         value >>= 7;\n     }\n \n-    if !buf.is_null() {\n-        std::ptr::copy_nonoverlapping(varint.as_ptr().add(pos), buf, varint.len() - pos);\n+    let len = varint.len() - pos;\n+\n+    if let Some(buf) = buf {\n+        buf[0..len].copy_from_slice(&varint[pos..pos + len]);\n     }\n \n-    (varint.len() - pos) as u8\n+    len\n }\n \n #[cfg(test)]\n mod tests {\n-    use super::*;\n+    use super::c::encode_varint;\n+    use super::decode_varint;\n \n     #[test]\n     fn test_decode_varint() {\n@@ -90,3 +92,23 @@ mod tests {\n         }\n     }\n }\n+\n+mod c {\n+    /// Encode `value` into `buf` as a variable-length integer unless `buf` is null.\n+    ///\n+    /// Returns the number of bytes written, or, if `buf` is null, the number of bytes that would be\n+    /// used to encode the integer.\n+    ///\n+    /// # Safety\n+    ///\n+    /// `buf` must either be null or point to at least 16 bytes of memory.\n+    #[no_mangle]\n+    pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n+        let buffer = if buf.is_null() {\n+            None\n+        } else {\n+            Some(std::slice::from_raw_parts_mut(buf, 16))\n+        };\n+        super::encode_varint(value, buffer) as u8\n+    }\n+}\n-- >% --\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"528200","messageId":"aOXsjnWBOt0qFGwc@pks.im","threadId":"64262","inReplyTo":"aOWXSO5GInJI8-NZ@fruit.crustytoothpaste.net","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-08T04:46:06Z","receivedAt":"2025-10-08T04:46:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Oct 07, 2025 at 10:42:16PM +0000, brian m. carlson wrote:\n> On 2025-10-07 at 17:13:18, Eric Sunshine wrote:\n> > Later in the same thread, I wrote[2]:\n> > \n> >     Project guidelines have long suggested 80 columns as a desirable\n> >     maximum not only for C code, but for pretty much all other\n> >     resources, including shell code, Perl code, and documentation\n> >     files. This suggested maximum works well for adherents of\n> >     80-columns and (presumably) hasn't been too onerous for developers\n> >     who use wider windows; at least we haven't heard people clamoring\n> >     to increase the suggested maximum column limit. As such, it does\n> >     not seem far-fetched to expect that the project guidelines\n> >     should/could/would also apply to Rust code.\n> \n> My preference is actually that we stick with the default.  I use (and\n> for a long time have used) a 132-character editor window and I find it\n> quite useful to have the extra space.  The DEC VT100 did 132 columns\n> (available on your local Linux system as `vt100-w`), so I think there's\n> plenty of precedent for that being an acceptable width[0].\n> \n> I did previously use 80-column terminals when I had a tiny laptop\n> screen, but modern display resolutions over the past decade, even on\n> smaller laptops, have made it entirely possible to get several wider\n> terminal windows (or in my case, tmux panes) on one screen.  One of my\n> current tmux panes is now 213×54 and I really enjoy the extra space.\n> \n> The default Rust behaviour is 100 characters[1], which I think is a fine\n> default.  I won't be enormously angsty if we say we still absolutely\n> must stick to 80-character lines, but I also think we should take this\n> opportunity to choose the Rust defaults for Rust.  C, Perl, and text\n> formats like AsciiDoc do not have rigid defaults about indentation\n> style, tabs vs. spaces, and line length; Rust does.  We wouldn't use\n> tabs in Rust (the default is four spaces) because we use it everywhere\n> else, so I think we should take the opportunity to use the Rust defaults\n> here as well.\n\nI am also slightly leaning into the direction of sticking with Rust's\ndefault of 100 characters. It's not substantially more than 80, should\nbe reasonable to accommodate for in most modern setups, and sticks with\nwhat the remainder of the ecosystem is doing.\n\nSo for now I'll leave it at 80 characters. But I don't feel strongly\nabout this, so if there is a majority in favor of 80 characters I'm\nhappy to adjust.\n\nThanks!\n\nPatrick\n"},{"id":"528201","messageId":"aOXsmIu1BEWzxlVE@pks.im","threadId":"64262","inReplyTo":"aOWwcqyithDKQzVs@fruit.crustytoothpaste.net","subject":"Re: [PATCH 3/6] rust/varint: add safety comments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-08T04:46:16Z","receivedAt":"2025-10-08T04:46:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 08, 2025 at 12:29:38AM +0000, brian m. carlson wrote:\n> On 2025-10-07 at 12:36:31, Patrick Steinhardt wrote:\n> > +/// # Safety\n> > +///\n> > +/// The provided buffer must be large enough to store the encoded varint. Callers may either provide\n> > +/// a `[u8; 16]` here, which is guaranteed to satisfy all encodable numbers. Or they can call this\n> > +/// function with a `NULL` pointer first to figure out array size.\n> >  #[no_mangle]\n> >  pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n> >      let mut varint: [u8; 16] = [0; 16];\n> \n> I'm planning to do something a little different with this code by\n> refactoring it out into a Rust function, so at that point it will no\n> longer be possible to provide a buffer smaller than 16 bytes.  Note that\n> all callers of this function pass a 16-byte buffer, so that should be\n> safe.\n\nAh, true. I just double-checked, and all callers pass in a 16 byte\nbuffer indeed. Also means that the NULL-pointer handling can go away in\ntheory, as we don't use it.\n\nIn any case, your direction makes sense once we have Rust-internal\ncallers of this functionality. We definitely don't want to propagate the\nunsafety to callers and should make sure that it is contained to the C\nAPI.\n\n> That doesn't mean that you can't send this patch (and I think your patch\n> is good), just that we shouldn't tell people we can use a buffer smaller\n> than 16 bytes, since that will at some point no longer be true.\n> \n> Here's the current version of the patch I'm planning on sending for\n> reference.  I can rebase onto your series once Junio picks it up.\n\nThe patch makes sense to me, thanks. I'll not pick it up yet though as\nthere is no justifiable need as part of my series, but I'm happy to\nadjust the comment.\n\nThanks!\n\nPatrick\n"},{"id":"528256","messageId":"xmqqms61h0g1.fsf@gitster.g","threadId":"64262","inReplyTo":"aOXsjnWBOt0qFGwc@pks.im","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-08T15:34:22Z","receivedAt":"2025-10-08T15:34:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> ... but I also think we should take this\n>> opportunity to choose the Rust defaults for Rust.  C, Perl, and text\n>> formats like AsciiDoc do not have rigid defaults about indentation\n>> style, tabs vs. spaces, and line length; Rust does.  We wouldn't use\n>> tabs in Rust (the default is four spaces) because we use it everywhere\n>> else, so I think we should take the opportunity to use the Rust defaults\n>> here as well.\n>\n> I am also slightly leaning into the direction of sticking with Rust's\n> default of 100 characters. It's not substantially more than 80, should\n> be reasonable to accommodate for in most modern setups, and sticks with\n> what the remainder of the ecosystem is doing.\n>\n> So for now I'll leave it at 80 characters. But I don't feel strongly\n> about this, so if there is a majority in favor of 80 characters I'm\n> happy to adjust.\n\nSo the question is if we want consistency across files regardless of\nwhat language they are written in (i.e. 80-columns everywhere) or we\ntreat our existing rules a \"fallback rules\" we have adopted while\ndealing with languages without their own strict rules, and use the\ndefault for a language with its own rule (i.e. whatever rustfmt\nwants is used for Rust, our own rules still apply to everything\nelse)?\n\nI actually am fine with the latter myself.\n\nIf people strongly prefer, I also can be talked into adopting\nslightly wider limit for our fallback rules for everything else, but\nthat is probably a separate discussion.  It is a bit unfriendly move\nagainst folks with aging eyeballs like myself, though.\n\nThanks.\n\n"},{"id":"528314","messageId":"aObPzzLtZzodZf+Q@szeder.dev","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-2-394502abe7ea@pks.im","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2025-10-08T20:55:43Z","receivedAt":"2025-10-08T20:55:47Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Oct 07, 2025 at 02:36:30PM +0200, Patrick Steinhardt wrote:\n> Introduce a CI check that verifies that our Rust code is well-formatted.\n> This check uses rustfmt(1), which is the de-facto standard in the Rust\n> world.\n> \n> The rustfmt(1) tool allows to tweak the final format in theory. In\n> practice though, the Rust ecosystem has aligned on style \"editions\".\n> These editions only exist to ensure that any potential changes to the\n> style don't cause reformats to existing code bases. Other than that,\n> most Rust projects out there accept this default style of a specific\n> edition.\n> \n> Let's do the same and use that default style. It may not be anyone's\n> favorite, but it is consistent and by making it part of our CI we also\n> enforce it right from the start.\n> \n> Note that we don't have to pick a specific style edition here, as the\n> edition is automatically derived from the edition we have specified in\n> our \"Cargo.toml\" file.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n\n> diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\n> new file mode 100755\n> index 0000000000..082eb52f11\n> --- /dev/null\n> +++ b/ci/run-rust-checks.sh\n> @@ -0,0 +1,12 @@\n> +#!/bin/sh\n> +\n> +. ${0%/*}/lib.sh\n> +\n> +set +x\n> +\n> +if ! group \"Check Rust formatting\" cargo fmt --all --check\n> +then\n> +\tRET=1\n> +fi\n> +\n> +exit $RET\n\nOur ci/*.sh scripts usually rely on 'set -e' to catch failed commands.\nEither this script should follow that convention as well, or the\ncommit message should justify the deviation from convention.\n\n"},{"id":"528351","messageId":"aOdIPjE2iTpN6L7q@pks.im","threadId":"64262","inReplyTo":"xmqqms61h0g1.fsf@gitster.g","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T05:29:34Z","receivedAt":"2025-10-09T05:29:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 08, 2025 at 08:34:22AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> >> ... but I also think we should take this\n> >> opportunity to choose the Rust defaults for Rust.  C, Perl, and text\n> >> formats like AsciiDoc do not have rigid defaults about indentation\n> >> style, tabs vs. spaces, and line length; Rust does.  We wouldn't use\n> >> tabs in Rust (the default is four spaces) because we use it everywhere\n> >> else, so I think we should take the opportunity to use the Rust defaults\n> >> here as well.\n> >\n> > I am also slightly leaning into the direction of sticking with Rust's\n> > default of 100 characters. It's not substantially more than 80, should\n> > be reasonable to accommodate for in most modern setups, and sticks with\n> > what the remainder of the ecosystem is doing.\n> >\n> > So for now I'll leave it at 80 characters. But I don't feel strongly\n> > about this, so if there is a majority in favor of 80 characters I'm\n> > happy to adjust.\n> \n> So the question is if we want consistency across files regardless of\n> what language they are written in (i.e. 80-columns everywhere) or we\n> treat our existing rules a \"fallback rules\" we have adopted while\n> dealing with languages without their own strict rules, and use the\n> default for a language with its own rule (i.e. whatever rustfmt\n> wants is used for Rust, our own rules still apply to everything\n> else)?\n\nYeah, exactly.\n\n> I actually am fine with the latter myself.\n\nOkay.\n\n> If people strongly prefer, I also can be talked into adopting\n> slightly wider limit for our fallback rules for everything else, but\n> that is probably a separate discussion.  It is a bit unfriendly move\n> against folks with aging eyeballs like myself, though.\n\nYup, this feels like a separate question indeed.\n\nThanks!\n\nPatrick\n"},{"id":"528352","messageId":"aOdIRnB-SGQwj935@pks.im","threadId":"64262","inReplyTo":"aObPzzLtZzodZf+Q@szeder.dev","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-09T05:29:42Z","receivedAt":"2025-10-09T05:29:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 08, 2025 at 10:55:43PM +0200, SZEDER Gábor wrote:\n> On Tue, Oct 07, 2025 at 02:36:30PM +0200, Patrick Steinhardt wrote:\n> > Introduce a CI check that verifies that our Rust code is well-formatted.\n> > This check uses rustfmt(1), which is the de-facto standard in the Rust\n> > world.\n> > \n> > The rustfmt(1) tool allows to tweak the final format in theory. In\n> > practice though, the Rust ecosystem has aligned on style \"editions\".\n> > These editions only exist to ensure that any potential changes to the\n> > style don't cause reformats to existing code bases. Other than that,\n> > most Rust projects out there accept this default style of a specific\n> > edition.\n> > \n> > Let's do the same and use that default style. It may not be anyone's\n> > favorite, but it is consistent and by making it part of our CI we also\n> > enforce it right from the start.\n> > \n> > Note that we don't have to pick a specific style edition here, as the\n> > edition is automatically derived from the edition we have specified in\n> > our \"Cargo.toml\" file.\n> > \n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> \n> > diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\n> > new file mode 100755\n> > index 0000000000..082eb52f11\n> > --- /dev/null\n> > +++ b/ci/run-rust-checks.sh\n> > @@ -0,0 +1,12 @@\n> > +#!/bin/sh\n> > +\n> > +. ${0%/*}/lib.sh\n> > +\n> > +set +x\n> > +\n> > +if ! group \"Check Rust formatting\" cargo fmt --all --check\n> > +then\n> > +\tRET=1\n> > +fi\n> > +\n> > +exit $RET\n> \n> Our ci/*.sh scripts usually rely on 'set -e' to catch failed commands.\n> Either this script should follow that convention as well, or the\n> commit message should justify the deviation from convention.\n\nAh, good point. The reason is that subsequent commits add more checks,\nand I want to make sure that they all run even if previous checks\nfailed. It's otherwise annoying to fix a first set of errors surfaced by\nthe CI only to then notice that later checks also fail.\n\nI'll mention this in the commit message.\n\nPatrick\n"},{"id":"528764","messageId":"rxdwxiokqn2vak4sm7yxzisolbugzr26ygcq4mue3fu5lmmfra@r2pj355wu5mf","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-1-394502abe7ea@pks.im","subject":"Re: [PATCH 1/6] ci: deduplicate calls to `apt-get update`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-14T20:56:59Z","receivedAt":"2025-10-14T20:57:00Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/07 02:36PM, Patrick Steinhardt wrote:\n> When installing dependencies we first check for the distribution that is\n> in use and then we check for the specific job. In the first step we\n> already install all dependencies required to build and test Git, whereas\n> the second step installs a couple of additional dependencies that are\n> only required to perform job-specific tasks.\n> \n> In both steps we use `apt-get update` to update our repository sources.\n> This is unecessary though: all platforms that use Aptitude would have\n> already executed this command in the distro-specific step anyway.\n\nThe distro-specific setup always executes first and does make these call\nredundant. Make sense.\n\nNot related to this change, but at a glance it looks like this job\nspecific setup relies on using an Aptitude based distro. This does seem\nslightly fragile if a job were to be configured with an unsupported\ndistro. Not anything we need to change here though.\n\n-Justin\n"},{"id":"528781","messageId":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im","subject":"[PATCH v3 0/6] ci: improvements to our Rust infrastructure","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:04Z","receivedAt":"2025-10-15T06:04:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series introduces some improvements for our Rust\ninfrastructure. Most importantly, it introduces a couple of static\nanalysis checks to verify consistent formatting, use Clippy for linting\nand to verify our minimum supported Rust version.\n\nFurthermore, this series also introduces support for building with Rust\nenabled on Windows.\n\nThe series is built on top of 45547b60ac (Merge branch 'master' of\nhttps://github.com/j6t/gitk, 2025-10-05) with ps/rust-balloon at\ne425c40aa0 (ci: enable Rust for breaking-changes jobs, 2025-10-02) and\nps/gitlab-ci-windows-improvements at 3c4925c3f5 (t8020: fix test failure\ndue to indeterministic tag sorting, 2025-10-02) merged into it.\n\nChanges in v3:\n  - Clarify why scripts don't use `set -e` exclusively for error\n    handling.\n  - Link to v2: https://lore.kernel.org/r/20251008-b4-pks-ci-rust-v2-0-d556ee83c381@pks.im\n\nChanges in v2:\n  - Adjust comments for `encode_varint()` and `decode_varint()` based on\n    brian's feedback.\n  - Some small improvements to commit messages.\n  - Not changed is the default column limit used by Rust. I think using\n    the column limit of 100 used by the Rust ecosystem is sensible, but\n    if there is a majority advocating for a limit of 80 I'll adapt this.\n  - Link to v1: https://lore.kernel.org/r/20251007-b4-pks-ci-rust-v1-0-394502abe7ea@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (6):\n      ci: deduplicate calls to `apt-get update`\n      ci: check formatting of our Rust code\n      rust/varint: add safety comments\n      ci: check for common Rust mistakes via Clippy\n      ci: verify minimum supported Rust version\n      rust: support for Windows\n\n .github/workflows/main.yml | 15 +++++++++++++++\n .gitlab-ci.yml             | 13 ++++++++++++-\n Cargo.toml                 |  1 +\n Makefile                   | 14 ++++++++++++--\n ci/install-dependencies.sh | 17 +++++++++++++----\n ci/run-rust-checks.sh      | 22 ++++++++++++++++++++++\n meson.build                |  4 ++++\n src/cargo-meson.sh         | 11 +++++++++--\n src/varint.rs              | 15 +++++++++++++++\n 9 files changed, 103 insertions(+), 9 deletions(-)\n\nRange-diff versus v2:\n\n1:  dc9d75f47c = 1:  cac74c6387 ci: deduplicate calls to `apt-get update`\n2:  8537190491 ! 2:  6164bbd971 ci: check formatting of our Rust code\n    @@ Commit message\n         edition is automatically derived from the edition we have specified in\n         our \"Cargo.toml\" file.\n     \n    +    The implemented script looks somewhat weird as we perfom manual error\n    +    handling instead of using something like `set -e`. The intent here is\n    +    that subsequent commits will add more checks, and we want to execute all\n    +    of these checks regardless of whether or not a previous check failed.\n    +\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n      ## .github/workflows/main.yml ##\n3:  8f7232e650 = 3:  1c87940646 rust/varint: add safety comments\n4:  09810edff2 = 4:  0b09774307 ci: check for common Rust mistakes via Clippy\n5:  bdb4e9df32 = 5:  93d6111ae7 ci: verify minimum supported Rust version\n6:  40edae19a8 = 6:  3f58a9b9df rust: support for Windows\n\n---\nbase-commit: 8c8e270f2aba359479c4c2b4ab3c62726e5dac9d\nchange-id: 20251007-b4-pks-ci-rust-8422e6a8196e\n\n"},{"id":"528782","messageId":"20251015-b4-pks-ci-rust-v3-1-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"[PATCH v3 1/6] ci: deduplicate calls to `apt-get update`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:05Z","receivedAt":"2025-10-15T06:04:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When installing dependencies we first check for the distribution that is\nin use and then we check for the specific job. In the first step we\nalready install all dependencies required to build and test Git, whereas\nthe second step installs a couple of additional dependencies that are\nonly required to perform job-specific tasks.\n\nIn both steps we use `apt-get update` to update our repository sources.\nThis is unnecessary though: all platforms that use Aptitude would have\nalready executed this command in the distro-specific step anyway.\n\nDrop the redundant calls.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/install-dependencies.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 0d3aa496fc..645d035250 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -120,21 +120,17 @@ esac\n \n case \"$jobname\" in\n ClangFormat)\n-\tsudo apt-get -q update\n \tsudo apt-get -q -y install clang-format\n \t;;\n StaticAnalysis)\n-\tsudo apt-get -q update\n \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n \t\tlibexpat-dev gettext make\n \t;;\n sparse)\n-\tsudo apt-get -q update -q\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n \t\tlibexpat-dev gettext zlib1g-dev sparse\n \t;;\n Documentation)\n-\tsudo apt-get -q update\n \tsudo apt-get -q -y install asciidoc xmlto docbook-xsl-ns make\n \n \ttest -n \"$ALREADY_HAVE_ASCIIDOCTOR\" ||\n\n-- \n2.51.0.869.ge66316f041.dirty\n\n"},{"id":"528783","messageId":"20251015-b4-pks-ci-rust-v3-2-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"[PATCH v3 2/6] ci: check formatting of our Rust code","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:06Z","receivedAt":"2025-10-15T06:04:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a CI check that verifies that our Rust code is well-formatted.\nThis check uses `cargo fmt`, which is a wrapper around rustfmt(1) that\nexecutes formatting for all Rust source files. rustfmt(1) itself is the\nde-facto standard for formatting code in the Rust ecosystem.\n\nThe rustfmt(1) tool allows to tweak the final format in theory. In\npractice though, the Rust ecosystem has aligned on style \"editions\".\nThese editions only exist to ensure that any potential changes to the\nstyle don't cause reformats to existing code bases. Other than that,\nmost Rust projects out there accept this default style of a specific\nedition.\n\nLet's do the same and use that default style. It may not be anyone's\nfavorite, but it is consistent and by making it part of our CI we also\nenforce it right from the start.\n\nNote that we don't have to pick a specific style edition here, as the\nedition is automatically derived from the edition we have specified in\nour \"Cargo.toml\" file.\n\nThe implemented script looks somewhat weird as we perfom manual error\nhandling instead of using something like `set -e`. The intent here is\nthat subsequent commits will add more checks, and we want to execute all\nof these checks regardless of whether or not a previous check failed.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .github/workflows/main.yml | 15 +++++++++++++++\n .gitlab-ci.yml             | 11 +++++++++++\n ci/install-dependencies.sh |  5 +++++\n ci/run-rust-checks.sh      | 12 ++++++++++++\n 4 files changed, 43 insertions(+)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 393ea4d1cc..9e36b5c5e3 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -458,6 +458,21 @@ jobs:\n     - run: ci/install-dependencies.sh\n     - run: ci/run-static-analysis.sh\n     - run: ci/check-directional-formatting.bash\n+  rust-analysis:\n+    needs: ci-config\n+    if: needs.ci-config.outputs.enabled == 'yes'\n+    env:\n+      jobname: RustAnalysis\n+      CI_JOB_IMAGE: ubuntu:rolling\n+    runs-on: ubuntu-latest\n+    container: ubuntu:rolling\n+    concurrency:\n+      group: rust-analysis-${{ github.ref }}\n+      cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}\n+    steps:\n+    - uses: actions/checkout@v4\n+    - run: ci/install-dependencies.sh\n+    - run: ci/run-rust-checks.sh\n   sparse:\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex f7d57d1ee9..a47d839e39 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -212,6 +212,17 @@ static-analysis:\n     - ./ci/run-static-analysis.sh\n     - ./ci/check-directional-formatting.bash\n \n+rust-analysis:\n+  image: ubuntu:rolling\n+  stage: analyze\n+  needs: [ ]\n+  variables:\n+    jobname: RustAnalysis\n+  before_script:\n+    - ./ci/install-dependencies.sh\n+  script:\n+    - ./ci/run-rust-checks.sh\n+\n check-whitespace:\n   image: ubuntu:latest\n   stage: analyze\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex 645d035250..a24b07edff 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -126,6 +126,11 @@ StaticAnalysis)\n \tsudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \\\n \t\tlibexpat-dev gettext make\n \t;;\n+RustAnalysis)\n+\tsudo apt-get -q -y install rustup\n+\trustup default stable\n+\trustup component add rustfmt\n+\t;;\n sparse)\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\n \t\tlibexpat-dev gettext zlib1g-dev sparse\ndiff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\nnew file mode 100755\nindex 0000000000..082eb52f11\n--- /dev/null\n+++ b/ci/run-rust-checks.sh\n@@ -0,0 +1,12 @@\n+#!/bin/sh\n+\n+. ${0%/*}/lib.sh\n+\n+set +x\n+\n+if ! group \"Check Rust formatting\" cargo fmt --all --check\n+then\n+\tRET=1\n+fi\n+\n+exit $RET\n\n-- \n2.51.0.869.ge66316f041.dirty\n\n"},{"id":"528784","messageId":"20251015-b4-pks-ci-rust-v3-3-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"[PATCH v3 3/6] rust/varint: add safety comments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:07Z","receivedAt":"2025-10-15T06:04:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `decode_varint()` and `encode_varint()` functions in our Rust crate\nare reimplementations of the respective C functions. As such, we are\nnaturally forced to use the same interface in both Rust and C, which\nmakes use of raw pointers. The consequence is that the code needs to be\nmarked as unsafe in Rust.\n\nIt is common practice in Rust to provide safety documentation for every\nblock that is marked as unsafe. This common practice is also enforced by\nClippy, Rust's static analyser. We don't have Clippy wired up yet, and\nwe could of course just disable this check. But we're about to wire it\nup, and it is reasonable to always enforce documentation for unsafe\nblocks.\n\nAdd such safety comments to already squelch those warnings now. While at\nit, also document the functions' behaviour.\n\nHelped-by: \"brian m. carlson\" <sandals@crustytoothpaste.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n src/varint.rs | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/src/varint.rs b/src/varint.rs\nindex 6e610bdd8e..06492dfc5e 100644\n--- a/src/varint.rs\n+++ b/src/varint.rs\n@@ -1,3 +1,10 @@\n+/// Decode the variable-length integer stored in `bufp` and return the decoded value.\n+///\n+/// Returns 0 in case the decoded integer would overflow u64::MAX.\n+///\n+/// # Safety\n+///\n+/// The buffer must be NUL-terminated to ensure safety.\n #[no_mangle]\n pub unsafe extern \"C\" fn decode_varint(bufp: *mut *const u8) -> u64 {\n     let mut buf = *bufp;\n@@ -22,6 +29,14 @@ pub unsafe extern \"C\" fn decode_varint(bufp: *mut *const u8) -> u64 {\n     val\n }\n \n+/// Encode `value` into `buf` as a variable-length integer unless `buf` is null.\n+///\n+/// Returns the number of bytes written, or, if `buf` is null, the number of bytes that would be\n+/// written to encode the integer.\n+///\n+/// # Safety\n+///\n+/// `buf` must either be null or point to at least 16 bytes of memory.\n #[no_mangle]\n pub unsafe extern \"C\" fn encode_varint(value: u64, buf: *mut u8) -> u8 {\n     let mut varint: [u8; 16] = [0; 16];\n\n-- \n2.51.0.869.ge66316f041.dirty\n\n"},{"id":"528785","messageId":"20251015-b4-pks-ci-rust-v3-4-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"[PATCH v3 4/6] ci: check for common Rust mistakes via Clippy","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:08Z","receivedAt":"2025-10-15T06:04:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a CI check that uses Clippy to perform checks for common\nmistakes and suggested code improvements. Clippy is the official static\nanalyser of the Rust project and thus the de-facto standard.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/install-dependencies.sh | 2 +-\n ci/run-rust-checks.sh      | 5 +++++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex a24b07edff..dcd22ddd95 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -129,7 +129,7 @@ StaticAnalysis)\n RustAnalysis)\n \tsudo apt-get -q -y install rustup\n \trustup default stable\n-\trustup component add rustfmt\n+\trustup component add clippy rustfmt\n \t;;\n sparse)\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\ndiff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\nindex 082eb52f11..fb5ea8991b 100755\n--- a/ci/run-rust-checks.sh\n+++ b/ci/run-rust-checks.sh\n@@ -9,4 +9,9 @@ then\n \tRET=1\n fi\n \n+if ! group \"Check for common Rust mistakes\" cargo clippy --all-targets --all-features -- -Dwarnings\n+then\n+\tRET=1\n+fi\n+\n exit $RET\n\n-- \n2.51.0.869.ge66316f041.dirty\n\n"},{"id":"528786","messageId":"20251015-b4-pks-ci-rust-v3-5-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"[PATCH v3 5/6] ci: verify minimum supported Rust version","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:09Z","receivedAt":"2025-10-15T06:04:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In the current state of our Rust code base we don't really have any\nrequirements for the minimum supported Rust version yet, as we don't use\nany features introduced by a recent version of Rust. Consequently, we\nhave decided that we want to aim for a rather old version and edition of\nRust, where the hope is that using an old version will make alternatives\nlike gccrs viable earlier for compiling Git.\n\nBut while we specify the Rust edition, we don't yet specify a Rust\nversion. And even if we did, the Rust version would only be enforced for\nour own code, but not for any of our dependencies.\n\nWe don't yet have any dependencies at the current point in time. But\nlet's add some safeguards by specifying the minimum supported Rust\nversion and using cargo-msrv(1) to verify that this version can be\nsatisfied for all of our dependencies.\n\nNote that we fix the version of cargo-msrv(1) at v0.18.1. This is the\nlatest release supported by Ubuntu's Rust version.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Cargo.toml                 | 1 +\n ci/install-dependencies.sh | 8 ++++++++\n ci/run-rust-checks.sh      | 5 +++++\n 3 files changed, 14 insertions(+)\n\ndiff --git a/Cargo.toml b/Cargo.toml\nindex 45c9b34981..2f51bf5d5f 100644\n--- a/Cargo.toml\n+++ b/Cargo.toml\n@@ -2,6 +2,7 @@\n name = \"gitcore\"\n version = \"0.1.0\"\n edition = \"2018\"\n+rust-version = \"1.49.0\"\n \n [lib]\n crate-type = [\"staticlib\"]\ndiff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh\nindex dcd22ddd95..29e558bb9c 100755\n--- a/ci/install-dependencies.sh\n+++ b/ci/install-dependencies.sh\n@@ -10,6 +10,8 @@ begin_group \"Install dependencies\"\n P4WHENCE=https://cdist2.perforce.com/perforce/r23.2\n LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION\n 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\n+CARGO_MSRV_VERSION=0.18.4\n+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\n \n # Make sudo a no-op and execute the command directly when running as root.\n # While using sudo would be fine on most platforms when we are root already,\n@@ -130,6 +132,12 @@ RustAnalysis)\n \tsudo apt-get -q -y install rustup\n \trustup default stable\n \trustup component add clippy rustfmt\n+\n+\twget -q \"$CARGO_MSRV_WHENCE\" -O \"cargo-msvc.tgz\"\n+\tsudo mkdir -p \"$CUSTOM_PATH\"\n+\tsudo tar -xf \"cargo-msvc.tgz\" --strip-components=1 \\\n+\t\t--directory \"$CUSTOM_PATH\" --wildcards \"*/cargo-msrv\"\n+\tsudo chmod a+x \"$CUSTOM_PATH/cargo-msrv\"\n \t;;\n sparse)\n \tsudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \\\ndiff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\nindex fb5ea8991b..b5ad9e8dc6 100755\n--- a/ci/run-rust-checks.sh\n+++ b/ci/run-rust-checks.sh\n@@ -14,4 +14,9 @@ then\n \tRET=1\n fi\n \n+if ! group \"Check for minimum required Rust version\" cargo msrv verify\n+then\n+\tRET=1\n+fi\n+\n exit $RET\n\n-- \n2.51.0.869.ge66316f041.dirty\n\n"},{"id":"528787","messageId":"20251015-b4-pks-ci-rust-v3-6-13810af33bd5@pks.im","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"[PATCH v3 6/6] rust: support for Windows","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-15T06:04:10Z","receivedAt":"2025-10-15T06:04:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The initial patch series that introduced Rust into the core of Git only\ncared about macOS and Linux. This specifically leaves out Windows, which\nindeed fails to build right now due to two issues:\n\n  - The Rust runtime requires `GetUserProfileDirectoryW()`, but we don't\n    link against \"userenv.dll\".\n\n  - The path of the Rust library built on Windows is different than on\n    most other systems systems.\n\nFix both of these issues to support Windows.\n\nNote that this commit fixes the Meson-based job in GitHub's CI. Meson\nauto-detects the availability of Rust, and as the Windows runner has\nRust installed by default it already enabled Rust support there. But due\nto the above issues that job fails consistently.\n\nInstall Rust on GitLab CI, as well, to improve test coverage there.\n\nBased-on-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nBased-on-patch-by: Ezekiel Newren <ezekielnewren@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml     |  2 +-\n Makefile           | 14 ++++++++++++--\n meson.build        |  4 ++++\n src/cargo-meson.sh | 11 +++++++++--\n 4 files changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex a47d839e39..b419a84e2c 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -161,7 +161,7 @@ test:mingw64:\n     - saas-windows-medium-amd64\n   before_script:\n     - *windows_before_script\n-    - choco install -y git meson ninja\n+    - choco install -y git meson ninja rust-ms\n     - Import-Module $env:ChocolateyInstall\\helpers\\chocolateyProfile.psm1\n     - refreshenv\n \ndiff --git a/Makefile b/Makefile\nindex 7ea149598d..366fd173e7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -929,10 +929,17 @@ TEST_SHELL_PATH = $(SHELL_PATH)\n LIB_FILE = libgit.a\n XDIFF_LIB = xdiff/lib.a\n REFTABLE_LIB = reftable/libreftable.a\n+\n ifdef DEBUG\n-RUST_LIB = target/debug/libgitcore.a\n+RUST_TARGET_DIR = target/debug\n else\n-RUST_LIB = target/release/libgitcore.a\n+RUST_TARGET_DIR = target/release\n+endif\n+\n+ifeq ($(uname_S),Windows)\n+RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib\n+else\n+RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a\n endif\n \n # xdiff and reftable libs may in turn depend on what is in libgit.a\n@@ -1538,6 +1545,9 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)\n ifdef WITH_RUST\n BASIC_CFLAGS += -DWITH_RUST\n GITLIBS += $(RUST_LIB)\n+ifeq ($(uname_S),Windows)\n+EXTLIBS += -luserenv\n+endif\n endif\n \n ifdef SANITIZE\ndiff --git a/meson.build b/meson.build\nindex ec55d6a5fd..a9c865b2af 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1707,6 +1707,10 @@ rust_option = get_option('rust').disable_auto_if(not cargo.found())\n if rust_option.allowed()\n   subdir('src')\n   libgit_c_args += '-DWITH_RUST'\n+\n+  if host_machine.system() == 'windows'\n+    libgit_dependencies += compiler.find_library('userenv')\n+  endif\n else\n   libgit_sources += [\n     'varint.c',\ndiff --git a/src/cargo-meson.sh b/src/cargo-meson.sh\nindex 99400986d9..3998db0435 100755\n--- a/src/cargo-meson.sh\n+++ b/src/cargo-meson.sh\n@@ -26,7 +26,14 @@ then\n \texit $RET\n fi\n \n-if ! cmp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n+case \"$(cargo -vV | sed -s 's/^host: \\(.*\\)$/\\1/')\" in\n+\t*-windows-*)\n+\t\tLIBNAME=gitcore.lib;;\n+\t*)\n+\t\tLIBNAME=libgitcore.a;;\n+esac\n+\n+if ! cmp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n then\n-\tcp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\"\n+\tcp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\"\n fi\n\n-- \n2.51.0.869.ge66316f041.dirty\n\n"},{"id":"528820","messageId":"xmqqy0pcur56.fsf@gitster.g","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-0-13810af33bd5@pks.im","subject":"Re: [PATCH v3 0/6] ci: improvements to our Rust infrastructure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-15T15:21:57Z","receivedAt":"2025-10-15T15:22:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> this small patch series introduces some improvements for our Rust\n> infrastructure. Most importantly, it introduces a couple of static\n> analysis checks to verify consistent formatting, use Clippy for linting\n> and to verify our minimum supported Rust version.\n>\n> Furthermore, this series also introduces support for building with Rust\n> enabled on Windows.\n\nThe updated structure is definite improvement (although I have no\nidea on the Windows part, but presumably that has been written for\nand tested on a real Windows box).\n\nWill replace.\n"},{"id":"529891","messageId":"aQKEz5bnPHuCSjlR@szeder.dev","threadId":"64262","inReplyTo":"aOdIRnB-SGQwj935@pks.im","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2025-10-29T21:19:11Z","receivedAt":"2025-10-29T21:19:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Oct 09, 2025 at 07:29:42AM +0200, Patrick Steinhardt wrote:\n> On Wed, Oct 08, 2025 at 10:55:43PM +0200, SZEDER Gábor wrote:\n> > On Tue, Oct 07, 2025 at 02:36:30PM +0200, Patrick Steinhardt wrote:\n> > > diff --git a/ci/run-rust-checks.sh b/ci/run-rust-checks.sh\n> > > new file mode 100755\n> > > index 0000000000..082eb52f11\n> > > --- /dev/null\n> > > +++ b/ci/run-rust-checks.sh\n> > > @@ -0,0 +1,12 @@\n> > > +#!/bin/sh\n> > > +\n> > > +. ${0%/*}/lib.sh\n> > > +\n> > > +set +x\n> > > +\n> > > +if ! group \"Check Rust formatting\" cargo fmt --all --check\n> > > +then\n> > > +\tRET=1\n> > > +fi\n> > > +\n> > > +exit $RET\n> > \n> > Our ci/*.sh scripts usually rely on 'set -e' to catch failed commands.\n> > Either this script should follow that convention as well, or the\n> > commit message should justify the deviation from convention.\n> \n> Ah, good point. The reason is that subsequent commits add more checks,\n> and I want to make sure that they all run even if previous checks\n> failed. It's otherwise annoying to fix a first set of errors surfaced by\n> the CI only to then notice that later checks also fail.\n\nWell, OTOH, it is annoying when the error messages from a failed CI\nrun are not at the bottom of the logs.\n\n"},{"id":"529909","messageId":"aQKbN157AMJi59rV@szeder.dev","threadId":"64262","inReplyTo":"xmqqms61h0g1.fsf@gitster.g","subject":"Re: [PATCH 2/6] ci: check formatting of our Rust code","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2025-10-29T22:54:47Z","receivedAt":"2025-10-29T22:54:50Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Oct 08, 2025 at 08:34:22AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> >> ... but I also think we should take this\n> >> opportunity to choose the Rust defaults for Rust.  C, Perl, and text\n> >> formats like AsciiDoc do not have rigid defaults about indentation\n> >> style, tabs vs. spaces, and line length; Rust does.  We wouldn't use\n> >> tabs in Rust (the default is four spaces) because we use it everywhere\n> >> else, so I think we should take the opportunity to use the Rust defaults\n> >> here as well.\n> >\n> > I am also slightly leaning into the direction of sticking with Rust's\n> > default of 100 characters. It's not substantially more than 80, should\n> > be reasonable to accommodate for in most modern setups, and sticks with\n> > what the remainder of the ecosystem is doing.\n> >\n> > So for now I'll leave it at 80 characters. But I don't feel strongly\n> > about this, so if there is a majority in favor of 80 characters I'm\n> > happy to adjust.\n> \n> So the question is if we want consistency across files regardless of\n> what language they are written in (i.e. 80-columns everywhere) or we\n> treat our existing rules a \"fallback rules\" we have adopted while\n> dealing with languages without their own strict rules, and use the\n> default for a language with its own rule (i.e. whatever rustfmt\n> wants is used for Rust, our own rules still apply to everything\n> else)?\n\nConsistency across files regardless of language was great, because I,\nfor one, prefer to use the same editor for all files regardless of\nlanguage.\n\nI find 100 columns much worse than 80, because on my laptop I can put\nthree 80 columns editors next to each other (and still have a bit of\nroom to spare), but with 100 columns there is only room for two.\n\n> I actually am fine with the latter myself.\n> \n> If people strongly prefer, I also can be talked into adopting\n> slightly wider limit for our fallback rules for everything else, but\n> that is probably a separate discussion.  It is a bit unfriendly move\n> against folks with aging eyeballs like myself, though.\n> \n> Thanks.\n> \n> \n"},{"id":"531076","messageId":"CAH=ZcbB8cRgCTp-Q_CxJ4VFNY1+w+C20zgx9bMre4-hNmPrD7g@mail.gmail.com","threadId":"64262","inReplyTo":"20251015-b4-pks-ci-rust-v3-6-13810af33bd5@pks.im","subject":"Re: [PATCH v3 6/6] rust: support for Windows","fromName":"Ezekiel Newren","fromEmail":"ezekielnewren@gmail.com","sentAt":"2025-11-20T19:45:34Z","receivedAt":"2025-11-20T19:45:47Z","isPatch":true,"sender":{"key":"ezekielnewren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/18313324?v=4"},"body":"This is a retrospective review. I completely missed this patch series,\nand only noticed its existence after it was merged into master. The\ncore problem is that these changes assume that windows builds only\never use the MSVC compiler, but that's not true.\n\nOn Wed, Oct 15, 2025 at 12:04 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> The initial patch series that introduced Rust into the core of Git only\n> cared about macOS and Linux. This specifically leaves out Windows, which\n> indeed fails to build right now due to two issues:\n>\n>   - The Rust runtime requires `GetUserProfileDirectoryW()`, but we don't\n>     link against \"userenv.dll\".\n>\n>   - The path of the Rust library built on Windows is different than on\n>     most other systems systems.\n\nThat is true, but the build systems also need to check if the C\ncompiler is gnu or msvc. Also you used the word \"systems\" twice.\n\n> diff --git a/Makefile b/Makefile\n> index 7ea149598d..366fd173e7 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -929,10 +929,17 @@ TEST_SHELL_PATH = $(SHELL_PATH)\n>  LIB_FILE = libgit.a\n>  XDIFF_LIB = xdiff/lib.a\n>  REFTABLE_LIB = reftable/libreftable.a\n> +\n>  ifdef DEBUG\n> -RUST_LIB = target/debug/libgitcore.a\n> +RUST_TARGET_DIR = target/debug\n>  else\n> -RUST_LIB = target/release/libgitcore.a\n> +RUST_TARGET_DIR = target/release\n> +endif\n> +\n> +ifeq ($(uname_S),Windows)\n> +RUST_LIB = $(RUST_TARGET_DIR)/gitcore.lib\n> +else\n> +RUST_LIB = $(RUST_TARGET_DIR)/libgitcore.a\n>  endif\n>\n>  # xdiff and reftable libs may in turn depend on what is in libgit.a\n> @@ -1538,6 +1545,9 @@ ALL_LDFLAGS = $(LDFLAGS) $(LDFLAGS_APPEND)\n>  ifdef WITH_RUST\n>  BASIC_CFLAGS += -DWITH_RUST\n>  GITLIBS += $(RUST_LIB)\n> +ifeq ($(uname_S),Windows)\n> +EXTLIBS += -luserenv\n> +endif\n>  endif\n>  ifdef SANITIZE\n\nThis is not fully correct for Makefile. If Windows AND using MSVC ->\ngitcore.lib. However this bug doesn't show up because github ci\ndoesn't test the windows+msvc+makefile combo.\n\n> diff --git a/meson.build b/meson.build\n> index ec55d6a5fd..a9c865b2af 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1707,6 +1707,10 @@ rust_option = get_option('rust').disable_auto_if(not cargo.found())\n>  if rust_option.allowed()\n>    subdir('src')\n>    libgit_c_args += '-DWITH_RUST'\n> +\n> +  if host_machine.system() == 'windows'\n> +    libgit_dependencies += compiler.find_library('userenv')\n> +  endif\n>  else\n>    libgit_sources += [\n>      'varint.c',\n\nSame issue as above, but it doesn't show up because the github ci\ndoesn't test the windows+gnu+meson combo.\n\n> diff --git a/src/cargo-meson.sh b/src/cargo-meson.sh\n> index 99400986d9..3998db0435 100755\n> --- a/src/cargo-meson.sh\n> +++ b/src/cargo-meson.sh\n> @@ -26,7 +26,14 @@ then\n>         exit $RET\n>  fi\n>\n> -if ! cmp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n> +case \"$(cargo -vV | sed -s 's/^host: \\(.*\\)$/\\1/')\" in\n> +       *-windows-*)\n> +               LIBNAME=gitcore.lib;;\n> +       *)\n> +               LIBNAME=libgitcore.a;;\n> +esac\n> +\n> +if ! cmp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n>  then\n> -       cp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\"\n> +       cp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\"\n>  fi\n\nSame issue again. This needs to test for windows AND msvc.\n"},{"id":"531130","messageId":"dc753c0e-eb93-948c-55f7-bb0e91772c83@gmx.de","threadId":"64262","inReplyTo":"CAH=ZcbB8cRgCTp-Q_CxJ4VFNY1+w+C20zgx9bMre4-hNmPrD7g@mail.gmail.com","subject":"Re: [PATCH v3 6/6] rust: support for Windows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-11-21T08:18:47Z","receivedAt":"2025-11-21T08:18:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ezekiel,\n\nOn Thu, 20 Nov 2025, Ezekiel Newren wrote:\n\n> This is a retrospective review. I completely missed this patch series,\n> and only noticed its existence after it was merged into master. The\n> core problem is that these changes assume that windows builds only\n> ever use the MSVC compiler, but that's not true.\n\nCorrect. I had actually worked on a solution to this in Git for Windows,\nbut due to time constraints (after factoring in the usual time tax, other\npriorities dictated that I wouldn't have time to see it through) I hadn't\nhad time to contribute it, let alone engage in reviewing Patrick's patches\n(I had actually not even seen them until I had written the patch and\nverified that it fixed the issue).\n\nHere is my patch (with proper handling of MSVC, but obviously it no longer\napplies without conflicts):\nhttps://github.com/git-for-windows/git/commit/0949ff2ad5d1d085b10c63029c65293416732851\n\n-- snipsnap --\nFrom 0949ff2ad5d1d085b10c63029c65293416732851 Mon Sep 17 00:00:00 2001\nFrom: Johannes Schindelin <johannes.schindelin@gmx.de>\nDate: Fri, 24 Oct 2025 14:49:22 +0200\nSubject: [PATCH] meson(cargo): support Windows again\n\nFor over a year, Git has been moved to a more modern build system than\nit had before (GNU make, or on Windows optionally CMake). Naturally,\nthis new system breaks Windows support left and right.\n\nFor example, c184795fc0e (meson: add infrastructure to build internal\nRust library, 2025-10-02) added support for building a Rust library, and\nit fails when Visual C is configured as compiler.\n\nThis is the reason that the `win+Meson` job of Git's `master` branch\nfails for the past 16 days, i.e. the latest 9 pushes of the `master`\nbranch as of time of writing. The symptom is:\n\n  [697/905] Generating src/git_rs with a custom command\n  FAILED: [code=1] src/libgitcore.a \"C:\\Program Files\\Git\\bin\\sh.exe\" \"D:/a/git/git/src/cargo-meson.sh\" \"D:/a/git/git\" \"D:/a/git/git/build/src\" \"--release\"\n  cp: cannot stat 'D:/a/git/git/build/src/release/libgitcore.a': No such file or directory\n\nThe reason is that Visual C's output is called `gitcore.lib`, not\n`libgitcore.a`. Let's special-case Visual C and use the correct filename\nin all cases.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n src/cargo-meson.sh |  7 +++++--\n src/meson.build    | 10 +++++++++-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/src/cargo-meson.sh b/src/cargo-meson.sh\nindex 99400986d93..c14a08c592d 100755\n--- a/src/cargo-meson.sh\n+++ b/src/cargo-meson.sh\n@@ -5,6 +5,9 @@ then\n \texit 1\n fi\n \n+target=\"$1\"\n+shift\n+\n SOURCE_DIR=\"$1\"\n BUILD_DIR=\"$2\"\n BUILD_TYPE=debug\n@@ -26,7 +29,7 @@ then\n \texit $RET\n fi\n \n-if ! cmp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n+if ! cmp \"$BUILD_DIR/$BUILD_TYPE/$target\" \"$BUILD_DIR/$target\" >/dev/null 2>&1\n then\n-\tcp \"$BUILD_DIR/$BUILD_TYPE/libgitcore.a\" \"$BUILD_DIR/libgitcore.a\"\n+\tcp \"$BUILD_DIR/$BUILD_TYPE/$target\" \"$BUILD_DIR/$target\"\n fi\ndiff --git a/src/meson.build b/src/meson.build\nindex 25b9ad5a147..b2473c46994 100644\n--- a/src/meson.build\n+++ b/src/meson.build\n@@ -3,6 +3,13 @@ libgit_rs_sources = [\n   'varint.rs',\n ]\n \n+# The exact file name depends on the compiler\n+if meson.get_compiler('c').get_id() == 'msvc'\n+  target = 'gitcore.lib'\n+else\n+  target = 'libgitcore.a'\n+endif\n+\n # Unfortunately we must use a wrapper command to move the output file into the\n # current build directory. This can fixed once `cargo build --artifact-dir`\n # stabilizes. See https://github.com/rust-lang/cargo/issues/6790 for that\n@@ -10,6 +17,7 @@ libgit_rs_sources = [\n cargo_command = [\n   shell,\n   meson.current_source_dir() / 'cargo-meson.sh',\n+  target,\n   meson.project_source_root(),\n   meson.current_build_dir(),\n ]\n@@ -21,7 +29,7 @@ libgit_rs = custom_target('git_rs',\n   input: libgit_rs_sources + [\n     meson.project_source_root() / 'Cargo.toml',\n   ],\n-  output: 'libgitcore.a',\n+  output: target,\n   command: cargo_command,\n )\n libgit_dependencies += declare_dependency(link_with: libgit_rs)\n-- \n2.51.1.windows.1\n\n"},{"id":"531154","messageId":"xmqq34673w22.fsf@gitster.g","threadId":"64262","inReplyTo":"dc753c0e-eb93-948c-55f7-bb0e91772c83@gmx.de","subject":"Re: [PATCH v3 6/6] rust: support for Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-21T21:39:01Z","receivedAt":"2025-11-21T21:39:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Here is my patch (with proper handling of MSVC, but obviously it no longer\n> applies without conflicts):\n\nThanks.  Here is my attempt to make it apply to 'master'.\n\nIt seems to pass win+Meson build & test for 'master'.\n\n  https://github.com/git/git/actions/runs/19583133885\n\nBut curiously, the tip of 'master' has been happy without this fix,\nand it does not help when brian's SHA-256 interop topic merged\nfurther on top, but I didn't look any further.\n\n--- >8 ---\nSubject: [PATCH] meson(cargo): Visual C produces gitcore.lib, not libgitcore.a\n\nOn Windows, when Visual C compiler is used to compile, the resulting\nlibrary that is created is called gitcore.lib instead of libgitcore.a\nlibrary archive.\n\nJohannes sent a fix in <dc753c0e-eb93-948c-55f7-bb0e91772c83@gmx.de>\nthat was based on an older code base, which I attempted to forward\nport it to apply to today's codebase.\n\nBased-on-the-patch-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n src/cargo-meson.sh | 14 ++++----------\n src/meson.build    | 11 ++++++++++-\n 2 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/src/cargo-meson.sh b/src/cargo-meson.sh\nindex 3998db0435..4a46f731d8 100755\n--- a/src/cargo-meson.sh\n+++ b/src/cargo-meson.sh\n@@ -7,9 +7,10 @@ fi\n \n SOURCE_DIR=\"$1\"\n BUILD_DIR=\"$2\"\n+LIBNAME=\"$3\"\n BUILD_TYPE=debug\n \n-shift 2\n+shift 3\n \n for arg\n do\n@@ -26,14 +27,7 @@ then\n \texit $RET\n fi\n \n-case \"$(cargo -vV | sed -s 's/^host: \\(.*\\)$/\\1/')\" in\n-\t*-windows-*)\n-\t\tLIBNAME=gitcore.lib;;\n-\t*)\n-\t\tLIBNAME=libgitcore.a;;\n-esac\n-\n-if ! cmp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\" >/dev/null 2>&1\n+if ! cmp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/$LIBNAME\" >/dev/null 2>&1\n then\n-\tcp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/libgitcore.a\"\n+\tcp \"$BUILD_DIR/$BUILD_TYPE/$LIBNAME\" \"$BUILD_DIR/$LIBNAME\"\n fi\ndiff --git a/src/meson.build b/src/meson.build\nindex 25b9ad5a14..f37f0a5f58 100644\n--- a/src/meson.build\n+++ b/src/meson.build\n@@ -3,6 +3,14 @@ libgit_rs_sources = [\n   'varint.rs',\n ]\n \n+# The exact file name depends on the compiler\n+if meson.get_compiler('c').get_id() == 'msvc'\n+  libname = 'gitcore.lib'\n+else\n+  libname = 'libgitcore.a'\n+endif\n+\n+\n # Unfortunately we must use a wrapper command to move the output file into the\n # current build directory. This can fixed once `cargo build --artifact-dir`\n # stabilizes. See https://github.com/rust-lang/cargo/issues/6790 for that\n@@ -12,6 +20,7 @@ cargo_command = [\n   meson.current_source_dir() / 'cargo-meson.sh',\n   meson.project_source_root(),\n   meson.current_build_dir(),\n+  libname,\n ]\n if get_option('buildtype') == 'release'\n   cargo_command += '--release'\n@@ -21,7 +30,7 @@ libgit_rs = custom_target('git_rs',\n   input: libgit_rs_sources + [\n     meson.project_source_root() / 'Cargo.toml',\n   ],\n-  output: 'libgitcore.a',\n+  output: libname,\n   command: cargo_command,\n )\n libgit_dependencies += declare_dependency(link_with: libgit_rs)\n-- \n2.52.0-168-gcf2c56d51e\n\n"}]}