git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[StGit PATCH 13/14] Log and undo external modifications

From
Karl Hasselström <kha@treskal.com>
Date
Jun 12, 2008, 05:35 UTC
Message-ID
<20080612053525.23549.49690.stgit@yoghurt>
In-Reply-To
<20080612052913.23549.69687.stgit@yoghurt>

At the beginning of every StGit command, quickly check if the branch head recorded in the log is the same as the actual branch head; if it's not, conclude that some non-StGit tool has modified the stack, and record a log entry that says so. (Additionally, if the log doesn't exist yet, create it.)

This introduces the possibility that a log entry specifies a head and a top that aren't equal. So teach undo, redo, and reset to deal with that case.

Signed-off-by: Karl Hasselström <kha@treskal.com>
---
 stgit/commands/common.py     |    1 
 stgit/commands/redo.py       |    4 +-
 stgit/commands/reset.py      |    8 +++-
 stgit/commands/undo.py       |    4 +-
 stgit/lib/log.py             |   88 ++++++++++++++++++++++++++++--------------
 stgit/lib/transaction.py     |   39 ++++++++++++-------
 t/t3105-undo-external-mod.sh |   68 ++++++++++++++++++++++++++++++++
 7 files changed, 162 insertions(+), 50 deletions(-)
 create mode 100755 t/t3105-undo-external-mod.sh
diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index fd02398..2689a42 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -525,6 +525,7 @@ class DirectoryAnywhere(_Directory):
 class DirectoryHasRepository(_Directory):
     def setup(self):
         self.git_dir # might throw an exception
+        log.compat_log_external_mods()
 
 class DirectoryInWorktree(DirectoryHasRepository):
     def setup(self):
diff --git a/stgit/commands/redo.py b/stgit/commands/redo.py
index 47221a5..a1075ec 100644
--- a/stgit/commands/redo.py
+++ b/stgit/commands/redo.py
@@ -46,7 +46,7 @@ def func(parser, options, args):
     trans = transaction.StackTransaction(stack, 'redo %d' % options.number,
                                          discard_changes = options.hard)
     try:
-        log.reset_stack(trans, stack.repository.default_iw, state, [])
+        log.reset_stack(trans, stack.repository.default_iw, state)
     except transaction.TransactionHalted:
         pass
-    return trans.run(stack.repository.default_iw)
+    return trans.run(stack.repository.default_iw, allow_bad_head = True)
diff --git a/stgit/commands/reset.py b/stgit/commands/reset.py
index 5ad9914..22d7731 100644
--- a/stgit/commands/reset.py
+++ b/stgit/commands/reset.py
@@ -57,7 +57,11 @@ def func(parser, options, args):
     trans = transaction.StackTransaction(stack, 'reset',
                                          discard_changes = options.hard)
     try:
-        log.reset_stack(trans, stack.repository.default_iw, state, patches)
+        if patches:
+            log.reset_stack_partially(trans, stack.repository.default_iw,
+                                      state, patches)
+        else:
+            log.reset_stack(trans, stack.repository.default_iw, state)
     except transaction.TransactionHalted:
         pass
-    return trans.run(stack.repository.default_iw)
+    return trans.run(stack.repository.default_iw, allow_bad_head = not patches)
diff --git a/stgit/commands/undo.py b/stgit/commands/undo.py
index b1d7de9..58f1da6 100644
--- a/stgit/commands/undo.py
+++ b/stgit/commands/undo.py
@@ -43,7 +43,7 @@ def func(parser, options, args):
     trans = transaction.StackTransaction(stack, 'undo %d' % options.number,
                                          discard_changes = options.hard)
     try:
-        log.reset_stack(trans, stack.repository.default_iw, state, [])
+        log.reset_stack(trans, stack.repository.default_iw, state)
     except transaction.TransactionHalted:
         pass
