Add --rietveld_XXX arguments to presubmit_support to the commit queue can use it.

presubmit_support.py can now creates a Rietveld object on its own.

This is necessary since the object needs to be recreated out of process. The
commit queue runs the presubmit check out of process for sanity reasons.

Added TODO to push the cookie instead of the password.

R=dpranke@chromium.org
BUG=
TEST=

Review URL: http://codereview.chromium.org/7058054

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

gcl.py

@@ -1235,7 +1235,7 @@ def DoPresubmitChecks(change_info, committing, may_prompt):
       default_presubmit=root_presubmit,
       may_prompt=may_prompt,
       tbr=False,
-      rietveld=change_info.RpcServer())
+      rietveld_obj=change_info.RpcServer())
   if not output.should_continue() and may_prompt:
     # TODO(dpranke): move into DoPresubmitChecks(), unify cmd line args.
     print "\nPresubmit errors, can't continue (use --no_presubmit to bypass)"

git_cl.py

@@ -548,7 +548,7 @@ or verify this branch is set up to track another (via the --track argument to
       output = presubmit_support.DoPresubmitChecks(change, committing,
           verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin,
           default_presubmit=None, may_prompt=may_prompt, tbr=tbr,
-          rietveld=self.RpcServer())
+          rietveld_obj=self.RpcServer())
     except presubmit_support.PresubmitFailure, e:
       DieWithError(
           ('%s\nMaybe your depot_tools is out of date?\n'

presubmit_support.py

@@ -47,6 +47,7 @@ import fix_encoding
 import gclient_utils
 import owners
 import presubmit_canned_checks
+import rietveld
 import scm
 import subprocess2 as subprocess  # Exposed through the API.
 
@@ -216,7 +217,7 @@ class InputApi(object):
   )
 
   def __init__(self, change, presubmit_path, is_committing, tbr,
-      rietveld, verbose):
+      rietveld_obj, verbose):
     """Builds an InputApi object.
 
     Args:
@@ -224,18 +225,18 @@ class InputApi(object):
       presubmit_path: The path to the presubmit script being processed.
       is_committing: True if the change is about to be committed.
       tbr: True if '--tbr' was passed to skip any reviewer/owner checks
-      rietveld: rietveld client object
+      rietveld_obj: rietveld.Rietveld client object
     """
     # Version number of the presubmit_support script.
     self.version = [int(x) for x in __version__.split('.')]
     self.change = change
     self.is_committing = is_committing
     self.tbr = tbr
-    self.rietveld = rietveld
+    self.rietveld = rietveld_obj
     # TBD
     self.host_url = 'http://codereview.chromium.org'
     if self.rietveld:
-      self.host_url = rietveld.url
+      self.host_url = self.rietveld.url
 
     # We expose various modules and functions as attributes of the input_api
     # so that presubmit scripts don't have to import them.
