[PATCH 00/12] t: exercise Git/JGit reftable compatibility

[PATCH 00/12] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

Hi,

while the reftable backend is a recent addition to Git, it has been part
of JGit since 2017 already. Given that there are essentially two
different implementations of the reftable format now there is a very
real risk of these two diverge and become incompatible with each other,
which would be a shame.

This patch series addresses this risk by introducing compatibility tests
which assert that both Git and JGit can access reftables written by the
respective other implementation.

The patch series is structured as follows:

  - Patches 1-8 merge "install-docker-dependencies.sh" into
    "install-dependencies.sh". This is done so that both CI job flavors
    have the same dependencies and thus the same test coverage available
    without always having to maintain them both.

  - Patch 9 makes JGit available.

  - Patch 10 starts running backend-specific tests in all jobs, and
    patch 11 addresses a portability issue surfaced by this.

  - Patch 12 adds a very basic compatibility test suite for Git/JGit
    reftables. I mostly consider this as a proof of concept, it should
    likely be extended over time.

These compatibility tests surface three findings:

  - JGit does not support reftables format v2, which was added to
    support the SHA256 object format.

  - JGit cannot read reflogs written by itself when starting from an
    unborn branch. This smells like a bug in JGit to me where it
    misinterprets reflog entries with a zero object ID as new OID, but I
    didn't dig any deeper yet.

  - JGit is incompatible with split indices because it cannot handle
    'link' DIRC entries. This is unrelated to reftables though.

I have tested the CI changes against both GitLab [1] and GitHub [2]. The
macOS test failures on GitHub are caused by the recent curl regression.

[1]: https://gitlab.com/gitlab-org/git/-/merge_requests/123
[2]: https://github.com/git/git/pull/1696

Patrick

Patrick Steinhardt (12):
  ci: rename "runs_on_pool" to "distro"
  ci: expose distro name in dockerized GitHub jobs
  ci: allow skipping sudo on dockerized jobs
  ci: drop duplicate package installation for "linux-gcc-default"
  ci: convert "install-dependencies.sh" to use "/bin/sh"
  ci: merge custom PATH directories
  ci: merge scripts which install dependencies
  ci: make Perforce binaries executable for all users
  ci: install JGit dependency
  t06xx: always execute backend-specific tests
  t0610: fix non-portable variable assignment
  t0612: add tests to exercise Git/JGit reftable compatibility

 .github/workflows/main.yml             |   8 +-
 .gitlab-ci.yml                         |   4 +-
 ci/install-dependencies.sh             | 100 ++++++++++++++-------
 ci/install-docker-dependencies.sh      |  46 ----------
 ci/lib.sh                              |  14 ++-
 t/t0600-reffiles-backend.sh            |   8 +-
 t/t0601-reffiles-pack-refs.sh          |   9 +-
 t/t0610-reftable-basics.sh             |  12 ++-
 t/t0612-reftable-jgit-compatibility.sh | 115 +++++++++++++++++++++++++
 9 files changed, 208 insertions(+), 108 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh


base-commit: 7774cfed6261ce2900c84e55906da708c711d601
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 01/12] ci: rename "runs_on_pool" to "distro"

[Patrick Steinhardt]

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

The "runs_on_pool" environment variable is used by our CI scripts to
distinguish the different kinds of operating systems. It is quite
specific to GitHub Actions though and not really a descriptive name.

Rename the variable to "distro" to clarify its intent.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 2 +-
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3428773b09..684ef5c00d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -303,7 +303,7 @@ jobs:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      distro: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v4
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index b4e22de3cb..7d247b5ef4 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,7 +11,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0a73fc7bd1..d882250db5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -279,7 +279,7 @@ then
 
 	cache_dir="$HOME/none"
 
-	runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
+	distro=$(echo "$CI_JOB_IMAGE" | tr : -)
 	JOBS=$(nproc)
 else
 	echo "Could not identify CI type" >&2
@@ -318,7 +318,7 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	if test "$jobname" = "linux-gcc-default"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 02/12] ci: expose distro name in dockerized GitHub jobs

[Patrick Steinhardt]

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

Expose a distro name in dockerized jobs. This will be used in a
subsequent commit where we merge the installation scripts for dockerized
and non-dockerized jobs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 684ef5c00d..71cd4e5486 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -342,12 +342,16 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
+          distro: alpine-latest
         - jobname: linux32
           image: daald/ubuntu32:xenial
+          distro: ubuntu32-16.04
         - jobname: pedantic
           image: fedora
+          distro: fedora-latest
     env:
       jobname: ${{matrix.vector.jobname}}
+      distro: ${{matrix.vector.distro}}
     runs-on: ubuntu-latest
     container: ${{matrix.vector.image}}
     steps:
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 03/12] ci: allow skipping sudo on dockerized jobs

[Patrick Steinhardt]

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

Our "install-dependencies.sh" script is executed by non-dockerized jobs
to install dependencies. These jobs don't run with "root" permissions,
but with a separate user. Consequently, we need to use sudo(8) there to
elevate permissions when installing packages.

We're about to merge "install-docker-dependencies.sh" into that script
though, and our Docker containers do run as "root". Using sudo(8) is
thus unnecessary there, even though it would be harmless. On some images
like Alpine Linux though there is no sudo(8) available by default, which
would consequently break the build.

Adapt the script to make "sudo" a no-op when running as "root" user.
This allows us to easily reuse the script for our dockerized jobs.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7d247b5ef4..7dfd3e50ed 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,6 +11,17 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
+# Make sudo a no-op and execute the command directly when running as root.
+# While using sudo would be fine on most platforms when we are root already,
+# some platforms like e.g. Alpine Linux do not have sudo available by default
+# and would thus break.
+if test "$(id -u)" -eq 0
+then
+	sudo () {
+		"$@"
+	}
+fi
+
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 04/12] ci: drop duplicate package installation for "linux-gcc-default"

[Patrick Steinhardt]

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

The "linux-gcc-default" job installs common Ubuntu packages. This is
already done in the distro-specific switch, so we basically duplicate
the effort here.

Drop the duplicate package installations and inline the variable that
contains those common packages.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7dfd3e50ed..fad53aac96 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -7,9 +7,6 @@
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
-UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
- tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
- libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -25,8 +22,13 @@ fi
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
+	sudo apt-get -q -y install \
+		language-pack-is libsvn-perl apache2 \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
+		$CC_PACKAGE $PYTHON_PACKAGE
+
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -34,6 +36,7 @@ ubuntu-*)
 		chmod u+x p4d
 		chmod u+x p4
 	popd
+
 	mkdir --parents "$GIT_LFS_PATH"
 	pushd "$GIT_LFS_PATH"
 		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
@@ -83,10 +86,6 @@ Documentation)
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
 	;;
-linux-gcc-default)
-	sudo apt-get -q update
-	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
-	;;
 esac
 
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 05/12] ci: convert "install-dependencies.sh" to use "/bin/sh"

[Patrick Steinhardt]

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

We're about to merge the "install-docker-dependencies.sh" script into
"install-dependencies.sh". This will also move our Alpine-based jobs
over to use the latter script. This script uses the Bash shell though,
which is not available by default on Alpine Linux.

Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash.
This requires us to get rid of the pushd/popd invocations, which are
replaced by some more elaborate commands that download or extract
executables right to where they are needed.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index fad53aac96..7bcccc96fd 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 #
 # Install dependencies required to build and test Git on Linux and macOS
 #
@@ -30,19 +30,14 @@ ubuntu-*)
 		$CC_PACKAGE $PYTHON_PACKAGE
 
 	mkdir --parents "$P4_PATH"
-	pushd "$P4_PATH"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
-		chmod u+x p4d
-		chmod u+x p4
-	popd
+	wget --quiet --directory-prefix="$P4_PATH" \
+		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
+	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
 
 	mkdir --parents "$GIT_LFS_PATH"
-	pushd "$GIT_LFS_PATH"
-		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
-	popd
+	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
@@ -53,11 +48,10 @@ macos-*)
 	brew link --force gettext
 
 	mkdir -p "$P4_PATH"
-	pushd "$P4_PATH"
-		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-		tar -xf helix-core-server.tgz &&
-		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
-	popd
+	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
+	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 06/12] ci: merge custom PATH directories

[Patrick Steinhardt]

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

We're downloading various executables required by our tests. Each of
these executables goes into its own directory, which is then appended to
the PATH variable. Consequently, whenever we add a new dependency and
thus a new directory, we would have to adapt to this change in several
places.

Refactor this to instead put all binaries into a single directory.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 +++++++--------
 ci/lib.sh                  | 10 +++-------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7bcccc96fd..46b9efb2d5 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -29,14 +29,13 @@ ubuntu-*)
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
 		$CC_PACKAGE $PYTHON_PACKAGE
 
-	mkdir --parents "$P4_PATH"
-	wget --quiet --directory-prefix="$P4_PATH" \
+	mkdir --parents "$CUSTOM_PATH"
+	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
+	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
-	mkdir --parents "$GIT_LFS_PATH"
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
@@ -47,10 +46,10 @@ macos-*)
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
 
-	mkdir -p "$P4_PATH"
+	mkdir -p "$CUSTOM_PATH"
 	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
-	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	tar -xf helix-core-server.tgz -C "$CUSTOM_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
 	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
diff --git a/ci/lib.sh b/ci/lib.sh
index d882250db5..4cce854bad 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -340,10 +340,6 @@ ubuntu-*)
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
 	export LINUX_GIT_LFS_VERSION="1.5.2"
-
-	P4_PATH="$HOME/custom/p4"
-	GIT_LFS_PATH="$HOME/custom/git-lfs"
-	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
 macos-*)
 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
@@ -351,12 +347,12 @@ macos-*)
 	then
 		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
 	fi
-
-	P4_PATH="$HOME/custom/p4"
-	export PATH="$P4_PATH:$PATH"
 	;;
 esac
 
+CUSTOM_PATH="$HOME/path"
+export PATH="$CUSTOM_PATH:$PATH"
+
 case "$jobname" in
 linux32)
 	CC=gcc
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 07/12] ci: merge scripts which install dependencies

[Patrick Steinhardt]

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

We have two different scripts which install dependencies, one for
dockerized jobs and one for non-dockerized ones. Naturally, these
scripts have quite some duplication. Furthermore, either of these
scripts is missing some test dependencies that the respective other
script has, thus reducing test coverage.

Merge those two scripts such that there is a single source of truth for
test dependencies, only.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml        |  2 +-
 .gitlab-ci.yml                    |  4 +--
 ci/install-dependencies.sh        | 32 ++++++++++++++++++---
 ci/install-docker-dependencies.sh | 46 -------------------------------
 4 files changed, 31 insertions(+), 53 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 71cd4e5486..5838986895 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -359,7 +359,7 @@ jobs:
       if: matrix.vector.jobname != 'linux32'
     - uses: actions/checkout@v1 # cannot be upgraded because Node.js Actions aren't supported in this container
       if: matrix.vector.jobname == 'linux32'
-    - run: ci/install-docker-dependencies.sh
+    - run: ci/install-dependencies.sh
     - run: ci/run-build-and-tests.sh
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c0fa2fe90b..8ce0e1b4bd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -10,7 +10,7 @@ workflow:
 test:linux:
   image: $image
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - useradd builder --create-home
     - chown -R builder "${CI_PROJECT_DIR}"
