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

[Patrick Steinhardt <ps@pks.im>]

[-- 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 --]