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

From: Junio C Hamano <[email-masked]>
Date: Fri, 15 Nov 2024 14:50:31 +0900

Bence Ferdinandy <[email-masked]> writes:

> 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).

> 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.

> +		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
	)
'