-    return trans.run(stack.repository.default_iw)
+    return trans.run(stack.repository.default_iw, allow_bad_head = True)
diff --git a/stgit/lib/log.py b/stgit/lib/log.py
index a8de06b..8c0516b 100644
--- a/stgit/lib/log.py
+++ b/stgit/lib/log.py
@@ -259,6 +259,7 @@ class Log(object):
             self.base = self.patches[self.applied[0]].data.parent
         else:
             self.base = self.head
+    all_patches = property(lambda self: self.applied + self.unapplied)
     @property
     def top(self):
         if self.applied:
@@ -297,34 +298,33 @@ def copy_log(repo, src_branch, dst_branch, msg):
 def default_repo():
     return stack.Repository.default()
 
-def reset_stack(trans, iw, state, only_patches):
-    """Reset the stack to a given previous state. If C{only_patches} is
-    not empty, touch only patches whose names appear in it.
+def reset_stack(trans, iw, state):
+    """Reset the stack to a given previous state."""
+    for pn in trans.all_patches:
+        trans.patches[pn] = None
+    for pn in state.all_patches:
+        trans.patches[pn] = state.patches[pn]
+    trans.applied = state.applied
+    trans.unapplied = state.unapplied
+    trans.base = state.base
+    trans.head = state.head
+
+def reset_stack_partially(trans, iw, state, only_patches):
+    """Reset the stack to a given previous state -- but only the given
+    patches, not anything else.
 
-    @param only_patches: Reset only these patches
+    @param only_patches: Touch only these patches
     @type only_patches: iterable"""
     only_patches = set(only_patches)
-    def mask(s):
-        if only_patches:
-            return s & only_patches
-        else:
-            return s
-    patches_to_reset = mask(set(state.applied + state.unapplied))
+    patches_to_reset = set(state.applied + state.unapplied) & only_patches
     existing_patches = set(trans.all_patches)
     original_applied_order = list(trans.applied)
-    to_delete = mask(existing_patches - patches_to_reset)
-
-    # If we have to change the stack base, we need to pop all patches
-    # first.
-    if not only_patches and trans.base != state.base:
-        trans.pop_patches(lambda pn: True)
-        out.info('Setting stack base to %s' % state.base.sha1)
-        trans.base = state.base
+    to_delete = (existing_patches - patches_to_reset) & only_patches
 
     # In one go, do all the popping we have to in order to pop the
     # patches we're going to delete or modify.
     def mod(pn):
-        if only_patches and not pn in only_patches:
+        if not pn in only_patches:
             return False
         if pn in to_delete:
             return True
@@ -347,17 +347,12 @@ def reset_stack(trans, iw, state, only_patches):
             out.info('Resurrecting %s' % pn)
         trans.patches[pn] = state.patches[pn]
 
-    # Push/pop patches as necessary.
-    if only_patches:
-        # Push all the patches that we've popped, if they still
-        # exist.
-        pushable = set(trans.unapplied)
-        for pn in original_applied_order:
-            if pn in pushable:
-                trans.push_patch(pn, iw)
-    else:
-        # Recreate the exact order specified by the goal state.
-        trans.reorder_patches(state.applied, state.unapplied, iw)
+    # Push all the patches that we've popped, if they still
+    # exist.
+    pushable = set(trans.unapplied)
+    for pn in original_applied_order:
+        if pn in pushable:
+            trans.push_patch(pn, iw)
 
 def undo_state(stack, undo_steps):
     """Find the log entry C{undo_steps} steps in the past. (Successive
@@ -396,3 +391,36 @@ def undo_state(stack, undo_steps):
             raise LogException('Not enough undo information available')
         log = Log(stack.repository, log.parents[0].sha1, log.parents[0])
     return log
+
+def log_external_mods(stack):
+    ref = log_ref(stack.name)
+    try:
+        log_commit = stack.repository.refs.get(ref)
+    except KeyError:
+        # No log exists yet.
+        log_entry(stack, 'start of log')
+        return
+    try:
+        log = Log(stack.repository, ref, log_commit)
+    except LogException:
+        # Something's wrong with the log, so don't bother.
+        return
+    if log.head == stack.head:
+        # No external modifications.
+        return
+    log_entry(stack, '\n'.join([
+                'external modifications', '',
+                'Modifications by tools other than StGit (e.g. git).']))
+
+def compat_log_external_mods():
+    try:
+        repo = default_repo()
+    except git.RepositoryException:
+        # No repository, so we couldn't log even if we wanted to.
+        return
+    try:
+        stack = repo.get_stack(repo.current_branch_name)
+    except exception.StgException:
+        # Stack doesn't exist, so we can't log.
+        return
+    log_external_mods(stack)
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 2003105..0c8d9a5 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -91,6 +91,7 @@ class StackTransaction(object):
         self.__current_tree = self.__stack.head.data.tree
         self.__base = self.__stack.base
         self.__discard_changes = discard_changes
+        self.__bad_head = None
         if isinstance(allow_conflicts, bool):
             self.__allow_conflicts = lambda trans: allow_conflicts
         else:
@@ -109,8 +110,22 @@ class StackTransaction(object):
                 or self.patches[self.applied[0]].data.parent == val)
         self.__base = val
     base = property(lambda self: self.__base, __set_base)
