threads / patch / 6889

patch, 5 partsMove stack-base querying into Series class.

Subject: [PATCH 2/5] Move stack-base querying into Series class.

## tl;dr

7 messages between Feb 20, 2007 and Feb 28, 2007. Diffs are folded; open one to read it.

replies: 6people: 1as markdown or json

Yann Dirson· Feb 20, 2007, 00:13 UTC · lore

[PATCH 0/5] stgit rebasing fixes

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    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>
Yann Dirson· Feb 20, 2007, 00:14 UTC · re: Yann Dirson · lore

[PATCH 1/5] Factorize rebasing behaviour.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/commands/common.py |   23 +++++++++++++++++++++++
 stgit/commands/pull.py   |   20 ++++----------------
 stgit/commands/rebase.py |   17 +++--------------
 3 files changed, 30 insertions(+), 30 deletions(-)
Show changes to 3 files +30 −30

stgit/commands/common.py, stgit/commands/pull.py, stgit/commands/rebase.py

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· Feb 20, 2007, 00:14 UTC · re: Yann Dirson · lore
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/commands/common.py |    2 +-
 stgit/stack.py           |    5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)
Show changes to 2 files +5 −2

stgit/commands/common.py, stgit/stack.py

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· Feb 20, 2007, 00:14 UTC · re: Yann Dirson · lore

[PATCH 3/5] Various cleanups for clarity.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/stack.py |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
Show changes to stgit/stack.py +1 −3
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· Feb 20, 2007, 00:14 UTC · re: Yann Dirson · lore

[PATCH 4/5] Keep track of safe base changes.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/commands/common.py |    2 ++
 stgit/stack.py           |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)
Show changes to 2 files +3 −0

stgit/commands/common.py, stgit/stack.py

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· Feb 20, 2007, 00:14 UTC · re: Yann Dirson · lore

[PATCH 5/5] Refuse to pull/rebase when rendered unsafe by (un)commits.

This can be overriden by a new --force flag.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
 stgit/commands/common.py |    9 ++++++++-
 stgit/commands/pull.py   |    8 ++++++--
 stgit/commands/rebase.py |    5 ++++-
 3 files changed, 18 insertions(+), 4 deletions(-)
Show changes to 3 files +18 −4

stgit/commands/common.py, stgit/commands/pull.py, stgit/commands/rebase.py

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· Feb 28, 2007, 22:15 UTC · lore

stgit "push --merged" woes

On Wed, Feb 21, 2007 at 10:58:13PM +0000, Catalin Marinas wrote:
Show 13 quoted lines
> >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 <ydirson@altern.org> # Subject to the GNU GPL, version 2.

usage()
{
    echo "Usage: $(basename $0) <patch1> <patch2>"
    exit 1
}
if [ "$#" != 2 ]; then
    usage
fi

colordiff -u <(stg show "$1") <(stg show "$2") | less -RFX ====================

← back to recent threads