Reland "Remove Rietveld support from presubmit"

Since no one can upload or land changes from Rietveld anymore,
PRESUBMIT and its support files no longer need to be able to
support it.

Bug: 770408
Change-Id: I4ca6391291e7b0755c78af453c3d006ad3666a17
Reviewed-on: https://chromium-review.googlesource.com/991053Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>

git_cl.py

@@ -1525,7 +1525,6 @@ class Changelist(object):
       return presubmit_support.DoPresubmitChecks(change, committing,
           verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin,
           default_presubmit=None, may_prompt=may_prompt,
-          rietveld_obj=self._codereview_impl.GetRietveldObjForPresubmit(),
           gerrit_obj=self._codereview_impl.GetGerritObjForPresubmit())
     except presubmit_support.PresubmitFailure as e:
       DieWithError('%s\nMaybe your depot_tools is out of date?' % e)
@@ -1807,11 +1806,6 @@ class _ChangelistCodereviewBase(object):
     """Which branch-specific properties to erase when unsetting issue."""
     return []
 
-  def GetRietveldObjForPresubmit(self):
-    # This is an unfortunate Rietveld-embeddedness in presubmit.
-    # For non-Rietveld code reviews, this probably should return a dummy object.
-    raise NotImplementedError()
-
   def GetGerritObjForPresubmit(self):
     # None is valid return value, otherwise presubmit_support.GerritAccessor.
     return None
@@ -2129,9 +2123,6 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
   def CodereviewServerConfigKey(cls):
     return 'rietveldserver'
 
-  def GetRietveldObjForPresubmit(self):
-    return self.RpcServer()
-
   def SetLabels(self, enable_auto_submit, use_commit_queue, cq_dry_run):
     raise NotImplementedError()
 
@@ -2504,24 +2495,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
     """Which branch-specific properties to erase when unsetting issue."""
     return ['gerritsquashhash']
 
-  def GetRietveldObjForPresubmit(self):
-    class ThisIsNotRietveldIssue(object):
-      def __nonzero__(self):
-        # This is a hack to make presubmit_support think that rietveld is not
-        # defined, yet still ensure that calls directly result in a decent
-        # exception message below.
-        return False
-
-      def __getattr__(self, attr):
-        print(
-            'You aren\'t using Rietveld at the moment, but Gerrit.\n'
-            'Using Rietveld in your PRESUBMIT scripts won\'t work.\n'
-            'Please, either change your PRESUBMIT to not use rietveld_obj.%s,\n'
-            'or use Rietveld for codereview.\n'
-            'See also http://crbug.com/579160.' % attr)
-        raise NotImplementedError()
-    return ThisIsNotRietveldIssue()
-
   def GetGerritObjForPresubmit(self):
     return presubmit_support.GerritAccessor(self._GetGerritHost())
 

presubmit_canned_checks.py

@@ -793,15 +793,6 @@ def RunPylint(input_api, *args, **kwargs):
   return input_api.RunTests(GetPylint(input_api, *args, **kwargs), False)
 
 
-def CheckRietveldTryJobExecution(dummy_input_api, output_api,
-                                 dummy_host_url, dummy_platforms,
-                                 dummy_owner):
-  return [
-    output_api.PresubmitNotifyResult(
-        'CheckRietveldTryJobExecution is deprecated, please remove it.')
-  ]
-
-
 def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
     ignored):
   try:
@@ -895,25 +886,25 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
     return [output_fn('Missing LGTM from someone other than %s' % owner_email)]
   return []
 
+
 def GetCodereviewOwnerAndReviewers(input_api, email_regexp, approval_needed):
   """Return the owner and reviewers of a change, if any.
 
   If approval_needed is True, only reviewers who have approved the change
   will be returned.
   """
-  # Rietveld is default.
-  func = _RietveldOwnerAndReviewers
-  if input_api.gerrit:
-    func = _GerritOwnerAndReviewers
-  return func(input_api, email_regexp, approval_needed)
-
-
-def _GetRietveldIssueProps(input_api, messages):
-  """Gets the issue properties from rietveld."""
   issue = input_api.change.issue
-  if issue and input_api.rietveld:
-    return input_api.rietveld.get_issue_properties(
-        issue=int(issue), messages=messages)
+  if not issue:
+    return None, (set() if approval_needed else
+                  _ReviewersFromChange(input_api.change))
+
+  owner_email = input_api.gerrit.GetChangeOwner(issue)
+  reviewers = set(
+      r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed)
+      if _match_reviewer_email(r, owner_email, email_regexp))
+  input_api.logging.debug('owner: %s; approvals given by: %s',
+                          owner_email, ', '.join(sorted(reviewers)))
+  return owner_email, reviewers
 
 
 def _ReviewersFromChange(change):
@@ -929,49 +920,6 @@ def _ReviewersFromChange(change):
 def _match_reviewer_email(r, owner_email, email_regexp):
   return email_regexp.match(r) and r != owner_email
 
-def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
-  """Return the owner and reviewers of a change, if any.
-
-  If approval_needed is True, only reviewers who have approved the change
-  will be returned.
-  """
-  issue_props = _GetRietveldIssueProps(input_api, True)
-  if not issue_props:
-    return None, (set() if approval_needed else
-                  _ReviewersFromChange(input_api.change))
-
-  if not approval_needed:
-    return issue_props['owner_email'], set(issue_props['reviewers'])
-
-  owner_email = issue_props['owner_email']
-
-  messages = issue_props.get('messages', [])
-  approvers = set(
-      m['sender'] for m in messages
-      if m.get('approval') and _match_reviewer_email(m['sender'], owner_email,
-                                                     email_regexp))
-  return owner_email, approvers
-
-
-def _GerritOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
-  """Return the owner and reviewers of a change, if any.
-
-  If approval_needed is True, only reviewers who have approved the change
-  will be returned.
-  """
-  issue = input_api.change.issue
-  if not issue:
-    return None, (set() if approval_needed else
-                  _ReviewersFromChange(input_api.change))
-
-  owner_email = input_api.gerrit.GetChangeOwner(issue)
-  reviewers = set(
-      r for r in input_api.gerrit.GetChangeReviewers(issue, approval_needed)
-      if _match_reviewer_email(r, owner_email, email_regexp))
-  input_api.logging.debug('owner: %s; approvals given by: %s',
-                          owner_email, ', '.join(sorted(reviewers)))
-  return owner_email, reviewers
-
 
 def CheckSingletonInHeaders(input_api, output_api, source_file_filter=None):
   """Deprecated, must be removed."""

presubmit_support.py

@@ -43,7 +43,6 @@ import urlparse
 from warnings import warn
 
 # Local imports.
-import auth
 import fix_encoding
 import gclient_utils
 import git_footers
@@ -51,7 +50,6 @@ import gerrit_util
 import owners
 import owners_finder
 import presubmit_canned_checks
-import rietveld
 import scm
 import subprocess2 as subprocess  # Exposed through the API.
 
@@ -534,14 +532,13 @@ class InputApi(object):
   )
 
   def __init__(self, change, presubmit_path, is_committing,
-      rietveld_obj, verbose, gerrit_obj=None, dry_run=None, thread_pool=None):
+      verbose, gerrit_obj, dry_run=None, thread_pool=None):
     """Builds an InputApi object.
 
     Args:
       change: A presubmit.Change object.
       presubmit_path: The path to the presubmit script being processed.
       is_committing: True if the change is about to be committed.
-      rietveld_obj: rietveld.Rietveld client object
       gerrit_obj: provides basic Gerrit codereview functionality.
       dry_run: if true, some Checks will be skipped.
     """
@@ -549,13 +546,8 @@ class InputApi(object):
     self.version = [int(x) for x in __version__.split('.')]
     self.change = change
     self.is_committing = is_committing
-    self.rietveld = rietveld_obj
     self.gerrit = gerrit_obj
     self.dry_run = dry_run
-    # TBD
-    self.host_url = 'http://codereview.chromium.org'
-    if self.rietveld:
-      self.host_url = self.rietveld.url
 
     self.thread_pool = thread_pool or ThreadPool()
 
@@ -1406,19 +1398,17 @@ def DoPostUploadExecuter(change,
 
 
 class PresubmitExecuter(object):
-  def __init__(self, change, committing, rietveld_obj, verbose,
-               gerrit_obj=None, dry_run=None, thread_pool=None):
+  def __init__(self, change, committing, verbose,
+               gerrit_obj, dry_run=None, thread_pool=None):
     """
     Args:
       change: The Change object.
       committing: True if 'git cl land' is running, False if 'git cl upload' is.
-      rietveld_obj: rietveld.Rietveld client object.
       gerrit_obj: provides basic Gerrit codereview functionality.
       dry_run: if true, some Checks will be skipped.
     """
     self.change = change
     self.committing = committing
-    self.rietveld = rietveld_obj
     self.gerrit = gerrit_obj
     self.verbose = verbose
     self.dry_run = dry_run
@@ -1443,9 +1433,8 @@ class PresubmitExecuter(object):
 
     # Load the presubmit script into context.
     input_api = InputApi(self.change, presubmit_path, self.committing,
-                         self.rietveld, self.verbose,
-                         gerrit_obj=self.gerrit, dry_run=self.dry_run,
-                         thread_pool=self.thread_pool)
+                         self.verbose, gerrit_obj=self.gerrit,
+                         dry_run=self.dry_run, thread_pool=self.thread_pool)
     output_api = OutputApi(self.committing)
     context = {}
     try:
