From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 16 May 2024 11:09:26 GMT
Subject: Re: [PATCH 3/6] update-ref: add support for 'symref-delete' command
Message-ID: <ZkXpZss_RZbIvZp2@tanuki>
In-Reply-To: <20240514124411.1037019-4-knayak@gitlab.com>

On Tue, May 14, 2024 at 02:44:08PM +0200, Karthik Nayak wrote:
> From: Karthik Nayak <karthik.188@gmail.com>
[snip]
> @@ -1317,14 +1317,18 @@ int ref_transaction_create(struct ref_transaction *transaction,
>  int ref_transaction_delete(struct ref_transaction *transaction,
>  			   const char *refname,
>  			   const struct object_id *old_oid,
> -			   unsigned int flags, const char *msg,
> +			   unsigned int flags,
> +			   const char *old_target,
> +			   const char *msg,
>  			   struct strbuf *err)
>  {
>  	if (old_oid && is_null_oid(old_oid))
>  		BUG("delete called with old_oid set to zeros");
> +	if (old_target && !(flags & REF_NO_DEREF))
> +		BUG("delete cannot operate on symrefs with deref mode");
>  	return ref_transaction_update(transaction, refname,
>  				      null_oid(), old_oid,
> -				      NULL, NULL, flags,
> +				      NULL, old_target, flags,
>  				      msg, err);
>  }

Same comment here, should we BUG on `old_oid && old_target`?

> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index 8784c59a53..fbc6b234cb 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -1689,7 +1689,7 @@ do
>  		test_cmp before after
>  	'
>  
> -	test_expect_success "stdin ${type} symref-verify no value is treated as zero value" '
> +	test_expect_success "stdin ${type} symref-verify fails with no value" '
>  		git symbolic-ref refs/heads/symref >expect &&
>  		create_stdin_buf ${type} "symref-verify refs/heads/symref" "" &&
>  		test_must_fail git update-ref --stdin ${type} --no-deref <stdin

This change belongs into the preceding commit.

Patrick