Make sure warnings fail the check when may_prompt=False.

presubmit_support.py

 #!/usr/bin/python
 # Copyright (c) 2010 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Enables directory-specific presubmit checks to run at upload and/or commit.
 """
 
-__version__ = '1.3.5'
+__version__ = '1.4'
 
 # TODO(joi) Add caching where appropriate/needed. The API is designed to allow
 # caching (between all different invocations of presubmit scripts for a given
 # change). We should add it as our presubmit scripts start feeling slow.
 
 import cPickle  # Exposed through the API.
 import cStringIO  # Exposed through the API.
 import exceptions
 import fnmatch
 import glob
@@ skipping 1056 matching lines @@
     if items:
       output.write('** Presubmit %s **\n' % name)
       for item in items:
         item.handle(output)
         output.write('\n')
 
   total_time = time.time() - start_time
   if total_time > 1.0:
     output.write("Presubmit checks took %.1fs to calculate.\n" % total_time)
 
-  if not errors and warnings and may_prompt:
-    output.prompt_yes_no('There were presubmit warnings. '
-                         'Are you sure you wish to continue? (y/N): ')
+  if not errors and warnings:
+    if may_prompt:
+      output.prompt_yes_no('There were presubmit warnings. '
+                          'Are you sure you wish to continue? (y/N): ')
+    else:
+      output.fail()
 
   global _ASKED_FOR_FEEDBACK
   # Ask for feedback one time out of 5.
   if (len(results) and random.randint(0, 4) == 0 and not _ASKED_FOR_FEEDBACK):
     output.write("Was the presubmit check useful? Please send feedback "
                  "& hate mail to maruel@chromium.org!\n")
     _ASKED_FOR_FEEDBACK = True
   return output
 
 
@@ skipping 14 matching lines @@
 
 
 def ParseFiles(args, recursive):
   logging.debug('Searching for %s' % args)
   files = []
   for arg in args:
     files.extend([('M', f) for f in ScanSubDirs(arg, recursive)])
   return files
 
 
+def DetermineScm(options, args):
+  """Tries to determine the SCM."""
+  files = []
+  if os.path.isdir(os.path.join(options.root, '.svn')):
+    change_class = SvnChange
+    status_fn = scm.SVN.CaptureStatus
+  else:
+    is_git = os.path.isdir(os.path.join(options.root, '.git'))
+    if not is_git:
+      is_git = (0 == subprocess.call(
+          ['git', 'rev-parse', '--show-cdup'],
+          stdout=subprocess.PIPE, cwd=options.root))
+    if is_git:
+      # Only look at the subdirectories below cwd.
+      change_class = GitChange
+      status_fn = scm.GIT.CaptureStatus
+    else:
+      logging.info('Doesn\'t seem under source control. Got %d files' %
+          len(args))
+      if not args:
+        return None, None
+      change_class = Change
+  if args:
+    files = ParseFiles(args, options.recursive)
+  else:
+    # Grab modified files.
+    files = status_fn([options.root])
+  return change_class, files
+
+

[Dirk Pranke, 2011/03/17 19:12:56]

Can we put this in scm.py? It doesn't feel like it belongs here.

 def Main(argv):
-  parser = optparse.OptionParser(usage="%prog [options]",
+  parser = optparse.OptionParser(usage="%prog [options] <files...>",
                                  version="%prog " + str(__version__))
   parser.add_option("-c", "--commit", action="store_true", default=False,
                    help="Use commit instead of upload checks")
   parser.add_option("-u", "--upload", action="store_false", dest='commit',
                    help="Use upload instead of commit checks")
   parser.add_option("-r", "--recursive", action="store_true",
                    help="Act recursively")
   parser.add_option("-v", "--verbose", action="store_true", default=False,
                    help="Verbose output")
-  parser.add_option("--files")
   parser.add_option("--name", default='no name')
   parser.add_option("--description", default='')
   parser.add_option("--issue", type='int', default=0)
   parser.add_option("--patchset", type='int', default=0)
   parser.add_option("--root", default=os.getcwd(),
                     help="Search for PRESUBMIT.py up to this directory. "
                     "If inherit-review-settings-ok is present in this "
                     "directory, parent directories up to the root file "
                     "system directories will also be searched.")
   parser.add_option("--default_presubmit")
   parser.add_option("--may_prompt", action='store_true', default=False)
   options, args = parser.parse_args(argv)
   if options.verbose:
     logging.basicConfig(level=logging.DEBUG)
-  if os.path.isdir(os.path.join(options.root, '.svn')):
-    change_class = SvnChange
-    if not options.files:
-      if args:
-        options.files = ParseFiles(args, options.recursive)
-      else:
-        # Grab modified files.
-        options.files = scm.SVN.CaptureStatus([options.root])
-  else:
-    is_git = os.path.isdir(os.path.join(options.root, '.git'))
-    if not is_git:
-      is_git = (0 == subprocess.call(
-          ['git', 'rev-parse', '--show-cdup'],
-          stdout=subprocess.PIPE, cwd=options.root))
-    if is_git:
-      # Only look at the subdirectories below cwd.
-      change_class = GitChange
-      if not options.files:
-        if args:
-          options.files = ParseFiles(args, options.recursive)
-        else:
-          # Grab modified files.
-          options.files = scm.GIT.CaptureStatus([options.root])
-    else:
-      logging.info('Doesn\'t seem under source control.')
-      change_class = Change
+  change_class, files = DetermineScm(options, args)
+  if not change_class:
+    parser.error('For unversioned directory, <files> is not optional.')
   if options.verbose:
-    if not options.files:
-      print "Found no files."
-    elif len(options.files) != 1:
-      print "Found %d files." % len(options.files)
-    else:
-      print "Found 1 file."
+    print "Found %d file(s)." % len(files)
   results = DoPresubmitChecks(change_class(options.name,
                                            options.description,
                                            options.root,
-                                           options.files,
+                                           files,
                                            options.issue,
                                            options.patchset),
                               options.commit,
                               options.verbose,
                               sys.stdout,
                               sys.stdin,
                               options.default_presubmit,
                               options.may_prompt)
   return not results.should_continue()
 
 
 if __name__ == '__main__':
   sys.exit(Main(None))