@@ -1491,8 +1480,7 @@ def DoPresubmitChecks(change,
                       input_stream,
                       default_presubmit,
                       may_prompt,
-                      rietveld_obj,
-                      gerrit_obj=None,
+                      gerrit_obj,
                       dry_run=None):
   """Runs all presubmit checks that apply to the files in the change.
 
@@ -1512,7 +1500,6 @@ def DoPresubmitChecks(change,
     default_presubmit: A default presubmit script to execute in any case.
     may_prompt: Enable (y/n) questions on warning or error. If False,
                 any questions are answered with yes by default.
-    rietveld_obj: rietveld.Rietveld object.
     gerrit_obj: provides basic Gerrit codereview functionality.
     dry_run: if true, some Checks will be skipped.
 
@@ -1542,7 +1529,7 @@ def DoPresubmitChecks(change,
       output.write("Warning, no PRESUBMIT.py found.\n")
     results = []
     thread_pool = ThreadPool()
-    executer = PresubmitExecuter(change, committing, rietveld_obj, verbose,
+    executer = PresubmitExecuter(change, committing, verbose,
                                  gerrit_obj, dry_run, thread_pool)
     if default_presubmit:
       if verbose:
@@ -1701,17 +1688,8 @@ def main(argv=None):
   parser.add_option("--gerrit_url", help=optparse.SUPPRESS_HELP)
   parser.add_option("--gerrit_fetch", action='store_true',
                     help=optparse.SUPPRESS_HELP)
-  parser.add_option("--rietveld_url", help=optparse.SUPPRESS_HELP)
-  parser.add_option("--rietveld_email", help=optparse.SUPPRESS_HELP)
-  parser.add_option("--rietveld_fetch", action='store_true', default=False,
-                    help=optparse.SUPPRESS_HELP)
-  # These are for OAuth2 authentication for bots. See also apply_issue.py
-  parser.add_option("--rietveld_email_file", help=optparse.SUPPRESS_HELP)
-  parser.add_option("--rietveld_private_key_file", help=optparse.SUPPRESS_HELP)
 
-  auth.add_auth_options(parser)
   options, args = parser.parse_args(argv)
-  auth_config = auth.extract_auth_config_from_options(options)
 
   if options.verbose >= 2:
     logging.basicConfig(level=logging.DEBUG)
@@ -1720,49 +1698,14 @@ def main(argv=None):
   else:
     logging.basicConfig(level=logging.ERROR)
 
-  if (any((options.rietveld_url, options.rietveld_email_file,
-           options.rietveld_fetch, options.rietveld_private_key_file))
-      and any((options.gerrit_url, options.gerrit_fetch))):
-    parser.error('Options for only codereview --rietveld_* or --gerrit_* '
-                 'allowed')
-
-  if options.rietveld_email and options.rietveld_email_file:
-    parser.error("Only one of --rietveld_email or --rietveld_email_file "
-                 "can be passed to this program.")
-  if options.rietveld_email_file:
-    with open(options.rietveld_email_file, "rb") as f:
-      options.rietveld_email = f.read().strip()
-
   change_class, files = load_files(options, args)
   if not change_class:
     parser.error('For unversioned directory, <files> is not optional.')
   logging.info('Found %d file(s).', len(files))
 
-  rietveld_obj, gerrit_obj = None, None
-
-  if options.rietveld_url:
-    # The empty password is permitted: '' is not None.
-    if options.rietveld_private_key_file:
-      rietveld_obj = rietveld.JwtOAuth2Rietveld(
-        options.rietveld_url,
-        options.rietveld_email,
-        options.rietveld_private_key_file)
-    else:
-      rietveld_obj = rietveld.CachingRietveld(
-        options.rietveld_url,
-        auth_config,
-        options.rietveld_email)
-    if options.rietveld_fetch:
-      assert options.issue
-      props = rietveld_obj.get_issue_properties(options.issue, False)
-      options.author = props['owner_email']
-      options.description = props['description']
-      logging.info('Got author: "%s"', options.author)
-      logging.info('Got description: """\n%s\n"""', options.description)
-
+  gerrit_obj = None
   if options.gerrit_url and options.gerrit_fetch:
     assert options.issue and options.patchset
-    rietveld_obj = None
     gerrit_obj = GerritAccessor(urlparse.urlparse(options.gerrit_url).netloc)
     options.author = gerrit_obj.GetChangeOwner(options.issue)
     options.description = gerrit_obj.GetChangeDescription(options.issue,
@@ -1787,7 +1730,6 @@ def main(argv=None):
           sys.stdin,
           options.default_presubmit,
           options.may_prompt,
-          rietveld_obj,
           gerrit_obj,
           options.dry_run)
     return not results.should_continue()