Subject: Re: [PATCH 3/6] update-ref: add support for 'symref-delete' command

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 16 May 2024 13:09:26 +0200

On Tue, May 14, 2024 at 02:44:08PM +0200, Karthik Nayak wrote:
> From: Karthik Nayak <[email-masked]>
[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