Support --target-branch option to git-cl upload for Rietveld

This is similar to the Gerrit behavior in that we default to master,
unless the remote upstream is a branch head.

BUG=435702

Review URL: https://codereview.chromium.org/822503005

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@293807 0039d316-1c4b-4281-b951-d872f2087c98

git_cl.py

@@ -1679,6 +1679,62 @@ def GerritUpload(options, args, cl, change):
   return 0
 
 
+def GetTargetRef(remote, remote_branch, target_branch, pending_prefix):
+  """Computes the remote branch ref to use for the CL.
+
+  Args:
+    remote (str): The git remote for the CL.
+    remote_branch (str): The git remote branch for the CL.
+    target_branch (str): The target branch specified by the user.
+    pending_prefix (str): The pending prefix from the settings.
+  """
+  if not (remote and remote_branch):
+    return None
+  
+  if target_branch:
+    # Cannonicalize branch references to the equivalent local full symbolic
+    # refs, which are then translated into the remote full symbolic refs
+    # below.
+    if '/' not in target_branch:
+      remote_branch = 'refs/remotes/%s/%s' % (remote, target_branch)
+    else:
+      prefix_replacements = (
+        ('^((refs/)?remotes/)?branch-heads/', 'refs/remotes/branch-heads/'),
+        ('^((refs/)?remotes/)?%s/' % remote,  'refs/remotes/%s/' % remote),
+        ('^(refs/)?heads/',                   'refs/remotes/%s/' % remote),
+      )
+      match = None
+      for regex, replacement in prefix_replacements:
+        match = re.search(regex, target_branch)
+        if match:
+          remote_branch = target_branch.replace(match.group(0), replacement)
+          break
+      if not match:
+        # This is a branch path but not one we recognize; use as-is.
+        remote_branch = target_branch
+  elif (not remote_branch.startswith('refs/remotes/branch-heads') and
+        not remote_branch.startswith('refs/remotes/%s/refs' % remote)):
+    # Default to master for refs that are not branches.
+    remote_branch = 'refs/remotes/%s/master' % remote
+    
+  # Create the true path to the remote branch.
+  # Does the following translation:
+  # * refs/remotes/origin/refs/diff/test -> refs/diff/test
+  # * refs/remotes/origin/master -> refs/heads/master
+  # * refs/remotes/branch-heads/test -> refs/branch-heads/test
+  if remote_branch.startswith('refs/remotes/%s/refs/' % remote):
+    remote_branch = remote_branch.replace('refs/remotes/%s/' % remote, '')
+  elif remote_branch.startswith('refs/remotes/%s/' % remote):
+    remote_branch = remote_branch.replace('refs/remotes/%s/' % remote,
+                                          'refs/heads/')
+  elif remote_branch.startswith('refs/remotes/branch-heads'):
+    remote_branch = remote_branch.replace('refs/remotes/', 'refs/')
+  # If a pending prefix exists then replace refs/ with it.
+  if pending_prefix:
+    remote_branch = remote_branch.replace('refs/', pending_prefix)
+  return remote_branch
+
+
 def RietveldUpload(options, args, cl, change):
   """upload the patch to rietveld."""
   upload_args = ['--assume_yes']  # Don't ask about untracked files.
@@ -1757,24 +1813,10 @@ def RietveldUpload(options, args, cl, change):
   if remote_url:
     upload_args.extend(['--base_url', remote_url])
     remote, remote_branch = cl.GetRemoteBranch()
-    if remote and remote_branch:
-      # Create the true path to the remote branch.
-      # Does the following translation:
-      # * refs/remotes/origin/refs/diff/test -> refs/diff/test
-      # * refs/remotes/origin/master -> refs/heads/master
-      # * refs/remotes/branch-heads/test -> refs/branch-heads/test
-      if remote_branch.startswith('refs/remotes/%s/refs/' % remote):
-        remote_branch = remote_branch.replace('refs/remotes/%s/' % remote, '')
-      elif remote_branch.startswith('refs/remotes/%s/' % remote):
-        remote_branch = remote_branch.replace('refs/remotes/%s/' % remote,
-                                              'refs/heads/')
-      elif remote_branch.startswith('refs/remotes/branch-heads'):
-        remote_branch = remote_branch.replace('refs/remotes/', 'refs/')
-      pending_prefix = settings.GetPendingRefPrefix()
-      # If a pending prefix exists then replace refs/ with it.
-      if pending_prefix:
-        remote_branch = remote_branch.replace('refs/', pending_prefix)
-      upload_args.extend(['--target_ref', remote_branch])
+    target_ref = GetTargetRef(remote, remote_branch, options.target_branch,
+                              settings.GetPendingRefPrefix())
+    if target_ref:
+      upload_args.extend(['--target_ref', target_ref])
 
   project = settings.GetProject()
   if project:
@@ -1851,8 +1893,9 @@ def CMDupload(parser, args):
                     help='set the review private (rietveld only)')
   parser.add_option('--target_branch',
                     '--target-branch',
-                    help='When uploading to gerrit, remote branch to '
-                         'use for CL.  Default: master')
+                    metavar='TARGET',
+                    help='Apply CL to remote ref TARGET.  ' +
+                         'Default: remote branch head, or master')
   parser.add_option('--email', default=None,
                     help='email address to use to connect to Rietveld')
   parser.add_option('--tbr-owners', dest='tbr_owners', action='store_true',
@@ -1861,9 +1904,6 @@ def CMDupload(parser, args):
   add_git_similarity(parser)
   (options, args) = parser.parse_args(args)
 
-  if options.target_branch and not settings.GetIsGerrit():
-    parser.error('Use --target_branch for non gerrit repository.')
-
   if is_dirty_git_tree('upload'):
     return 1
 

tests/git_cl_test.py

@@ -756,6 +756,57 @@ class TestGitCl(TestCase):
       actual.append(obj.description)
     self.assertEqual(expected, actual)
 
+  def test_get_target_ref(self):
+    # Check remote or remote branch not present.
+    self.assertEqual(None, git_cl.GetTargetRef('origin', None, 'master', None))
+    self.assertEqual(None, git_cl.GetTargetRef(None,
+                                               'refs/remotes/origin/master',
+                                               'master', None))
+    
+    # Check default target refs for branches.
+    self.assertEqual('refs/heads/master',
+                     git_cl.GetTargetRef('origin', 'refs/remotes/origin/master',
+                                         None, None))
+    self.assertEqual('refs/heads/master',
+                     git_cl.GetTargetRef('origin', 'refs/remotes/origin/lkgr',
+                                         None, None))
+    self.assertEqual('refs/heads/master',
+                     git_cl.GetTargetRef('origin', 'refs/remotes/origin/lkcr',
+                                         None, None))
+    self.assertEqual('refs/branch-heads/123',
+                     git_cl.GetTargetRef('origin',
+                                         'refs/remotes/branch-heads/123',
+                                         None, None))
+    self.assertEqual('refs/diff/test',
+                     git_cl.GetTargetRef('origin',
+                                         'refs/remotes/origin/refs/diff/test',
+                                         None, None))
+
+    # Check target refs for user-specified target branch.
+    for branch in ('branch-heads/123', 'remotes/branch-heads/123',
+                   'refs/remotes/branch-heads/123'):
+      self.assertEqual('refs/branch-heads/123',
+                       git_cl.GetTargetRef('origin',
+                                           'refs/remotes/origin/master',
+                                           branch, None))
+    for branch in ('origin/master', 'remotes/origin/master',
+                   'refs/remotes/origin/master'):
+      self.assertEqual('refs/heads/master',
+                       git_cl.GetTargetRef('origin',
+                                           'refs/remotes/branch-heads/123',
+                                           branch, None))
+    for branch in ('master', 'heads/master', 'refs/heads/master'):
+      self.assertEqual('refs/heads/master',
+                       git_cl.GetTargetRef('origin',
+                                           'refs/remotes/branch-heads/123',
+                                           branch, None))
+
+    # Check target refs for pending prefix.
+    self.assertEqual('prefix/heads/master',
+                     git_cl.GetTargetRef('origin', 'refs/remotes/origin/master',
+                                         None, 'prefix/'))
+
+
 if __name__ == '__main__':
   git_cl.logging.basicConfig(
       level=git_cl.logging.DEBUG if '-v' in sys.argv else git_cl.logging.ERROR)