git-squash-branch: handle empty squashes

Error out of the current tree is dirty (previously the dirty
content would be incorporated silently into the newly
squashed branch!).

Review URL: https://codereview.chromium.org/1064933004

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

git_cl.py

@@ -201,20 +201,6 @@ def add_git_similarity(parser):
   parser.parse_args = Parse
 
 
-def is_dirty_git_tree(cmd):
-  # Make sure index is up-to-date before running diff-index.
-  RunGit(['update-index', '--refresh', '-q'], error_ok=True)
-  dirty = RunGit(['diff-index', '--name-status', 'HEAD'])
-  if dirty:
-    print 'Cannot %s with a dirty tree. You must commit locally first.' % cmd
-    print 'Uncommitted files: (git diff-index --name-status HEAD)'
-    print dirty[:4096]
-    if len(dirty) > 4096:
-      print '... (run "git diff-index --name-status HEAD" to see full output).'
-    return True
-  return False
-
-
 def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards):
   """Return the corresponding git ref if |base_url| together with |glob_spec|
   matches the full |url|.
@@ -1655,7 +1641,7 @@ def CMDpresubmit(parser, args):
                     help='Run checks even if tree is dirty')
   (options, args) = parser.parse_args(args)
 
-  if not options.force and is_dirty_git_tree('presubmit'):
+  if not options.force and git_common.is_dirty_git_tree('presubmit'):
     print 'use --force to check even if tree is dirty.'
     return 1
 
@@ -2032,7 +2018,7 @@ def CMDupload(parser, args):
   add_git_similarity(parser)
   (options, args) = parser.parse_args(args)
 
-  if is_dirty_git_tree('upload'):
+  if git_common.is_dirty_git_tree('upload'):
     return 1
 
   options.reviewers = cleanup_list(options.reviewers)
@@ -2162,7 +2148,7 @@ def SendUpstream(parser, args, cmd):
   base_branch = args[0]
   base_has_submodules = IsSubmoduleMergeCommit(base_branch)
 
-  if is_dirty_git_tree(cmd):
+  if git_common.is_dirty_git_tree(cmd):
     return 1
 
   # This rev-list syntax means "show all commits not in my branch that
@@ -2538,7 +2524,7 @@ def CMDpatch(parser, args):
   issue_arg = args[0]
 
   # We don't want uncommitted changes mixed up with the patch.
-  if is_dirty_git_tree('patch'):
+  if git_common.is_dirty_git_tree('patch'):
     return 1
 
   # TODO(maruel): Use apply_issue.py
@@ -2558,7 +2544,7 @@ def CMDpatch(parser, args):
 def PatchIssue(issue_arg, reject, nocommit, directory):
   # There's a "reset --hard" when failing to apply the patch. In order
   # not to destroy users' data, make sure the tree is not dirty here.
-  assert(not is_dirty_git_tree('apply'))
+  assert(not git_common.is_dirty_git_tree('apply'))
 
   if type(issue_arg) is int or issue_arg.isdigit():
     # Input is an issue id.  Figure out the URL.
@@ -2921,7 +2907,7 @@ def CMDdiff(parser, args):
   # Staged changes would be committed along with the patch from last
   # upload, hence counted toward the "last upload" side in the final
   # diff output, and this is not what we want.
-  if is_dirty_git_tree('diff'):
+  if git_common.is_dirty_git_tree('diff'):
     return 1
 
   cl = Changelist()

git_common.py

@@ -602,6 +602,24 @@ def set_config(option, value, scope='local'):
   run('config', '--' + scope, option, value)
 
 
+def get_dirty_files():
+  # Make sure index is up-to-date before running diff-index.
+  run_with_retcode('update-index', '--refresh', '-q')
+  return run('diff-index', '--name-status', 'HEAD')
+
+
+def is_dirty_git_tree(cmd):
+  dirty = get_dirty_files()
+  if dirty:
+    print 'Cannot %s with a dirty tree. You must commit locally first.' % cmd
+    print 'Uncommitted files: (git diff-index --name-status HEAD)'
+    print dirty[:4096]
+    if len(dirty) > 4096: # pragma: no cover
+      print '... (run "git diff-index --name-status HEAD" to see full output).'
+    return True
+  return False
+
+
 def squash_current_branch(header=None, merge_base=None):
   header = header or 'git squash commit.'
   merge_base = merge_base or get_or_create_merge_base(current_branch())
@@ -610,7 +628,14 @@ def squash_current_branch(header=None, merge_base=None):
     log_msg += '\n'
   log_msg += run('log', '--reverse', '--format=%H%n%B', '%s..HEAD' % merge_base)
   run('reset', '--soft', merge_base)
+
+  if not get_dirty_files():
+    # Sometimes the squash can result in the same tree, meaning that there is
+    # nothing to commit at this point.
+    print 'Nothing to commit; squashed branch is empty'
+    return False
   run('commit', '-a', '-F', '-', indata=log_msg)
+  return True
 
 
 def tags(*args):

git_squash_branch.py

@@ -6,7 +6,7 @@
 import argparse
 import sys
 
-from git_common import squash_current_branch
+import git_common
 
 def main(args):
   parser = argparse.ArgumentParser()
@@ -14,7 +14,9 @@ def main(args):
       '-m', '--message', metavar='<msg>', default='git squash commit.',
       help='Use the given <msg> as the first line of the commit message.')
   opts = parser.parse_args(args)
-  squash_current_branch(opts.message)
+  if git_common.is_dirty_git_tree('squash-branch'):
+    return 1
+  git_common.squash_current_branch(opts.message)
   return 0
 
 

testing_support/git_test_utils.py

@@ -349,7 +349,6 @@ class GitRepo(object):
         env['GIT_%s' % singleton] = str(val)
     return env
 
-
   def git(self, *args, **kwargs):
     """Runs a git command specified by |args| in this repo."""
     assert self.repo_path is not None

tests/git_cl_test.py

@@ -75,7 +75,7 @@ class TestGitCl(TestCase):
     self.mock(subprocess2, 'check_call', self._mocked_call)
     self.mock(subprocess2, 'check_output', self._mocked_call)
     self.mock(subprocess2, 'communicate', self._mocked_call)
-    self.mock(subprocess2, 'Popen', self._mocked_call)
+    self.mock(git_common, 'is_dirty_git_tree', lambda x: False)
     self.mock(git_common, 'get_or_create_merge_base',
               lambda *a: (
                   self._mocked_call(['get_or_create_merge_base']+list(a))))
@@ -156,8 +156,6 @@ class TestGitCl(TestCase):
       similarity_call,
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
       find_copies_call,
-      ((['git', 'update-index', '--refresh', '-q'],), ''),
-      ((['git', 'diff-index', '--name-status', 'HEAD'],), ''),
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
       ((['git', 'config', 'branch.master.merge'],), 'master'),
       ((['git', 'config', 'branch.master.remote'],), 'origin'),
@@ -280,8 +278,6 @@ class TestGitCl(TestCase):
       ((['git', 'rev-list', '--merges',
          '--grep=^SVN changes up to revision [0-9]*$',
          'refs/remotes/origin/master^!'],), ''),
-      ((['git', 'update-index', '--refresh', '-q'],), ''),
-      ((['git', 'diff-index', '--name-status', 'HEAD'],), ''),
       ((['git', 'rev-list', '^refs/heads/working',
          'refs/remotes/origin/master'],),
          ''),
@@ -544,8 +540,6 @@ class TestGitCl(TestCase):
         ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
         ((['git', 'config', '--int', '--get',
           'branch.master.git-find-copies'],), ''),
-        ((['git', 'update-index', '--refresh', '-q'],), ''),
-        ((['git', 'diff-index', '--name-status', 'HEAD'],), ''),
         ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
         ((['git', 'config', 'branch.master.merge'],), 'master'),
         ((['git', 'config', 'branch.master.remote'],), 'origin'),

tests/git_common_test.py

@@ -561,10 +561,17 @@ class GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase,
       ('root_A', 'root_X'),
     ])
 
+  def testIsGitTreeDirty(self):
+    self.assertEquals(False, self.repo.run(self.gc.is_dirty_git_tree, 'foo'))
+    self.repo.open('test.file', 'w').write('test data')
+    self.repo.git('add', 'test.file')
+    self.assertEquals(True, self.repo.run(self.gc.is_dirty_git_tree, 'foo'))
+
   def testSquashBranch(self):
     self.repo.git('checkout', 'branch_K')
 
-    self.repo.run(self.gc.squash_current_branch, 'cool message')
+    self.assertEquals(True, self.repo.run(self.gc.squash_current_branch,
+                                          'cool message'))
 
     lines = ['cool message', '']
     for l in 'HIJK':
@@ -580,6 +587,14 @@ class GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase,
       'K'
     )
 
+  def testSquashBranchEmpty(self):
+    self.repo.git('checkout', 'branch_K')
+    self.repo.git('checkout', 'branch_G', '.')
+    self.repo.git('commit', '-m', 'revert all changes no branch')
+    # Should return False since the quash would result in an empty commit
+    stdout = self.repo.capture_stdio(self.gc.squash_current_branch)[0]
+    self.assertEquals(stdout, 'Nothing to commit; squashed branch is empty\n')
+
   def testRebase(self):
     self.assertSchema("""
     A B C D E F G