Disallow applying patches when git tree is dirty

git_cl.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.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld."""
 
 from distutils.version import LooseVersion
@@ skipping 2383 matching lines @@
                     help='failed patches spew .rej files rather than '
                         'attempting a 3-way merge')
   parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit',
                     help="don't commit after patch applies")
   (options, args) = parser.parse_args(args)
   if len(args) != 1:
     parser.print_help()
     return 1
   issue_arg = args[0]
 
+  if is_dirty_git_tree('patch'):
+    return 1
+
   # TODO(maruel): Use apply_issue.py
   # TODO(ukai): use gerrit-cherry-pick for gerrit repository?
 
   if options.newbranch:
     if options.force:
       RunGit(['branch', '-D', options.newbranch],
           stderr=subprocess2.PIPE, error_ok=True)
     RunGit(['checkout', '-b', options.newbranch,
             Changelist().GetUpstreamBranch()])
 
@@ skipping 43 matching lines @@
   if directory:
     cmd.extend(('--directory', directory))
   if reject:
     cmd.append('--reject')
   elif IsGitVersionAtLeast('1.7.12'):
     cmd.append('--3way')
   try:
     subprocess2.check_call(cmd, env=GetNoGitPagerEnv(),
                            stdin=patch_data, stdout=subprocess2.VOID)
   except subprocess2.CalledProcessError:
+    RunGit(['reset', '--hard'])
     DieWithError('Failed to apply the patch')
 
   # If we had an issue, commit the current state and register the issue.
   if not nocommit:
     RunGit(['commit', '-m', ('patch from issue %(i)s at patchset '
                              '%(p)s (http://crrev.com/%(i)s#ps%(p)s)'
                              % {'i': issue, 'p': patchset})])
     cl = Changelist()
     cl.SetIssue(issue)
     cl.SetPatchset(patchset)
@@ skipping 285 matching lines @@
     parser.error('Unrecognized args: %s' % ' '.join(args))
   cl = Changelist()
   # Ensure there actually is an issue to close.
   cl.GetDescription()
   cl.CloseIssue()
   return 0
 
 
 def CMDdiff(parser, args):
   """shows differences between local tree and last upload."""
+  if is_dirty_git_tree('diff'):
+    return 1

[iannucci, 2015/02/02 15:49:42]

Hm... why is this necessary?

[wychen, 2015/02/02 17:09:25]

This is basically a safety measure. If the user have local uncommitted changes
(dirty tree), these changes might get destroyed by "reset --hard" if the
patching step has merging conflict.

I thought about automatically "git stash" first so the user don't have to do
anything due to this change. However, if the user has partially staged changes,
it could also be slightly harmful because the user has to do "add -p" again.

If some local uncommitted changes are staged, the behavior of "git cl diff"
would be unexpected (staged changes count towards last upload).

I should probably add some comments in the code to explain this.

+
   cl = Changelist()
   issue = cl.GetIssue()
   branch = cl.GetBranch()
   if not issue:
     DieWithError('No issue found for current branch (%s)' % branch)
   TMP_BRANCH = 'git-cl-diff'
   base_branch = cl.GetCommonAncestorWithUpstream()
 
   # Create a new branch based on the merge-base
   RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch])
@@ skipping 192 matching lines @@
         ('AppEngine is misbehaving and returned HTTP %d, again. Keep faith '
           'and retry or visit go/isgaeup.\n%s') % (e.code, str(e)))
 
 
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   colorama.init()
   sys.exit(main(sys.argv[1:]))