-    def __checkout(self, tree, iw):
-        if not self.__stack.head_top_equal():
+    @property
+    def top(self):
+        if self.__applied:
+            return self.__patches[self.__applied[-1]]
+        else:
+            return self.__base
+    def __get_head(self):
+        if self.__bad_head:
+            return self.__bad_head
+        else:
+            return self.top
+    def __set_head(self, val):
+        self.__bad_head = val
+    head = property(__get_head, __set_head)
+    def __checkout(self, tree, iw, allow_bad_head):
+        if not (allow_bad_head or self.__stack.head_top_equal()):
             out.error(
                 'HEAD and top are not the same.',
                 'This can happen if you modify a branch with git.',
@@ -143,26 +158,22 @@ class StackTransaction(object):
                 assert self.__stack.patches.exists(pn)
             else:
                 assert pn in remaining
-    @property
-    def __head(self):
-        if self.__applied:
-            return self.__patches[self.__applied[-1]]
-        else:
-            return self.__base
     def abort(self, iw = None):
         # The only state we need to restore is index+worktree.
         if iw:
-            self.__checkout(self.__stack.head.data.tree, iw)
-    def run(self, iw = None):
+            self.__checkout(self.__stack.head.data.tree, iw,
+                            allow_bad_head = True)
+    def run(self, iw = None, allow_bad_head = False):
         """Execute the transaction. Will either succeed, or fail (with an
         exception) and do nothing."""
         self.__check_consistency()
-        new_head = self.__head
+        log.log_external_mods(self.__stack)
+        new_head = self.head
 
         # Set branch head.
         if iw:
             try:
-                self.__checkout(new_head.data.tree, iw)
+                self.__checkout(new_head.data.tree, iw, allow_bad_head)
             except git.CheckoutException:
                 # We have to abort the transaction.
                 self.abort(iw)
@@ -257,7 +268,7 @@ class StackTransaction(object):
         cd = orig_cd.set_committer(None)
         s = ['', ' (empty)'][cd.is_nochange()]
         oldparent = cd.parent
-        cd = cd.set_parent(self.__head)
+        cd = cd.set_parent(self.top)
         base = oldparent.data.tree
         ours = cd.parent.data.tree
         theirs = cd.tree
@@ -267,7 +278,7 @@ class StackTransaction(object):
             if iw == None:
                 self.__halt('%s does not apply cleanly' % pn)
             try:
-                self.__checkout(ours, iw)
+                self.__checkout(ours, iw, allow_bad_head = False)
             except git.CheckoutException:
                 self.__halt('Index/worktree dirty')
             try:
diff --git a/t/t3105-undo-external-mod.sh b/t/t3105-undo-external-mod.sh
new file mode 100755
index 0000000..289a42a
--- /dev/null
+++ b/t/t3105-undo-external-mod.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='Undo external modifications of the stack'
+
+. ./test-lib.sh
+
+# Ignore our own output files.
+cat > .git/info/exclude <<EOF
+/expected.txt
+/head?.txt
+EOF
+
+test_expect_success 'Initialize StGit stack' '
+    stg init &&
+    echo 000 >> a &&
+    git add a &&
+    git commit -m p0 &&
+    echo 111 >> a &&
+    git add a &&
+    git commit -m p1 &&
+    stg uncommit -n 1
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success 'Make a git commit and turn it into a patch' '
+    git rev-parse HEAD > head0.txt &&
+    echo 222 >> a &&
+    git add a &&
+    git commit -m p2 &&
+    git rev-parse HEAD > head1.txt &&
+    stg repair &&
+    test "$(echo $(stg applied))" = "p1 p2" &&
+    test "$(echo $(stg unapplied))" = "" &&
+    test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+222
+EOF
+test_expect_success 'Undo the patchification' '
+    stg undo &&
+    git rev-parse HEAD > head2.txt &&
+    test_cmp head1.txt head2.txt &&
+    test "$(echo $(stg applied))" = "p1" &&
+    test "$(echo $(stg unapplied))" = "" &&
+    test_cmp expected.txt a
+'
+
+cat > expected.txt <<EOF
+000
+111
+EOF
+test_expect_success 'Undo the commit' '
+    stg undo &&
+    git rev-parse HEAD > head3.txt &&
+    test_cmp head0.txt head3.txt &&
+    test "$(echo $(stg applied))" = "p1" &&
+    test "$(echo $(stg unapplied))" = "" &&
+    test_cmp expected.txt a
+'
+
+test_done
Previous: Karl HasselströmNext: Karl Hasselström
Message 31 of 32 in “Undo series”
  1. 00/14 Undo seriesKarl Hasselström, Jun 12, 2008
  2. 01/14 Fix typoKarl Hasselström, Jun 12, 2008
  3. 02/14 Library functions for tree and blob manipulationKarl Hasselström, Jun 12, 2008
  4. 03/14 Write to a stack log when stack is modifiedKarl Hasselström, Jun 12, 2008
  5. Catalin MarinasJun 17, 2008
  6. Karl HasselströmJun 17, 2008
  7. Karl HasselströmJun 17, 2008
  8. Catalin MarinasJun 17, 2008
  9. Karl HasselströmJun 17, 2008
  10. Catalin MarinasJun 18, 2008
  11. Karl HasselströmJun 18, 2008
  12. Catalin MarinasJun 18, 2008
  13. Karl HasselströmJun 18, 2008
  14. Catalin MarinasJun 19, 2008
  15. Karl HasselströmJun 19, 2008
  16. Catalin MarinasJun 20, 2008
  17. Karl HasselströmJun 23, 2008
  18. Catalin MarinasJul 12, 2008
  19. Karl HasselströmJul 14, 2008
  20. Karl HasselströmJul 1, 2008
  21. Catalin MarinasJul 3, 2008
  22. 04/14 Add utility function for reordering patchesKarl Hasselström, Jun 12, 2008
  23. 05/14 New command: stg resetKarl Hasselström, Jun 12, 2008
  24. 06/14 Log conflicts separatelyKarl Hasselström, Jun 12, 2008
  25. 07/14 Log conflicts separately for all commandsKarl Hasselström, Jun 12, 2008
  26. 08/14 Add a --hard flag to stg resetKarl Hasselström, Jun 12, 2008
  27. 09/14 Don't write a log entry if there were no changesKarl Hasselström, Jun 12, 2008
  28. 10/14 Move stack reset function to a shared locationKarl Hasselström, Jun 12, 2008
  29. 11/14 New command: stg undoKarl Hasselström, Jun 12, 2008
  30. 12/14 New command: stg redoKarl Hasselström, Jun 12, 2008
  31. 13/14 Log and undo external modificationsKarl Hasselström, Jun 12, 2008
  32. 14/14 Make "stg log" show stack log instead of patch logKarl Hasselström, Jun 12, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.