git cl: use project~number on Gerrit for better routing when setting reviewers

R=ehmaldonado@chromium.org

Testing
  patched my own depot_tools in $PATH and uploaded
  https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1186072
  which set reviewers and ccs as expected.

Bug: 876910
Change-Id: I43c0f2284941cf703133bb51132226d4a0472d8e
Reviewed-on: https://chromium-review.googlesource.com/1186068
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

gerrit_util.py

@@ -900,3 +900,14 @@ def tempdir():
   finally:
     if tdir:
       gclient_utils.rmtree(tdir)
+
+
+def ChangeIdentifier(project, change_number):
+  """Returns change identifier "project~number" suitable for |chagne| arg of
+  this module API.
+
+  Such format is allows for more efficient Gerrit routing of HTTP requests,
+  comparing to specifying just change_number.
+  """
+  assert int(change_number)
+  return '%s~%s' % (urllib.quote(project, safe=''), change_number)

git_cl.py

@@ -2399,6 +2399,15 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
         self._gerrit_server = 'https://%s' % self._gerrit_host
     return self._gerrit_server
 
+  def _GetGerritProject(self, remote_url=None):
+    """Returns Gerrit project name based on remote git URL."""
+    if remote_url is None:
+      remote_url = self.GetRemoteUrl()
+    project = urlparse.urlparse(remote_url).path.strip('/')
+    if project.endswith('.git'):
+      project = project[:-len('.git')]
+    return project
+
   @classmethod
   def IssueConfigKey(cls):
     return 'gerritissue'
@@ -3075,9 +3084,13 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
           'spaces not allowed in refspec: "%s"' % refspec_suffix)
     refspec = '%s:refs/for/%s%s' % (ref_to_push, branch, refspec_suffix)
 
+    # TODO(tandrii): hack to avoid messing with tests while resolving
+    # https://crbug.com/876910. Instead, remote_url should be cached inside this
+    # class, just like GetIssue caches issue.
+    remote_url = self.GetRemoteUrl()
     try:
       push_stdout = gclient_utils.CheckCallAndFilter(
-          ['git', 'push', self.GetRemoteUrl(), refspec],
+          ['git', 'push', remote_url, refspec],
           print_stdout=True,
           # Flush after every line: useful for seeing progress when running as
           # recipe.
@@ -3115,9 +3128,15 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
     if change_desc.get_cced():
       cc.extend(change_desc.get_cced())
 
-    gerrit_util.AddReviewers(
-        self._GetGerritHost(), self.GetIssue(), reviewers, cc,
-        notify=bool(options.send_mail))
+    if self.GetIssue():
+      # GetIssue() is not set in case of non-squash uploads according to tests.
+      # TODO(agable): non-squash uploads in git cl should be removed.
+      gerrit_util.AddReviewers(
+          self._GetGerritHost(),
+          gerrit_util.ChangeIdentifier(
+              self._GetGerritProject(remote_url), self.GetIssue()),
+          reviewers, cc,
+          notify=bool(options.send_mail))
 
     if change_desc.get_reviewers(tbr_only=True):
       labels = self._GetChangeDetail(['LABELS']).get('labels', {})
@@ -3126,7 +3145,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
         score = max([int(x) for x in labels['Code-Review']['values'].keys()])
       print('Adding self-LGTM (Code-Review +%d) because of TBRs.' % score)
       gerrit_util.SetReview(
-          self._GetGerritHost(), self.GetIssue(),
+          self._GetGerritHost(),
+          gerrit_util.ChangeIdentifier(
+              self._GetGerritProject(remote_url), self.GetIssue()),
           msg='Self-approving for TBR',
           labels={'Code-Review': score})
 

tests/git_cl_test.py

@@ -1311,25 +1311,25 @@ class TestGitCl(TestCase):
            'https://chromium.googlesource.com/yyy/zzz'),
       ]
 
-    calls += [
-        ((['git', 'push',
-           'https://chromium.googlesource.com/yyy/zzz',
-           ref_to_push + ':refs/for/refs/heads/master' + ref_suffix],),
-         ('remote:\n'
-         'remote: Processing changes: (\)\n'
-         'remote: Processing changes: (|)\n'
-         'remote: Processing changes: (/)\n'
-         'remote: Processing changes: (-)\n'
-         'remote: Processing changes: new: 1 (/)\n'
-         'remote: Processing changes: new: 1, done\n'
-         'remote:\n'
-         'remote: New Changes:\n'
-         'remote:   https://chromium-review.googlesource.com/#/c/foo/+/123456 '
-             'XXX\n'
-         'remote:\n'
-         'To https://chromium.googlesource.com/yyy/zzz\n'
-         ' * [new branch]      hhhh -> refs/for/refs/heads/master\n')),
-        ]
+    calls.append((
+      (['git', 'push',
+        'https://chromium.googlesource.com/yyy/zzz',
+        ref_to_push + ':refs/for/refs/heads/master' + ref_suffix],),
+      ('remote:\n'
+       'remote: Processing changes: (\)\n'
+       'remote: Processing changes: (|)\n'
+       'remote: Processing changes: (/)\n'
+       'remote: Processing changes: (-)\n'
+       'remote: Processing changes: new: 1 (/)\n'
+       'remote: Processing changes: new: 1, done\n'
+       'remote:\n'
+       'remote: New Changes:\n'
+       'remote:   https://chromium-review.googlesource.com/#/c/yyy/zzz/+/123456'
+           ' XXX\n'
+       'remote:\n'
+       'To https://chromium.googlesource.com/yyy/zzz\n'
+       ' * [new branch]      hhhh -> refs/for/refs/heads/master\n')
+    ))
     if squash:
       calls += [
           ((['git', 'config', 'branch.master.gerritissue', '123456'],),
@@ -1341,11 +1341,15 @@ class TestGitCl(TestCase):
       ]
     calls += [
         ((['git', 'config', 'rietveld.cc'],), ''),
-        (('AddReviewers', 'chromium-review.googlesource.com', 123456
-          if squash else None, sorted(reviewers),
-          ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] +
-          cc, notify), ''),
     ]
+    if squash:
+      calls += [
+          (('AddReviewers',
+            'chromium-review.googlesource.com', 'yyy%2Fzzz~123456',
+            sorted(reviewers),
+            ['joe@example.com', 'chromium-reviews+test-more-cc@chromium.org'] +
+            cc, notify), ''),
+      ]
     if tbr:
       calls += [
         (('GetChangeDetail', 'chromium-review.googlesource.com', '123456',
@@ -1363,8 +1367,10 @@ class TestGitCl(TestCase):
                  }
               }
           }),
-        (('SetReview', 'chromium-review.googlesource.com',
-          123456 if squash else None, 'Self-approving for TBR',
+        (('SetReview',
+          'chromium-review.googlesource.com',
+          'yyy%2Fzzz~123456',
+          'Self-approving for TBR',
           {'Code-Review': 2}, None), ''),
       ]
     calls += cls._git_post_upload_calls()