From patchwork Mon Apr 8 06:45:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13620600 Received: from fhigh3-smtp.messagingengine.com (fhigh3-smtp.messagingengine.com [103.168.172.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A0782555B for ; Mon, 8 Apr 2024 06:45:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.154 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712558744; cv=none; b=ZB03FwagUJ5bYg2I/UPTEM+I+2ZGE1aEVhmolzamabfWyAtag0F6E2HnbtmNFuX3zx1z1bEwXFq/HJZTIs0YrZ3wZ1RRKfXkM+hDA2PR3y+cSPvbkVDcImqEq2n0tIsb7o/1jGpZZ2kWYoaSUoxZiRpE0ZG1r5meu17B1E4PivI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712558744; c=relaxed/simple; bh=FYsyCa0d3BBV+UdSjHjNaKxSf++eoHTrlcixP5OnrSI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rKB/OloTecYY5KB4zB10HzioyM/xjcyNxDNg89TzFNJt2NNPLpmutQI3fjJoqR3RonYa25kuy/R33GNvzfxwXYmxZ+9eaT00L5RBGwTG7+TjPgLgzG7xqiuoq544W78cHp/OgtvCk/XsUtMDrQ9MCvDdgPhiaI1wOqtpdnRk1nc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=dTtWsKDV; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=wqF1xc6Z; arc=none smtp.client-ip=103.168.172.154 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="dTtWsKDV"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="wqF1xc6Z" Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfhigh.nyi.internal (Postfix) with ESMTP id 79CD911400D3; Mon, 8 Apr 2024 02:45:41 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Mon, 08 Apr 2024 02:45:41 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1712558741; x=1712645141; bh=9B0pjmOrZt rmcfR1RPEgaypsH+m0QiK9bkmkGu1NxaM=; b=dTtWsKDV59VQIfKIspzw8smdQR Q+IPm4kJ3MtagmSbDgmwb0MBGKoImNR8Z+RRV1D9UepHWoWgkBAT+n4SDYq5fJhU XrVnVZbOaU5u9un8U2q3AOlk2/mj8AaMzGJfBKrVj9mLR+qcspuZkv5yN7kGSxcs G3TLii767qyE+JAZ1sPSAOmuYiaKx1qZ1okIiA+3fTU3KO0egJwl3VOESfDYTeJf ZCShDPquKBikuLN6qyX5dJ41kvF9y+LQrRAkTlXWX33yCmQfrMOFuqAsuBxaWmtU 41xZQtF5BUYTDda4M5sxWGFQsgwRCTACZAJnWRGg29mPnR8yxVICzUGJ1uyA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1712558741; x=1712645141; bh=9B0pjmOrZtrmcfR1RPEgaypsH+m0 QiK9bkmkGu1NxaM=; b=wqF1xc6ZWrY0y4EYQ7CT02DEsNmFeA/ltfPt1F988l/t yD+PR2X5qTPOm3PsQHHTlFfxSeFDZk3ulylyl/XqD8A12jcwqbiaTOF8GS/LzwRC zd02QRIDK5TvQzXIteHV6yI/zt8ZGKEYN0AsWbKqbuFAAk0jPDMucUIum1CiD3m1 kuaQSu7HGeda3l0gwrrAHc6QqVCqabvTWPR6c7SIygUGxLpSM0E8VrRpviaSC9e4 JU606U+pqWnyrk0tyaaZC+03IbiR3ll0lXe/5qOFBCKJe7heopoG2iZ3vHp05uUf NsJGRrVnvI3FdaYeLvz328ldBa1L7nUx/R+CNhxBOg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrudeghedguddutdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrght rhhitghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtth gvrhhnpedvgfevueffffeghfeuvdegteekgefftefgtefghedutedvvddvueeifeelleff feenucffohhmrghinhepghhithhhuhgsrdgtohhmpdhgihhtlhgrsgdrtghomhenucevlh hushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdr ihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 8 Apr 2024 02:45:40 -0400 (EDT) Received: by localhost (OpenSMTPD) with ESMTPSA id 0aaf8179 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 8 Apr 2024 06:45:32 +0000 (UTC) Date: Mon, 8 Apr 2024 08:45:36 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: Han-Wen Nienhuys , Josh Steadmon , Luca Milanesio , Eric Sunshine Subject: [PATCH v2 00/12] t: exercise Git/JGit reftable compatibility Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: 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 ## 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