From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 02 Oct 2025 11:44:46 GMT
Subject: Re: [PATCH v4 7/7] refs/reftable: add fsck check for checking the table name
Message-ID: <aN5lrhCLQFnw7qUB@pks.im>
In-Reply-To: <20250926-228-reftable-introduce-consistency-checks-v4-7-c96fd8551c0d@gmail.com>

On Fri, Sep 26, 2025 at 09:25:50AM +0200, Karthik Nayak wrote:
> diff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh
> new file mode 100755
> index 0000000000..250d244e66
> --- /dev/null
> +++ b/t/t0614-reftable-fsck.sh
> @@ -0,0 +1,38 @@
> +#!/bin/sh
> +
> +test_description='Test reftable backend consistency check'
> +
> +GIT_TEST_DEFAULT_REF_FORMAT=reftable
> +export GIT_TEST_DEFAULT_REF_FORMAT
> +
> +. ./test-lib.sh
> +
> +for TABLE_NAME in "foo-bar-e4d12d59.ref" \
> +	"0x00000000zzzz-0x00000000zzzz-e4d12d59.ref" \
> +	"0x000000000001-0x000000000002-e4d12d59.abc" \
> +	"0x000000000001-0x000000000002-e4d12d59.refabc"; do
> +	test_expect_success "table name $TABLE_NAME should be checked" '
> +		test_when_finished "rm -rf repo" &&
> +		git init repo &&
> +		(
> +			cd repo &&
> +			git commit --allow-empty -m initial &&
> +
> +			git refs verify 2>err &&
> +			test_must_be_empty err &&
> +
> +			EXISTING_TABLE=$(head -n1 .git/reftable/tables.list) &&
> +			mv ".git/reftable/$EXISTING_TABLE" ".git/reftable/$TABLE_NAME" &&
> +			sed "s/${EXISTING_TABLE}/${TABLE_NAME}/g" .git/reftable/tables.list > tables.list &&
> +			mv tables.list .git/reftable/tables.list &&
> +
> +			git refs verify 2>err &&
> +			cat >expect <<-EOF &&
> +			warning: ${TABLE_NAME}: badReftableTableName: invalid reftable table name
> +			EOF
> +			test_cmp expect err
> +		)
> +	'
> +done
> +
> +test_done

Nit: we don't have any test that verifies that `git refs verify` doesn't
complain with a well-formed stack.

Other than that this series looks good to me, thanks! I think we might
want to have one final reroll, but once that's out I think this should
be ready to be merged down.

Patrick