Re: [StGit PATCH] Check for local changes with "goto"

[Catalin Marinas <catalin.marinas@gmail.com>]

2009/1/29 Karl Hasselström <kha@treskal.com>:
> On 2009-01-28 23:13:05 +0000, Catalin Marinas wrote:
>> +        # Check for not clean index
>> +        if check_clean and iw and not iw.index.is_clean():
>> +            self.__halt('Repository not clean. Use "refresh" or '
>> +                        '"status --reset"')
>
> ... this, which doesn't do what I think you think it does.
>
> Index.is_clean() calls "git update-index --refresh", which checks for
> changes in the worktree relative to the index. It's bad design to have
> it in Index rather than IndexAndWorktree, but that's my fault, not
> yours. ;-) But the point that breaks your patch is that it doesn't
> check for changes between index and HEAD -- try it and see.
>
> The fix I'd suggest is to move the existing is_clean() method to
> IndexAndWorktree, and call it maybe worktree_clean(). And create a
> method in Index() called is_clean(tree) that checks whether the index
> is clean with respect to the given Tree (I think this method should
> just call "git diff-index --quiet --cached <tree>".). Then call both
> of these methods.

What about this (only pasting the relevant hunks, though they may be
wrapped by the web interface):

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index e2b4266..7e1b9dd 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -706,9 +706,15 @@ class Index(RunWithEnv):
                     ).output_one_line())
         except run.RunException:
             raise MergeException('Conflicting merge')
-    def is_clean(self):
+    def is_clean(self, tree = None):
+        """Check whether the index is clean relative to the given tree."""
+        if tree:
+            sha1 = tree.sha1
+        else:
+            sha1 = 'HEAD'
         try:
-            self.run(['git', 'update-index', '--refresh']).discard_output()
+            self.run(['git', 'diff-index', '--quiet', '--cached', sha1]
+                    ).discard_output()
         except run.RunException:
             return False
         else:
@@ -858,6 +864,15 @@ class IndexAndWorktree(RunWithEnvCwd):
         cmd = ['git', 'update-index', '--remove']
         self.run(cmd + ['-z', '--stdin']
                  ).input_nulterm(paths).discard_output()
+    def worktree_clean(self):
+        """Check whether the worktree is clean relative and no updates or
+        merges are needed."""
+        try:
+            self.run(['git', 'update-index', '--refresh']).discard_output()
+        except run.RunException:
+            return False
+        else:
+            return True

 class Branch(object):
     """Represents a Git branch."""
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 54de127..8abf296 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -183,13 +183,22 @@ class StackTransaction(object):
             self.__checkout(self.__stack.head.data.tree, iw,
                             allow_bad_head = True)
     def run(self, iw = None, set_head = True, allow_bad_head = False,
-            print_current_patch = True):
+            print_current_patch = True, check_clean = False):
         """Execute the transaction. Will either succeed, or fail (with an
         exception) and do nothing."""
         self.__check_consistency()
         log.log_external_mods(self.__stack)
         new_head = self.head

+        # Check for not clean index and worktree
+        if check_clean and iw:
+            if not iw.worktree_clean():
+                self.__halt('Repository not clean. Use "refresh" or '
+                            '"status --reset"')
+            elif not iw.index.is_clean()):
+                self.__halt('Index and HEAD different. Use "repair" to '
+                            'recover additional commits')
+
         # Set branch head.
         if set_head:
             if iw:


Thanks.

-- 
Catalin