# [PATCH 2/5] Move stack-base querying into Series class. 7 messages from 2007-02-20 to 2007-02-28. Participants: Yann Dirson. Thread: https://gitlist.dev/t/6889 ## Yann Dirson, 2007-02-20 00:13 Subject: [PATCH 0/5] stgit rebasing fixes Message-ID: <20070220000908.31139.36841.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20070220000908.31139.36841.stgit%40gandelf.nowhere.earth ``` The following series stores a backup of the stack-base to detect when it is safe to rebase or not (by "stg rebase" or "stg pull" in fetch+rebase mode). The final patch in the series is the one implementing this, but is not tested enough for 0.12.1. The previous ones, however, may be good to have in 0.12.1. Especially the patch that adds tracking of the orig-base parameter, since that will make things safer for more people when the final patch gets shipped (if no orig-base is found, rebasing is currently done without checking, to stay backwards-compatible). -- Yann Dirson | Debian-related: | Support Debian GNU/Linux: | Freedom, Power, Stability, Gratis http://ydirson.free.fr/ | Check ``` ## Yann Dirson, 2007-02-20 00:14 Subject: [PATCH 1/5] Factorize rebasing behaviour. Message-ID: <20070220001403.31139.19775.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20070220001403.31139.19775.stgit%40gandelf.nowhere.earth In-Reply-To: <20070220000908.31139.36841.stgit@gandelf.nowhere.earth> ``` Signed-off-by: Yann Dirson --- stgit/commands/common.py | 23 +++++++++++++++++++++++ stgit/commands/pull.py | 20 ++++---------------- stgit/commands/rebase.py | 17 +++-------------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/stgit/commands/common.py b/stgit/commands/common.py index 432edce..a352d89 100644 --- a/stgit/commands/common.py +++ b/stgit/commands/common.py @@ -346,3 +346,26 @@ def make_patch_name(msg, unacceptable, default_name = 'patch', suffix += 1 patchname = '%s-%d' % (patchname, suffix) return patchname + +def prepare_rebase(): + # pop all patches + applied = crt_series.get_applied() + if len(applied) > 0: + print 'Popping all applied patches...', + sys.stdout.flush() + crt_series.pop_patch(applied[0]) + print 'done' + return applied + +def rebase(target): + if target == git.get_head(): + print 'Already at "%s", no need for rebasing.' % target + return + + print 'Rebasing to "%s"...' % target + git.reset(tree_id = git_id(target)) + +def post_rebase(applied, nopush, merged): + # push the patches back + if not nopush: + push_patches(applied, merged) diff --git a/stgit/commands/pull.py b/stgit/commands/pull.py index 2d4a782..83a2725 100644 --- a/stgit/commands/pull.py +++ b/stgit/commands/pull.py @@ -61,27 +61,15 @@ def func(parser, options, args): check_conflicts() check_head_top_equal() - # pop all patches - applied = crt_series.get_applied() - if len(applied) > 0: - print 'Popping all applied patches...', - sys.stdout.flush() - crt_series.pop_patch(applied[0]) - print 'done' + applied = prepare_rebase() # pull the remote changes print 'Pulling from "%s"...' % repository git.fetch(repository) if (config.get('stgit.pull-does-rebase') == 'yes'): - fetch_head = git.fetch_head() - if fetch_head != git.get_head(): - print 'rebasing to "%s"...' % fetch_head - git.reset(tree_id = fetch_head) - print 'done' - - # push the patches back - if not options.nopush: - push_patches(applied, options.merged) + rebase(git.fetch_head()) + + post_rebase(applied, options.nopush, options.merged) # maybe tidy up if config.get('stgit.keepoptimized') == 'yes': diff --git a/stgit/commands/rebase.py b/stgit/commands/rebase.py index 2951421..63f18ec 100644 --- a/stgit/commands/rebase.py +++ b/stgit/commands/rebase.py @@ -49,19 +49,8 @@ def func(parser, options, args): check_conflicts() check_head_top_equal() - # pop all patches - applied = crt_series.get_applied() - if len(applied) > 0: - print 'Popping all applied patches...', - sys.stdout.flush() - crt_series.pop_patch(applied[0]) - print 'done' - - print 'Rebasing to "%s"...' % args[0] - git.reset(tree_id = git_id(args[0])) - - # push the patches back - if not options.nopush: - push_patches(applied, options.merged) + applied = prepare_rebase() + rebase(args[0]) + post_rebase(applied, options.nopush, options.merged) print_crt_patch() ``` ## Yann Dirson, 2007-02-20 00:14 Subject: [PATCH 2/5] Move stack-base querying into Series class. Message-ID: <20070220001408.31139.3657.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20070220001408.31139.3657.stgit%40gandelf.nowhere.earth In-Reply-To: <20070220000908.31139.36841.stgit@gandelf.nowhere.earth> ``` Signed-off-by: Yann Dirson --- stgit/commands/common.py | 2 +- stgit/stack.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/stgit/commands/common.py b/stgit/commands/common.py index a352d89..240d003 100644 --- a/stgit/commands/common.py +++ b/stgit/commands/common.py @@ -101,7 +101,7 @@ def git_id(rev): elif patch_id == 'log': return series.get_patch(patch).get_log() if patch == 'base' and patch_id == None: - return read_string(series.get_base_file()) + return series.get_base() except RevParseException: pass return git.rev_parse(rev + '^{commit}') diff --git a/stgit/stack.py b/stgit/stack.py index 3632aa1..dc6caa6 100644 --- a/stgit/stack.py +++ b/stgit/stack.py @@ -394,6 +394,9 @@ class Series(StgitObject): self.__begin_stack_check() return self.__base_file + def get_base(self): + return read_string(self.get_base_file()) + def get_protected(self): return os.path.isfile(os.path.join(self._dir(), 'protected')) @@ -609,7 +612,7 @@ class Series(StgitObject): """ try: # allow cloning of branches not under StGIT control - base = read_string(self.get_base_file()) + base = self.get_base() except: base = git.get_head() Series(target_series).init(create_at = base) ``` ## Yann Dirson, 2007-02-20 00:14 Subject: [PATCH 3/5] Various cleanups for clarity. Message-ID: <20070220001413.31139.18688.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20070220001413.31139.18688.stgit%40gandelf.nowhere.earth In-Reply-To: <20070220000908.31139.36841.stgit@gandelf.nowhere.earth> ``` Signed-off-by: Yann Dirson --- stgit/stack.py | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/stgit/stack.py b/stgit/stack.py index dc6caa6..3185d64 100644 --- a/stgit/stack.py +++ b/stgit/stack.py @@ -518,8 +518,6 @@ class Series(StgitObject): def init(self, create_at=False, parent_remote=None, parent_branch=None): """Initialises the stgit series """ - bases_dir = os.path.join(self.__base_dir, 'refs', 'bases') - if os.path.exists(self.__patch_dir): raise StackException, self.__patch_dir + ' already exists' if os.path.exists(self.__refs_dir): @@ -534,7 +532,7 @@ class Series(StgitObject): self.set_parent(parent_remote, parent_branch) - create_dirs(bases_dir) + create_dirs(os.path.join(self.__base_dir, 'refs', 'bases')) self.create_empty_field('applied') self.create_empty_field('unapplied') ``` ## Yann Dirson, 2007-02-20 00:14 Subject: [PATCH 4/5] Keep track of safe base changes. Message-ID: <20070220001418.31139.49033.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20070220001418.31139.49033.stgit%40gandelf.nowhere.earth In-Reply-To: <20070220000908.31139.36841.stgit@gandelf.nowhere.earth> ``` Signed-off-by: Yann Dirson --- stgit/commands/common.py | 2 ++ stgit/stack.py | 1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/stgit/commands/common.py b/stgit/commands/common.py index 240d003..56cb517 100644 --- a/stgit/commands/common.py +++ b/stgit/commands/common.py @@ -366,6 +366,8 @@ def rebase(target): git.reset(tree_id = git_id(target)) def post_rebase(applied, nopush, merged): + # memorize that we rebased to here + crt_series._set_field('orig-base', git.get_head()) # push the patches back if not nopush: push_patches(applied, merged) diff --git a/stgit/stack.py b/stgit/stack.py index 3185d64..0f5d868 100644 --- a/stgit/stack.py +++ b/stgit/stack.py @@ -540,6 +540,7 @@ class Series(StgitObject): os.makedirs(os.path.join(self._dir(), 'patches')) os.makedirs(self.__refs_dir) self.__begin_stack_check() + self._set_field('orig-base', git.get_head()) def convert(self): """Either convert to use a separate patch directory, or ``` ## Yann Dirson, 2007-02-20 00:14 Subject: [PATCH 5/5] Refuse to pull/rebase when rendered unsafe by (un)commits. Message-ID: <20070220001424.31139.33562.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20070220001424.31139.33562.stgit%40gandelf.nowhere.earth In-Reply-To: <20070220000908.31139.36841.stgit@gandelf.nowhere.earth> ``` This can be overriden by a new --force flag. Signed-off-by: Yann Dirson --- stgit/commands/common.py | 9 ++++++++- stgit/commands/pull.py | 8 ++++++-- stgit/commands/rebase.py | 5 ++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/stgit/commands/common.py b/stgit/commands/common.py index 56cb517..398231a 100644 --- a/stgit/commands/common.py +++ b/stgit/commands/common.py @@ -347,7 +347,14 @@ def make_patch_name(msg, unacceptable, default_name = 'patch', patchname = '%s-%d' % (patchname, suffix) return patchname -def prepare_rebase(): +def prepare_rebase(real_rebase, force=None): + if not force: + # Be sure we won't loose results of stg-(un)commit by error. + # Do not require an existing orig-base for compatibility with 0.12 and earlier. + origbase = crt_series._get_field('orig-base') + if origbase and crt_series.get_base() != origbase: + raise CmdException, 'Rebasing would possibly lose data' + # pop all patches applied = crt_series.get_applied() if len(applied) > 0: diff --git a/stgit/commands/pull.py b/stgit/commands/pull.py index 83a2725..990244e 100644 --- a/stgit/commands/pull.py +++ b/stgit/commands/pull.py @@ -41,6 +41,9 @@ options = [make_option('-n', '--nopush', action = 'store_true'), make_option('-m', '--merged', help = 'check for patches merged upstream', + action = 'store_true'), + make_option('--force', + help = 'force rebase even if the stack based was moved by (un)commits', action = 'store_true')] def func(parser, options, args): @@ -61,12 +64,13 @@ def func(parser, options, args): check_conflicts() check_head_top_equal() - applied = prepare_rebase() + must_rebase = (config.get('stgit.pull-does-rebase') == 'yes') + applied = prepare_rebase(real_rebase=must_rebase, force=options.force) # pull the remote changes print 'Pulling from "%s"...' % repository git.fetch(repository) - if (config.get('stgit.pull-does-rebase') == 'yes'): + if must_rebase: rebase(git.fetch_head()) post_rebase(applied, options.nopush, options.merged) diff --git a/stgit/commands/rebase.py b/stgit/commands/rebase.py index 63f18ec..d132b60 100644 --- a/stgit/commands/rebase.py +++ b/stgit/commands/rebase.py @@ -34,6 +34,9 @@ options = [make_option('-n', '--nopush', action = 'store_true'), make_option('-m', '--merged', help = 'check for patches merged upstream', + action = 'store_true'), + make_option('--force', + help = 'force rebase even if the stack based was moved by (un)commits', action = 'store_true')] def func(parser, options, args): @@ -49,7 +52,7 @@ def func(parser, options, args): check_conflicts() check_head_top_equal() - applied = prepare_rebase() + applied = prepare_rebase(real_rebase=True, force=options.force) rebase(args[0]) post_rebase(applied, options.nopush, options.merged) ``` ## Yann Dirson, 2007-02-28 22:15 Subject: stgit "push --merged" woes Message-ID: <20070228221552.GL4045@nan92-1-81-57-214-146.fbx.proxad.net> URL: https://gitlist.dev/e/20070228221552.GL4045%40nan92-1-81-57-214-146.fbx.proxad.net In-Reply-To: ``` On Wed, Feb 21, 2007 at 10:58:13PM +0000, Catalin Marinas wrote: > >Oh, while I'm at it, "push -m" does not seem to work as expected. > >After pulling from your tree, which contains my changes, and rebasing > >to your head, "push -m" of the "Factorize..." patch results in a > >conflict, while rebasing onto this particular commit and then pushing > >my original patch gets the merge properly detected. Sounds like a > >candidate for a quick 0.12.2 if we cannot kill it before 0.12.1. > > Unless you deleted the empty patch, you could do a push --undo and try > to reverse apply the diff (that's what push -m does) and see whether > it fails or not. There might be a bug in StGIT and not be able to cope > with new files while doing the merge check (this patch has a new test > script). If I have time tomorrow, I'll try to put this in the push > test-case. Just had a similar case, trying to "push -m" my Documentation patches. The patch creating Documentation/ fails with conflict on Makefile. Trying to reverse-apply the diff does not work, since the Makefile has been modified by another commit. I verified that reverse-applying the 2 patches touching that file in my stack does work. That seems to imply that added files are not the only ones causing problems to the current "push -m". They just exhibit the problem more often. "push -m" should probably try other techniques if that one fails. Reverse-apply on top of every newly-pulled commit that touches the problematic files would be a possibility, but that's expensive. Trying a meta-diff[1] between the patch to be pushed and candidate newly-pulled commits could be cheaper. But both techniques would miss a case that current "push -m" seems to handle: when a patch was applied and then reversed upstream. [1] FWIW, my (naive) stg-mdiff script: ==================== #!/bin/bash set -e # stg-mdiff - display meta-diffs, ie. diffs of diffs # Main use: show evolutions of a patch. # eg. stg-mdiff foo@stable foo # stg-mdiff foo 012345567ABCD # sha1 for "foo" as integrated upstream # Copyright (c) 2007 Yann Dirson # Subject to the GNU GPL, version 2. usage() { echo "Usage: $(basename $0) " exit 1 } if [ "$#" != 2 ]; then usage fi colordiff -u <(stg show "$1") <(stg show "$2") | less -RFX ==================== ```