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@xxxxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html