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