Re: [PATCH v12 4/8] remote set-head: better output for --auto

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 15, 2024, 05:50 UTC
Message-ID: <xmqqcyix11w8.fsf@gitster.g>
In-Reply-To: <20241023153736.257733-5-bence@ferdinandy.com>

Bence Ferdinandy <bence@ferdinandy.com> writes:

Show 8 quoted lines

> Currently, set-head --auto will print a message saying "remote/HEAD set
> to branch", which implies something was changed.
>
> Change the output of --auto, so the output actually reflects what was
> done: a) set a previously unset HEAD, b) change HEAD because remote
> changed or c) no updates. As a fourth output, if HEAD is changed from
> a previous value that was not a remote branch, explicitly call attention
> to this fact.

OK.  That's sensible.

There is a slight variant of the fourth case. HEAD may have been a symbolic ref that pointed at an unexpected place (which you addressed), or HEAD may have been a non-symbolic ref (which the new code would mistakenly say "HEAD is now created", if I am reading the patch correctly).

Show 12 quoted lines

> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 9b50276646..0ea86d51a4 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -432,12 +432,51 @@ test_expect_success 'set-head --auto' '
>  	)
>  '
>  
> +test_expect_success 'set-head --auto detects creation' '
> +	(
> +		cd test &&
> +		git symbolic-ref -d refs/remotes/origin/HEAD &&

Are we sure refs/remotes/origin/HEAD exists at this point in the test, regardless of which earlier tests were skipped or failed? If not, perhaps

		git update-ref --no-deref -d refs/remotes/origin/HEAD &&

is a better alternative.

Show 5 quoted lines

> +		git remote set-head --auto origin >output &&
> +		echo "${SQ}origin/HEAD${SQ} is now created and points to ${SQ}main${SQ}" >expect &&
> +		test_cmp expect output
> +	)
> +'

Here, we could insert another one:

test_expect_success 'set-head --auto to update a non symbolic ref' '
	(
		cd test &&
		git update-ref --no-deref -d refs/remotes/origin/HEAD &&
		git update-ref refs/remotes/origin/HEAD HEAD &&
		git remote set-head --auto origin >output &&

I'd imagine "output" should at least say that we are setting up a symref origin/HEAD to point at some ref the --auto option figured out, and if we wanted to report its previous state, it was a non symbolic ref that pointed at some commit. In any case,

		echo "${SQ}origin/HEAD${SQ} is now created and points to ${SQ}main${SQ}" >expect &&

is not what we want to see here, I suspect.

Can we detect the case where we overwrite a non symref with a symref without going back to step 2/8 and doing a major surgery?

		test_cmp expect output
	)
'