Re: [PATCH 1/2] refs/reftable: don't fail empty transactions in repo without HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Patrick Steinhardt <ps@xxxxxx> writes:

> Under normal circumstances, it shouldn't ever happen that a repository
> has no HEAD reference. In fact, git-update-ref(1) would fail any request
> to delete the HEAD reference, and a newly initialized repository always
> pre-creates it, too.
>
> But in the next commit, we are going to change git-clone(1) to partially
> initialize the refdb just up to the point where remote helpers can find
> the repository. With that change, we are going to run into a situation
> where repositories have no refs at all.
>
> Now there is a very particular edge case in this situation: when
> preparing an empty ref transacton, we end up returning whatever value
> `read_ref_without_reload()` returned to the caller. Under normal
> conditions this would be fine: "HEAD" should usually exist, and thus the
> function would return `0`. But if "HEAD" doesn't exist, the function
> returns a positive value which we end up returning to the caller.
>
> Fix this bug by resetting the return code to `0` and add a test.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
>  refs/reftable-backend.c    |  1 +
>  t/t0610-reftable-basics.sh | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index a14f2ad7f4..45568818f0 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -821,6 +821,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
>  				      &head_referent, &head_type);
>  	if (ret < 0)
>  		goto done;
> +	ret = 0;
>

So this is not really a problem in this function, it's more of that
`refs.c:ref_transaction_prepare` checks if `ret` is non-zero.

Nit: would be nice to have a comment about why overriding this value is
ok.

>  	for (i = 0; i < transaction->nr; i++) {
>  		struct ref_update *u = transaction->updates[i];
> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> index 6a131e40b8..c5f4d23433 100755
> --- a/t/t0610-reftable-basics.sh
> +++ b/t/t0610-reftable-basics.sh
> @@ -328,6 +328,19 @@ test_expect_success 'ref transaction: writes are synced' '
>  	EOF
>  '
>
> +test_expect_success 'ref transaction: empty transaction in empty repo' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	test_commit -C repo --no-tag A &&
> +	COMMIT=$(git -C repo rev-parse HEAD) &&

why do we do this?

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux