superproject: support a new revision attribute.

Tested:
$ ./run_tests

Verified that a manifest that specified superproject revision would use
the specified revision, and superproject will use the default revision.

Note that this is a slight behavior change from earlier repo versions,
which would always use the branch name of the manifest itself. However,
the new behavior would be more consisitent with regular "project"
element and would allow superproject be used even if it is not enabled
for the particular manifest branch, so we have decided to make the
change as it would provide more flexibility and better matches what
other elements would do.

Bug: [google internal] b/187868160
Change-Id: I35255ee347aff6e65179f7879d52931f168b477e
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/317643
Tested-by: Xin Li <delphij@google.com>
Reviewed-by: Raman Tenneti <rtenneti@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
diff --git a/docs/manifest-format.md b/docs/manifest-format.md
index 854e5e1..ed297ae 100644
--- a/docs/manifest-format.md
+++ b/docs/manifest-format.md
@@ -103,8 +103,9 @@
   <!ATTLIST repo-hooks enabled-list CDATA #REQUIRED>
 
   <!ELEMENT superproject EMPTY>
-  <!ATTLIST superproject name    CDATA #REQUIRED>
-  <!ATTLIST superproject remote  IDREF #IMPLIED>
+  <!ATTLIST superproject name     CDATA #REQUIRED>
+  <!ATTLIST superproject remote   IDREF #IMPLIED>
+  <!ATTLIST superproject revision CDATA #IMPLIED>
 
   <!ELEMENT contactinfo EMPTY>
   <!ATTLIST contactinfo bugurl  CDATA #REQUIRED>
@@ -432,6 +433,11 @@
 Attribute `remote`: Name of a previously defined remote element.
 If not supplied the remote given by the default element is used.
 
+Attribute `revision`: Name of the Git branch the manifest wants
+to track for this superproject. If not supplied the revision given
+by the remote element is used if applicable, else the default
+element is used.
+
 ### Element contactinfo
 
 ***
diff --git a/git_superproject.py b/git_superproject.py
index 935e125..ce68690 100644
--- a/git_superproject.py
+++ b/git_superproject.py
@@ -90,7 +90,7 @@
     self._git_event_log = git_event_log
     self._quiet = quiet
     self._print_messages = print_messages
-    self._branch = self._GetBranch()
+    self._branch = manifest.branch
     self._repodir = os.path.abspath(repodir)
     self._superproject_dir = superproject_dir
     self._superproject_path = os.path.join(self._repodir, superproject_dir)
@@ -100,6 +100,7 @@
     if self._manifest.superproject:
       remote = self._manifest.superproject['remote']
       git_name = hashlib.md5(remote.name.encode('utf8')).hexdigest() + '-'
+      self._branch = self._manifest.superproject['revision']
       self._remote_url = remote.url
     else:
       self._remote_url = None
@@ -116,17 +117,6 @@
     """Returns the manifest path if the path exists or None."""
     return self._manifest_path if os.path.exists(self._manifest_path) else None
 
-  def _GetBranch(self):
-    """Returns the branch name for getting the approved manifest."""
-    p = self._manifest.manifestProject
-    b = p.GetBranch(p.CurrentBranch)
-    if not b:
-      return None
-    branch = b.merge
-    if branch and branch.startswith(R_HEADS):
-      branch = branch[len(R_HEADS):]
-    return branch
-
   def _LogMessage(self, message):
     """Logs message to stderr and _git_event_log."""
     if self._print_messages:
diff --git a/manifest_xml.py b/manifest_xml.py
index 7099d5f..135c91f 100644
--- a/manifest_xml.py
+++ b/manifest_xml.py
@@ -507,6 +507,9 @@
       if not d.remote or remote.orig_name != remoteName:
         remoteName = remote.orig_name
         e.setAttribute('remote', remoteName)
+      revision = remote.revision or d.revisionExpr
+      if not revision or revision != self._superproject['revision']:
+        e.setAttribute('revision', self._superproject['revision'])
       root.appendChild(e)
 
     if self._contactinfo.bugurl != Wrapper().BUG_URL:
@@ -914,6 +917,13 @@
           raise ManifestParseError("no remote for superproject %s within %s" %
                                    (name, self.manifestFile))
         self._superproject['remote'] = remote.ToRemoteSpec(name)
+        revision = node.getAttribute('revision') or remote.revision
+        if not revision:
+          revision = self._default.revisionExpr
+        if not revision:
+          raise ManifestParseError('no revision for superproject %s within %s' %
+                                   (name, self.manifestFile))
+        self._superproject['revision'] = revision
       if node.nodeName == 'contactinfo':
         bugurl = self._reqatt(node, 'bugurl')
         # This element can be repeated, later entries will clobber earlier ones.
diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py
index e9b824d..a24fc7f 100644
--- a/tests/test_git_superproject.py
+++ b/tests/test_git_superproject.py
@@ -157,7 +157,7 @@
 """)
     self._superproject = git_superproject.Superproject(manifest, self.repodir,
                                                        self.git_event_log)
-    with mock.patch.object(self._superproject, '_GetBranch', return_value='junk'):
+    with mock.patch.object(self._superproject, '_branch', 'junk'):
       sync_result = self._superproject.Sync()
       self.assertFalse(sync_result.success)
       self.assertTrue(sync_result.fatal)
diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py
index 20459d1..ce42253 100644
--- a/tests/test_manifest_xml.py
+++ b/tests/test_manifest_xml.py
@@ -572,6 +572,7 @@
     self.assertEqual(manifest.superproject['name'], 'superproject')
     self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
     self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject['revision'], 'refs/heads/main')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -580,6 +581,72 @@
         '<superproject name="superproject"/>'
         '</manifest>')
 
+  def test_superproject_revision(self):
+    """Check superproject settings with a different revision attribute"""
+    self.maxDiff = None
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <superproject name="superproject" revision="refs/heads/stable" />
+</manifest>
+""")
+    self.assertEqual(manifest.superproject['name'], 'superproject')
+    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
+    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable')
+    self.assertEqual(
+        sort_attributes(manifest.ToXml().toxml()),
+        '<?xml version="1.0" ?><manifest>'
+        '<remote fetch="http://localhost" name="test-remote"/>'
+        '<default remote="test-remote" revision="refs/heads/main"/>'
+        '<superproject name="superproject" revision="refs/heads/stable"/>'
+        '</manifest>')
+
+  def test_superproject_revision_default_negative(self):
+    """Check superproject settings with a same revision attribute"""
+    self.maxDiff = None
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" />
+  <default remote="test-remote" revision="refs/heads/stable" />
+  <superproject name="superproject" revision="refs/heads/stable" />
+</manifest>
+""")
+    self.assertEqual(manifest.superproject['name'], 'superproject')
+    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
+    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable')
+    self.assertEqual(
+        sort_attributes(manifest.ToXml().toxml()),
+        '<?xml version="1.0" ?><manifest>'
+        '<remote fetch="http://localhost" name="test-remote"/>'
+        '<default remote="test-remote" revision="refs/heads/stable"/>'
+        '<superproject name="superproject"/>'
+        '</manifest>')
+
+  def test_superproject_revision_remote(self):
+    """Check superproject settings with a same revision attribute"""
+    self.maxDiff = None
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" revision="refs/heads/main" />
+  <default remote="test-remote" />
+  <superproject name="superproject" revision="refs/heads/stable" />
+</manifest>
+""")
+    self.assertEqual(manifest.superproject['name'], 'superproject')
+    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
+    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
+    self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable')
+    self.assertEqual(
+        sort_attributes(manifest.ToXml().toxml()),
+        '<?xml version="1.0" ?><manifest>'
+        '<remote fetch="http://localhost" name="test-remote" revision="refs/heads/main"/>'
+        '<default remote="test-remote"/>'
+        '<superproject name="superproject" revision="refs/heads/stable"/>'
+        '</manifest>')
+
   def test_remote(self):
     """Check superproject settings with a remote."""
     manifest = self.getXmlManifest("""
@@ -593,6 +660,7 @@
     self.assertEqual(manifest.superproject['name'], 'platform/superproject')
     self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote')
     self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject')
+    self.assertEqual(manifest.superproject['revision'], 'refs/heads/main')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
@@ -613,6 +681,7 @@
 """)
     self.assertEqual(manifest.superproject['name'], 'superproject')
     self.assertEqual(manifest.superproject['remote'].name, 'default-remote')
+    self.assertEqual(manifest.superproject['revision'], 'refs/heads/main')
     self.assertEqual(
         sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'