@@ -98,7 +98,7 @@ static-analysis:
   variables:
     jobname: StaticAnalysis
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - ./ci/run-static-analysis.sh
     - ./ci/check-directional-formatting.bash
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 46b9efb2d5..f4eb125fd2 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,6 +5,8 @@
 
 . ${0%/*}/lib.sh
 
+begin_group "Install dependencies"
+
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 
@@ -20,14 +22,27 @@ then
 fi
 
 case "$distro" in
+alpine-*)
+	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
+		pcre2-dev python3 musl-libintl perl-utils ncurses \
+		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+	;;
+fedora-*)
+	dnf -yq update >/dev/null &&
+	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
+	;;
 ubuntu-*)
+	# Required so that apt doesn't wait for user input on certain packages.
+	export DEBIAN_FRONTEND=noninteractive
+
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
-		language-pack-is libsvn-perl apache2 \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
-		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
-		$CC_PACKAGE $PYTHON_PACKAGE
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
+		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
 
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
@@ -38,6 +53,13 @@ ubuntu-*)
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
+ubuntu32-*)
+	sudo linux32 --32bit i386 sh -c '
+		apt update >/dev/null &&
+		apt install -y build-essential libcurl4-openssl-dev \
+			libssl-dev libexpat-dev gettext python >/dev/null
+	'
+	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
@@ -97,3 +119,5 @@ then
 else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
+
+end_group "Install dependencies"
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
deleted file mode 100755
index eb2c9e1eca..0000000000
--- a/ci/install-docker-dependencies.sh
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/bin/sh
-#
-# Install dependencies required to build and test Git inside container
-#
-
-. ${0%/*}/lib.sh
-
-begin_group "Install dependencies"
-
-case "$jobname" in
-linux32)
-	linux32 --32bit i386 sh -c '
-		apt update >/dev/null &&
-		apt install -y build-essential libcurl4-openssl-dev \
-			libssl-dev libexpat-dev gettext python >/dev/null
-	'
-	;;
-linux-musl)
-	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
-		pcre2-dev python3 musl-libintl perl-utils ncurses \
-		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
-		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
-	;;
-linux-*|StaticAnalysis)
-	# Required so that apt doesn't wait for user input on certain packages.
-	export DEBIAN_FRONTEND=noninteractive
-
-	apt update -q &&
-	apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
-		libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
-		perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
-		libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
-		apache2 cvs cvsps gnupg libcgi-pm-perl subversion
-
-	if test "$jobname" = StaticAnalysis
-	then
-		apt install -q -y coccinelle
-	fi
-	;;
-pedantic)
-	dnf -yq update >/dev/null &&
-	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
-	;;
-esac
-
-end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 08/12] ci: make Perforce binaries executable for all users

[Patrick Steinhardt]

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

The Perforce binaries are only made executable for the current user. On
GitLab CI though we execute tests as a different user than "root", and
thus these binaries may not be executable by that test user.

Fix the setup so that we set the executable bits for all users.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index f4eb125fd2..068c478025 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -47,7 +47,7 @@ ubuntu-*)
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
+	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 09/12] ci: install JGit dependency

[Patrick Steinhardt]

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

We have some tests in t5310 that use JGit to verify that bitmaps can be
read both by Git and by JGit. We do not execute these tests in our CI
jobs though because we don't make JGit available there. Consequently,
the tests basically bitrot because almost nobody is ever going to have
JGit in their path.

Install JGit to plug this test gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 068c478025..3fefe134ea 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -9,6 +9,7 @@ begin_group "Install dependencies"
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
+JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -39,7 +40,7 @@ ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
 		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
 		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
@@ -52,6 +53,9 @@ ubuntu-*)
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+
+	wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit"
+	chmod a+x "$CUSTOM_PATH/jgit"
 	;;
 ubuntu32-*)
 	sudo linux32 --32bit i386 sh -c '
@@ -112,6 +116,7 @@ then
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
+
 if type git-lfs >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
@@ -120,4 +125,12 @@ else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
 
+if type jgit >/dev/null 2>&1
+then
+	echo "$(tput setaf 6)JGit Version$(tput sgr0)"
+	jgit version
+else
+	echo >&2 "WARNING: JGit wasn't installed, see above for clues why"
+fi
+
 end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 10/12] t06xx: always execute backend-specific tests

[Patrick Steinhardt]

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

The tests in t06xx exercise specific ref formats. Next to probing some
basic functionality, these tests also exercise other low-level details
specific to the format. Those tests are only executed though in case
`GIT_TEST_DEFAULT_REF_FORMAT` is set to the ref format of the respective
backend-under-test.

Ideally, we would run the full test matrix for ref formats such that our
complete test suite is executed with every supported format on every
supported platform. This is quite an expensive undertaking though, and
thus we only execute e.g. the "reftable" tests on macOS and Linux. As a
result, we basically have no test coverage for the "reftable" format at
all on other platforms like Windows.

Adapt these tests so that they override `GIT_TEST_DEFAULT_REF_FORMAT`,
which means that they'll always execute. This increases test coverage on
platforms that don't run the full test matrix, which at least gives us
some basic test coverage on those platforms for the "reftable" format.

This of course comes at the cost of running those tests multiple times
on platforms where we do run the full test matrix. But arguably, this is
a good thing because it will also cause us to e.g. run those tests with
the address sanitizer and other non-standard parameters.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0600-reffiles-backend.sh   | 8 ++------
 t/t0601-reffiles-pack-refs.sh | 9 +++------
 t/t0610-reftable-basics.sh    | 9 +++------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 64214340e7..a390cffc80 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -4,16 +4,12 @@ test_description='Test reffiles backend'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	git commit --allow-empty -m Initial &&
 	C=$(git rev-parse HEAD) &&
diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index c309d2bae8..157f79fe52 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -9,18 +9,15 @@ test_description='git pack-refs should not change the branch semantic
 This test runs git pack-refs and git show-ref and checks that the branch
 semantic is still the same.
 '
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'enable reflogs' '
 	git config core.logallrefupdates true
 '
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 686781192e..aa9282007c 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -4,17 +4,14 @@
 #
 
 test_description='reftable basics'
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 . ./test-lib.sh
 
-if ! test_have_prereq REFTABLE
-then
-	skip_all='skipping reftable tests; set GIT_TEST_DEFAULT_REF_FORMAT=reftable'
-	test_done
-fi
-
 INVALID_OID=$(test_oid 001)
 
 test_expect_success 'init: creates basic reftable structures' '
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 11/12] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

In `test_expect_perms()` we assign the output of a command to a variable
declared via `local`. To assert that the command is actually successful
we also chain it with `&&`. This construct is seemingly not portable and
may fail with "local: 1: bad variable name".

Split up the variable declaration and assignment to fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0610-reftable-basics.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index aa9282007c..3b1aa99e7c 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -80,8 +80,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
 test_expect_perms () {
 	local perms="$1"
 	local file="$2"
-	local actual=$(ls -l "$file") &&
+	local actual
 
+	actual=$(ls -l "$file") &&
 	case "$actual" in
 	$perms*)
 		: happy
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

While the reftable format is a recent introduction in Git, JGit already
knows to read and write reftables since 2017. Given the complexity of
the format there is a very real risk of incompatibilities between those
two implementations, which is something that we really want to avoid.

Add some basic tests that verify that reftables written by Git and JGit
can be read by the respective other implementation. For now this test
suite is rather small, only covering basic functionality. But it serves
as a good starting point and can be extended over time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0612-reftable-jgit-compatibility.sh | 115 +++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
new file mode 100755
index 0000000000..f25ea880a0
--- /dev/null
+++ b/t/t0612-reftable-jgit-compatibility.sh
@@ -0,0 +1,115 @@
+#!/bin/sh
+
+test_description='reftables are compatible with JGit'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
+
+# JGit does not support the 'link' DIRC extension.
+GIT_TEST_SPLIT_INDEX=0
+export GIT_TEST_SPLIT_INDEX
+
+. ./test-lib.sh
+
+if ! test_have_prereq JGIT
+then
+	skip_all='skipping reftable JGit tests'
+	test_done
+fi
+
+if ! test_have_prereq SHA1
+then
+	skip_all='skipping reftable JGit tests; JGit does not support SHA256 reftables'
+	test_done
+fi
+
+test_commit_jgit () {
+	touch "$1" &&
+	jgit add "$1" &&
+	jgit commit -m "$1"
+}
+
+test_same_refs () {
+	git show-ref --head >cgit.actual &&
+	jgit show-ref >jgit-tabs.actual &&
+	tr "\t" " " <jgit-tabs.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_reflog () {
+	git reflog "$*" >cgit.actual &&
+	jgit reflog "$*" >jgit-newline.actual &&
+	sed '/^$/d' <jgit-newline.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_expect_success 'CGit repository can be read by JGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_same_refs &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit repository can be read by CGit' '
+	test_when_finished "rm -rf repo" &&
+	# JGit does not provide a way to create a reftable-enabled repository.
+	git init repo &&
+	(
+		cd repo &&
+		touch file &&
+		jgit add file &&
+		jgit commit -m "initial commit" &&
+
+		test_same_refs &&
+		# Interestingly, JGit cannot read its own reflog here. CGit can
+		# though.
+		printf "%s HEAD@{0}: commit (initial): initial commit" "$(git rev-parse --short HEAD)" >expect &&
+		git reflog HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'mixed writes from JGit and CGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		test_commit_jgit B &&
+		test_commit C &&
+		test_commit_jgit D &&
+
+		test_same_refs &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit can read multi-level index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		awk "
+		    BEGIN {
+			print \"start\";
+			for (i = 0; i < 10000; i++)
+			    printf \"create refs/heads/branch-%d HEAD\n\", i;
+			print \"commit\";
+		    }
+		" >input &&
+		git update-ref --stdin <input &&
+
+		test_same_refs
+	)
+'
+
+test_done
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 00/12] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Thu, Apr 04, 2024 at 03:25:00PM +0200, Patrick Steinhardt wrote:
> Hi,
> 
> while the reftable backend is a recent addition to Git, it has been part
> of JGit since 2017 already. Given that there are essentially two
> different implementations of the reftable format now there is a very
> real risk of these two diverge and become incompatible with each other,
> which would be a shame.
> 
> This patch series addresses this risk by introducing compatibility tests
> which assert that both Git and JGit can access reftables written by the
> respective other implementation.
> 
> The patch series is structured as follows:
> 
>   - Patches 1-8 merge "install-docker-dependencies.sh" into
>     "install-dependencies.sh". This is done so that both CI job flavors
>     have the same dependencies and thus the same test coverage available
>     without always having to maintain them both.
> 
>   - Patch 9 makes JGit available.
> 
>   - Patch 10 starts running backend-specific tests in all jobs, and
>     patch 11 addresses a portability issue surfaced by this.
> 
>   - Patch 12 adds a very basic compatibility test suite for Git/JGit
>     reftables. I mostly consider this as a proof of concept, it should
>     likely be extended over time.
> 
> These compatibility tests surface three findings:
> 
>   - JGit does not support reftables format v2, which was added to
>     support the SHA256 object format.
> 
>   - JGit cannot read reflogs written by itself when starting from an
>     unborn branch. This smells like a bug in JGit to me where it
>     misinterprets reflog entries with a zero object ID as new OID, but I
>     didn't dig any deeper yet.
> 
>   - JGit is incompatible with split indices because it cannot handle
>     'link' DIRC entries. This is unrelated to reftables though.
> 
> I have tested the CI changes against both GitLab [1] and GitHub [2]. The
> macOS test failures on GitHub are caused by the recent curl regression.
> 
> [1]: https://gitlab.com/gitlab-org/git/-/merge_requests/123
> [2]: https://github.com/git/git/pull/1696
> 
> Patrick

Hrmpf, Cc'ing the jgit-dev@ list didn't work out as all my mails got
refused there due to myself not being signed up. Oh, well..

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Han-Wen Nienhuys]

On Thu, Apr 4, 2024 at 5:01 PM Patrick Steinhardt <ps@pks.im> wrote:
> +
> +test_same_refs () {
> +       git show-ref --head >cgit.actual &&
> +       jgit show-ref >jgit-tabs.actual &&

This seeks to the start and then iterates to the end, likely skipping
indexes. If you want to test for indexes etc. you should also sample
random refs from the namespace and see if they are returned correctly.

> +test_expect_success 'JGit repository can be read by CGit' '
> +       test_when_finished "rm -rf repo" &&
> +       # JGit does not provide a way to create a reftable-enabled repository.

You can do "jgit init && jgit convert-refstorage --format=reftable"

-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: [PATCH 11/12] t0610: fix non-portable variable assignment

[Eric Sunshine]

On Fri, Apr 5, 2024 at 3:51 AM Patrick Steinhardt <ps@pks.im> wrote:
> In `test_expect_perms()` we assign the output of a command to a variable
> declared via `local`. To assert that the command is actually successful
> we also chain it with `&&`. This construct is seemingly not portable and
> may fail with "local: 1: bad variable name".
>
> Split up the variable declaration and assignment to fix this.

Under what configuration, circumstances, conditions did you encounter
this problem? I ask because this isn't the only such case in the test
suite, as shown by:

    git grep -nP 'local ..*=\$\(..*&&' -- t

What makes this case distinct from the others? Including such
information would improve the commit message and help future readers.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> @@ -80,8 +80,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
>  test_expect_perms () {
>         local perms="$1"
>         local file="$2"
> -       local actual=$(ls -l "$file") &&
> +       local actual
>
> +       actual=$(ls -l "$file") &&

Re: [PATCH 11/12] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

On Fri, Apr 05, 2024 at 05:14:34AM -0400, Eric Sunshine wrote:
> On Fri, Apr 5, 2024 at 3:51 AM Patrick Steinhardt <ps@pks.im> wrote:
> > In `test_expect_perms()` we assign the output of a command to a variable
> > declared via `local`. To assert that the command is actually successful
> > we also chain it with `&&`. This construct is seemingly not portable and
> > may fail with "local: 1: bad variable name".
> >
> > Split up the variable declaration and assignment to fix this.
> 
> Under what configuration, circumstances, conditions did you encounter
> this problem? I ask because this isn't the only such case in the test
> suite, as shown by:
> 
>     git grep -nP 'local ..*=\$\(..*&&' -- t
> 
> What makes this case distinct from the others? Including such
> information would improve the commit message and help future readers.

I only ever saw this in some subset of GitHub CI jobs. See e.g. [1],
where the logs [2] show the following error:

```
ok 6 - init: reinitializing reftable with files backend fails
+ test_when_finished rm -rf repo
+ test 0 = 0
+ test_cleanup={ rm -rf repo
                } && (exit "$eval_ret"); eval_ret=$?; :
+ umask 002
+ git init --shared=true repo
Initialized empty shared Git repository in /home/runner/work/git/git/t/trash directory.t0610-reftable-basics/repo/.git/
+ git -C repo config core.sharedrepository
+ test 1 = 1
+ test_expect_perms -rw-rw-r-- repo/.git/reftable/tables.list
+ local perms=-rw-rw-r--
+ local file=repo/.git/reftable/tables.list
+ ls -l repo/.git/reftable/tables.list
+ local actual=-rw-rw-r-- 1 runner docker 43 Apr 4 11:55 repo/.git/reftable/tables.list
t0610-reftable-basics.sh: 83: local: 1: bad variable name
+ die
+ code=2
+ test_atexit_handler
+ test : != :
+ return 0
+ test -n
+ echo FATAL: Unexpected exit with code 2
FATAL: Unexpected exit with code 2
```

I was a bit surprised by this, too, and cannot reproduce it with any of
my systems. But I would bet this is dependent on the actual version of
your shell  because it only happened with the Ubuntu 20.04 and 16.04
jobs.

I didn't dig much deeper though as the fix is easy enough, even though
it is surprising.

[1]: https://github.com/git/git/actions/runs/8554157462/job/23438877643
[2]: https://github.com/git/git/actions/runs/8554157462/artifacts/1384620962

Patrick

> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> > @@ -80,8 +80,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
> >  test_expect_perms () {
> >         local perms="$1"
> >         local file="$2"
> > -       local actual=$(ls -l "$file") &&
> > +       local actual
> >
> > +       actual=$(ls -l "$file") &&

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Thu, Apr 04, 2024 at 11:40:29PM +0200, Han-Wen Nienhuys wrote:
> On Thu, Apr 4, 2024 at 5:01 PM Patrick Steinhardt <ps@pks.im> wrote:
> > +
> > +test_same_refs () {
> > +       git show-ref --head >cgit.actual &&
> > +       jgit show-ref >jgit-tabs.actual &&
> 
> This seeks to the start and then iterates to the end, likely skipping
> indexes. If you want to test for indexes etc. you should also sample
> random refs from the namespace and see if they are returned correctly.

True, I'll add that.

> > +test_expect_success 'JGit repository can be read by CGit' '
> > +       test_when_finished "rm -rf repo" &&
> > +       # JGit does not provide a way to create a reftable-enabled repository.
> 
> You can do "jgit init && jgit convert-refstorage --format=reftable"

Perfect, thanks for this hint!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 11/12] t0610: fix non-portable variable assignment

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>  test_expect_perms () {
>  	local perms="$1"
>  	local file="$2"
> -	local actual=$(ls -l "$file") &&
> +	local actual
>  
> +	actual=$(ls -l "$file") &&

Isn't this the same as what ebee5580 (parallel-checkout: avoid dash
local bug in tests, 2021-06-06) fixed?

The rule for variable assignment is mishandled when local is
involved by some shells.

	perms=$1
	file=$2
	actual=$(ls -l "$file")

would be perfectly fine, and should be fine with "local" prefixed on
these lines, but the last one with local without "" qround $(...)
incorrectly makes the substitution subject to field splitting.

I think the right fix should look rather like this, instead.

 t/t0610-reftable-basics.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git i/t/t0610-reftable-basics.sh w/t/t0610-reftable-basics.sh
index 686781192e..894896933e 100755
--- i/t/t0610-reftable-basics.sh
+++ w/t/t0610-reftable-basics.sh
@@ -81,9 +81,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
 '
 
 test_expect_perms () {
-	local perms="$1"
-	local file="$2"
-	local actual=$(ls -l "$file") &&
+	local perms="$1" &&
+	local file="$2" &&
+	local actual="$(ls -l "$file")" &&
 
 	case "$actual" in
 	$perms*)

[PATCH] CodingGuidelines: quote assigned value with "local" and "export"

[Junio C Hamano]

Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
lets the shell erroneously perform field splitting on the expansion
of a command substitution during declaration of a local or an extern
variable.

The explanation was stolen from ebee5580 (parallel-checkout: avoid
dash local bug in tests, 2021-06-06).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * The coding guidelines now forbids use of local/export var=$val
   without having $val quoted inside a pair of double quotes to
   avoid portability bugs.

> Isn't this the same as what ebee5580 (parallel-checkout: avoid dash
> local bug in tests, 2021-06-06) fixed?

 Documentation/CodingGuidelines | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git i/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
index 32e69f798e..8aa76094ca 100644
--- i/Documentation/CodingGuidelines
+++ w/Documentation/CodingGuidelines
@@ -188,6 +188,20 @@ For shell scripts specifically (not exhaustive):
    hopefully nobody starts using "local" before they are reimplemented
    in C ;-)
 
+ - Some versions of dash has broken variable assignment when prefixed
+   with "local", "export", and "readonly", in that the value to be
+   assigned goes through field splitting at $IFS unless quoted.  
+
+   DO NOT write:
+
+     local variable=$value           ;# wrong
+     export variable=$(command args) ;# wrong
+
+   and instead write:
+
+     local variable="$value"
+     export variable="$(command args)"
+
  - Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g.
    "\xc2\xa2") in printf format strings, since hexadecimal escape
    sequences are not portable.

Re: [PATCH] CodingGuidelines: quote assigned value with "local" and "export"

[Junio C Hamano]

Somebody may want to inspect the output from

    $ git grep -E -i -e '^	*(local|export) [a-z0-9_]*=\$'

and fix those that can be used with dash.  I made a cursory scan and
removed obviously safe ones whose RHS will never have $IFS whitespaces,
and the following are remainders.  #leftoverbits

t/t0610-reftable-basics.sh:	local actual=$(ls -l "$file") &&
t/test-lib-functions.sh:	local file=${2:-"$1.t"} &&
t/test-lib-functions.sh:	local basename=${1#??}
t/test-lib-functions.sh:	local var=$1 port
t/test-lib-functions.sh:	local expr=$(printf '"%s",' "$@")
t/test-lib-functions.sh:	local inc=${2:-0} &&
t/test-lib-functions.sh:	local inc=${2:-0} &&
t/test-lib-functions.sh:	local ret=$?

Re: [PATCH 11/12] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

On Fri, Apr 05, 2024 at 09:02:47AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >  test_expect_perms () {
> >  	local perms="$1"
> >  	local file="$2"
> > -	local actual=$(ls -l "$file") &&
> > +	local actual
> >  
> > +	actual=$(ls -l "$file") &&
> 
> Isn't this the same as what ebee5580 (parallel-checkout: avoid dash
> local bug in tests, 2021-06-06) fixed?
> 
> The rule for variable assignment is mishandled when local is
> involved by some shells.
> 
> 	perms=$1
> 	file=$2
> 	actual=$(ls -l "$file")
> 
> would be perfectly fine, and should be fine with "local" prefixed on
> these lines, but the last one with local without "" qround $(...)
> incorrectly makes the substitution subject to field splitting.
> 
> I think the right fix should look rather like this, instead.

Ah, interesting, thanks for the pointer! I'll send v2 of this series
early next week.

Patrick

>  t/t0610-reftable-basics.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git i/t/t0610-reftable-basics.sh w/t/t0610-reftable-basics.sh
> index 686781192e..894896933e 100755
> --- i/t/t0610-reftable-basics.sh
> +++ w/t/t0610-reftable-basics.sh
> @@ -81,9 +81,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
>  '
>  
>  test_expect_perms () {
> -	local perms="$1"
> -	local file="$2"
> -	local actual=$(ls -l "$file") &&
> +	local perms="$1" &&
> +	local file="$2" &&
> +	local actual="$(ls -l "$file")" &&
>  
>  	case "$actual" in
>  	$perms*)
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] CodingGuidelines: quote assigned value with "local" and "export"

[Jeff King]

On Fri, Apr 05, 2024 at 09:12:34AM -0700, Junio C Hamano wrote:

> Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
> lets the shell erroneously perform field splitting on the expansion
> of a command substitution during declaration of a local or an extern
> variable.
> 
> The explanation was stolen from ebee5580 (parallel-checkout: avoid
> dash local bug in tests, 2021-06-06).

Thanks for digging up that commit. I read the earlier part of the thread
and went off on a wild goose chase in the archive. :)

> + - Some versions of dash has broken variable assignment when prefixed
> +   with "local", "export", and "readonly", in that the value to be
> +   assigned goes through field splitting at $IFS unless quoted.  
> +
> +   DO NOT write:
> +
> +     local variable=$value           ;# wrong
> +     export variable=$(command args) ;# wrong
> +
> +   and instead write:
> +
> +     local variable="$value"
> +     export variable="$(command args)"

I think that is a good rule for "local", but I thought we did not allow
"export foo=bar" at all, and required:

  foo=bar
  export foo

If that was only because of this bug, it would be nice to loosen the
rules a bit.

-Peff

Re: [PATCH] CodingGuidelines: quote assigned value with "local" and "export"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> + - Some versions of dash has broken variable assignment when prefixed
>> +   with "local", "export", and "readonly", in that the value to be
>> +   assigned goes through field splitting at $IFS unless quoted.  
>> +
>> +   DO NOT write:
>> +
>> +     local variable=$value           ;# wrong
>> +     export variable=$(command args) ;# wrong
>> +
>> +   and instead write:
>> +
>> +     local variable="$value"
>> +     export variable="$(command args)"
>
> I think that is a good rule for "local", but I thought we did not allow
> "export foo=bar" at all, and required:
>
>   foo=bar
>   export foo
>
> If that was only because of this bug, it would be nice to loosen the
> rules a bit.

That rule in Documentation/CodingGuidelines predates the discovery
of this bug.  I have this vague feeling that it was for the shell on
old Solaris, which would not matter to us anymore, but I do not
remember.

As we are not showing "readonly" in the "DO NOT/DO" example above,
we should probably drop the "export" example and discuss it
separately and decide if it makes sense to loosen the "export var"
vs "export var=val" rule.

Thanks.

Re: [PATCH 08/12] ci: make Perforce binaries executable for all users

[Josh Steadmon]

On 2024.04.04 15:25, Patrick Steinhardt wrote:
> The Perforce binaries are only made executable for the current user. On
> GitLab CI though we execute tests as a different user than "root", and
> thus these binaries may not be executable by that test user.
> 
> Fix the setup so that we set the executable bits for all users.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  ci/install-dependencies.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index f4eb125fd2..068c478025 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -47,7 +47,7 @@ ubuntu-*)
>  	mkdir --parents "$CUSTOM_PATH"
>  	wget --quiet --directory-prefix="$CUSTOM_PATH" \
>  		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
> -	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
> +	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
>  
>  	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
>  	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
> -- 
> 2.44.GIT
> 

Do we break CI in patch 6 and 7 until we get this fix? Perhaps we should
just squash this into patch 6?

Re: [PATCH] CodingGuidelines: quote assigned value with "local" and "export"

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> I think that is a good rule for "local", but I thought we did not allow
>> "export foo=bar" at all, and required:
>>
>>   foo=bar
>>   export foo
>>
>> If that was only because of this bug, it would be nice to loosen the
>> rules a bit.
>
> That rule in Documentation/CodingGuidelines predates the discovery
> of this bug.  I have this vague feeling that it was for the shell on
> old Solaris, which would not matter to us anymore, but I do not
> remember.

Heh, I do not even see any such rule in the guidelines.  What
enforces it is actually in t/check-non-portable-shell.pl script.  It
came from 9968ffff (test-lint: detect 'export FOO=bar', 2013-07-08),
which in turn comes from https://lore.kernel.org/git/201307081121.22769.tboegi@web.de/

We make multiple uses of it in ci/*.sh but the environments ci/
scripts are used in are rather sterile, so they do not quite count
as a proof that the problematic shells no longer exist.

We may instead want to add a separate rule e.g.,

	/\blocal\s+[a-zA-z0-9_]*=\$/ and err q(quote "$val" in 'local var=$val');

to the check script.

Re: [PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Han-Wen Nienhuys]

On Fri, Apr 5, 2024 at 11:41 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > +test_expect_success 'JGit repository can be read by CGit' '
> > > +       test_when_finished "rm -rf repo" &&
> > > +       # JGit does not provide a way to create a reftable-enabled repository.
> >
> > You can do "jgit init && jgit convert-refstorage --format=reftable"
>
> Perfect, thanks for this hint!

on a tangent: I wrote this a long time ago, and it does no locking,
which probably leads to unpleasant surprises if it is run in parallel
with other ref updates. What would be a foolproof way to stop other
processes from stomping over the repo while the conversion is ongoing?
Maybe create a packed-refs.lock file?


-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen

Re: [PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Sat, Apr 06, 2024 at 03:03:20PM +0200, Han-Wen Nienhuys wrote:
> On Fri, Apr 5, 2024 at 11:41 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > > +test_expect_success 'JGit repository can be read by CGit' '
> > > > +       test_when_finished "rm -rf repo" &&
> > > > +       # JGit does not provide a way to create a reftable-enabled repository.
> > >
> > > You can do "jgit init && jgit convert-refstorage --format=reftable"
> >
> > Perfect, thanks for this hint!
> 
> on a tangent: I wrote this a long time ago, and it does no locking,
> which probably leads to unpleasant surprises if it is run in parallel
> with other ref updates. What would be a foolproof way to stop other
> processes from stomping over the repo while the conversion is ongoing?
> Maybe create a packed-refs.lock file?

Yes, this is indeed a problem that I have been thinking about quite a
lot recently as I want to introduce migration tooling in the next
release cycle. I was quite happy to learn that you've got something in
JGit and was hoping that you found a way to avoid races. Too bad you
seem to have the same issue as I do right now.

In any case, creating a "packed-refs.lock" is not sufficient. While it
does protect against ref deletion and repacking refs, it doesn't protect
against ref creation and updates. And in fact there is no way to guard
against these at all as all that the "files" backend does is to create
or update a single loose file without taking any central lock.

I was briefly thinking about a "migration" repository extension. Once
the migration starts we add "extensions.ref-migration" to the repo,
which is an extension that no client knows how to handle. Consequently,
while the extension is set, any _new_ Git processes would bail out and
refuse to update the repository.

That only solves part of the problem though. What it doesn't protect
against is already-running Git processes that have already read the
config.

There are of course very hacky ways to get closer to the goal. E.g.:

    1. Repack all refs.

    2. Delete the now-empty "refs/" hierarchy.

    3. Create a file "refs" with invalid contents.

    4. Lock the "packed-refs" file.

    5. Lock the "HEAD" file.

Now you can be sure that there is no way to create or update normal
refs. But it's still incomplete as pseudo-refs can be written to. And it
does feel very fragile overall, especially if you need to roll back the
migration due to whatever reason.

So I dunno, until now I didn't yet find any way to reliably avoid racy
ref updates during a conversion. If you or anybody else got some ideas
I'd be very happy to hear about them.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] CodingGuidelines: quote assigned value with "local" and "export"

[Jeff King]

On Fri, Apr 05, 2024 at 04:15:57PM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> >> I think that is a good rule for "local", but I thought we did not allow
> >> "export foo=bar" at all, and required:
> >>
> >>   foo=bar
> >>   export foo
> >>
> >> If that was only because of this bug, it would be nice to loosen the
> >> rules a bit.
> >
> > That rule in Documentation/CodingGuidelines predates the discovery
> > of this bug.  I have this vague feeling that it was for the shell on
> > old Solaris, which would not matter to us anymore, but I do not
> > remember.
> 
> Heh, I do not even see any such rule in the guidelines.  What
> enforces it is actually in t/check-non-portable-shell.pl script.  It
> came from 9968ffff (test-lint: detect 'export FOO=bar', 2013-07-08),
> which in turn comes from https://lore.kernel.org/git/201307081121.22769.tboegi@web.de/

Yeah, I noticed it was not in CodingGuidelines, but did not even realize
it was in the linter. Thanks for digging up the link, though sadly it
does not say which shell had a problem. I could very well believe there
is no such modern shell, but I don't know how to test that without a
weather balloon patch.

> We make multiple uses of it in ci/*.sh but the environments ci/
> scripts are used in are rather sterile, so they do not quite count
> as a proof that the problematic shells no longer exist.
> 
> We may instead want to add a separate rule e.g.,
> 
> 	/\blocal\s+[a-zA-z0-9_]*=\$/ and err q(quote "$val" in 'local var=$val');
> 
> to the check script.

Yeah. I think matching \$ is probably enough to catch most relevant
cases without complaining about the innocuous:

  local foo=bar

It would miss:

  local foo="bar/$1"

I guess "=[^"]*\$" would be a bit more aggressive.

-Peff

Re: [PATCH 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Thu, Apr 04, 2024 at 11:40:29PM +0200, Han-Wen Nienhuys wrote:
> On Thu, Apr 4, 2024 at 5:01 PM Patrick Steinhardt <ps@pks.im> wrote:
> > +test_expect_success 'JGit repository can be read by CGit' '
> > +       test_when_finished "rm -rf repo" &&
> > +       # JGit does not provide a way to create a reftable-enabled repository.
> 
> You can do "jgit init && jgit convert-refstorage --format=reftable"

By the way, the above command does not work as-is as it will end up with
a repository that has no "HEAD" reference. I can work around that
though by first writing the default branch and then converting the ref
storage.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/12] ci: make Perforce binaries executable for all users

[Patrick Steinhardt]

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

On Fri, Apr 05, 2024 at 01:01:52PM -0700, Josh Steadmon wrote:
> On 2024.04.04 15:25, Patrick Steinhardt wrote:
> > The Perforce binaries are only made executable for the current user. On
> > GitLab CI though we execute tests as a different user than "root", and
> > thus these binaries may not be executable by that test user.
> > 
> > Fix the setup so that we set the executable bits for all users.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  ci/install-dependencies.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> > index f4eb125fd2..068c478025 100755
> > --- a/ci/install-dependencies.sh
> > +++ b/ci/install-dependencies.sh
> > @@ -47,7 +47,7 @@ ubuntu-*)
> >  	mkdir --parents "$CUSTOM_PATH"
> >  	wget --quiet --directory-prefix="$CUSTOM_PATH" \
> >  		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
> > -	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
> > +	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
> >  
> >  	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> >  	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
> > -- 
> > 2.44.GIT
> > 
> 
> Do we break CI in patch 6 and 7 until we get this fix? Perhaps we should
> just squash this into patch 6?

No, this was broken before already as we had the same "chmod u+x" even
without this patch series. This doesn't lead to a broken CI system
though as Perforce is an optional dependency. Instead it causes us to
skip all Perforce tests because we won't be able to look up these
binaries via PATH.

I'll try to clarify the commit message.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 00/12] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

Hi,

this is the second version of my patch series that adds compatibility
tests for reftables to check whether the Git and JGit implementations
are compatible with each other.

Changes compared to v1:

    - Patch 8: Clarified that this has been broken for a rather long
      time, but that it was "silently" broken.

    - Patch 11: adapt the fix for the non-portable "local" variable
      assignment based on the discussion.

    - Patch 12: extend tests to do some basic ref comparisons, which
      should exercise indices more thoroughly.

CI runs for this series:

    - https://github.com/git/git/actions/runs/8595241646/
    - https://gitlab.com/gitlab-org/git/-/pipelines/1243766428

Thanks!

Patrick

Patrick Steinhardt (12):
  ci: rename "runs_on_pool" to "distro"
  ci: expose distro name in dockerized GitHub jobs
  ci: allow skipping sudo on dockerized jobs
  ci: drop duplicate package installation for "linux-gcc-default"
  ci: convert "install-dependencies.sh" to use "/bin/sh"
  ci: merge custom PATH directories
  ci: merge scripts which install dependencies
  ci: make Perforce binaries executable for all users
  ci: install JGit dependency
  t06xx: always execute backend-specific tests
  t0610: fix non-portable variable assignment
  t0612: add tests to exercise Git/JGit reftable compatibility

 .github/workflows/main.yml             |   8 +-
 .gitlab-ci.yml                         |   4 +-
 ci/install-dependencies.sh             | 100 +++++++++++++------
 ci/install-docker-dependencies.sh      |  46 ---------
 ci/lib.sh                              |  14 +--
 t/t0600-reffiles-backend.sh            |   8 +-
 t/t0601-reffiles-pack-refs.sh          |   9 +-
 t/t0610-reftable-basics.sh             |  15 ++-
 t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
 9 files changed, 226 insertions(+), 110 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

Range-diff against v1:
 1:  e618129549 =  1:  89723b6812 ci: rename "runs_on_pool" to "distro"
 2:  e3e2b7cd50 =  2:  e60a40bd65 ci: expose distro name in dockerized GitHub jobs
 3:  8abc9ad6a7 =  3:  16603d40fd ci: allow skipping sudo on dockerized jobs
 4:  7cf2538625 =  4:  b4f6d6d3bf ci: drop duplicate package installation for "linux-gcc-default"
 5:  38e64224e2 =  5:  6abc53bf51 ci: convert "install-dependencies.sh" to use "/bin/sh"
 6:  196dab460a =  6:  d9be4db56f ci: merge custom PATH directories
 7:  668553e18f =  7:  4a90c003d1 ci: merge scripts which install dependencies
 8:  22f86f8ccb !  8:  5240046a0f ci: make Perforce binaries executable for all users
    @@ Commit message
     
         The Perforce binaries are only made executable for the current user. On
         GitLab CI though we execute tests as a different user than "root", and
    -    thus these binaries may not be executable by that test user.
    +    thus these binaries may not be executable by that test user at all. This
    +    has gone unnoticed so far because those binaries are optional -- in case
    +    they don't exist we simply skip over tests requiring them.
     
         Fix the setup so that we set the executable bits for all users.
     
 9:  1deded615e =  9:  29ceb623b9 ci: install JGit dependency
10:  51c45c879f = 10:  fc3472cdf3 t06xx: always execute backend-specific tests
11:  c2c2747ff5 ! 11:  cedf5929d1 t0610: fix non-portable variable assignment
    @@ Metadata
      ## Commit message ##
         t0610: fix non-portable variable assignment
     
    -    In `test_expect_perms()` we assign the output of a command to a variable
    -    declared via `local`. To assert that the command is actually successful
    -    we also chain it with `&&`. This construct is seemingly not portable and
    -    may fail with "local: 1: bad variable name".
    +    Older versions of the Dash shell fail to parse `local var=val`
    +    assignments in some cases when `val` is unquoted. Such failures can be
    +    observed e.g. with Ubuntu 20.04 and older, which has a Dash version that
    +    still has this bug.
     
    -    Split up the variable declaration and assignment to fix this.
    +    Such an assignment has been introduced in t0610. The issue wasn't
    +    detected for a while because this test used to only run when the
    +    GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "refatble".
    +    We have dropped that requirement now though, meaning that it runs
    +    unconditionally, inclluding on jobs which use such older versions of
    +    Ubuntu.
    +
    +    We have worked around such issues in the past, e.g. in ebee5580ca
    +    (parallel-checkout: avoid dash local bug in tests, 2021-06-06), by
    +    quoting the `val` side. Apply the same fix to t0610.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## t/t0610-reftable-basics.sh ##
     @@ t/t0610-reftable-basics.sh: test_expect_success 'init: reinitializing reftable with files backend fails' '
    + '
    + 
      test_expect_perms () {
    - 	local perms="$1"
    - 	local file="$2"
    +-	local perms="$1"
    +-	local file="$2"
     -	local actual=$(ls -l "$file") &&
    -+	local actual
    ++	local perms="$1" &&
    ++	local file="$2" &&
    ++	local actual="$(ls -l "$file")" &&
      
    -+	actual=$(ls -l "$file") &&
      	case "$actual" in
      	$perms*)
    - 		: happy
12:  db66dd4155 ! 12:  160b026e69 t0612: add tests to exercise Git/JGit reftable compatibility
    @@ t/t0612-reftable-jgit-compatibility.sh (new)
     +	test_cmp cgit.actual jgit.actual
     +}
     +
    ++test_same_ref () {
    ++	git rev-parse "$1" >cgit.actual &&
    ++	jgit rev-parse "$1" >jgit.actual &&
    ++	test_cmp cgit.actual jgit.actual
    ++}
    ++
     +test_same_reflog () {
     +	git reflog "$*" >cgit.actual &&
     +	jgit reflog "$*" >jgit-newline.actual &&
    @@ t/t0612-reftable-jgit-compatibility.sh (new)
     +		cd repo &&
     +		test_commit A &&
     +		test_same_refs &&
    ++		test_same_ref HEAD &&
     +		test_same_reflog HEAD
     +	)
     +'
     +
     +test_expect_success 'JGit repository can be read by CGit' '
     +	test_when_finished "rm -rf repo" &&
    -+	# JGit does not provide a way to create a reftable-enabled repository.
    -+	git init repo &&
    ++	jgit init repo &&
     +	(
     +		cd repo &&
    ++
     +		touch file &&
     +		jgit add file &&
     +		jgit commit -m "initial commit" &&
     +
    ++		# Note that we must convert the ref storage after we have
    ++		# written the default branch. Otherwise JGit will end up with
    ++		# no HEAD at all.
    ++		jgit convert-ref-storage --format=reftable &&
    ++
     +		test_same_refs &&
    ++		test_same_ref HEAD &&
     +		# Interestingly, JGit cannot read its own reflog here. CGit can
     +		# though.
     +		printf "%s HEAD@{0}: commit (initial): initial commit" "$(git rev-parse --short HEAD)" >expect &&
    @@ t/t0612-reftable-jgit-compatibility.sh (new)
     +		test_commit_jgit D &&
     +
     +		test_same_refs &&
    ++		test_same_ref HEAD &&
     +		test_same_reflog HEAD
     +	)
     +'
    @@ t/t0612-reftable-jgit-compatibility.sh (new)
     +		" >input &&
     +		git update-ref --stdin <input &&
     +
    -+		test_same_refs
    ++		test_same_refs &&
    ++		test_same_ref refs/heads/branch-1 &&
    ++		test_same_ref refs/heads/branch-5738 &&
    ++		test_same_ref refs/heads/branch-9999
     +	)
     +'
     +

base-commit: 7774cfed6261ce2900c84e55906da708c711d601
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 02/12] ci: expose distro name in dockerized GitHub jobs

[Patrick Steinhardt]

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

Expose a distro name in dockerized jobs. This will be used in a
subsequent commit where we merge the installation scripts for dockerized
and non-dockerized jobs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 684ef5c00d..71cd4e5486 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -342,12 +342,16 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
+          distro: alpine-latest
         - jobname: linux32
           image: daald/ubuntu32:xenial
+          distro: ubuntu32-16.04
         - jobname: pedantic
           image: fedora
+          distro: fedora-latest
     env:
       jobname: ${{matrix.vector.jobname}}
+      distro: ${{matrix.vector.distro}}
     runs-on: ubuntu-latest
     container: ${{matrix.vector.image}}
     steps:
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs

[Patrick Steinhardt]

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

Our "install-dependencies.sh" script is executed by non-dockerized jobs
to install dependencies. These jobs don't run with "root" permissions,
but with a separate user. Consequently, we need to use sudo(8) there to
elevate permissions when installing packages.

We're about to merge "install-docker-dependencies.sh" into that script
though, and our Docker containers do run as "root". Using sudo(8) is
thus unnecessary there, even though it would be harmless. On some images
like Alpine Linux though there is no sudo(8) available by default, which
would consequently break the build.

Adapt the script to make "sudo" a no-op when running as "root" user.
This allows us to easily reuse the script for our dockerized jobs.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7d247b5ef4..7dfd3e50ed 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,6 +11,17 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
+# Make sudo a no-op and execute the command directly when running as root.
+# While using sudo would be fine on most platforms when we are root already,
+# some platforms like e.g. Alpine Linux do not have sudo available by default
+# and would thus break.
+if test "$(id -u)" -eq 0
+then
+	sudo () {
+		"$@"
+	}
+fi
+
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 04/12] ci: drop duplicate package installation for "linux-gcc-default"

[Patrick Steinhardt]

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

The "linux-gcc-default" job installs common Ubuntu packages. This is
already done in the distro-specific switch, so we basically duplicate
the effort here.

Drop the duplicate package installations and inline the variable that
contains those common packages.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7dfd3e50ed..fad53aac96 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -7,9 +7,6 @@
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
-UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
- tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
- libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -25,8 +22,13 @@ fi
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
+	sudo apt-get -q -y install \
+		language-pack-is libsvn-perl apache2 \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
+		$CC_PACKAGE $PYTHON_PACKAGE
+
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -34,6 +36,7 @@ ubuntu-*)
 		chmod u+x p4d
 		chmod u+x p4
 	popd
+
 	mkdir --parents "$GIT_LFS_PATH"
 	pushd "$GIT_LFS_PATH"
 		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
@@ -83,10 +86,6 @@ Documentation)
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
 	;;
-linux-gcc-default)
-	sudo apt-get -q update
-	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
-	;;
 esac
 
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 05/12] ci: convert "install-dependencies.sh" to use "/bin/sh"

[Patrick Steinhardt]

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

We're about to merge the "install-docker-dependencies.sh" script into
"install-dependencies.sh". This will also move our Alpine-based jobs
over to use the latter script. This script uses the Bash shell though,
which is not available by default on Alpine Linux.

Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash.
This requires us to get rid of the pushd/popd invocations, which are
replaced by some more elaborate commands that download or extract
executables right to where they are needed.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index fad53aac96..7bcccc96fd 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 #
 # Install dependencies required to build and test Git on Linux and macOS
 #
@@ -30,19 +30,14 @@ ubuntu-*)
 		$CC_PACKAGE $PYTHON_PACKAGE
 
 	mkdir --parents "$P4_PATH"
-	pushd "$P4_PATH"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
-		chmod u+x p4d
-		chmod u+x p4
-	popd
+	wget --quiet --directory-prefix="$P4_PATH" \
+		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
+	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
 
 	mkdir --parents "$GIT_LFS_PATH"
-	pushd "$GIT_LFS_PATH"
-		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
-	popd
+	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
@@ -53,11 +48,10 @@ macos-*)
 	brew link --force gettext
 
 	mkdir -p "$P4_PATH"
-	pushd "$P4_PATH"
-		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-		tar -xf helix-core-server.tgz &&
-		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
-	popd
+	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
+	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 06/12] ci: merge custom PATH directories

[Patrick Steinhardt]

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

We're downloading various executables required by our tests. Each of
these executables goes into its own directory, which is then appended to
the PATH variable. Consequently, whenever we add a new dependency and
thus a new directory, we would have to adapt to this change in several
places.

Refactor this to instead put all binaries into a single directory.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 +++++++--------
 ci/lib.sh                  | 10 +++-------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7bcccc96fd..46b9efb2d5 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -29,14 +29,13 @@ ubuntu-*)
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
 		$CC_PACKAGE $PYTHON_PACKAGE
 
-	mkdir --parents "$P4_PATH"
-	wget --quiet --directory-prefix="$P4_PATH" \
+	mkdir --parents "$CUSTOM_PATH"
+	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
+	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
-	mkdir --parents "$GIT_LFS_PATH"
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
@@ -47,10 +46,10 @@ macos-*)
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
 
-	mkdir -p "$P4_PATH"
+	mkdir -p "$CUSTOM_PATH"
 	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
-	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	tar -xf helix-core-server.tgz -C "$CUSTOM_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
 	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
diff --git a/ci/lib.sh b/ci/lib.sh
index d882250db5..4cce854bad 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -340,10 +340,6 @@ ubuntu-*)
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
 	export LINUX_GIT_LFS_VERSION="1.5.2"
-
-	P4_PATH="$HOME/custom/p4"
-	GIT_LFS_PATH="$HOME/custom/git-lfs"
-	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
 macos-*)
 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
@@ -351,12 +347,12 @@ macos-*)
 	then
 		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
 	fi
-
-	P4_PATH="$HOME/custom/p4"
-	export PATH="$P4_PATH:$PATH"
 	;;
 esac
 
+CUSTOM_PATH="$HOME/path"
+export PATH="$CUSTOM_PATH:$PATH"
+
 case "$jobname" in
 linux32)
 	CC=gcc
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 07/12] ci: merge scripts which install dependencies

[Patrick Steinhardt]

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

We have two different scripts which install dependencies, one for
dockerized jobs and one for non-dockerized ones. Naturally, these
scripts have quite some duplication. Furthermore, either of these
scripts is missing some test dependencies that the respective other
script has, thus reducing test coverage.

Merge those two scripts such that there is a single source of truth for
test dependencies, only.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml        |  2 +-
 .gitlab-ci.yml                    |  4 +--
 ci/install-dependencies.sh        | 32 ++++++++++++++++++---
 ci/install-docker-dependencies.sh | 46 -------------------------------
 4 files changed, 31 insertions(+), 53 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 71cd4e5486..5838986895 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -359,7 +359,7 @@ jobs:
       if: matrix.vector.jobname != 'linux32'
     - uses: actions/checkout@v1 # cannot be upgraded because Node.js Actions aren't supported in this container
       if: matrix.vector.jobname == 'linux32'
-    - run: ci/install-docker-dependencies.sh
+    - run: ci/install-dependencies.sh
     - run: ci/run-build-and-tests.sh
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c0fa2fe90b..8ce0e1b4bd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -10,7 +10,7 @@ workflow:
 test:linux:
   image: $image
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - useradd builder --create-home
     - chown -R builder "${CI_PROJECT_DIR}"
@@ -98,7 +98,7 @@ static-analysis:
   variables:
     jobname: StaticAnalysis
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - ./ci/run-static-analysis.sh
     - ./ci/check-directional-formatting.bash
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 46b9efb2d5..f4eb125fd2 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,6 +5,8 @@
 
 . ${0%/*}/lib.sh
 
+begin_group "Install dependencies"
+
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 
@@ -20,14 +22,27 @@ then
 fi
 
 case "$distro" in
+alpine-*)
+	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
+		pcre2-dev python3 musl-libintl perl-utils ncurses \
+		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+	;;
+fedora-*)
+	dnf -yq update >/dev/null &&
+	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
+	;;
 ubuntu-*)
+	# Required so that apt doesn't wait for user input on certain packages.
+	export DEBIAN_FRONTEND=noninteractive
+
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
-		language-pack-is libsvn-perl apache2 \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
-		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
-		$CC_PACKAGE $PYTHON_PACKAGE
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
+		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
 
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
@@ -38,6 +53,13 @@ ubuntu-*)
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
+ubuntu32-*)
+	sudo linux32 --32bit i386 sh -c '
+		apt update >/dev/null &&
+		apt install -y build-essential libcurl4-openssl-dev \
+			libssl-dev libexpat-dev gettext python >/dev/null
+	'
+	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
@@ -97,3 +119,5 @@ then
 else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
+
+end_group "Install dependencies"
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
deleted file mode 100755
index eb2c9e1eca..0000000000
--- a/ci/install-docker-dependencies.sh
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/bin/sh
-#
-# Install dependencies required to build and test Git inside container
-#
-
-. ${0%/*}/lib.sh
-
-begin_group "Install dependencies"
-
-case "$jobname" in
-linux32)
-	linux32 --32bit i386 sh -c '
-		apt update >/dev/null &&
-		apt install -y build-essential libcurl4-openssl-dev \
-			libssl-dev libexpat-dev gettext python >/dev/null
-	'
-	;;
-linux-musl)
-	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
-		pcre2-dev python3 musl-libintl perl-utils ncurses \
-		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
-		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
-	;;
-linux-*|StaticAnalysis)
-	# Required so that apt doesn't wait for user input on certain packages.
-	export DEBIAN_FRONTEND=noninteractive
-
-	apt update -q &&
-	apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
-		libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
-		perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
-		libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
-		apache2 cvs cvsps gnupg libcgi-pm-perl subversion
-
-	if test "$jobname" = StaticAnalysis
-	then
-		apt install -q -y coccinelle
-	fi
-	;;
-pedantic)
-	dnf -yq update >/dev/null &&
-	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
-	;;
-esac
-
-end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 08/12] ci: make Perforce binaries executable for all users

[Patrick Steinhardt]

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

The Perforce binaries are only made executable for the current user. On
GitLab CI though we execute tests as a different user than "root", and
thus these binaries may not be executable by that test user at all. This
has gone unnoticed so far because those binaries are optional -- in case
they don't exist we simply skip over tests requiring them.

Fix the setup so that we set the executable bits for all users.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index f4eb125fd2..068c478025 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -47,7 +47,7 @@ ubuntu-*)
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
+	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 09/12] ci: install JGit dependency

[Patrick Steinhardt]

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

We have some tests in t5310 that use JGit to verify that bitmaps can be
read both by Git and by JGit. We do not execute these tests in our CI
jobs though because we don't make JGit available there. Consequently,
the tests basically bitrot because almost nobody is ever going to have
JGit in their path.

Install JGit to plug this test gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 068c478025..3fefe134ea 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -9,6 +9,7 @@ begin_group "Install dependencies"
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
+JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -39,7 +40,7 @@ ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
 		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
 		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
@@ -52,6 +53,9 @@ ubuntu-*)
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+
+	wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit"
+	chmod a+x "$CUSTOM_PATH/jgit"
 	;;
 ubuntu32-*)
 	sudo linux32 --32bit i386 sh -c '
@@ -112,6 +116,7 @@ then
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
+
 if type git-lfs >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
@@ -120,4 +125,12 @@ else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
 
+if type jgit >/dev/null 2>&1
+then
+	echo "$(tput setaf 6)JGit Version$(tput sgr0)"
+	jgit version
+else
+	echo >&2 "WARNING: JGit wasn't installed, see above for clues why"
+fi
+
 end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 10/12] t06xx: always execute backend-specific tests

[Patrick Steinhardt]

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

The tests in t06xx exercise specific ref formats. Next to probing some
basic functionality, these tests also exercise other low-level details
specific to the format. Those tests are only executed though in case
`GIT_TEST_DEFAULT_REF_FORMAT` is set to the ref format of the respective
backend-under-test.

Ideally, we would run the full test matrix for ref formats such that our
complete test suite is executed with every supported format on every
supported platform. This is quite an expensive undertaking though, and
thus we only execute e.g. the "reftable" tests on macOS and Linux. As a
result, we basically have no test coverage for the "reftable" format at
all on other platforms like Windows.

Adapt these tests so that they override `GIT_TEST_DEFAULT_REF_FORMAT`,
which means that they'll always execute. This increases test coverage on
platforms that don't run the full test matrix, which at least gives us
some basic test coverage on those platforms for the "reftable" format.

This of course comes at the cost of running those tests multiple times
on platforms where we do run the full test matrix. But arguably, this is
a good thing because it will also cause us to e.g. run those tests with
the address sanitizer and other non-standard parameters.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0600-reffiles-backend.sh   | 8 ++------
 t/t0601-reffiles-pack-refs.sh | 9 +++------
 t/t0610-reftable-basics.sh    | 9 +++------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 64214340e7..a390cffc80 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -4,16 +4,12 @@ test_description='Test reffiles backend'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	git commit --allow-empty -m Initial &&
 	C=$(git rev-parse HEAD) &&
diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index c309d2bae8..157f79fe52 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -9,18 +9,15 @@ test_description='git pack-refs should not change the branch semantic
 This test runs git pack-refs and git show-ref and checks that the branch
 semantic is still the same.
 '
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'enable reflogs' '
 	git config core.logallrefupdates true
 '
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 686781192e..aa9282007c 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -4,17 +4,14 @@
 #
 
 test_description='reftable basics'
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 . ./test-lib.sh
 
-if ! test_have_prereq REFTABLE
-then
-	skip_all='skipping reftable tests; set GIT_TEST_DEFAULT_REF_FORMAT=reftable'
-	test_done
-fi
-
 INVALID_OID=$(test_oid 001)
 
 test_expect_success 'init: creates basic reftable structures' '
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 11/12] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

Older versions of the Dash shell fail to parse `local var=val`
assignments in some cases when `val` is unquoted. Such failures can be
observed e.g. with Ubuntu 20.04 and older, which has a Dash version that
still has this bug.

Such an assignment has been introduced in t0610. The issue wasn't
detected for a while because this test used to only run when the
GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "refatble".
We have dropped that requirement now though, meaning that it runs
unconditionally, inclluding on jobs which use such older versions of
Ubuntu.

We have worked around such issues in the past, e.g. in ebee5580ca
(parallel-checkout: avoid dash local bug in tests, 2021-06-06), by
quoting the `val` side. Apply the same fix to t0610.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0610-reftable-basics.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index aa9282007c..dfa7e274ea 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -78,9 +78,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
 '
 
 test_expect_perms () {
-	local perms="$1"
-	local file="$2"
-	local actual=$(ls -l "$file") &&
+	local perms="$1" &&
+	local file="$2" &&
+	local actual="$(ls -l "$file")" &&
 
 	case "$actual" in
 	$perms*)
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

While the reftable format is a recent introduction in Git, JGit already
knows to read and write reftables since 2017. Given the complexity of
the format there is a very real risk of incompatibilities between those
two implementations, which is something that we really want to avoid.

Add some basic tests that verify that reftables written by Git and JGit
can be read by the respective other implementation. For now this test
suite is rather small, only covering basic functionality. But it serves
as a good starting point and can be extended over time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
new file mode 100755
index 0000000000..222464e360
--- /dev/null
+++ b/t/t0612-reftable-jgit-compatibility.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='reftables are compatible with JGit'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
+
+# JGit does not support the 'link' DIRC extension.
+GIT_TEST_SPLIT_INDEX=0
+export GIT_TEST_SPLIT_INDEX
+
+. ./test-lib.sh
+
+if ! test_have_prereq JGIT
+then
+	skip_all='skipping reftable JGit tests'
+	test_done
+fi
+
+if ! test_have_prereq SHA1
+then
+	skip_all='skipping reftable JGit tests; JGit does not support SHA256 reftables'
+	test_done
+fi
+
+test_commit_jgit () {
+	touch "$1" &&
+	jgit add "$1" &&
+	jgit commit -m "$1"
+}
+
+test_same_refs () {
+	git show-ref --head >cgit.actual &&
+	jgit show-ref >jgit-tabs.actual &&
+	tr "\t" " " <jgit-tabs.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_ref () {
+	git rev-parse "$1" >cgit.actual &&
+	jgit rev-parse "$1" >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_reflog () {
+	git reflog "$*" >cgit.actual &&
+	jgit reflog "$*" >jgit-newline.actual &&
+	sed '/^$/d' <jgit-newline.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_expect_success 'CGit repository can be read by JGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_same_refs &&
+		test_same_ref HEAD &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit repository can be read by CGit' '
+	test_when_finished "rm -rf repo" &&
+	jgit init repo &&
+	(
+		cd repo &&
+
+		touch file &&
+		jgit add file &&
+		jgit commit -m "initial commit" &&
+
+		# Note that we must convert the ref storage after we have
+		# written the default branch. Otherwise JGit will end up with
+		# no HEAD at all.
+		jgit convert-ref-storage --format=reftable &&
+
+		test_same_refs &&
+		test_same_ref HEAD &&
+		# Interestingly, JGit cannot read its own reflog here. CGit can
+		# though.
+		printf "%s HEAD@{0}: commit (initial): initial commit" "$(git rev-parse --short HEAD)" >expect &&
+		git reflog HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'mixed writes from JGit and CGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		test_commit_jgit B &&
+		test_commit C &&
+		test_commit_jgit D &&
+
+		test_same_refs &&
+		test_same_ref HEAD &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit can read multi-level index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		awk "
+		    BEGIN {
+			print \"start\";
+			for (i = 0; i < 10000; i++)
+			    printf \"create refs/heads/branch-%d HEAD\n\", i;
+			print \"commit\";
+		    }
+		" >input &&
+		git update-ref --stdin <input &&
+
+		test_same_refs &&
+		test_same_ref refs/heads/branch-1 &&
+		test_same_ref refs/heads/branch-5738 &&
+		test_same_ref refs/heads/branch-9999
+	)
+'
+
+test_done
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 01/12] ci: rename "runs_on_pool" to "distro"

[Patrick Steinhardt]

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

The "runs_on_pool" environment variable is used by our CI scripts to
distinguish the different kinds of operating systems. It is quite
specific to GitHub Actions though and not really a descriptive name.

Rename the variable to "distro" to clarify its intent.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 2 +-
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3428773b09..684ef5c00d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -303,7 +303,7 @@ jobs:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      distro: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v4
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index b4e22de3cb..7d247b5ef4 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,7 +11,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0a73fc7bd1..d882250db5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -279,7 +279,7 @@ then
 
 	cache_dir="$HOME/none"
 
-	runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
+	distro=$(echo "$CI_JOB_IMAGE" | tr : -)
 	JOBS=$(nproc)
 else
 	echo "Could not identify CI type" >&2
@@ -318,7 +318,7 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	if test "$jobname" = "linux-gcc-default"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 11/12] t0610: fix non-portable variable assignment

[Eric Sunshine]

On Mon, Apr 8, 2024 at 2:47 AM Patrick Steinhardt <ps@pks.im> wrote:
> Older versions of the Dash shell fail to parse `local var=val`
> assignments in some cases when `val` is unquoted. Such failures can be
> observed e.g. with Ubuntu 20.04 and older, which has a Dash version that
> still has this bug.
>
> Such an assignment has been introduced in t0610. The issue wasn't
> detected for a while because this test used to only run when the
> GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "refatble".
> We have dropped that requirement now though, meaning that it runs
> unconditionally, inclluding on jobs which use such older versions of
> Ubuntu.

s/refatble/reftable/
s/inclluding/including/

> We have worked around such issues in the past, e.g. in ebee5580ca
> (parallel-checkout: avoid dash local bug in tests, 2021-06-06), by
> quoting the `val` side. Apply the same fix to t0610.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Eric Sunshine]

On Mon, Apr 8, 2024 at 2:47 AM Patrick Steinhardt <ps@pks.im> wrote:
> While the reftable format is a recent introduction in Git, JGit already
> knows to read and write reftables since 2017. Given the complexity of
> the format there is a very real risk of incompatibilities between those
> two implementations, which is something that we really want to avoid.
>
> Add some basic tests that verify that reftables written by Git and JGit
> can be read by the respective other implementation. For now this test
> suite is rather small, only covering basic functionality. But it serves
> as a good starting point and can be extended over time.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
> @@ -0,0 +1,132 @@
> +test_expect_success 'JGit can read multi-level index' '
> +               ...
> +               awk "
> +                   BEGIN {
> +                       print \"start\";
> +                       for (i = 0; i < 10000; i++)
> +                           printf \"create refs/heads/branch-%d HEAD\n\", i;
> +                       print \"commit\";
> +                   }
> +               " >input &&

I was going to suggest that you could accomplish this more easily
directly in shell (without employing `awk`):

    {
        echo start &&
        printf "create refs/heads/branch-%d HEAD\n" $(test_seq 0 9999) &&
        echo commit
    } >input &&

but then I realized that that could potentially run afoul of
command-line length limit on some platform due to the 0-9999 sequence.

Re: [PATCH v2 11/12] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

On Mon, Apr 08, 2024 at 02:57:04AM -0400, Eric Sunshine wrote:
> On Mon, Apr 8, 2024 at 2:47 AM Patrick Steinhardt <ps@pks.im> wrote:
> > Older versions of the Dash shell fail to parse `local var=val`
> > assignments in some cases when `val` is unquoted. Such failures can be
> > observed e.g. with Ubuntu 20.04 and older, which has a Dash version that
> > still has this bug.
> >
> > Such an assignment has been introduced in t0610. The issue wasn't
> > detected for a while because this test used to only run when the
> > GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "refatble".
> > We have dropped that requirement now though, meaning that it runs
> > unconditionally, inclluding on jobs which use such older versions of
> > Ubuntu.
> 
> s/refatble/reftable/
> s/inclluding/including/

Thanks, fixed locally. I'll wait for more feedback before sending out a
new version.

Patrick

> > We have worked around such issues in the past, e.g. in ebee5580ca
> > (parallel-checkout: avoid dash local bug in tests, 2021-06-06), by
> > quoting the `val` side. Apply the same fix to t0610.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> I was going to suggest that you could accomplish this more easily
> directly in shell (without employing `awk`):
>
>     {
>         echo start &&
>         printf "create refs/heads/branch-%d HEAD\n" $(test_seq 0 9999) &&
>         echo commit
>     } >input &&
>
> but then I realized that that could potentially run afoul of
> command-line length limit on some platform due to the 0-9999 sequence.

As xargs is supposed to know the system limit, perhaps

	test_seq 0 9999 | xargs printf "...%d...\n"

should work?

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Eric Sunshine]

On Mon, Apr 8, 2024 at 12:07 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > I was going to suggest that you could accomplish this more easily
> > directly in shell (without employing `awk`):
> >
> >     {
> >         echo start &&
> >         printf "create refs/heads/branch-%d HEAD\n" $(test_seq 0 9999) &&
> >         echo commit
> >     } >input &&
> >
> > but then I realized that that could potentially run afoul of
> > command-line length limit on some platform due to the 0-9999 sequence.
>
> As xargs is supposed to know the system limit, perhaps
>
>         test_seq 0 9999 | xargs printf "...%d...\n"
>
> should work?

Hmm, yes, that should work nicely.

Whether or not such a change is worthwhile is a different matter.
Although it is perhaps simpler and easier to read, Windows folk might
not appreciate it since it spawns at least three processes (and
perhaps more depending upon how test_seq is implemented), whereas the
`awk` approach spawns a single process.

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Mon, Apr 08, 2024 at 09:07:32AM -0700, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > I was going to suggest that you could accomplish this more easily
> > directly in shell (without employing `awk`):
> >
> >     {
> >         echo start &&
> >         printf "create refs/heads/branch-%d HEAD\n" $(test_seq 0 9999) &&
> >         echo commit
> >     } >input &&
> >
> > but then I realized that that could potentially run afoul of
> > command-line length limit on some platform due to the 0-9999 sequence.
> 
> As xargs is supposed to know the system limit, perhaps
> 
> 	test_seq 0 9999 | xargs printf "...%d...\n"
> 
> should work?

Is there a reason why we want to avoid using awk(1) in the first place?
We use it in several other tests and I don't think that the resulting
code is all that bad.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Eric Sunshine]

On Mon, Apr 8, 2024 at 12:19 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Mon, Apr 8, 2024 at 12:07 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> > > I was going to suggest that you could accomplish this more easily
> > > directly in shell (without employing `awk`):
> > >
> > >     {
> > >         echo start &&
> > >         printf "create refs/heads/branch-%d HEAD\n" $(test_seq 0 9999) &&
> > >         echo commit
> > >     } >input &&
> > >
> > > but then I realized that that could potentially run afoul of
> > > command-line length limit on some platform due to the 0-9999 sequence.
> >
> > As xargs is supposed to know the system limit, perhaps
> >
> >         test_seq 0 9999 | xargs printf "...%d...\n"
> >
> > should work?
>
> Hmm, yes, that should work nicely.
>
> Whether or not such a change is worthwhile is a different matter.
> Although it is perhaps simpler and easier to read, Windows folk might
> not appreciate it since it spawns at least three processes (and
> perhaps more depending upon how test_seq is implemented), whereas the
> `awk` approach spawns a single process.

And, to be clear, I have no objection to this patch's use of `awk`. It
is "good enough" as is; I was not asking Patrick to make the change,
but rather made the comment in case the idea hadn't occurred to him.

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Jeff King]

On Mon, Apr 08, 2024 at 06:22:10PM +0200, Patrick Steinhardt wrote:

> On Mon, Apr 08, 2024 at 09:07:32AM -0700, Junio C Hamano wrote:
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> > 
> > > I was going to suggest that you could accomplish this more easily
> > > directly in shell (without employing `awk`):
> > >
> > >     {
> > >         echo start &&
> > >         printf "create refs/heads/branch-%d HEAD\n" $(test_seq 0 9999) &&
> > >         echo commit
> > >     } >input &&
> > >
> > > but then I realized that that could potentially run afoul of
> > > command-line length limit on some platform due to the 0-9999 sequence.
> > 
> > As xargs is supposed to know the system limit, perhaps
> > 
> > 	test_seq 0 9999 | xargs printf "...%d...\n"
> > 
> > should work?
> 
> Is there a reason why we want to avoid using awk(1) in the first place?
> We use it in several other tests and I don't think that the resulting
> code is all that bad.

Using awk is fine, but I think in general if we can do something just as
easily without an extra process, that is preferable. Using xargs here
does not to me count as "just as easily". However, using a shell loop
might actually be more readable because you'd avoid the extra quoting.
I.e. either:

  for i in $(test_seq 10000)
  do
	echo "create refs/heads/branch-$i HEAD"
  done

or:

  i=0
  while test $i -lt 10000
  do
	echo "create refs/heads/branch-$i HEAD"
	i=$((i+1))
  done

I think the first one actually incurs an extra process anyway because of
the $(). The second one would not. Of course, the second one probably
needs &&-chaining and a "|| return 1" to work in our test snippet.

IMHO the nicest thing would be if our test_seq could take a format
parameter, like:

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2eccf100c0..c8f32eb409 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1404,6 +1404,13 @@ test_cmp_fspath () {
 # from 1.
 
 test_seq () {
+	local fmt="%d"
+	case "$1" in
+	-f)
+		fmt="$2"
+		shift 2
+		;;
+	esac
 	case $# in
 	1)	set 1 "$@" ;;
 	2)	;;
@@ -1412,7 +1419,7 @@ test_seq () {
 	test_seq_counter__=$1
 	while test "$test_seq_counter__" -le "$2"
 	do
-		echo "$test_seq_counter__"
+		printf "$fmt\n" "$test_seq_counter__"
 		test_seq_counter__=$(( $test_seq_counter__ + 1 ))
 	done
 }

Then you could just write:

  test_seq -f "create refs/heads/branch-%d HEAD" 0 9999

and I suspect there are several other spots which could be simplified as
well.

Anyway, I don't think it is worth derailing your series for this, but
just general food for thought.

-Peff

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> As xargs is supposed to know the system limit, perhaps
>> 
>> 	test_seq 0 9999 | xargs printf "...%d...\n"
>> 
>> should work?
>
> Is there a reason why we want to avoid using awk(1) in the first place?
> We use it in several other tests and I don't think that the resulting
> code is all that bad.

There is no reason.  It was a canned reaction against "gee we will
bust shell's limit" to "use xargs then".

Of course, if we can do the loop in the shell and everything we do
in the loop with shell builtins (printf is often builtin in modern
shells but not always, if I recall correctly), that would be best,
and if the job is _that_ simple that we could do in the shell, it
would make "awk" a horrible choice ;-)

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I.e. either:
>
>   for i in $(test_seq 10000)
>   do
> 	echo "create refs/heads/branch-$i HEAD"
>   done
>
> or:
>
>   i=0
>   while test $i -lt 10000
>   do
> 	echo "create refs/heads/branch-$i HEAD"
> 	i=$((i+1))
>   done
>
> I think the first one actually incurs an extra process anyway because of
> the $(). The second one would not. Of course, the second one probably
> needs &&-chaining and a "|| return 1" to work in our test snippet.
> ...
> Then you could just write:
>
>   test_seq -f "create refs/heads/branch-%d HEAD" 0 9999
>
> and I suspect there are several other spots which could be simplified as
> well.
>
> Anyway, I don't think it is worth derailing your series for this, but
> just general food for thought.

Yup, I agree with everything you said.

Thanks.

Re: [PATCH v2 00/12] t: exercise Git/JGit reftable compatibility

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> CI runs for this series:
>
>     - https://github.com/git/git/actions/runs/8595241646/
>     - https://gitlab.com/gitlab-org/git/-/pipelines/1243766428

Thanks.

When this is queued on 'seen', I seem to be getting test errors from
t0610.  I suspect that it is not a breakage in this series, but the
fact that this series enables reftable tests without setting any
GIT_TEST_ environment variables, that causes an existing breakage
(or two) more visible.  I did not have time to locate which (other)
topic introduces the breakage, though.

Re: [PATCH v2 00/12] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Mon, Apr 08, 2024 at 07:17:08PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > CI runs for this series:
> >
> >     - https://github.com/git/git/actions/runs/8595241646/
> >     - https://gitlab.com/gitlab-org/git/-/pipelines/1243766428
> 
> Thanks.
> 
> When this is queued on 'seen', I seem to be getting test errors from
> t0610.  I suspect that it is not a breakage in this series, but the
> fact that this series enables reftable tests without setting any
> GIT_TEST_ environment variables, that causes an existing breakage
> (or two) more visible.  I did not have time to locate which (other)
> topic introduces the breakage, though.

Interesting, I cannot reproduce any failures with the current state of
seen (17ff004052 (Merge branch 'ps/ci-test-with-jgit' into seen,
2024-04-08)). Was this on your local machine or in CI? Which platform
are you using? Could you maybe provide logs of the failing tests?

Patric

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/12] t: exercise Git/JGit reftable compatibility

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Apr 08, 2024 at 07:17:08PM -0700, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> 
>> > CI runs for this series:
>> >
>> >     - https://github.com/git/git/actions/runs/8595241646/
>> >     - https://gitlab.com/gitlab-org/git/-/pipelines/1243766428
>> 
>> Thanks.
>> 
>> When this is queued on 'seen', I seem to be getting test errors from
>> t0610.  I suspect that it is not a breakage in this series, but the
>> fact that this series enables reftable tests without setting any
>> GIT_TEST_ environment variables, that causes an existing breakage
>> (or two) more visible.  I did not have time to locate which (other)
>> topic introduces the breakage, though.
>
> Interesting, I cannot reproduce any failures with the current state of
> seen (17ff004052 (Merge branch 'ps/ci-test-with-jgit' into seen,
> 2024-04-08)). Was this on your local machine or in CI? Which platform
> are you using? Could you maybe provide logs of the failing tests?

Local execution on some variant of Debian testing.

The tests that fail are t0610.29 and t0610.30, both of which are
"honors core.sharedRepository" with umask 002 and umask 022.
They expect "-rw-rw-r--", but actually get "-rw-rw----" when
the user's umask is 077 (or 007 for that matter).

If you add "umask 002" or "umask 022" near the beginning of the
script (e.g. before including test-lib.sh), the tests pass even when
the user's umask is 077 or 007 when the tests are run, of course, so
there is something the test must be doing in the test repository
before it forces the umask to 002/022.

Re: [PATCH v2 00/12] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Mon, Apr 08, 2024 at 10:34:53PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Mon, Apr 08, 2024 at 07:17:08PM -0700, Junio C Hamano wrote:
> >> Patrick Steinhardt <ps@pks.im> writes:
> >> 
> >> > CI runs for this series:
> >> >
> >> >     - https://github.com/git/git/actions/runs/8595241646/
> >> >     - https://gitlab.com/gitlab-org/git/-/pipelines/1243766428
> >> 
> >> Thanks.
> >> 
> >> When this is queued on 'seen', I seem to be getting test errors from
> >> t0610.  I suspect that it is not a breakage in this series, but the
> >> fact that this series enables reftable tests without setting any
> >> GIT_TEST_ environment variables, that causes an existing breakage
> >> (or two) more visible.  I did not have time to locate which (other)
> >> topic introduces the breakage, though.
> >
> > Interesting, I cannot reproduce any failures with the current state of
> > seen (17ff004052 (Merge branch 'ps/ci-test-with-jgit' into seen,
> > 2024-04-08)). Was this on your local machine or in CI? Which platform
> > are you using? Could you maybe provide logs of the failing tests?
> 
> Local execution on some variant of Debian testing.
> 
> The tests that fail are t0610.29 and t0610.30, both of which are
> "honors core.sharedRepository" with umask 002 and umask 022.
> They expect "-rw-rw-r--", but actually get "-rw-rw----" when
> the user's umask is 077 (or 007 for that matter).
> 
> If you add "umask 002" or "umask 022" near the beginning of the
> script (e.g. before including test-lib.sh), the tests pass even when
> the user's umask is 077 or 007 when the tests are run, of course, so
> there is something the test must be doing in the test repository
> before it forces the umask to 002/022.

Indeed, I can reproduce the failures when I change my own umask. This
issue is unrelated to this patch series -- as you mentioned, it is only
being unmasked now because we increase test exposure by dropping the
prereq.

I'll send a separate patch series to fix this. Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 0/2] t0610: fix umask tests

[Patrick Steinhardt]

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

Hi,

these patches address the issue reported by Junio where running t0610
with an umask of 0027 breaks some tests. The root cause of this is quite
simple: we executed git-pack-refs(1) outside of the shell that had the
modified umask, and thus we ran tests with the umask of the host shell
by accident.

The series fixes this issue and extends the tests to exercise more
umasks than just 002 and 022, which would have caught the issue earlier.

Patrick

Patrick Steinhardt (2):
  t0610: make `--shared=` tests reusable
  t0610: execute git-pack-refs(1) with specified umask

 t/t0610-reftable-basics.sh | 67 ++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 28 deletions(-)

-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/2] t0610: make `--shared=` tests reusable

[Patrick Steinhardt]

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

We have two kinds of `--shared=` tests, one for git-init(1) and one for
git-pack-refs(1). Merge them into a reusable function such that we can
easily add additional testcases with different umasks and flags for the
`--shared=` switch.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0610-reftable-basics.sh | 57 ++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 686781192e..a3269c20e0 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -96,23 +96,46 @@ test_expect_perms () {
 	esac
 }
 
-for umask in 002 022
-do
-	test_expect_success POSIXPERM 'init: honors core.sharedRepository' '
+test_expect_reftable_perms () {
+	local umask="$1"
+	local shared="$2"
+	local expect="$3"
+
+	test_expect_success POSIXPERM "init: honors --shared=$shared with umask $umask" '
 		test_when_finished "rm -rf repo" &&
 		(
 			umask $umask &&
-			git init --shared=true repo &&
+			git init --shared=$shared repo &&
 			test 1 = "$(git -C repo config core.sharedrepository)"
 		) &&
-		test_expect_perms "-rw-rw-r--" repo/.git/reftable/tables.list &&
+		test_expect_perms "$expect" repo/.git/reftable/tables.list &&
 		for table in repo/.git/reftable/*.ref
 		do
-			test_expect_perms "-rw-rw-r--" "$table" ||
+			test_expect_perms "$expect" "$table" ||
 			return 1
 		done
 	'
-done
+
+	test_expect_success POSIXPERM "pack-refs: honors --shared=$shared with umask $umask" '
+		test_when_finished "rm -rf repo" &&
+		(
+			umask $umask &&
+			git init --shared=$shared repo &&
+			test_commit -C repo A &&
+			test_line_count = 3 repo/.git/reftable/tables.list
+		) &&
+		git -C repo pack-refs &&
+		test_expect_perms "$expect" repo/.git/reftable/tables.list &&
+		for table in repo/.git/reftable/*.ref
+		do
+			test_expect_perms "$expect" "$table" ||
+			return 1
+		done
+	'
+}
+
+test_expect_reftable_perms 002 true "-rw-rw-r--"
+test_expect_reftable_perms 022 true "-rw-rw-r--"
 
 test_expect_success 'clone: can clone reftable repository' '
 	test_when_finished "rm -rf repo clone" &&
@@ -371,26 +394,6 @@ test_expect_success 'pack-refs: does not prune non-table files' '
 	test_path_is_file repo/.git/reftable/garbage
 '
 
-for umask in 002 022
-do
-	test_expect_success POSIXPERM 'pack-refs: honors core.sharedRepository' '
-		test_when_finished "rm -rf repo" &&
-		(
-			umask $umask &&
-			git init --shared=true repo &&
-			test_commit -C repo A &&
-			test_line_count = 3 repo/.git/reftable/tables.list
-		) &&
-		git -C repo pack-refs &&
-		test_expect_perms "-rw-rw-r--" repo/.git/reftable/tables.list &&
-		for table in repo/.git/reftable/*.ref
-		do
-			test_expect_perms "-rw-rw-r--" "$table" ||
-			return 1
-		done
-	'
-done
-
 test_expect_success 'packed-refs: writes are synced' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/2] t0610: execute git-pack-refs(1) with specified umask

[Patrick Steinhardt]

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

The tests for git-pack-refs(1) with the `core.sharedRepository` config
execute git-pack-refs(1) outside of the shell that has the expected
umask set. This is wrong because we want to test the behaviour of that
command with different umasks. The issue went unnoticed because most
distributions have a default umask of 0022, and we only ever test with
`--shared=true`, which re-adds the group write bit.

Fix the issue by moving git-pack-refs(1) into the umask'd shell and add
a bunch of test cases that exercise behaviour more thoroughly.

Note that we drop the check for whether `core.sharedRepository` was set
to the correct value to make the test setup a bit easier. We should be
able to rely on git-init(1) doing its thing correctly. Furthermore, to
help readability, we convert tests that pass `--shared=true` to instead
pass the equivalent `--shared=group`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0610-reftable-basics.sh | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index a3269c20e0..8ffecefadc 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -105,8 +105,7 @@ test_expect_reftable_perms () {
 		test_when_finished "rm -rf repo" &&
 		(
 			umask $umask &&
-			git init --shared=$shared repo &&
-			test 1 = "$(git -C repo config core.sharedrepository)"
+			git init --shared=$shared repo
 		) &&
 		test_expect_perms "$expect" repo/.git/reftable/tables.list &&
 		for table in repo/.git/reftable/*.ref
@@ -122,9 +121,9 @@ test_expect_reftable_perms () {
 			umask $umask &&
 			git init --shared=$shared repo &&
 			test_commit -C repo A &&
-			test_line_count = 3 repo/.git/reftable/tables.list
+			test_line_count = 3 repo/.git/reftable/tables.list &&
+			git -C repo pack-refs
 		) &&
-		git -C repo pack-refs &&
 		test_expect_perms "$expect" repo/.git/reftable/tables.list &&
 		for table in repo/.git/reftable/*.ref
 		do
@@ -134,8 +133,17 @@ test_expect_reftable_perms () {
 	'
 }
 
-test_expect_reftable_perms 002 true "-rw-rw-r--"
-test_expect_reftable_perms 022 true "-rw-rw-r--"
+test_expect_reftable_perms 002 umask "-rw-rw-r--"
+test_expect_reftable_perms 022 umask "-rw-r--r--"
+test_expect_reftable_perms 027 umask "-rw-r-----"
+
+test_expect_reftable_perms 002 group "-rw-rw-r--"
+test_expect_reftable_perms 022 group "-rw-rw-r--"
+test_expect_reftable_perms 027 group "-rw-rw----"
+
+test_expect_reftable_perms 002 world "-rw-rw-r--"
+test_expect_reftable_perms 022 world "-rw-rw-r--"
+test_expect_reftable_perms 027 world "-rw-rw-r--"
 
 test_expect_success 'clone: can clone reftable repository' '
 	test_when_finished "rm -rf repo clone" &&
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs

[Toon claes]

> [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs

I think this title is somewhat misleading. While it's true, I don't
think it's limited to dockerized jobs. Something more along the lines of
"allow running install-dependencies.sh as root" would make more sense to
me.

That's the only tiny remark I have on this patch series.

-- 
Toon

Re: [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs

[Junio C Hamano]

Toon claes <toon@iotcl.com> writes:

>> [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs
>
> I think this title is somewhat misleading. While it's true, I don't
> think it's limited to dockerized jobs. Something more along the lines of
> "allow running install-dependencies.sh as root" would make more sense to
> me.

It is true that dockerized is not the essential part of this change;
and it is that we skip sudo when we are already root.  So I agree
with you that the original title is misleading.

"allow running as root" is somehow a bit unsatisfactory, though.  It
sounds as if _we_ were not allowing it before---what was preventing
us from doing so was a system without sudo.

Stepping back a bit, I wonder if install-dependencies.sh will stay
to be the only one among ci/ scripts that need to run things as
root.  If we moved the new logic in this patch to ci/lib.sh that is
included by everybody, then the title of the patch can become a
short and sweet

    ci: skip sudo when we are already root

> That's the only tiny remark I have on this patch series.

Thanks.

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Justin Tobler]

On 24/04/08 08:47AM, Patrick Steinhardt wrote:
> While the reftable format is a recent introduction in Git, JGit already
> knows to read and write reftables since 2017. Given the complexity of
> the format there is a very real risk of incompatibilities between those
> two implementations, which is something that we really want to avoid.
> 
> Add some basic tests that verify that reftables written by Git and JGit
> can be read by the respective other implementation. For now this test
> suite is rather small, only covering basic functionality. But it serves
> as a good starting point and can be extended over time.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100755 t/t0612-reftable-jgit-compatibility.sh
> 
> diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
> new file mode 100755
> index 0000000000..222464e360
> --- /dev/null
> +++ b/t/t0612-reftable-jgit-compatibility.sh
> @@ -0,0 +1,132 @@
> +#!/bin/sh
> +
> +test_description='reftables are compatible with JGit'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +GIT_TEST_DEFAULT_REF_FORMAT=reftable
> +export GIT_TEST_DEFAULT_REF_FORMAT
> +
> +# JGit does not support the 'link' DIRC extension.
> +GIT_TEST_SPLIT_INDEX=0
> +export GIT_TEST_SPLIT_INDEX
> +
> +. ./test-lib.sh
> +
> +if ! test_have_prereq JGIT

Do we want these tests to run in CI? As far as I can tell these tests
would always be skipped.

-Justin

> +then
> +	skip_all='skipping reftable JGit tests'
> +	test_done
> +fi
...

Re: [PATCH v2 05/12] ci: convert "install-dependencies.sh" to use "/bin/sh"

[Justin Tobler]

On 24/04/08 08:46AM, Patrick Steinhardt wrote:
> We're about to merge the "install-docker-dependencies.sh" script into
> "install-dependencies.sh". This will also move our Alpine-based jobs
> over to use the latter script. This script uses the Bash shell though,
> which is not available by default on Alpine Linux.
> 
> Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash.
> This requires us to get rid of the pushd/popd invocations, which are
> replaced by some more elaborate commands that download or extract
> executables right to where they are needed.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  ci/install-dependencies.sh | 28 +++++++++++-----------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index fad53aac96..7bcccc96fd 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env bash
> +#!/bin/sh
>  #
>  # Install dependencies required to build and test Git on Linux and macOS
>  #
> @@ -30,19 +30,14 @@ ubuntu-*)
>  		$CC_PACKAGE $PYTHON_PACKAGE
>  
>  	mkdir --parents "$P4_PATH"
> -	pushd "$P4_PATH"
> -		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
> -		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
> -		chmod u+x p4d
> -		chmod u+x p4
> -	popd
> +	wget --quiet --directory-prefix="$P4_PATH" \
> +		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
> +	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
>  
>  	mkdir --parents "$GIT_LFS_PATH"
> -	pushd "$GIT_LFS_PATH"
> -		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> -		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> -		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
> -	popd
> +	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> +	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"

Do we want to fold this line since it is rather long?

-Justin

Re: [PATCH 0/2] t0610: fix umask tests

[Justin Tobler]

On 24/04/09 11:57AM, Patrick Steinhardt wrote:
> Hi,
> 
> these patches address the issue reported by Junio where running t0610
> with an umask of 0027 breaks some tests. The root cause of this is quite
> simple: we executed git-pack-refs(1) outside of the shell that had the
> modified umask, and thus we ran tests with the umask of the host shell
> by accident.
> 
> The series fixes this issue and extends the tests to exercise more
> umasks than just 002 and 022, which would have caught the issue earlier.
> 
> Patrick
> 
> Patrick Steinhardt (2):
>   t0610: make `--shared=` tests reusable
>   t0610: execute git-pack-refs(1) with specified umask
> 
>  t/t0610-reftable-basics.sh | 67 ++++++++++++++++++++++----------------
>  1 file changed, 39 insertions(+), 28 deletions(-)
> 
> -- 
> 2.44.GIT
> 

Nice fix! I like how the tests were made more extensible too. This looks
good to me :)

-Justin

Re: [PATCH v2 05/12] ci: convert "install-dependencies.sh" to use "/bin/sh"

[Patrick Steinhardt]

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

On Wed, Apr 10, 2024 at 03:46:49PM -0500, Justin Tobler wrote:
> On 24/04/08 08:46AM, Patrick Steinhardt wrote:
> > We're about to merge the "install-docker-dependencies.sh" script into
> > "install-dependencies.sh". This will also move our Alpine-based jobs
> > over to use the latter script. This script uses the Bash shell though,
> > which is not available by default on Alpine Linux.
> > 
> > Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash.
> > This requires us to get rid of the pushd/popd invocations, which are
> > replaced by some more elaborate commands that download or extract
> > executables right to where they are needed.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  ci/install-dependencies.sh | 28 +++++++++++-----------------
> >  1 file changed, 11 insertions(+), 17 deletions(-)
> > 
> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> > index fad53aac96..7bcccc96fd 100755
> > --- a/ci/install-dependencies.sh
> > +++ b/ci/install-dependencies.sh
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/env bash
> > +#!/bin/sh
> >  #
> >  # Install dependencies required to build and test Git on Linux and macOS
> >  #
> > @@ -30,19 +30,14 @@ ubuntu-*)
> >  		$CC_PACKAGE $PYTHON_PACKAGE
> >  
> >  	mkdir --parents "$P4_PATH"
> > -	pushd "$P4_PATH"
> > -		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
> > -		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
> > -		chmod u+x p4d
> > -		chmod u+x p4
> > -	popd
> > +	wget --quiet --directory-prefix="$P4_PATH" \
> > +		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
> > +	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
> >  
> >  	mkdir --parents "$GIT_LFS_PATH"
> > -	pushd "$GIT_LFS_PATH"
> > -		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> > -		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> > -		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
> > -	popd
> > +	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
> > +	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
> 
> Do we want to fold this line since it is rather long?
> 
> -Justin

Yeah, let's.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs

[Patrick Steinhardt]

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

On Wed, Apr 10, 2024 at 10:21:48AM -0700, Junio C Hamano wrote:
> Toon claes <toon@iotcl.com> writes:
> 
> >> [PATCH v2 03/12] ci: allow skipping sudo on dockerized jobs
> >
> > I think this title is somewhat misleading. While it's true, I don't
> > think it's limited to dockerized jobs. Something more along the lines of
> > "allow running install-dependencies.sh as root" would make more sense to
> > me.
> 
> It is true that dockerized is not the essential part of this change;
> and it is that we skip sudo when we are already root.  So I agree
> with you that the original title is misleading.
> 
> "allow running as root" is somehow a bit unsatisfactory, though.  It
> sounds as if _we_ were not allowing it before---what was preventing
> us from doing so was a system without sudo.
> 
> Stepping back a bit, I wonder if install-dependencies.sh will stay
> to be the only one among ci/ scripts that need to run things as
> root.  If we moved the new logic in this patch to ci/lib.sh that is
> included by everybody, then the title of the patch can become a
> short and sweet
> 
>     ci: skip sudo when we are already root

Agreed, this title reads much better than what I had.

> > That's the only tiny remark I have on this patch series.
> 
> Thanks.

Thanks to both of you!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 12/12] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Wed, Apr 10, 2024 at 03:43:58PM -0500, Justin Tobler wrote:
> On 24/04/08 08:47AM, Patrick Steinhardt wrote:
> > While the reftable format is a recent introduction in Git, JGit already
> > knows to read and write reftables since 2017. Given the complexity of
> > the format there is a very real risk of incompatibilities between those
> > two implementations, which is something that we really want to avoid.
> > 
> > Add some basic tests that verify that reftables written by Git and JGit
> > can be read by the respective other implementation. For now this test
> > suite is rather small, only covering basic functionality. But it serves
> > as a good starting point and can be extended over time.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
> >  1 file changed, 132 insertions(+)
> >  create mode 100755 t/t0612-reftable-jgit-compatibility.sh
> > 
> > diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
> > new file mode 100755
> > index 0000000000..222464e360
> > --- /dev/null
> > +++ b/t/t0612-reftable-jgit-compatibility.sh
> > @@ -0,0 +1,132 @@
> > +#!/bin/sh
> > +
> > +test_description='reftables are compatible with JGit'
> > +
> > +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> > +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> > +GIT_TEST_DEFAULT_REF_FORMAT=reftable
> > +export GIT_TEST_DEFAULT_REF_FORMAT
> > +
> > +# JGit does not support the 'link' DIRC extension.
> > +GIT_TEST_SPLIT_INDEX=0
> > +export GIT_TEST_SPLIT_INDEX
> > +
> > +. ./test-lib.sh
> > +
> > +if ! test_have_prereq JGIT
> 
> Do we want these tests to run in CI? As far as I can tell these tests
> would always be skipped.

They aren't skipped on Linux-based jobs at least. The JGIT prerequisite
is defined in "t/test-lib.sh" like so:

```
test_lazy_prereq JGIT '
	jgit --version
'
```

And because we have adapted "install-dependencies.sh" to install the
"jgit" executable into our custom PATH directory it is available,
meaning that the prereq is fulfilled.

But you're actually onto something: while the tests run on GitHub, they
don't run on GitLab. And this is caused by the unprivileged build that
we use on GitLab, where we install dependencies as a different user than
what we run tests with. Consequently, as the custom path is derived from
HOME, and HOME changes, we cannot find these binaries.

This is not a new issue, it's been there since the inception of the
GitLab pipelines. I'll include another patch on top to fix this.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 00/13] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

Hi,

this is the third version of my patch series that introduces
compatibility checks for the CGit/JGit refatble implementations.

Changes compared to v2:

    - Adjusted commit subject for the skipping-sudo patch.

    - Broken an overly long line in "install-dependencies.sh".

    - Fixed tests that depend on custom binaries not running on GitLab
      CI due to the unprivileged-user setup.

    - Fixed some typos.

CI jobs for this version:

    - https://gitlab.com/gitlab-org/git/-/pipelines/1248875682
    - https://github.com/git/git/actions/runs/8643984413

Thanks for all the feedback!

Patrick

Patrick Steinhardt (13):
  ci: rename "runs_on_pool" to "distro"
  ci: expose distro name in dockerized GitHub jobs
  ci: skip sudo when we are already root
  ci: drop duplicate package installation for "linux-gcc-default"
  ci: convert "install-dependencies.sh" to use "/bin/sh"
  ci: merge custom PATH directories
  ci: fix setup of custom path for GitLab CI
  ci: merge scripts which install dependencies
  ci: make Perforce binaries executable for all users
  ci: install JGit dependency
  t06xx: always execute backend-specific tests
  t0610: fix non-portable variable assignment
  t0612: add tests to exercise Git/JGit reftable compatibility

 .github/workflows/main.yml             |   8 +-
 .gitlab-ci.yml                         |   6 +-
 ci/install-dependencies.sh             | 101 +++++++++++++------
 ci/install-docker-dependencies.sh      |  46 ---------
 ci/lib.sh                              |  14 +--
 t/t0600-reffiles-backend.sh            |   8 +-
 t/t0601-reffiles-pack-refs.sh          |   9 +-
 t/t0610-reftable-basics.sh             |  15 ++-
 t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
 9 files changed, 229 insertions(+), 110 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

Range-diff against v2:
 1:  89723b6812 =  1:  46502bbe22 ci: rename "runs_on_pool" to "distro"
 2:  e60a40bd65 =  2:  d076ed9857 ci: expose distro name in dockerized GitHub jobs
 3:  16603d40fd !  3:  cc0c29052f ci: allow skipping sudo on dockerized jobs
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    ci: allow skipping sudo on dockerized jobs
    +    ci: skip sudo when we are already root
     
         Our "install-dependencies.sh" script is executed by non-dockerized jobs
         to install dependencies. These jobs don't run with "root" permissions,
 4:  b4f6d6d3bf =  4:  803f5020e0 ci: drop duplicate package installation for "linux-gcc-default"
 5:  6abc53bf51 !  5:  d2745e9438 ci: convert "install-dependencies.sh" to use "/bin/sh"
    @@ ci/install-dependencies.sh: ubuntu-*)
     -		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
     -	popd
     +	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
    -+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    ++	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
    ++		-C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
     +	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
      	;;
      macos-*)
 6:  d9be4db56f !  6:  61f108d954 ci: merge custom PATH directories
    @@ ci/install-dependencies.sh: ubuntu-*)
      
     -	mkdir --parents "$GIT_LFS_PATH"
      	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
    --	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    -+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    + 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
    +-		-C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    ++		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
      	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
      	;;
      macos-*)
 -:  ---------- >  7:  ef61b578da ci: fix setup of custom path for GitLab CI
 7:  4a90c003d1 !  8:  7748f87f8c ci: merge scripts which install dependencies
    @@ .github/workflows/main.yml: jobs:
            if: failure() && env.FAILED_TEST_ARTIFACTS != ''
     
      ## .gitlab-ci.yml ##
    -@@ .gitlab-ci.yml: workflow:
    - test:linux:
    -   image: $image
    +@@ .gitlab-ci.yml: test:linux:
    +   variables:
    +     CUSTOM_PATH: "/custom"
        before_script:
     -    - ./ci/install-docker-dependencies.sh
     +    - ./ci/install-dependencies.sh
    @@ ci/install-dependencies.sh: then
      	mkdir --parents "$CUSTOM_PATH"
      	wget --quiet --directory-prefix="$CUSTOM_PATH" \
     @@ ci/install-dependencies.sh: ubuntu-*)
    - 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    + 		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
      	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
      	;;
     +ubuntu32-*)
 8:  5240046a0f !  9:  f7399382f2 ci: make Perforce binaries executable for all users
    @@ ci/install-dependencies.sh: ubuntu-*)
     +	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
      
      	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
    - 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    + 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
 9:  29ceb623b9 ! 10:  b835ff8b78 ci: install JGit dependency
    @@ ci/install-dependencies.sh: ubuntu-*)
      		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
      		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
     @@ ci/install-dependencies.sh: ubuntu-*)
    - 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
    - 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" -C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
    + 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
    + 		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
      	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
     +
     +	wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit"
10:  fc3472cdf3 = 11:  7136c8b6c2 t06xx: always execute backend-specific tests
11:  cedf5929d1 ! 12:  cf4ee9c427 t0610: fix non-portable variable assignment
    @@ Commit message
     
         Such an assignment has been introduced in t0610. The issue wasn't
         detected for a while because this test used to only run when the
    -    GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "refatble".
    +    GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "reftable".
         We have dropped that requirement now though, meaning that it runs
    -    unconditionally, inclluding on jobs which use such older versions of
    +    unconditionally, including on jobs which use such older versions of
         Ubuntu.
     
         We have worked around such issues in the past, e.g. in ebee5580ca
12:  160b026e69 ! 13:  a9cd20eebc t0612: add tests to exercise Git/JGit reftable compatibility
    @@ t/t0612-reftable-jgit-compatibility.sh (new)
     +
     +if ! test_have_prereq JGIT
     +then
    -+	skip_all='skipping reftable JGit tests'
    ++	skip_all='skipping reftable JGit tests; JGit is not present in PATH'
     +	test_done
     +fi
     +
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 01/13] ci: rename "runs_on_pool" to "distro"

[Patrick Steinhardt]

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

The "runs_on_pool" environment variable is used by our CI scripts to
distinguish the different kinds of operating systems. It is quite
specific to GitHub Actions though and not really a descriptive name.

Rename the variable to "distro" to clarify its intent.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 2 +-
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3428773b09..684ef5c00d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -303,7 +303,7 @@ jobs:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      distro: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v4
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index b4e22de3cb..7d247b5ef4 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,7 +11,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0a73fc7bd1..d882250db5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -279,7 +279,7 @@ then
 
 	cache_dir="$HOME/none"
 
-	runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
+	distro=$(echo "$CI_JOB_IMAGE" | tr : -)
 	JOBS=$(nproc)
 else
 	echo "Could not identify CI type" >&2
@@ -318,7 +318,7 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	if test "$jobname" = "linux-gcc-default"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 02/13] ci: expose distro name in dockerized GitHub jobs

[Patrick Steinhardt]

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

Expose a distro name in dockerized jobs. This will be used in a
subsequent commit where we merge the installation scripts for dockerized
and non-dockerized jobs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 684ef5c00d..71cd4e5486 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -342,12 +342,16 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
+          distro: alpine-latest
         - jobname: linux32
           image: daald/ubuntu32:xenial
+          distro: ubuntu32-16.04
         - jobname: pedantic
           image: fedora
+          distro: fedora-latest
     env:
       jobname: ${{matrix.vector.jobname}}
+      distro: ${{matrix.vector.distro}}
     runs-on: ubuntu-latest
     container: ${{matrix.vector.image}}
     steps:
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 03/13] ci: skip sudo when we are already root

[Patrick Steinhardt]

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

Our "install-dependencies.sh" script is executed by non-dockerized jobs
to install dependencies. These jobs don't run with "root" permissions,
but with a separate user. Consequently, we need to use sudo(8) there to
elevate permissions when installing packages.

We're about to merge "install-docker-dependencies.sh" into that script
though, and our Docker containers do run as "root". Using sudo(8) is
thus unnecessary there, even though it would be harmless. On some images
like Alpine Linux though there is no sudo(8) available by default, which
would consequently break the build.

Adapt the script to make "sudo" a no-op when running as "root" user.
This allows us to easily reuse the script for our dockerized jobs.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7d247b5ef4..7dfd3e50ed 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,6 +11,17 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
+# Make sudo a no-op and execute the command directly when running as root.
+# While using sudo would be fine on most platforms when we are root already,
+# some platforms like e.g. Alpine Linux do not have sudo available by default
+# and would thus break.
+if test "$(id -u)" -eq 0
+then
+	sudo () {
+		"$@"
+	}
+fi
+
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 04/13] ci: drop duplicate package installation for "linux-gcc-default"

[Patrick Steinhardt]

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

The "linux-gcc-default" job installs common Ubuntu packages. This is
already done in the distro-specific switch, so we basically duplicate
the effort here.

Drop the duplicate package installations and inline the variable that
contains those common packages.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7dfd3e50ed..fad53aac96 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -7,9 +7,6 @@
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
-UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
- tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
- libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -25,8 +22,13 @@ fi
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
+	sudo apt-get -q -y install \
+		language-pack-is libsvn-perl apache2 \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
+		$CC_PACKAGE $PYTHON_PACKAGE
+
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -34,6 +36,7 @@ ubuntu-*)
 		chmod u+x p4d
 		chmod u+x p4
 	popd
+
 	mkdir --parents "$GIT_LFS_PATH"
 	pushd "$GIT_LFS_PATH"
 		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
@@ -83,10 +86,6 @@ Documentation)
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
 	;;
-linux-gcc-default)
-	sudo apt-get -q update
-	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
-	;;
 esac
 
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 05/13] ci: convert "install-dependencies.sh" to use "/bin/sh"

[Patrick Steinhardt]

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

We're about to merge the "install-docker-dependencies.sh" script into
"install-dependencies.sh". This will also move our Alpine-based jobs
over to use the latter script. This script uses the Bash shell though,
which is not available by default on Alpine Linux.

Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash.
This requires us to get rid of the pushd/popd invocations, which are
replaced by some more elaborate commands that download or extract
executables right to where they are needed.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index fad53aac96..2d6af876d6 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 #
 # Install dependencies required to build and test Git on Linux and macOS
 #
@@ -30,19 +30,15 @@ ubuntu-*)
 		$CC_PACKAGE $PYTHON_PACKAGE
 
 	mkdir --parents "$P4_PATH"
-	pushd "$P4_PATH"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
-		chmod u+x p4d
-		chmod u+x p4
-	popd
+	wget --quiet --directory-prefix="$P4_PATH" \
+		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
+	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
 
 	mkdir --parents "$GIT_LFS_PATH"
-	pushd "$GIT_LFS_PATH"
-		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
-	popd
+	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
+		-C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
@@ -53,11 +49,10 @@ macos-*)
 	brew link --force gettext
 
 	mkdir -p "$P4_PATH"
-	pushd "$P4_PATH"
-		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-		tar -xf helix-core-server.tgz &&
-		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
-	popd
+	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
+	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 06/13] ci: merge custom PATH directories

[Patrick Steinhardt]

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

We're downloading various executables required by our tests. Each of
these executables goes into its own directory, which is then appended to
the PATH variable. Consequently, whenever we add a new dependency and
thus a new directory, we would have to adapt to this change in several
places.

Refactor this to instead put all binaries into a single directory.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 +++++++--------
 ci/lib.sh                  | 10 +++-------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 2d6af876d6..bafe37f2d1 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -29,15 +29,14 @@ ubuntu-*)
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
 		$CC_PACKAGE $PYTHON_PACKAGE
 
-	mkdir --parents "$P4_PATH"
-	wget --quiet --directory-prefix="$P4_PATH" \
+	mkdir --parents "$CUSTOM_PATH"
+	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
+	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
-	mkdir --parents "$GIT_LFS_PATH"
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
-		-C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
@@ -48,10 +47,10 @@ macos-*)
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
 
-	mkdir -p "$P4_PATH"
+	mkdir -p "$CUSTOM_PATH"
 	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
-	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	tar -xf helix-core-server.tgz -C "$CUSTOM_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
 	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
diff --git a/ci/lib.sh b/ci/lib.sh
index d882250db5..4cce854bad 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -340,10 +340,6 @@ ubuntu-*)
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
 	export LINUX_GIT_LFS_VERSION="1.5.2"
-
-	P4_PATH="$HOME/custom/p4"
-	GIT_LFS_PATH="$HOME/custom/git-lfs"
-	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
 macos-*)
 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
@@ -351,12 +347,12 @@ macos-*)
 	then
 		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
 	fi
-
-	P4_PATH="$HOME/custom/p4"
-	export PATH="$P4_PATH:$PATH"
 	;;
 esac
 
+CUSTOM_PATH="$HOME/path"
+export PATH="$CUSTOM_PATH:$PATH"
+
 case "$jobname" in
 linux32)
 	CC=gcc
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 07/13] ci: fix setup of custom path for GitLab CI

[Patrick Steinhardt]

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

Part of "install-dependencies.sh" is to install some binaries required
for tests into a custom directory that gets added to the PATH. This
directory is located at "$HOME/path" and thus depends on the current
user that the script executes as.

This creates problems for GitLab CI, which installs dependencies as the
root user, but runs tests as a separate, unprivileged user. As their
respective home directories are different, we will end up using two
different custom path directories. Consequently, the unprivileged user
will not be able eto find the binaries that were setu up as root user.

Fix this issue by allowing CI to override the custom path, which allows
GitLab to set up a constant value that isn't derived from "$HOME".
---
 .gitlab-ci.yml | 2 ++
 ci/lib.sh      | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c0fa2fe90b..3a0ef4d4d4 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -9,6 +9,8 @@ workflow:
 
 test:linux:
   image: $image
+  variables:
+    CUSTOM_PATH: "/custom"
   before_script:
     - ./ci/install-docker-dependencies.sh
   script:
diff --git a/ci/lib.sh b/ci/lib.sh
index 4cce854bad..473a2d0348 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -350,7 +350,7 @@ macos-*)
 	;;
 esac
 
-CUSTOM_PATH="$HOME/path"
+CUSTOM_PATH="${CUSTOM_PATH:-$HOME/path}"
 export PATH="$CUSTOM_PATH:$PATH"
 
 case "$jobname" in
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 08/13] ci: merge scripts which install dependencies

[Patrick Steinhardt]

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

We have two different scripts which install dependencies, one for
dockerized jobs and one for non-dockerized ones. Naturally, these
scripts have quite some duplication. Furthermore, either of these
scripts is missing some test dependencies that the respective other
script has, thus reducing test coverage.

Merge those two scripts such that there is a single source of truth for
test dependencies, only.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml        |  2 +-
 .gitlab-ci.yml                    |  4 +--
 ci/install-dependencies.sh        | 32 ++++++++++++++++++---
 ci/install-docker-dependencies.sh | 46 -------------------------------
 4 files changed, 31 insertions(+), 53 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 71cd4e5486..5838986895 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -359,7 +359,7 @@ jobs:
       if: matrix.vector.jobname != 'linux32'
     - uses: actions/checkout@v1 # cannot be upgraded because Node.js Actions aren't supported in this container
       if: matrix.vector.jobname == 'linux32'
-    - run: ci/install-docker-dependencies.sh
+    - run: ci/install-dependencies.sh
     - run: ci/run-build-and-tests.sh
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3a0ef4d4d4..ba65f50aac 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -12,7 +12,7 @@ test:linux:
   variables:
     CUSTOM_PATH: "/custom"
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - useradd builder --create-home
     - chown -R builder "${CI_PROJECT_DIR}"
@@ -100,7 +100,7 @@ static-analysis:
   variables:
     jobname: StaticAnalysis
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - ./ci/run-static-analysis.sh
     - ./ci/check-directional-formatting.bash
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index bafe37f2d1..e673797115 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,6 +5,8 @@
 
 . ${0%/*}/lib.sh
 
+begin_group "Install dependencies"
+
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 
@@ -20,14 +22,27 @@ then
 fi
 
 case "$distro" in
+alpine-*)
+	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
+		pcre2-dev python3 musl-libintl perl-utils ncurses \
+		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+	;;
+fedora-*)
+	dnf -yq update >/dev/null &&
+	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
+	;;
 ubuntu-*)
+	# Required so that apt doesn't wait for user input on certain packages.
+	export DEBIAN_FRONTEND=noninteractive
+
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
-		language-pack-is libsvn-perl apache2 \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
-		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
-		$CC_PACKAGE $PYTHON_PACKAGE
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
+		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
 
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
@@ -39,6 +54,13 @@ ubuntu-*)
 		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
+ubuntu32-*)
+	sudo linux32 --32bit i386 sh -c '
+		apt update >/dev/null &&
+		apt install -y build-essential libcurl4-openssl-dev \
+			libssl-dev libexpat-dev gettext python >/dev/null
+	'
+	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
@@ -98,3 +120,5 @@ then
 else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
+
+end_group "Install dependencies"
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
deleted file mode 100755
index eb2c9e1eca..0000000000
--- a/ci/install-docker-dependencies.sh
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/bin/sh
-#
-# Install dependencies required to build and test Git inside container
-#
-
-. ${0%/*}/lib.sh
-
-begin_group "Install dependencies"
-
-case "$jobname" in
-linux32)
-	linux32 --32bit i386 sh -c '
-		apt update >/dev/null &&
-		apt install -y build-essential libcurl4-openssl-dev \
-			libssl-dev libexpat-dev gettext python >/dev/null
-	'
-	;;
-linux-musl)
-	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
-		pcre2-dev python3 musl-libintl perl-utils ncurses \
-		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
-		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
-	;;
-linux-*|StaticAnalysis)
-	# Required so that apt doesn't wait for user input on certain packages.
-	export DEBIAN_FRONTEND=noninteractive
-
-	apt update -q &&
-	apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
-		libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
-		perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
-		libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
-		apache2 cvs cvsps gnupg libcgi-pm-perl subversion
-
-	if test "$jobname" = StaticAnalysis
-	then
-		apt install -q -y coccinelle
-	fi
-	;;
-pedantic)
-	dnf -yq update >/dev/null &&
-	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
-	;;
-esac
-
-end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 09/13] ci: make Perforce binaries executable for all users

[Patrick Steinhardt]

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

The Perforce binaries are only made executable for the current user. On
GitLab CI though we execute tests as a different user than "root", and
thus these binaries may not be executable by that test user at all. This
has gone unnoticed so far because those binaries are optional -- in case
they don't exist we simply skip over tests requiring them.

Fix the setup so that we set the executable bits for all users.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index e673797115..0b9bb686d8 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -47,7 +47,7 @@ ubuntu-*)
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
+	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 10/13] ci: install JGit dependency

[Patrick Steinhardt]

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

We have some tests in t5310 that use JGit to verify that bitmaps can be
read both by Git and by JGit. We do not execute these tests in our CI
jobs though because we don't make JGit available there. Consequently,
the tests basically bitrot because almost nobody is ever going to have
JGit in their path.

Install JGit to plug this test gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 0b9bb686d8..c196e56762 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -9,6 +9,7 @@ begin_group "Install dependencies"
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
+JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -39,7 +40,7 @@ ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
 		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
 		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
@@ -53,6 +54,9 @@ ubuntu-*)
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
 		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+
+	wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit"
+	chmod a+x "$CUSTOM_PATH/jgit"
 	;;
 ubuntu32-*)
 	sudo linux32 --32bit i386 sh -c '
@@ -113,6 +117,7 @@ then
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
+
 if type git-lfs >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
@@ -121,4 +126,12 @@ else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
 
+if type jgit >/dev/null 2>&1
+then
+	echo "$(tput setaf 6)JGit Version$(tput sgr0)"
+	jgit version
+else
+	echo >&2 "WARNING: JGit wasn't installed, see above for clues why"
+fi
+
 end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 11/13] t06xx: always execute backend-specific tests

[Patrick Steinhardt]

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

The tests in t06xx exercise specific ref formats. Next to probing some
basic functionality, these tests also exercise other low-level details
specific to the format. Those tests are only executed though in case
`GIT_TEST_DEFAULT_REF_FORMAT` is set to the ref format of the respective
backend-under-test.

Ideally, we would run the full test matrix for ref formats such that our
complete test suite is executed with every supported format on every
supported platform. This is quite an expensive undertaking though, and
thus we only execute e.g. the "reftable" tests on macOS and Linux. As a
result, we basically have no test coverage for the "reftable" format at
all on other platforms like Windows.

Adapt these tests so that they override `GIT_TEST_DEFAULT_REF_FORMAT`,
which means that they'll always execute. This increases test coverage on
platforms that don't run the full test matrix, which at least gives us
some basic test coverage on those platforms for the "reftable" format.

This of course comes at the cost of running those tests multiple times
on platforms where we do run the full test matrix. But arguably, this is
a good thing because it will also cause us to e.g. run those tests with
the address sanitizer and other non-standard parameters.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0600-reffiles-backend.sh   | 8 ++------
 t/t0601-reffiles-pack-refs.sh | 9 +++------
 t/t0610-reftable-basics.sh    | 9 +++------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 64214340e7..a390cffc80 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -4,16 +4,12 @@ test_description='Test reffiles backend'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	git commit --allow-empty -m Initial &&
 	C=$(git rev-parse HEAD) &&
diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index 7d4ab0b91a..60a544b8ee 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -9,18 +9,15 @@ test_description='git pack-refs should not change the branch semantic
 This test runs git pack-refs and git show-ref and checks that the branch
 semantic is still the same.
 '
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'enable reflogs' '
 	git config core.logallrefupdates true
 '
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 931d888bbb..fd0ddb96ae 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -4,17 +4,14 @@
 #
 
 test_description='reftable basics'
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 . ./test-lib.sh
 
-if ! test_have_prereq REFTABLE
-then
-	skip_all='skipping reftable tests; set GIT_TEST_DEFAULT_REF_FORMAT=reftable'
-	test_done
-fi
-
 INVALID_OID=$(test_oid 001)
 
 test_expect_success 'init: creates basic reftable structures' '
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 12/13] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

Older versions of the Dash shell fail to parse `local var=val`
assignments in some cases when `val` is unquoted. Such failures can be
observed e.g. with Ubuntu 20.04 and older, which has a Dash version that
still has this bug.

Such an assignment has been introduced in t0610. The issue wasn't
detected for a while because this test used to only run when the
GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "reftable".
We have dropped that requirement now though, meaning that it runs
unconditionally, including on jobs which use such older versions of
Ubuntu.

We have worked around such issues in the past, e.g. in ebee5580ca
(parallel-checkout: avoid dash local bug in tests, 2021-06-06), by
quoting the `val` side. Apply the same fix to t0610.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0610-reftable-basics.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index fd0ddb96ae..b6e67724ce 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -78,9 +78,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
 '
 
 test_expect_perms () {
-	local perms="$1"
-	local file="$2"
-	local actual=$(ls -l "$file") &&
+	local perms="$1" &&
+	local file="$2" &&
+	local actual="$(ls -l "$file")" &&
 
 	case "$actual" in
 	$perms*)
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 13/13] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

While the reftable format is a recent introduction in Git, JGit already
knows to read and write reftables since 2017. Given the complexity of
the format there is a very real risk of incompatibilities between those
two implementations, which is something that we really want to avoid.

Add some basic tests that verify that reftables written by Git and JGit
can be read by the respective other implementation. For now this test
suite is rather small, only covering basic functionality. But it serves
as a good starting point and can be extended over time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
new file mode 100755
index 0000000000..d0d7e80b49
--- /dev/null
+++ b/t/t0612-reftable-jgit-compatibility.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='reftables are compatible with JGit'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
+
+# JGit does not support the 'link' DIRC extension.
+GIT_TEST_SPLIT_INDEX=0
+export GIT_TEST_SPLIT_INDEX
+
+. ./test-lib.sh
+
+if ! test_have_prereq JGIT
+then
+	skip_all='skipping reftable JGit tests; JGit is not present in PATH'
+	test_done
+fi
+
+if ! test_have_prereq SHA1
+then
+	skip_all='skipping reftable JGit tests; JGit does not support SHA256 reftables'
+	test_done
+fi
+
+test_commit_jgit () {
+	touch "$1" &&
+	jgit add "$1" &&
+	jgit commit -m "$1"
+}
+
+test_same_refs () {
+	git show-ref --head >cgit.actual &&
+	jgit show-ref >jgit-tabs.actual &&
+	tr "\t" " " <jgit-tabs.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_ref () {
+	git rev-parse "$1" >cgit.actual &&
+	jgit rev-parse "$1" >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_reflog () {
+	git reflog "$*" >cgit.actual &&
+	jgit reflog "$*" >jgit-newline.actual &&
+	sed '/^$/d' <jgit-newline.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_expect_success 'CGit repository can be read by JGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_same_refs &&
+		test_same_ref HEAD &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit repository can be read by CGit' '
+	test_when_finished "rm -rf repo" &&
+	jgit init repo &&
+	(
+		cd repo &&
+
+		touch file &&
+		jgit add file &&
+		jgit commit -m "initial commit" &&
+
+		# Note that we must convert the ref storage after we have
+		# written the default branch. Otherwise JGit will end up with
+		# no HEAD at all.
+		jgit convert-ref-storage --format=reftable &&
+
+		test_same_refs &&
+		test_same_ref HEAD &&
+		# Interestingly, JGit cannot read its own reflog here. CGit can
+		# though.
+		printf "%s HEAD@{0}: commit (initial): initial commit" "$(git rev-parse --short HEAD)" >expect &&
+		git reflog HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'mixed writes from JGit and CGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		test_commit_jgit B &&
+		test_commit C &&
+		test_commit_jgit D &&
+
+		test_same_refs &&
+		test_same_ref HEAD &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit can read multi-level index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		awk "
+		    BEGIN {
+			print \"start\";
+			for (i = 0; i < 10000; i++)
+			    printf \"create refs/heads/branch-%d HEAD\n\", i;
+			print \"commit\";
+		    }
+		" >input &&
+		git update-ref --stdin <input &&
+
+		test_same_refs &&
+		test_same_ref refs/heads/branch-1 &&
+		test_same_ref refs/heads/branch-5738 &&
+		test_same_ref refs/heads/branch-9999
+	)
+'
+
+test_done
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 07/13] ci: fix setup of custom path for GitLab CI

[Eric Sunshine]

On Thu, Apr 11, 2024 at 5:10 AM Patrick Steinhardt <ps@pks.im> wrote:
> Part of "install-dependencies.sh" is to install some binaries required
> for tests into a custom directory that gets added to the PATH. This
> directory is located at "$HOME/path" and thus depends on the current
> user that the script executes as.
>
> This creates problems for GitLab CI, which installs dependencies as the
> root user, but runs tests as a separate, unprivileged user. As their
> respective home directories are different, we will end up using two
> different custom path directories. Consequently, the unprivileged user
> will not be able eto find the binaries that were setu up as root user.

s/eto/to/
s/setu/setup/

> Fix this issue by allowing CI to override the custom path, which allows
> GitLab to set up a constant value that isn't derived from "$HOME".
> ---

Missing sign-off.

Re: [PATCH v3 07/13] ci: fix setup of custom path for GitLab CI

[Patrick Steinhardt]

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

On Thu, Apr 11, 2024 at 06:06:05AM -0400, Eric Sunshine wrote:
> On Thu, Apr 11, 2024 at 5:10 AM Patrick Steinhardt <ps@pks.im> wrote:
> > Part of "install-dependencies.sh" is to install some binaries required
> > for tests into a custom directory that gets added to the PATH. This
> > directory is located at "$HOME/path" and thus depends on the current
> > user that the script executes as.
> >
> > This creates problems for GitLab CI, which installs dependencies as the
> > root user, but runs tests as a separate, unprivileged user. As their
> > respective home directories are different, we will end up using two
> > different custom path directories. Consequently, the unprivileged user
> > will not be able eto find the binaries that were setu up as root user.
> 
> s/eto/to/
> s/setu/setup/

Ugh. Well, to my excuse, my keyboard is a bit wonky recently.

> > Fix this issue by allowing CI to override the custom path, which allows
> > GitLab to set up a constant value that isn't derived from "$HOME".
> > ---
> 
> Missing sign-off.

Indeed. Sorry, I'm somewhat distracted today.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

Hi,

here's the (hopefully last) version of my patch series that introduces
Git/JGit compatibility tests for the reftable format. Only a single
commit changed, where I fixed two typos and added a missing signoff.

Thanks!

Patrick

Patrick Steinhardt (13):
  ci: rename "runs_on_pool" to "distro"
  ci: expose distro name in dockerized GitHub jobs
  ci: skip sudo when we are already root
  ci: drop duplicate package installation for "linux-gcc-default"
  ci: convert "install-dependencies.sh" to use "/bin/sh"
  ci: merge custom PATH directories
  ci: fix setup of custom path for GitLab CI
  ci: merge scripts which install dependencies
  ci: make Perforce binaries executable for all users
  ci: install JGit dependency
  t06xx: always execute backend-specific tests
  t0610: fix non-portable variable assignment
  t0612: add tests to exercise Git/JGit reftable compatibility

 .github/workflows/main.yml             |   8 +-
 .gitlab-ci.yml                         |   6 +-
 ci/install-dependencies.sh             | 101 +++++++++++++------
 ci/install-docker-dependencies.sh      |  46 ---------
 ci/lib.sh                              |  14 +--
 t/t0600-reffiles-backend.sh            |   8 +-
 t/t0601-reffiles-pack-refs.sh          |   9 +-
 t/t0610-reftable-basics.sh             |  15 ++-
 t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
 9 files changed, 229 insertions(+), 110 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

Range-diff against v3:
 1:  46502bbe22 =  1:  99b0b60359 ci: rename "runs_on_pool" to "distro"
 2:  d076ed9857 =  2:  e1d4e1320d ci: expose distro name in dockerized GitHub jobs
 3:  cc0c29052f =  3:  44a0e07600 ci: skip sudo when we are already root
 4:  803f5020e0 =  4:  8e58ce38d9 ci: drop duplicate package installation for "linux-gcc-default"
 5:  d2745e9438 =  5:  b8f56a5e67 ci: convert "install-dependencies.sh" to use "/bin/sh"
 6:  61f108d954 =  6:  ce4f0c766c ci: merge custom PATH directories
 7:  ef61b578da !  7:  9fc462b578 ci: fix setup of custom path for GitLab CI
    @@ Commit message
         root user, but runs tests as a separate, unprivileged user. As their
         respective home directories are different, we will end up using two
         different custom path directories. Consequently, the unprivileged user
    -    will not be able eto find the binaries that were setu up as root user.
    +    will not be able to find the binaries that were set up as root user.
     
         Fix this issue by allowing CI to override the custom path, which allows
         GitLab to set up a constant value that isn't derived from "$HOME".
     
    +    Signed-off-by: Patrick Steinhardt <ps@pks.im>
    +
      ## .gitlab-ci.yml ##
     @@ .gitlab-ci.yml: workflow:
      
 8:  7748f87f8c =  8:  e7a17d57e7 ci: merge scripts which install dependencies
 9:  f7399382f2 =  9:  720d5a4682 ci: make Perforce binaries executable for all users
10:  b835ff8b78 = 10:  77f6d6ecaa ci: install JGit dependency
11:  7136c8b6c2 = 11:  acf0c28550 t06xx: always execute backend-specific tests
12:  cf4ee9c427 = 12:  a9b71e8eea t0610: fix non-portable variable assignment
13:  a9cd20eebc = 13:  218c694d2e t0612: add tests to exercise Git/JGit reftable compatibility

base-commit: 436d4e5b14df49870a897f64fe92c0ddc7017e4c
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 01/13] ci: rename "runs_on_pool" to "distro"

[Patrick Steinhardt]

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

The "runs_on_pool" environment variable is used by our CI scripts to
distinguish the different kinds of operating systems. It is quite
specific to GitHub Actions though and not really a descriptive name.

Rename the variable to "distro" to clarify its intent.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 2 +-
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 3428773b09..684ef5c00d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -303,7 +303,7 @@ jobs:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      distro: ${{matrix.vector.pool}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v4
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index b4e22de3cb..7d247b5ef4 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,7 +11,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
diff --git a/ci/lib.sh b/ci/lib.sh
index 0a73fc7bd1..d882250db5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -279,7 +279,7 @@ then
 
 	cache_dir="$HOME/none"
 
-	runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
+	distro=$(echo "$CI_JOB_IMAGE" | tr : -)
 	JOBS=$(nproc)
 else
 	echo "Could not identify CI type" >&2
@@ -318,7 +318,7 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
+case "$distro" in
 ubuntu-*)
 	if test "$jobname" = "linux-gcc-default"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 02/13] ci: expose distro name in dockerized GitHub jobs

[Patrick Steinhardt]

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

Expose a distro name in dockerized jobs. This will be used in a
subsequent commit where we merge the installation scripts for dockerized
and non-dockerized jobs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 684ef5c00d..71cd4e5486 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -342,12 +342,16 @@ jobs:
         vector:
         - jobname: linux-musl
           image: alpine
+          distro: alpine-latest
         - jobname: linux32
           image: daald/ubuntu32:xenial
+          distro: ubuntu32-16.04
         - jobname: pedantic
           image: fedora
+          distro: fedora-latest
     env:
       jobname: ${{matrix.vector.jobname}}
+      distro: ${{matrix.vector.distro}}
     runs-on: ubuntu-latest
     container: ${{matrix.vector.image}}
     steps:
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 03/13] ci: skip sudo when we are already root

[Patrick Steinhardt]

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

Our "install-dependencies.sh" script is executed by non-dockerized jobs
to install dependencies. These jobs don't run with "root" permissions,
but with a separate user. Consequently, we need to use sudo(8) there to
elevate permissions when installing packages.

We're about to merge "install-docker-dependencies.sh" into that script
though, and our Docker containers do run as "root". Using sudo(8) is
thus unnecessary there, even though it would be harmless. On some images
like Alpine Linux though there is no sudo(8) available by default, which
would consequently break the build.

Adapt the script to make "sudo" a no-op when running as "root" user.
This allows us to easily reuse the script for our dockerized jobs.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7d247b5ef4..7dfd3e50ed 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,6 +11,17 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
+# Make sudo a no-op and execute the command directly when running as root.
+# While using sudo would be fine on most platforms when we are root already,
+# some platforms like e.g. Alpine Linux do not have sudo available by default
+# and would thus break.
+if test "$(id -u)" -eq 0
+then
+	sudo () {
+		"$@"
+	}
+fi
+
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 04/13] ci: drop duplicate package installation for "linux-gcc-default"

[Patrick Steinhardt]

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

The "linux-gcc-default" job installs common Ubuntu packages. This is
already done in the distro-specific switch, so we basically duplicate
the effort here.

Drop the duplicate package installations and inline the variable that
contains those common packages.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 7dfd3e50ed..fad53aac96 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -7,9 +7,6 @@
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
-UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
- tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
- libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -25,8 +22,13 @@ fi
 case "$distro" in
 ubuntu-*)
 	sudo apt-get -q update
-	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
+	sudo apt-get -q -y install \
+		language-pack-is libsvn-perl apache2 \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
+		$CC_PACKAGE $PYTHON_PACKAGE
+
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
@@ -34,6 +36,7 @@ ubuntu-*)
 		chmod u+x p4d
 		chmod u+x p4
 	popd
+
 	mkdir --parents "$GIT_LFS_PATH"
 	pushd "$GIT_LFS_PATH"
 		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
@@ -83,10 +86,6 @@ Documentation)
 	test -n "$ALREADY_HAVE_ASCIIDOCTOR" ||
 	sudo gem install --version 1.5.8 asciidoctor
 	;;
-linux-gcc-default)
-	sudo apt-get -q update
-	sudo apt-get -q -y install $UBUNTU_COMMON_PKGS
-	;;
 esac
 
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 05/13] ci: convert "install-dependencies.sh" to use "/bin/sh"

[Patrick Steinhardt]

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

We're about to merge the "install-docker-dependencies.sh" script into
"install-dependencies.sh". This will also move our Alpine-based jobs
over to use the latter script. This script uses the Bash shell though,
which is not available by default on Alpine Linux.

Refactor "install-dependencies.sh" to use "/bin/sh" instead of Bash.
This requires us to get rid of the pushd/popd invocations, which are
replaced by some more elaborate commands that download or extract
executables right to where they are needed.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index fad53aac96..2d6af876d6 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 #
 # Install dependencies required to build and test Git on Linux and macOS
 #
@@ -30,19 +30,15 @@ ubuntu-*)
 		$CC_PACKAGE $PYTHON_PACKAGE
 
 	mkdir --parents "$P4_PATH"
-	pushd "$P4_PATH"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
-		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4"
-		chmod u+x p4d
-		chmod u+x p4
-	popd
+	wget --quiet --directory-prefix="$P4_PATH" \
+		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
+	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
 
 	mkdir --parents "$GIT_LFS_PATH"
-	pushd "$GIT_LFS_PATH"
-		wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		tar --extract --gunzip --file "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
-		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
-	popd
+	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
+		-C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
@@ -53,11 +49,10 @@ macos-*)
 	brew link --force gettext
 
 	mkdir -p "$P4_PATH"
-	pushd "$P4_PATH"
-		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-		tar -xf helix-core-server.tgz &&
-		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
-	popd
+	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
+	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
 	then
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 06/13] ci: merge custom PATH directories

[Patrick Steinhardt]

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

We're downloading various executables required by our tests. Each of
these executables goes into its own directory, which is then appended to
the PATH variable. Consequently, whenever we add a new dependency and
thus a new directory, we would have to adapt to this change in several
places.

Refactor this to instead put all binaries into a single directory.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 +++++++--------
 ci/lib.sh                  | 10 +++-------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 2d6af876d6..bafe37f2d1 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -29,15 +29,14 @@ ubuntu-*)
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
 		$CC_PACKAGE $PYTHON_PACKAGE
 
-	mkdir --parents "$P4_PATH"
-	wget --quiet --directory-prefix="$P4_PATH" \
+	mkdir --parents "$CUSTOM_PATH"
+	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$P4_PATH/p4d" "$P4_PATH/p4"
+	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
-	mkdir --parents "$GIT_LFS_PATH"
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
-		-C "$GIT_LFS_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
+		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
 macos-*)
@@ -48,10 +47,10 @@ macos-*)
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
 
-	mkdir -p "$P4_PATH"
+	mkdir -p "$CUSTOM_PATH"
 	wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
-	tar -xf helix-core-server.tgz -C "$P4_PATH" p4 p4d &&
-	sudo xattr -d com.apple.quarantine "$P4_PATH/p4" "$P4_PATH/p4d" 2>/dev/null || true
+	tar -xf helix-core-server.tgz -C "$CUSTOM_PATH" p4 p4d &&
+	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
 	rm helix-core-server.tgz
 
 	if test -n "$CC_PACKAGE"
diff --git a/ci/lib.sh b/ci/lib.sh
index d882250db5..4cce854bad 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -340,10 +340,6 @@ ubuntu-*)
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
 	export LINUX_GIT_LFS_VERSION="1.5.2"
-
-	P4_PATH="$HOME/custom/p4"
-	GIT_LFS_PATH="$HOME/custom/git-lfs"
-	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
 macos-*)
 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
@@ -351,12 +347,12 @@ macos-*)
 	then
 		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
 	fi
-
-	P4_PATH="$HOME/custom/p4"
-	export PATH="$P4_PATH:$PATH"
 	;;
 esac
 
+CUSTOM_PATH="$HOME/path"
+export PATH="$CUSTOM_PATH:$PATH"
+
 case "$jobname" in
 linux32)
 	CC=gcc
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 07/13] ci: fix setup of custom path for GitLab CI

[Patrick Steinhardt]

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

Part of "install-dependencies.sh" is to install some binaries required
for tests into a custom directory that gets added to the PATH. This
directory is located at "$HOME/path" and thus depends on the current
user that the script executes as.

This creates problems for GitLab CI, which installs dependencies as the
root user, but runs tests as a separate, unprivileged user. As their
respective home directories are different, we will end up using two
different custom path directories. Consequently, the unprivileged user
will not be able to find the binaries that were set up as root user.

Fix this issue by allowing CI to override the custom path, which allows
GitLab to set up a constant value that isn't derived from "$HOME".

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .gitlab-ci.yml | 2 ++
 ci/lib.sh      | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c0fa2fe90b..3a0ef4d4d4 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -9,6 +9,8 @@ workflow:
 
 test:linux:
   image: $image
+  variables:
+    CUSTOM_PATH: "/custom"
   before_script:
     - ./ci/install-docker-dependencies.sh
   script:
diff --git a/ci/lib.sh b/ci/lib.sh
index 4cce854bad..473a2d0348 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -350,7 +350,7 @@ macos-*)
 	;;
 esac
 
-CUSTOM_PATH="$HOME/path"
+CUSTOM_PATH="${CUSTOM_PATH:-$HOME/path}"
 export PATH="$CUSTOM_PATH:$PATH"
 
 case "$jobname" in
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 08/13] ci: merge scripts which install dependencies

[Patrick Steinhardt]

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

We have two different scripts which install dependencies, one for
dockerized jobs and one for non-dockerized ones. Naturally, these
scripts have quite some duplication. Furthermore, either of these
scripts is missing some test dependencies that the respective other
script has, thus reducing test coverage.

Merge those two scripts such that there is a single source of truth for
test dependencies, only.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 .github/workflows/main.yml        |  2 +-
 .gitlab-ci.yml                    |  4 +--
 ci/install-dependencies.sh        | 32 ++++++++++++++++++---
 ci/install-docker-dependencies.sh | 46 -------------------------------
 4 files changed, 31 insertions(+), 53 deletions(-)
 delete mode 100755 ci/install-docker-dependencies.sh

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 71cd4e5486..5838986895 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -359,7 +359,7 @@ jobs:
       if: matrix.vector.jobname != 'linux32'
     - uses: actions/checkout@v1 # cannot be upgraded because Node.js Actions aren't supported in this container
       if: matrix.vector.jobname == 'linux32'
-    - run: ci/install-docker-dependencies.sh
+    - run: ci/install-dependencies.sh
     - run: ci/run-build-and-tests.sh
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3a0ef4d4d4..ba65f50aac 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -12,7 +12,7 @@ test:linux:
   variables:
     CUSTOM_PATH: "/custom"
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - useradd builder --create-home
     - chown -R builder "${CI_PROJECT_DIR}"
@@ -100,7 +100,7 @@ static-analysis:
   variables:
     jobname: StaticAnalysis
   before_script:
-    - ./ci/install-docker-dependencies.sh
+    - ./ci/install-dependencies.sh
   script:
     - ./ci/run-static-analysis.sh
     - ./ci/check-directional-formatting.bash
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index bafe37f2d1..e673797115 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,6 +5,8 @@
 
 . ${0%/*}/lib.sh
 
+begin_group "Install dependencies"
+
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 
@@ -20,14 +22,27 @@ then
 fi
 
 case "$distro" in
+alpine-*)
+	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
+		pcre2-dev python3 musl-libintl perl-utils ncurses \
+		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+	;;
+fedora-*)
+	dnf -yq update >/dev/null &&
+	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
+	;;
 ubuntu-*)
+	# Required so that apt doesn't wait for user input on certain packages.
+	export DEBIAN_FRONTEND=noninteractive
+
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
-		language-pack-is libsvn-perl apache2 \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev \
+		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
-		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl \
-		$CC_PACKAGE $PYTHON_PACKAGE
+		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
+		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
 
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
@@ -39,6 +54,13 @@ ubuntu-*)
 		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	;;
+ubuntu32-*)
+	sudo linux32 --32bit i386 sh -c '
+		apt update >/dev/null &&
+		apt install -y build-essential libcurl4-openssl-dev \
+			libssl-dev libexpat-dev gettext python >/dev/null
+	'
+	;;
 macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
@@ -98,3 +120,5 @@ then
 else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
+
+end_group "Install dependencies"
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
deleted file mode 100755
index eb2c9e1eca..0000000000
--- a/ci/install-docker-dependencies.sh
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/bin/sh
-#
-# Install dependencies required to build and test Git inside container
-#
-
-. ${0%/*}/lib.sh
-
-begin_group "Install dependencies"
-
-case "$jobname" in
-linux32)
-	linux32 --32bit i386 sh -c '
-		apt update >/dev/null &&
-		apt install -y build-essential libcurl4-openssl-dev \
-			libssl-dev libexpat-dev gettext python >/dev/null
-	'
-	;;
-linux-musl)
-	apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
-		pcre2-dev python3 musl-libintl perl-utils ncurses \
-		apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
-		bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
-	;;
-linux-*|StaticAnalysis)
-	# Required so that apt doesn't wait for user input on certain packages.
-	export DEBIAN_FRONTEND=noninteractive
-
-	apt update -q &&
-	apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
-		libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
-		perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
-		libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
-		apache2 cvs cvsps gnupg libcgi-pm-perl subversion
-
-	if test "$jobname" = StaticAnalysis
-	then
-		apt install -q -y coccinelle
-	fi
-	;;
-pedantic)
-	dnf -yq update >/dev/null &&
-	dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
-	;;
-esac
-
-end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 09/13] ci: make Perforce binaries executable for all users

[Patrick Steinhardt]

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

The Perforce binaries are only made executable for the current user. On
GitLab CI though we execute tests as a different user than "root", and
thus these binaries may not be executable by that test user at all. This
has gone unnoticed so far because those binaries are optional -- in case
they don't exist we simply skip over tests requiring them.

Fix the setup so that we set the executable bits for all users.

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

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index e673797115..0b9bb686d8 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -47,7 +47,7 @@ ubuntu-*)
 	mkdir --parents "$CUSTOM_PATH"
 	wget --quiet --directory-prefix="$CUSTOM_PATH" \
 		"$P4WHENCE/bin.linux26x86_64/p4d" "$P4WHENCE/bin.linux26x86_64/p4"
-	chmod u+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
+	chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4"
 
 	wget --quiet "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 10/13] ci: install JGit dependency

[Patrick Steinhardt]

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

We have some tests in t5310 that use JGit to verify that bitmaps can be
read both by Git and by JGit. We do not execute these tests in our CI
jobs though because we don't make JGit available there. Consequently,
the tests basically bitrot because almost nobody is ever going to have
JGit in their path.

Install JGit to plug this test gap.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/install-dependencies.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 0b9bb686d8..c196e56762 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -9,6 +9,7 @@ begin_group "Install dependencies"
 
 P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
+JGITWHENCE=https://repo.eclipse.org/content/groups/releases//org/eclipse/jgit/org.eclipse.jgit.pgm/6.8.0.202311291450-r/org.eclipse.jgit.pgm-6.8.0.202311291450-r.sh
 
 # Make sudo a no-op and execute the command directly when running as root.
 # While using sudo would be fine on most platforms when we are root already,
@@ -39,7 +40,7 @@ ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install \
 		language-pack-is libsvn-perl apache2 cvs cvsps git gnupg subversion \
-		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo \
+		make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \
 		tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \
 		libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \
 		${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
@@ -53,6 +54,9 @@ ubuntu-*)
 	tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
 		-C "$CUSTOM_PATH" --strip-components=1 "git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs"
 	rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz"
+
+	wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit"
+	chmod a+x "$CUSTOM_PATH/jgit"
 	;;
 ubuntu32-*)
 	sudo linux32 --32bit i386 sh -c '
@@ -113,6 +117,7 @@ then
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
+
 if type git-lfs >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
@@ -121,4 +126,12 @@ else
 	echo >&2 "WARNING: git-lfs wasn't installed, see above for clues why"
 fi
 
+if type jgit >/dev/null 2>&1
+then
+	echo "$(tput setaf 6)JGit Version$(tput sgr0)"
+	jgit version
+else
+	echo >&2 "WARNING: JGit wasn't installed, see above for clues why"
+fi
+
 end_group "Install dependencies"
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 11/13] t06xx: always execute backend-specific tests

[Patrick Steinhardt]

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

The tests in t06xx exercise specific ref formats. Next to probing some
basic functionality, these tests also exercise other low-level details
specific to the format. Those tests are only executed though in case
`GIT_TEST_DEFAULT_REF_FORMAT` is set to the ref format of the respective
backend-under-test.

Ideally, we would run the full test matrix for ref formats such that our
complete test suite is executed with every supported format on every
supported platform. This is quite an expensive undertaking though, and
thus we only execute e.g. the "reftable" tests on macOS and Linux. As a
result, we basically have no test coverage for the "reftable" format at
all on other platforms like Windows.

Adapt these tests so that they override `GIT_TEST_DEFAULT_REF_FORMAT`,
which means that they'll always execute. This increases test coverage on
platforms that don't run the full test matrix, which at least gives us
some basic test coverage on those platforms for the "reftable" format.

This of course comes at the cost of running those tests multiple times
on platforms where we do run the full test matrix. But arguably, this is
a good thing because it will also cause us to e.g. run those tests with
the address sanitizer and other non-standard parameters.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0600-reffiles-backend.sh   | 8 ++------
 t/t0601-reffiles-pack-refs.sh | 9 +++------
 t/t0610-reftable-basics.sh    | 9 +++------
 3 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 64214340e7..a390cffc80 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -4,16 +4,12 @@ test_description='Test reffiles backend'
 
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'setup' '
 	git commit --allow-empty -m Initial &&
 	C=$(git rev-parse HEAD) &&
diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index 7d4ab0b91a..60a544b8ee 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -9,18 +9,15 @@ test_description='git pack-refs should not change the branch semantic
 This test runs git pack-refs and git show-ref and checks that the branch
 semantic is still the same.
 '
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=files
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-if ! test_have_prereq REFFILES
-then
-	skip_all='skipping reffiles specific tests'
-	test_done
-fi
-
 test_expect_success 'enable reflogs' '
 	git config core.logallrefupdates true
 '
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 931d888bbb..fd0ddb96ae 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -4,17 +4,14 @@
 #
 
 test_description='reftable basics'
+
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
 
 . ./test-lib.sh
 
-if ! test_have_prereq REFTABLE
-then
-	skip_all='skipping reftable tests; set GIT_TEST_DEFAULT_REF_FORMAT=reftable'
-	test_done
-fi
-
 INVALID_OID=$(test_oid 001)
 
 test_expect_success 'init: creates basic reftable structures' '
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 12/13] t0610: fix non-portable variable assignment

[Patrick Steinhardt]

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

Older versions of the Dash shell fail to parse `local var=val`
assignments in some cases when `val` is unquoted. Such failures can be
observed e.g. with Ubuntu 20.04 and older, which has a Dash version that
still has this bug.

Such an assignment has been introduced in t0610. The issue wasn't
detected for a while because this test used to only run when the
GIT_TEST_DEFAULT_REF_FORMAT environment variable was set to "reftable".
We have dropped that requirement now though, meaning that it runs
unconditionally, including on jobs which use such older versions of
Ubuntu.

We have worked around such issues in the past, e.g. in ebee5580ca
(parallel-checkout: avoid dash local bug in tests, 2021-06-06), by
quoting the `val` side. Apply the same fix to t0610.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0610-reftable-basics.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index fd0ddb96ae..b6e67724ce 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -78,9 +78,9 @@ test_expect_success 'init: reinitializing reftable with files backend fails' '
 '
 
 test_expect_perms () {
-	local perms="$1"
-	local file="$2"
-	local actual=$(ls -l "$file") &&
+	local perms="$1" &&
+	local file="$2" &&
+	local actual="$(ls -l "$file")" &&
 
 	case "$actual" in
 	$perms*)
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 13/13] t0612: add tests to exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

While the reftable format is a recent introduction in Git, JGit already
knows to read and write reftables since 2017. Given the complexity of
the format there is a very real risk of incompatibilities between those
two implementations, which is something that we really want to avoid.

Add some basic tests that verify that reftables written by Git and JGit
can be read by the respective other implementation. For now this test
suite is rather small, only covering basic functionality. But it serves
as a good starting point and can be extended over time.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0612-reftable-jgit-compatibility.sh | 132 +++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100755 t/t0612-reftable-jgit-compatibility.sh

diff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh
new file mode 100755
index 0000000000..d0d7e80b49
--- /dev/null
+++ b/t/t0612-reftable-jgit-compatibility.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='reftables are compatible with JGit'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_REF_FORMAT=reftable
+export GIT_TEST_DEFAULT_REF_FORMAT
+
+# JGit does not support the 'link' DIRC extension.
+GIT_TEST_SPLIT_INDEX=0
+export GIT_TEST_SPLIT_INDEX
+
+. ./test-lib.sh
+
+if ! test_have_prereq JGIT
+then
+	skip_all='skipping reftable JGit tests; JGit is not present in PATH'
+	test_done
+fi
+
+if ! test_have_prereq SHA1
+then
+	skip_all='skipping reftable JGit tests; JGit does not support SHA256 reftables'
+	test_done
+fi
+
+test_commit_jgit () {
+	touch "$1" &&
+	jgit add "$1" &&
+	jgit commit -m "$1"
+}
+
+test_same_refs () {
+	git show-ref --head >cgit.actual &&
+	jgit show-ref >jgit-tabs.actual &&
+	tr "\t" " " <jgit-tabs.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_ref () {
+	git rev-parse "$1" >cgit.actual &&
+	jgit rev-parse "$1" >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_same_reflog () {
+	git reflog "$*" >cgit.actual &&
+	jgit reflog "$*" >jgit-newline.actual &&
+	sed '/^$/d' <jgit-newline.actual >jgit.actual &&
+	test_cmp cgit.actual jgit.actual
+}
+
+test_expect_success 'CGit repository can be read by JGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		test_commit A &&
+		test_same_refs &&
+		test_same_ref HEAD &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit repository can be read by CGit' '
+	test_when_finished "rm -rf repo" &&
+	jgit init repo &&
+	(
+		cd repo &&
+
+		touch file &&
+		jgit add file &&
+		jgit commit -m "initial commit" &&
+
+		# Note that we must convert the ref storage after we have
+		# written the default branch. Otherwise JGit will end up with
+		# no HEAD at all.
+		jgit convert-ref-storage --format=reftable &&
+
+		test_same_refs &&
+		test_same_ref HEAD &&
+		# Interestingly, JGit cannot read its own reflog here. CGit can
+		# though.
+		printf "%s HEAD@{0}: commit (initial): initial commit" "$(git rev-parse --short HEAD)" >expect &&
+		git reflog HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'mixed writes from JGit and CGit' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		test_commit_jgit B &&
+		test_commit C &&
+		test_commit_jgit D &&
+
+		test_same_refs &&
+		test_same_ref HEAD &&
+		test_same_reflog HEAD
+	)
+'
+
+test_expect_success 'JGit can read multi-level index' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+
+		test_commit A &&
+		awk "
+		    BEGIN {
+			print \"start\";
+			for (i = 0; i < 10000; i++)
+			    printf \"create refs/heads/branch-%d HEAD\n\", i;
+			print \"commit\";
+		    }
+		" >input &&
+		git update-ref --stdin <input &&
+
+		test_same_refs &&
+		test_same_ref refs/heads/branch-1 &&
+		test_same_ref refs/heads/branch-5738 &&
+		test_same_ref refs/heads/branch-9999
+	)
+'
+
+test_done
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Justin Tobler]

On 24/04/12 06:43AM, Patrick Steinhardt wrote:
> Hi,
> 
> here's the (hopefully last) version of my patch series that introduces
> Git/JGit compatibility tests for the reftable format. Only a single
> commit changed, where I fixed two typos and added a missing signoff.

With 35e293e6 (Merge branch 'ps/ci-test-with-jgit' into next,
2024-05-01), I've noticed that a subset of the `test:linux` GitLab CI
jobs have started failing on next.

https://gitlab.com/gitlab-org/git/-/pipelines/1277877090

I'll take a look.

-Justin

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Fri, May 03, 2024 at 01:42:32PM -0500, Justin Tobler wrote:
> On 24/04/12 06:43AM, Patrick Steinhardt wrote:
> > Hi,
> > 
> > here's the (hopefully last) version of my patch series that introduces
> > Git/JGit compatibility tests for the reftable format. Only a single
> > commit changed, where I fixed two typos and added a missing signoff.
> 
> With 35e293e6 (Merge branch 'ps/ci-test-with-jgit' into next,
> 2024-05-01), I've noticed that a subset of the `test:linux` GitLab CI
> jobs have started failing on next.
> 
> https://gitlab.com/gitlab-org/git/-/pipelines/1277877090
> 
> I'll take a look.

Are you sure it's related to this merge? All failures are on
"ubuntu:latest", and the complaints are about the "python2" package
being missing. Given the recent release of Ubuntu 24.04, maybe the root
cause is that "python2" does not exist there anymore?

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Fri, May 03, 2024 at 08:48:03PM +0200, Patrick Steinhardt wrote:
> On Fri, May 03, 2024 at 01:42:32PM -0500, Justin Tobler wrote:
> > On 24/04/12 06:43AM, Patrick Steinhardt wrote:
> > > Hi,
> > > 
> > > here's the (hopefully last) version of my patch series that introduces
> > > Git/JGit compatibility tests for the reftable format. Only a single
> > > commit changed, where I fixed two typos and added a missing signoff.
> > 
> > With 35e293e6 (Merge branch 'ps/ci-test-with-jgit' into next,
> > 2024-05-01), I've noticed that a subset of the `test:linux` GitLab CI
> > jobs have started failing on next.
> > 
> > https://gitlab.com/gitlab-org/git/-/pipelines/1277877090
> > 
> > I'll take a look.
> 
> Are you sure it's related to this merge? All failures are on
> "ubuntu:latest", and the complaints are about the "python2" package
> being missing. Given the recent release of Ubuntu 24.04, maybe the root
> cause is that "python2" does not exist there anymore?
> 
> Patrick

Maybe we should do something like below patch. Basically, we start to
acknowledge the fact that Python 2 is end of life and always use Python
3 on ubuntu:latest. We might go even further than that and only use
Python 2 on ubuntu:20.04 and slowly phase out support for it.

diff --git a/ci/lib.sh b/ci/lib.sh
index 473a2d0348..3967a5af85 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -325,11 +325,18 @@ ubuntu-*)
 		break
 	fi
 
-	PYTHON_PACKAGE=python2
-	if test "$jobname" = linux-gcc
-	then
+	case "$distro" in
+	ubuntu-latest)
 		PYTHON_PACKAGE=python3
-	fi
+		;;
+	*)
+		PYTHON_PACKAGE=python2
+		if test "$jobname" = linux-gcc
+		then
+			PYTHON_PACKAGE=python3
+		fi
+		;;
+	esac
 	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
 
 	export GIT_TEST_HTTPD=true

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Justin Tobler]

On 24/05/03 08:57PM, Patrick Steinhardt wrote:
> On Fri, May 03, 2024 at 08:48:03PM +0200, Patrick Steinhardt wrote:
> > On Fri, May 03, 2024 at 01:42:32PM -0500, Justin Tobler wrote:
> > > On 24/04/12 06:43AM, Patrick Steinhardt wrote:
> > > > Hi,
> > > > 
> > > > here's the (hopefully last) version of my patch series that introduces
> > > > Git/JGit compatibility tests for the reftable format. Only a single
> > > > commit changed, where I fixed two typos and added a missing signoff.
> > > 
> > > With 35e293e6 (Merge branch 'ps/ci-test-with-jgit' into next,
> > > 2024-05-01), I've noticed that a subset of the `test:linux` GitLab CI
> > > jobs have started failing on next.
> > > 
> > > https://gitlab.com/gitlab-org/git/-/pipelines/1277877090
> > > 
> > > I'll take a look.
> > 
> > Are you sure it's related to this merge? All failures are on
> > "ubuntu:latest", and the complaints are about the "python2" package
> > being missing. Given the recent release of Ubuntu 24.04, maybe the root
> > cause is that "python2" does not exist there anymore?

Ya, you are right. That appears to be the problem here.

> Maybe we should do something like below patch. Basically, we start to
> acknowledge the fact that Python 2 is end of life and always use Python
> 3 on ubuntu:latest. We might go even further than that and only use
> Python 2 on ubuntu:20.04 and slowly phase out support for it.
> 
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 473a2d0348..3967a5af85 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -325,11 +325,18 @@ ubuntu-*)
>  		break
>  	fi
>  
> -	PYTHON_PACKAGE=python2
> -	if test "$jobname" = linux-gcc
> -	then
> +	case "$distro" in
> +	ubuntu-latest)
>  		PYTHON_PACKAGE=python3
> -	fi
> +		;;
> +	*)
> +		PYTHON_PACKAGE=python2
> +		if test "$jobname" = linux-gcc
> +		then
> +			PYTHON_PACKAGE=python3
> +		fi
> +		;;
> +	esac
>  	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
>  
>  	export GIT_TEST_HTTPD=true

This seems reasonable to me :)

-Justin

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Fri, May 03, 2024 at 02:35:46PM -0500, Justin Tobler wrote:
> On 24/05/03 08:57PM, Patrick Steinhardt wrote:
> > On Fri, May 03, 2024 at 08:48:03PM +0200, Patrick Steinhardt wrote:
> > > On Fri, May 03, 2024 at 01:42:32PM -0500, Justin Tobler wrote:
> > > > On 24/04/12 06:43AM, Patrick Steinhardt wrote:
[snip]
> > Maybe we should do something like below patch. Basically, we start to
> > acknowledge the fact that Python 2 is end of life and always use Python
> > 3 on ubuntu:latest. We might go even further than that and only use
> > Python 2 on ubuntu:20.04 and slowly phase out support for it.
> > 
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index 473a2d0348..3967a5af85 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -325,11 +325,18 @@ ubuntu-*)
> >  		break
> >  	fi
> >  
> > -	PYTHON_PACKAGE=python2
> > -	if test "$jobname" = linux-gcc
> > -	then
> > +	case "$distro" in
> > +	ubuntu-latest)
> >  		PYTHON_PACKAGE=python3
> > -	fi
> > +		;;
> > +	*)
> > +		PYTHON_PACKAGE=python2
> > +		if test "$jobname" = linux-gcc
> > +		then
> > +			PYTHON_PACKAGE=python3
> > +		fi
> > +		;;
> > +	esac
> >  	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
> >  
> >  	export GIT_TEST_HTTPD=true
> 
> This seems reasonable to me :)

Please feel free to adopt and adapt this fix. I probably shouldn't be
reading mails at this time of the day in the first place :)

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Justin Tobler]

On 24/05/03 09:49PM, Patrick Steinhardt wrote:
> On Fri, May 03, 2024 at 02:35:46PM -0500, Justin Tobler wrote:
> > On 24/05/03 08:57PM, Patrick Steinhardt wrote:
> > > On Fri, May 03, 2024 at 08:48:03PM +0200, Patrick Steinhardt wrote:
> > > > On Fri, May 03, 2024 at 01:42:32PM -0500, Justin Tobler wrote:
> > > > > On 24/04/12 06:43AM, Patrick Steinhardt wrote:
> [snip]
> > > Maybe we should do something like below patch. Basically, we start to
> > > acknowledge the fact that Python 2 is end of life and always use Python
> > > 3 on ubuntu:latest. We might go even further than that and only use
> > > Python 2 on ubuntu:20.04 and slowly phase out support for it.
> > > 
> > > diff --git a/ci/lib.sh b/ci/lib.sh
> > > index 473a2d0348..3967a5af85 100755
> > > --- a/ci/lib.sh
> > > +++ b/ci/lib.sh
> > > @@ -325,11 +325,18 @@ ubuntu-*)
> > >  		break
> > >  	fi
> > >  
> > > -	PYTHON_PACKAGE=python2
> > > -	if test "$jobname" = linux-gcc
> > > -	then
> > > +	case "$distro" in
> > > +	ubuntu-latest)
> > >  		PYTHON_PACKAGE=python3
> > > -	fi
> > > +		;;
> > > +	*)
> > > +		PYTHON_PACKAGE=python2
> > > +		if test "$jobname" = linux-gcc
> > > +		then
> > > +			PYTHON_PACKAGE=python3
> > > +		fi
> > > +		;;
> > > +	esac
> > >  	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
> > >  
> > >  	export GIT_TEST_HTTPD=true
> > 
> > This seems reasonable to me :)
> 
> Please feel free to adopt and adapt this fix. I probably shouldn't be
> reading mails at this time of the day in the first place :)

Thanks, I'll give it a go!

-Justin

Re: [PATCH v4 00/13] t: exercise Git/JGit reftable compatibility

[Patrick Steinhardt]

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

On Sat, May 04, 2024 at 12:32:31PM -0500, Justin Tobler wrote:
> On 24/05/03 09:49PM, Patrick Steinhardt wrote:
> > On Fri, May 03, 2024 at 02:35:46PM -0500, Justin Tobler wrote:
> > > On 24/05/03 08:57PM, Patrick Steinhardt wrote:
> > > > On Fri, May 03, 2024 at 08:48:03PM +0200, Patrick Steinhardt wrote:
> > > > > On Fri, May 03, 2024 at 01:42:32PM -0500, Justin Tobler wrote:
> > > > > > On 24/04/12 06:43AM, Patrick Steinhardt wrote:
> > [snip]
> > > > Maybe we should do something like below patch. Basically, we start to
> > > > acknowledge the fact that Python 2 is end of life and always use Python
> > > > 3 on ubuntu:latest. We might go even further than that and only use
> > > > Python 2 on ubuntu:20.04 and slowly phase out support for it.
> > > > 
> > > > diff --git a/ci/lib.sh b/ci/lib.sh
> > > > index 473a2d0348..3967a5af85 100755
> > > > --- a/ci/lib.sh
> > > > +++ b/ci/lib.sh
> > > > @@ -325,11 +325,18 @@ ubuntu-*)
> > > >  		break
> > > >  	fi
> > > >  
> > > > -	PYTHON_PACKAGE=python2
> > > > -	if test "$jobname" = linux-gcc
> > > > -	then
> > > > +	case "$distro" in
> > > > +	ubuntu-latest)
> > > >  		PYTHON_PACKAGE=python3
> > > > -	fi
> > > > +		;;
> > > > +	*)
> > > > +		PYTHON_PACKAGE=python2
> > > > +		if test "$jobname" = linux-gcc
> > > > +		then
> > > > +			PYTHON_PACKAGE=python3
> > > > +		fi
> > > > +		;;
> > > > +	esac
> > > >  	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
> > > >  
> > > >  	export GIT_TEST_HTTPD=true
> > > 
> > > This seems reasonable to me :)
> > 
> > Please feel free to adopt and adapt this fix. I probably shouldn't be
> > reading mails at this time of the day in the first place :)
> 
> Thanks, I'll give it a go!
> 
> -Justin

I've sent out a fix for this via [1] now so that we can hopefully
fast-track this.

Patrick

[1]: https://lore.kernel.org/git/cb8cefc20f373a3516695e7cbee975132553ea95.1714973381.git.ps@pks.im/T/#u

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]