On Fri Nov 15, 2024 at 23:49, Bence Ferdinandy [off-list ref] wrote:

On Fri Nov 15, 2024 at 06:50, Junio C Hamano [off-list ref] wrote:

quoted

Bence Ferdinandy [off-list ref] writes:

quoted

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

Good point, and yes, that is what happens. (Although I'm not quite sure how
valid that state is where a remote's HEAD is not a branch).

quoted

quoted

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.

Ack.

quoted

quoted

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

I agree, adding this makes sense. And this also takes us back to the question
of what we should do in 2/8 when refs_read_symbolic_ref exits with 1. I now
tested the behaviour and if origin/HEAD is gibberish, git already dies before
with 

error: cannot lock ref 'refs/remotes/origin/HEAD': unable to resolve reference 'refs/remotes/origin/HEAD': reference broken

so refs_read_symbolic_ref -> 1 only happens if there's a valid non-symbolic ref
in origin/HEAD. So maybe if we put "Not a symbolic reference." in the referent
(which should be an invalid symref), the caller could check for that and then
should be able to distinguish this special case?

On second thought, it would maybe make even more sense to get the reference
hash and put that into referent. In that case the output could still be

"'%s/HEAD' has changed from '%s' and now points to '%s'\n"

but with a non-symref after from.