presubmit_support: Add flags for Gerrit project and target branch.

Will be used in a follow-up CL to initialize code-owners client.

Recipe-Nontrivial-Roll: build
Change-Id: Iefe9176320b4d1ae7715e88a8db132e815be76ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2717979
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>

git_cl.py

@@ -1290,14 +1290,17 @@ class Changelist(object):
 
     args.extend(['--verbose'] * verbose)
 
+    remote, remote_branch = self.GetRemoteBranch()
+    target_ref = GetTargetRef(remote, remote_branch, None)
+    args.extend(['--gerrit_url', self.GetCodereviewServer()])
+    args.extend(['--gerrit_project', self.GetGerritProject()])
+    args.extend(['--gerrit_branch', target_ref])
+
     author = self.GetAuthor()
-    gerrit_url = self.GetCodereviewServer()
     issue = self.GetIssue()
     patchset = self.GetPatchset()
     if author:
       args.extend(['--author', author])
-    if gerrit_url:
-      args.extend(['--gerrit_url', gerrit_url])
     if issue:
       args.extend(['--issue', str(issue)])
     if patchset:
@@ -1319,11 +1322,9 @@ class Changelist(object):
 
     with gclient_utils.temporary_file() as description_file:
       with gclient_utils.temporary_file() as json_output:
-
         gclient_utils.FileWrite(description_file, description)
         args.extend(['--json_output', json_output])
         args.extend(['--description_file', description_file])
-        args.extend(['--gerrit_project', self.GetGerritProject()])
 
         start = time_time()
         cmd = ['vpython', PRESUBMIT_SUPPORT] + args

presubmit_support.py

@@ -376,8 +376,10 @@ class GerritAccessor(object):
   To avoid excessive Gerrit calls, caches the results.
   """
 
-  def __init__(self, host):
-    self.host = host
+  def __init__(self, url=None, project=None, branch=None):
+    self.host = urlparse.urlparse(url).netloc if url else None
+    self.project = project
+    self.branch = branch
     self.cache = {}
 
   def _FetchChangeDetail(self, issue):
@@ -1507,7 +1509,7 @@ def DoPostUploadExecuter(change,
 
 class PresubmitExecuter(object):
   def __init__(self, change, committing, verbose, gerrit_obj, dry_run=None,
-               thread_pool=None, parallel=False, gerrit_project=None):
+               thread_pool=None, parallel=False):
     """
     Args:
       change: The Change object.
@@ -1525,7 +1527,6 @@ class PresubmitExecuter(object):
     self.more_cc = []
     self.thread_pool = thread_pool
     self.parallel = parallel
-    self.gerrit_project = gerrit_project
 
   def ExecPresubmitScript(self, script_text, presubmit_path):
     """Executes a single presubmit script.
@@ -1567,10 +1568,10 @@ class PresubmitExecuter(object):
     rel_path = rel_path.replace(os.path.sep, '/')
 
     # Get the URL of git remote origin and use it to identify host and project
-    host = ''
-    if self.gerrit and self.gerrit.host:
-      host = self.gerrit.host
-    project = self.gerrit_project or ''
+    host = project = ''
+    if self.gerrit:
+      host = self.gerrit.host or ''
+      project = self.gerrit.project or ''
 
     # Prefix for test names
     prefix = 'presubmit:%s/%s:%s/' % (host, project, rel_path)
@@ -1669,8 +1670,7 @@ def DoPresubmitChecks(change,
                       gerrit_obj,
                       dry_run=None,
                       parallel=False,
-                      json_output=None,
-                      gerrit_project=None):
+                      json_output=None):
   """Runs all presubmit checks that apply to the files in the change.
 
   This finds all PRESUBMIT.py files in directories enclosing the files in the
@@ -1713,7 +1713,7 @@ def DoPresubmitChecks(change,
     results = []
     thread_pool = ThreadPool()
     executer = PresubmitExecuter(change, committing, verbose, gerrit_obj,
-                                 dry_run, thread_pool, parallel, gerrit_project)
+                                 dry_run, thread_pool, parallel)
     if default_presubmit:
       if verbose:
         sys.stdout.write('Running default presubmit script.\n')
