Re: [PATCH v2 3/7] update-ref: add support for symref-delete

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 19, 2024, 09:40 UTC
Message-ID: <ZiI8E2ZJT51akoAk@tanuki>
In-Reply-To: <20240412095908.1134387-4-knayak@gitlab.com>

On Fri, Apr 12, 2024 at 11:59:04AM +0200, Karthik Nayak wrote:

> From: Karthik Nayak <karthik.188@gmail.com>

[snip]

Show 12 quoted lines

> @@ -302,6 +302,37 @@ static void parse_cmd_delete(struct ref_transaction *transaction,
>  	strbuf_release(&err);
>  }
>  
> +static void parse_cmd_symref_delete(struct ref_transaction *transaction,
> +				    const char *next, const char *end)
> +{
> +	struct strbuf err = STRBUF_INIT;
> +	char *refname, *old_ref;
> +
> +	if (!(update_flags & REF_NO_DEREF))
> +                die("symref-delete: cannot operate with deref mode");

Again, I'm a bit on the fence regarding this restriction. I feel like it ought to be possible to delete both plain and symbolic refs in a single git-update-ref(1) command.

Show 5 quoted lines

> +	refname = parse_refname(&next);
> +	if (!refname)
> +		die("symref-delete: missing <ref>");
> +
> +        old_ref = parse_next_refname(&next);

This line is indented with spaces and not tabs.

[snip]

Show 34 quoted lines

> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -1715,6 +1715,45 @@ test_expect_success "stdin ${type} symref-verify fails for mistaken null value"
>  	test_cmp expect actual
>  '
>  
> +test_expect_success "stdin ${type} symref-delete fails without --no-deref" '
> +	git symbolic-ref refs/heads/symref $a &&
> +	create_stdin_buf ${type} "symref-delete refs/heads/symref" "$a" &&
> +	test_must_fail git update-ref --stdin ${type} <stdin 2>err &&
> +	grep "fatal: symref-delete: cannot operate with deref mode" err
> +'
> +
> +test_expect_success "stdin ${type} fails symref-delete with no ref" '
> +	create_stdin_buf ${type} "symref-delete " &&
> +	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
> +	grep "fatal: symref-delete: missing <ref>" err
> +'
> +
> +test_expect_success "stdin ${type} fails symref-delete with too many arguments" '
> +	create_stdin_buf ${type} "symref-delete refs/heads/symref" "$a" "$a" &&
> +	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
> +	if test "$type" = "-z"
> +	then
> +		grep "fatal: unknown command: $a" err
> +	else
> +		grep "fatal: symref-delete refs/heads/symref: extra input:  $a" err
> +	fi
> +'
> +
> +test_expect_success "stdin ${type} symref-delete ref fails with wrong old value" '
> +	create_stdin_buf ${type} "symref-delete refs/heads/symref" "$m" &&
> +	test_must_fail git update-ref --stdin ${type} --no-deref <stdin 2>err &&
> +	grep "fatal: cannot lock ref '"'"'refs/heads/symref'"'"'" err &&

You can use "${SQ}" to insert single quotes.

Patrick

Show 17 quoted lines

> +	git symbolic-ref refs/heads/symref >expect &&
> +	echo $a >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success "stdin ${type} symref-delete ref works with right old value" '
> +	create_stdin_buf ${type} "symref-delete refs/heads/symref" "$a" &&
> +	git update-ref --stdin ${type} --no-deref <stdin &&
> +	test_must_fail git rev-parse --verify -q $b
> +'
> +
>  done
>  
>  test_done
> -- 
> 2.43.GIT
>