From 139352ecee9bd1447a53572d7e66b1ebc724618b Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Fri, 10 Feb 2023 13:25:14 -0800
Subject: [PATCH] Deref tags on ls-remote

A previous commit (2f7335868eec846f12ea90342d67f1ba870caa07) introduced
a quiet bug which results in the "update needed" condition triggering
every loop.  e2e passed for me by sheer luck of winning races.  Until it
didn't.  Walking thru with the debugger to figure it out, I realized
this issue.

The short story: `ls-remote` for a tag gets us the SHA of the tag, but
`rev-parse HEAD` gets us the SHA of the commit to which that tag is
attached.  Those are never equal, so we detect "update needed" every
loop.

Now we ask `ls-remote` for the rev and the dereferenced rev.  If that
rev is a branch, the deref does nothing.  If that rev is a tag it
produces both results.  ls-remote does its own sort, so the deref (if
found) comes after the non-deref.  This means that, in both cases, the
last line is the one we want.
---
 cmd/git-sync/main.go | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go
index 65f13bb..eb5ee66 100644
--- a/cmd/git-sync/main.go
+++ b/cmd/git-sync/main.go
@@ -1393,14 +1393,29 @@ func (git *repoSync) LocalHashForRev(ctx context.Context, rev string) (string, e
 
 // RemoteHashForRef returns the upstream hash for a given ref.
 func (git *repoSync) RemoteHashForRef(ctx context.Context, ref string) (string, error) {
-	output, err := git.run.Run(ctx, git.root, nil, git.cmd, "ls-remote", "-q", git.repo, ref)
+	// Fetch both the bare and dereferenced rev. git sorts the results and
+	// prints the dereferenced result, if present, after the bare result, so we
+	// always want the last result it produces.
+	output, err := git.run.Run(ctx, git.root, nil, git.cmd, "ls-remote", "-q", git.repo, ref, ref+"^{}")
 	if err != nil {
 		return "", err
 	}
-	parts := strings.Split(string(output), "\t")
+	lines := strings.Split(string(output), "\n") // guaranteed to have at least 1 element
+	line := lastNonEmpty(lines)
+	parts := strings.Split(line, "\t") // guaranteed to have at least 1 element
 	return parts[0], nil
 }
 
+func lastNonEmpty(lines []string) string {
+	last := ""
+	for _, line := range lines {
+		if line != "" {
+			last = line
+		}
+	}
+	return last
+}
+
 func (git *repoSync) RevIsHash(ctx context.Context) (bool, error) {
 	hash, err := git.ResolveRef(ctx, git.rev)
 	if err != nil {