From 0667b8ec4d8b10ebc57271642953de6852c3331b Mon Sep 17 00:00:00 2001
From: Ivan Frade <ifrade@google.com>
Date: Mon, 24 May 2021 17:31:25 -0700
Subject: RepoCommand: Do not set 'branch' if the revision is a tag

The "branch" field in the .gitmodules is the signal for gerrit to keep
the superproject autoupdated. Tags are immutable and there is no need to
track them, plus the cgit client requires the field to be a "remote
branch name" but not a tag.

Do not set the "branch" field if the revision is a tag. Keep those tags
in another field ("ref") as they help other tools to find the commit in
the destination repository.

We can still have false negatives when a refname is not fully qualified,
but this check covers e.g. the most common case in android.

Note that the javadoc of #setRecordRemoteBranch already mentions that
"submodules that request a tag will not have branch name recorded".

Change-Id: Ib1c321a4d3b7f8d51ca2ea204f72dc0cfed50c37
Signed-off-by: Ivan Frade <ifrade@google.com>
---
 .../org/eclipse/jgit/gitrepo/RepoCommandTest.java  | 48 ++++++++++++++++++++++
 .../src/org/eclipse/jgit/gitrepo/RepoCommand.java  |  8 +++-
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
index ae4db0b7d8..509adc2cb2 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java
@@ -46,6 +46,8 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.util.FS;
+import org.eclipse.jgit.util.IO;
+import org.eclipse.jgit.util.RawParseUtils;
 import org.junit.Test;
 
 public class RepoCommandTest extends RepositoryTestCase {
@@ -749,7 +751,53 @@ public class RepoCommandTest extends RepositoryTestCase {
 			String gitlink = localDb.resolve(Constants.HEAD + ":foo").name();
 			assertEquals("The gitlink is same as remote head",
 					oldCommitId.name(), gitlink);
+
+			File dotmodules = new File(localDb.getWorkTree(),
+					Constants.DOT_GIT_MODULES);
+			assertTrue(dotmodules.exists());
+			// The .gitmodules file should have "branch" lines
+			String gitModulesContents = RawParseUtils
+					.decode(IO.readFully(dotmodules));
+			assertTrue(gitModulesContents.contains("branch = branch"));
+		}
+	}
+
+	@Test
+	public void testRevisionBare_ignoreTags() throws Exception {
+		Repository remoteDb = createBareRepository();
+		Repository tempDb = createWorkRepository();
+
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+				.append("<manifest>")
+				.append("<remote name=\"remote1\" fetch=\".\" />")
+				.append("<default revision=\"").append("refs/tags/" + TAG)
+				.append("\" remote=\"remote1\" />")
+				.append("<project path=\"foo\" name=\"")
+				.append(defaultUri)
+				.append("\" />").append("</manifest>");
+		JGitTestUtil.writeTrashFile(tempDb, "manifest.xml",
+				xmlContent.toString());
+		RepoCommand command = new RepoCommand(remoteDb);
+		command.setPath(
+				tempDb.getWorkTree().getAbsolutePath() + "/manifest.xml")
+				.setURI(rootUri).call();
+		// Clone it
+		File directory = createTempDirectory("testReplaceManifestBare");
+		File dotmodules;
+		try (Repository localDb = Git.cloneRepository().setDirectory(directory)
+				.setURI(remoteDb.getDirectory().toURI().toString()).call()
+				.getRepository()) {
+			dotmodules = new File(localDb.getWorkTree(),
+					Constants.DOT_GIT_MODULES);
+			assertTrue(dotmodules.exists());
 		}
+
+		// The .gitmodules file should not have "branch" lines
+		String gitModulesContents = RawParseUtils
+				.decode(IO.readFully(dotmodules));
+		assertFalse(gitModulesContents.contains("branch"));
+		assertTrue(gitModulesContents.contains("ref = refs/tags/" + TAG));
 	}
 
 	@Test
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
index c039aaffa9..5bf95def95 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java
@@ -12,6 +12,7 @@ package org.eclipse.jgit.gitrepo;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jgit.lib.Constants.DEFAULT_REMOTE_NAME;
 import static org.eclipse.jgit.lib.Constants.R_REMOTES;
+import static org.eclipse.jgit.lib.Constants.R_TAGS;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -587,8 +588,11 @@ public class RepoCommand extends GitCommand<RevCommit> {
 							throw new RemoteUnavailableException(url);
 						}
 						if (recordRemoteBranch) {
-							// can be branch or tag
-							cfg.setString("submodule", name, "branch", //$NON-NLS-1$ //$NON-NLS-2$
+							// "branch" field is only for non-tag references.
+							// Keep tags in "ref" field as hint for other tools.
+							String field = proj.getRevision().startsWith(
+									R_TAGS) ? "ref" : "branch"; //$NON-NLS-1$ //$NON-NLS-2$
+							cfg.setString("submodule", name, field, //$NON-NLS-1$
 									proj.getRevision());
 						}
 
-- 
cgit v1.2.3