Make git try warn on a dirty index.

trychange.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """Client-side script to send a try job to the try server. It communicates to
 the try server by either writting to a svn repository or by directly connecting
 to the server by HTTP.
 """
 
@@ skipping 46 matching lines @@
   Use svn to store the try job, specify an alternate email address and use a
   premade diff file on the local drive:
     %(prog)s --email user@example.com
             --svn_repo svn://svn.chromium.org/chrome-try/try --diff foo.diff
 
   Running only on a 'mac' slave with revision 123 and clobber first; specify
   manually the 3 source files to use for the try job:
     %(prog)s --bot mac --revision 123 --clobber -f src/a.cc -f src/a.h
             -f include/b.h
 """
 

[cmp, 2012/08/10 23:51:50]

nit: insert an empty line before line 67

[gavinp, 2012/08/12 16:52:16]

Done.

+def RunCommand(args, error_ok=False, error_message=None, **kwargs):
+  try:
+    return subprocess2.check_output(args, shell=False, **kwargs)
+  except subprocess2.CalledProcessError, e:
+    if not error_ok:
+      DieWithError(
+          'Command "%s" failed.\n%s' % (
+            ' '.join(args), error_message or e.stdout or ''))
+    return e.stdout
+
+
+def RunGit(args, **kwargs):
+  """Returns stdout."""
+  return RunCommand(['git'] + args, **kwargs)
+

[cmp, 2012/08/10 23:51:50]

nit: insert an empty line before line 82

[gavinp, 2012/08/12 16:52:16]

Done.

 class InvalidScript(Exception):
   def __str__(self):
     return self.args[0] + '\n' + HELP_STRING
 
 
 class NoTryServerAccess(Exception):
   def __str__(self):
     return self.args[0] + '\n' + HELP_STRING
 
 
@@ skipping 187 matching lines @@
             "(via the --track argument to \"git checkout -b ...\"")
     logging.info("GIT(%s)" % self.checkout_root)
 
   def CaptureStatus(self):
     return scm.GIT.CaptureStatus(
         [],
         self.checkout_root.replace(os.sep, '/'),
         self.diff_against)
 
   def GenerateDiff(self):
+    if RunGit(['diff-index', 'HEAD']):
+      # TODO(gavinp): Can we just include the index?

[cmp, 2012/08/10 23:51:50]

safe to remove this line, we can't include the index, since our workflow assumes
that devs will be committing their changes to their repos.

[gavinp, 2012/08/12 16:52:16]

I've removed the comment, and I'll think about this more.

But commit-first is definitely not my ideal workflow for try jobs, although the
tool currently requires this. I frequently have changes that fail on one
platform, and rather than do a local build, I send a try job to see if the
obvious fix worked, and move on to other work. The reason that I made this
change is that I frequently confused myself by running my commits only, without
my index, and I wouldn't understand why my changes had no effect.

Although perhaps the nonce commit makes sense; to change repos I need to commit
anyway, or use the unreliable stash feature. I have been known to redownload my
try diff though, and just commit that later.

So I see both sides, especially now that I realise I need to switch to another
repo anyway while i wait for the try job. Hrm.

+      print 'Cannot try with a dirty tree.  You must commit locally first.'
+      return None
     return scm.GIT.GenerateDiff(
         self.checkout_root,
         files=self.files,
         full_move=True,
         branch=self.diff_against)
 
 
 def _ParseSendChangeOptions(options):
   """Parse common options passed to _SendChangeHTTP and _SendChangeSVN."""
   values = [
@@ skipping 458 matching lines @@
       diff_url = ('%s/download/issue%d_%d.diff' %
           (options.rietveld_url,  options.issue, options.patchset))
       diff = GetMungedDiff('', urllib.urlopen(diff_url).readlines())
       options.diff = ''.join(diff[0])
       changed_files = diff[1]
     else:
       # Use this as the base.
       root = checkouts[0].checkout_root
       diffs = []
       for checkout in checkouts:
-        diff = checkout.GenerateDiff().splitlines(True)
+        raw_diff = checkout.GenerateDiff()
+        if not raw_diff:
+          return 1
+        diff = raw_diff.splitlines(True)
         path_diff = gclient_utils.PathDifference(root, checkout.checkout_root)
         # Munge it.
         diffs.extend(GetMungedDiff(path_diff, diff)[0])
       options.diff = ''.join(diffs)
 
     if not options.name:
       if options.issue:
         options.name = 'Issue %s' % options.issue
       else:
         options.name = 'Unnamed'
@@ skipping 74 matching lines @@
     return 1
   except (gclient_utils.Error, subprocess2.CalledProcessError), e:
     print >> sys.stderr, e
     return 1
   return 0
 
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
   sys.exit(TryChange(None, None, False))