Subject: Re: [PATCH v4 2/5] set-head: better output for --auto

From: Junio C Hamano <[email-masked]>
Date: Tue, 01 Oct 2024 15:54:50 -0700

Bence Ferdinandy <[email-masked]> writes:

> +static void report_auto(const char *remote, const char *head_name,
> +			struct strbuf *buf_prev) {
> +	struct strbuf buf_prefix = STRBUF_INIT;
> +	const char *prev_head;
> +
> +	strbuf_addf(&buf_prefix, "refs/remotes/%s/", remote);
> +	skip_prefix(buf_prev->buf, buf_prefix.buf, &prev_head);
> +
> +	if (prev_head && !strcmp(prev_head, head_name))
> +		printf("'%s/HEAD' is unchanged and points to '%s'\n",
> +			remote, head_name);
> +	else if (prev_head)
> +		printf("'%s/HEAD' has changed from '%s' and now points to '%s'\n",
> +			remote, prev_head, head_name);
> +	else
> +		printf("'%s/HEAD' is now created and points to '%s'\n",
> +			remote, head_name);
> +}

OK.

>  static int set_head(int argc, const char **argv, const char *prefix)
>  {
>  	int i, opt_a = 0, opt_d = 0, result = 0;
> -	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
> +	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT,
> +		buf_prev = STRBUF_INIT;
>  	char *head_name = NULL;
>  
> +	struct ref_store *refs = get_main_ref_store(the_repository);
> +

What does the blank line between "head_name" and "refs" declaration
indicate?  Is "refs" so special that it must stand separately from
all others?

The primary purpose of this step is to pass a strbuf to
refs_update_symref() so that we can learn what the original value of
the HEAD was, so that report_auto() can give a more intelligent
message.  The introduction of "refs" to avoid repetitive calls to
get_main_ref_store() is orthogonal to that.  It is a good change,
but it should stand on its own, as a separate "preparatory clean-up"
patch.

> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 532035933f..711d6e5598 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -434,7 +434,7 @@ test_expect_success 'set-head --auto has no problem w/multiple HEADs' '
>  		cd test &&
>  		git fetch two "refs/heads/*:refs/remotes/two/*" &&
>  		git remote set-head --auto two >output 2>&1 &&
> -		echo "two/HEAD set to main" >expect &&
> +		echo "'\''two/HEAD'\'' is now created and points to '\''main'\''" >expect &&
>  		test_cmp expect output
>  	)
>  '
> @@ -453,6 +453,17 @@ test_expect_success 'set-head explicit' '
>  	)
>  '
>  
> +
> +test_expect_success 'set-head --auto reports change' '
> +	(
> +		cd test &&
> +		git remote set-head origin side2 &&
> +		git remote set-head --auto origin >output 2>&1 &&
> +		echo "'\''origin/HEAD'\'' has changed from '\''side2'\'' and now points to '\''main'\''" >expect &&
> +		test_cmp expect output
> +	)
> +'
> +
>  cat >test/expect <<EOF
>  Pruning origin
>  URL: $(pwd)/one