@@ -1874,7 +1874,10 @@ def _parse_gerrit_options(parser, options):
   """
   gerrit_obj = None
   if options.gerrit_url:
-    gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc)
+    gerrit_obj = GerritAccessor(
+        url=options.gerrit_url,
+        project=options.gerrit_project,
+        branch=options.gerrit_branch)
 
   if not options.gerrit_fetch:
     return gerrit_obj
@@ -1946,6 +1949,8 @@ def main(argv=None):
                       'to skip multiple canned checks.')
   parser.add_argument('--dry_run', action='store_true', help=argparse.SUPPRESS)
   parser.add_argument('--gerrit_url', help=argparse.SUPPRESS)
+  parser.add_argument('--gerrit_project', help=argparse.SUPPRESS)
+  parser.add_argument('--gerrit_branch', help=argparse.SUPPRESS)
   parser.add_argument('--gerrit_fetch', action='store_true',
                       help=argparse.SUPPRESS)
   parser.add_argument('--parallel', action='store_true',
@@ -1959,7 +1964,6 @@ def main(argv=None):
                       help='List of files to be marked as modified when '
                       'executing presubmit or post-upload hooks. fnmatch '
                       'wildcards can also be used.')
-  parser.add_argument('--gerrit_project', help=argparse.SUPPRESS)
   options = parser.parse_args(argv)
 
   log_level = logging.ERROR
@@ -1992,8 +1996,7 @@ def main(argv=None):
           gerrit_obj,
           options.dry_run,
           options.parallel,
-          options.json_output,
-          options.gerrit_project)
+          options.json_output)
   except PresubmitFailure as e:
     print(e, file=sys.stderr)
     print('Maybe your depot_tools is out of date?', file=sys.stderr)

recipes/recipe_modules/presubmit/api.py

@@ -98,6 +98,8 @@ class PresubmitApi(recipe_api.RecipeApi):
       '--issue', self.m.tryserver.gerrit_change.change,
       '--patchset', self.m.tryserver.gerrit_change.patchset,
       '--gerrit_url', 'https://%s' % self.m.tryserver.gerrit_change.host,
+      '--gerrit_project', self.m.tryserver.gerrit_change.project,
+      '--gerrit_branch', self.m.tryserver.gerrit_change_target_ref,
       '--gerrit_fetch',
     ]
     if self.m.cq.state == self.m.cq.DRY:

tests/git_cl_test.py

@@ -2958,13 +2958,16 @@ class ChangelistTest(unittest.TestCase):
     mock.patch('git_cl.Changelist.GetAuthor', return_value='author').start()
     mock.patch('git_cl.Changelist.GetIssue', return_value=123456).start()
     mock.patch('git_cl.Changelist.GetPatchset', return_value=7).start()
+    mock.patch(
+        'git_cl.Changelist.GetRemoteBranch',
+        return_value=('origin', 'refs/remotes/origin/main')).start()
     mock.patch('git_cl.PRESUBMIT_SUPPORT', 'PRESUBMIT_SUPPORT').start()
     mock.patch('git_cl.Settings.GetRoot', return_value='root').start()
     mock.patch('git_cl.time_time').start()
     mock.patch('metrics.collector').start()
     mock.patch('subprocess2.Popen').start()
-    mock.patch('git_cl.Changelist.GetGerritProject',
-        return_value='https://chromium-review.googlesource.com').start()
+    mock.patch(
+        'git_cl.Changelist.GetGerritProject', return_value='project').start()
     self.addCleanup(mock.patch.stopall)
     self.temp_count = 0
 
@@ -2996,8 +2999,10 @@ class ChangelistTest(unittest.TestCase):
         '--root', 'root',
         '--upstream', 'upstream',
         '--verbose', '--verbose',
-        '--author', 'author',
         '--gerrit_url', 'https://chromium-review.googlesource.com',
+        '--gerrit_project', 'project',
+        '--gerrit_branch', 'refs/heads/main',
+        '--author', 'author',
         '--issue', '123456',
         '--patchset', '7',
         '--commit',
@@ -3006,7 +3011,6 @@ class ChangelistTest(unittest.TestCase):
         '--all_files',
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
-        '--gerrit_project', 'https://chromium-review.googlesource.com',
     ])
     gclient_utils.FileWrite.assert_called_once_with(
         '/tmp/fake-temp1', 'description')
@@ -3030,7 +3034,6 @@ class ChangelistTest(unittest.TestCase):
     git_cl.Changelist.GetAuthor.return_value = None
     git_cl.Changelist.GetIssue.return_value = None
     git_cl.Changelist.GetPatchset.return_value = None
-    git_cl.Changelist.GetCodereviewServer.return_value = None
 
     cl = git_cl.Changelist()
     results = cl.RunHook(
@@ -3048,10 +3051,12 @@ class ChangelistTest(unittest.TestCase):
         'vpython', 'PRESUBMIT_SUPPORT',
         '--root', 'root',
         '--upstream', 'upstream',
+        '--gerrit_url', 'https://chromium-review.googlesource.com',
+        '--gerrit_project', 'project',
+        '--gerrit_branch', 'refs/heads/main',
         '--upload',
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
-        '--gerrit_project', 'https://chromium-review.googlesource.com',
     ])
     gclient_utils.FileWrite.assert_called_once_with(
         '/tmp/fake-temp1', 'description')
@@ -3075,7 +3080,6 @@ class ChangelistTest(unittest.TestCase):
     git_cl.Changelist.GetAuthor.return_value = None
     git_cl.Changelist.GetIssue.return_value = None
     git_cl.Changelist.GetPatchset.return_value = None
-    git_cl.Changelist.GetCodereviewServer.return_value = None
 
     cl = git_cl.Changelist()
     results = cl.RunHook(
@@ -3095,10 +3099,12 @@ class ChangelistTest(unittest.TestCase):
         'vpython', 'PRESUBMIT_SUPPORT',
         '--root', 'root',
         '--upstream', 'upstream',
+        '--gerrit_url', 'https://chromium-review.googlesource.com',
+        '--gerrit_project', 'project',
+        '--gerrit_branch', 'refs/heads/main',
         '--upload',
         '--json_output', '/tmp/fake-temp2',
         '--description_file', '/tmp/fake-temp1',
-        '--gerrit_project', 'https://chromium-review.googlesource.com',
     ])
 
   @mock.patch('sys.exit', side_effect=SystemExitMock)
@@ -3131,8 +3137,10 @@ class ChangelistTest(unittest.TestCase):
         '--root', 'root',
         '--upstream', 'upstream',
         '--verbose', '--verbose',
-        '--author', 'author',
         '--gerrit_url', 'https://chromium-review.googlesource.com',
+        '--gerrit_project', 'project',
+        '--gerrit_branch', 'refs/heads/main',
+        '--author', 'author',
         '--issue', '123456',
         '--patchset', '7',
         '--post_upload',

tests/presubmit_unittest.py

@@ -500,7 +500,8 @@ class PresubmitUnittest(PresubmitTestsBase):
         0,
         0,
         None)
-    executer = presubmit.PresubmitExecuter(change, False, None, False)
+    executer = presubmit.PresubmitExecuter(
+        change, False, None, presubmit.GerritAccessor())
     self.assertFalse(executer.ExecPresubmitScript('', fake_presubmit))
     # No error if no on-upload entry point
     self.assertFalse(executer.ExecPresubmitScript(
@@ -509,7 +510,8 @@ class PresubmitUnittest(PresubmitTestsBase):
       fake_presubmit
     ))
 
-    executer = presubmit.PresubmitExecuter(change, True, None, False)
+    executer = presubmit.PresubmitExecuter(
+        change, True, None, presubmit.GerritAccessor())
     # No error if no on-commit entry point
     self.assertFalse(executer.ExecPresubmitScript(
       ('def CheckChangeOnUpload(input_api, output_api):\n'
@@ -556,7 +558,8 @@ class PresubmitUnittest(PresubmitTestsBase):
     fake_presubmit = os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
     change = presubmit.Change('mychange', '\n'.join(description_lines),
                               self.fake_root_dir, files, 0, 0, None)
-    executer = presubmit.PresubmitExecuter(change, True, None, False)
+    executer = presubmit.PresubmitExecuter(
+        change, True, None, presubmit.GerritAccessor())
     sink = self.rdb_client.__enter__.return_value = mock.MagicMock()
 
     # STATUS_PASS on success
@@ -591,7 +594,7 @@ class PresubmitUnittest(PresubmitTestsBase):
 
     fake_presubmit = os.path.join(self.fake_root_dir, 'PRESUBMIT.py')
     executer = presubmit.PresubmitExecuter(
-        self.fake_change, False, None, False)
+        self.fake_change, False, None, presubmit.GerritAccessor())
 
     self.assertEqual([], executer.ExecPresubmitScript(
       ('def CheckChangeOnUpload(input_api, output_api):\n'
@@ -1156,40 +1159,43 @@ def CheckChangeOnCommit(input_api, output_api):
     self.assertEqual('author', options.author)
     self.assertEqual('description', options.description)
 
-  @mock.patch('presubmit_support.GerritAccessor', mock.Mock())
   def testParseGerritOptions_NoGerritFetch(self):
     options = mock.Mock(
         gerrit_url='https://foo-review.googlesource.com/bar',
+        gerrit_project='project',
+        gerrit_branch='refs/heads/main',
         gerrit_fetch=False,
         author='author',
         description='description')
 
     gerrit_obj = presubmit._parse_gerrit_options(None, options)
 
-    presubmit.GerritAccessor.assert_called_once_with(
-        'foo-review.googlesource.com')
-    self.assertEqual(presubmit.GerritAccessor.return_value, gerrit_obj)
+    self.assertEqual('foo-review.googlesource.com', gerrit_obj.host)
+    self.assertEqual('project', gerrit_obj.project)
+    self.assertEqual('refs/heads/main', gerrit_obj.branch)
     self.assertEqual('author', options.author)
     self.assertEqual('description', options.description)
 
-  @mock.patch('presubmit_support.GerritAccessor', mock.Mock())
-  def testParseGerritOptions_GerritFetch(self):
-    accessor = mock.Mock()
-    accessor.GetChangeOwner.return_value = 'new owner'
-    accessor.GetChangeDescription.return_value = 'new description'
-    presubmit.GerritAccessor.return_value = accessor
+  @mock.patch('presubmit_support.GerritAccessor.GetChangeOwner')
+  @mock.patch('presubmit_support.GerritAccessor.GetChangeDescription')
+  def testParseGerritOptions_GerritFetch(
+      self, mockDescription, mockOwner):
+    mockDescription.return_value = 'new description'
+    mockOwner.return_value = 'new owner'
 
     options = mock.Mock(
         gerrit_url='https://foo-review.googlesource.com/bar',
+        gerrit_project='project',
+        gerrit_branch='refs/heads/main',
         gerrit_fetch=True,
         issue=123,
         patchset=4)
 
     gerrit_obj = presubmit._parse_gerrit_options(None, options)
 
-    presubmit.GerritAccessor.assert_called_once_with(
-        'foo-review.googlesource.com')
-    self.assertEqual(presubmit.GerritAccessor.return_value, gerrit_obj)
+    self.assertEqual('foo-review.googlesource.com', gerrit_obj.host)
+    self.assertEqual('project', gerrit_obj.project)
+    self.assertEqual('refs/heads/main', gerrit_obj.branch)
     self.assertEqual('new owner', options.author)
     self.assertEqual('new description', options.description)