@@ -955,18 +956,18 @@ def DoGetTrySlaves(changed_files,
 
 
 class PresubmitExecuter(object):
-  def __init__(self, change, committing, tbr, rietveld, verbose):
+  def __init__(self, change, committing, tbr, rietveld_obj, verbose):
     """
     Args:
       change: The Change object.
       committing: True if 'gcl commit' is running, False if 'gcl upload' is.
       tbr: True if '--tbr' was passed to skip any reviewer/owner checks
-      rietveld: rietveld client object.
+      rietveld_obj: rietveld.Rietveld client object.
     """
     self.change = change
     self.committing = committing
     self.tbr = tbr
-    self.rietveld = rietveld
+    self.rietveld = rietveld_obj
     self.verbose = verbose
 
   def ExecPresubmitScript(self, script_text, presubmit_path):
@@ -1030,7 +1031,7 @@ def DoPresubmitChecks(change,
                       default_presubmit,
                       may_prompt,
                       tbr,
-                      rietveld):
+                      rietveld_obj):
   """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
@@ -1049,7 +1050,7 @@ def DoPresubmitChecks(change,
     default_presubmit: A default presubmit script to execute in any case.
     may_prompt: Enable (y/n) questions on warning or error.
     tbr: was --tbr specified to skip any reviewer/owner checks?
-    rietveld: rietveld object.
+    rietveld_obj: rietveld.Rietveld object.
 
   Warning:
     If may_prompt is true, output_stream SHOULD be sys.stdout and input_stream
@@ -1076,7 +1077,7 @@ def DoPresubmitChecks(change,
     if not presubmit_files and verbose:
       output.write("Warning, no presubmit.py found.\n")
     results = []
-    executer = PresubmitExecuter(change, committing, tbr, rietveld, verbose)
+    executer = PresubmitExecuter(change, committing, tbr, rietveld_obj, verbose)
     if default_presubmit:
       if verbose:
         output.write("Running default presubmit script.\n")
@@ -1205,6 +1206,9 @@ def Main(argv):
                     "system directories will also be searched.")
   parser.add_option("--default_presubmit")
   parser.add_option("--may_prompt", action='store_true', default=False)
+  parser.add_option("--rietveld_url", help=optparse.SUPPRESS_HELP)
+  parser.add_option("--rietveld_email", help=optparse.SUPPRESS_HELP)
+  parser.add_option("--rietveld_password", help=optparse.SUPPRESS_HELP)
   options, args = parser.parse_args(argv)
   if options.verbose >= 2:
     logging.basicConfig(level=logging.DEBUG)
@@ -1216,6 +1220,12 @@ def Main(argv):
   if not change_class:
     parser.error('For unversioned directory, <files> is not optional.')
   logging.info('Found %d file(s).' % len(files))
+  rietveld_obj = None
+  if options.rietveld_url:
+    rietveld_obj = rietveld.Rietveld(
+        options.rietveld_url,
+        options.rietveld_email,
+        options.rietveld_password)
   try:
     results = DoPresubmitChecks(
         change_class(options.name,
@@ -1232,7 +1242,7 @@ def Main(argv):
         options.default_presubmit,
         options.may_prompt,
         False,
-        None)
+        rietveld_obj)
     return not results.should_continue()
   except PresubmitFailure, e:
     print >> sys.stderr, e

rietveld.py

@@ -42,6 +42,12 @@ class Rietveld(object):
   """Accesses rietveld."""
   def __init__(self, url, email, password, extra_headers=None):
     self.url = url
+    # TODO(maruel): It's not awesome but maybe necessary to retrieve the value.
+    # It happens when the presubmit check is ran out of process, the cookie
+    # needed to be recreated from the credentials. Instead, it should pass the
+    # email and the cookie.
+    self.email = email
+    self.password = password
     if email and password:
       get_creds = lambda: (email, password)
       self.rpc_server = upload.HttpRpcServer(

tests/presubmit_unittest.py

@@ -154,7 +154,8 @@ class PresubmitUnittest(PresubmitTestsBase):
       'fix_encoding', 'fnmatch', 'gclient_utils', 'glob', 'json',
       'load_files',
       'logging', 'marshal', 'normpath', 'optparse', 'os', 'owners', 'pickle',
-      'presubmit_canned_checks', 'random', 're', 'scm', 'subprocess',
+      'presubmit_canned_checks', 'random', 're', 'rietveld', 'scm',
+      'subprocess',
       'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest', 'urllib2',
       'warn',
     ]
@@ -838,7 +839,7 @@ class InputApiUnittest(PresubmitTestsBase):
     api = presubmit.InputApi(
         self.fake_change,
         presubmit_path='foo/path/PRESUBMIT.py',
-        is_committing=False, tbr=False, rietveld=None, verbose=False)
+        is_committing=False, tbr=False, rietveld_obj=None, verbose=False)
     self.assertEquals(api.PresubmitLocalPath(), 'foo/path')
     self.assertEquals(api.change, self.fake_change)
     self.assertEquals(api.host_url, 'http://codereview.chromium.org')
@@ -1082,7 +1083,7 @@ class InputApiUnittest(PresubmitTestsBase):
     presubmit_path = join(self.fake_root_dir, 'isdir', 'PRESUBMIT.py')
     api = presubmit.InputApi(
         change=change, presubmit_path=presubmit_path,
-        is_committing=True, tbr=False, rietveld=None, verbose=False)
+        is_committing=True, tbr=False, rietveld_obj=None, verbose=False)
     paths_from_api = api.AbsoluteLocalPaths(include_dirs=True)
     self.assertEqual(len(paths_from_api), 2)
     for absolute_paths in [paths_from_change, paths_from_api]: