{"thread":{"id":"8410","subject":"[StGIT RFC PATCH 0/2] Fixing the rebase safeguard","startedAt":"2007-06-03T13:41:46Z","lastAt":"2007-06-04T22:41:36Z","messageCount":5,"participants":["Yann Dirson","David Kågedal","Catalin Marinas"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"43869","messageId":"20070603133006.1681.1742.stgit@gandelf.nowhere.earth","threadId":"8410","inReplyTo":null,"subject":"[StGIT RFC PATCH 0/2] Fixing the rebase safeguard","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-06-03T13:41:46Z","receivedAt":"2007-06-03T13:41:46Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"This is an attempt at using a reachability test to replace the use of\norig-base, to decide when it is safe to accept a rebase.\n\nThis implementation passes the testcase posted earlier by Karl (resent\nhere as 2nd patch in this series), BUT fails the 4th test of t2100.\nThat is, it fails to deal with the case of a rewinding commit occuring\nin the upstream branch, and being first git-fetch'd before being\nstg-pull'd in fetch-rebase mode. In this case, the former upstream\ncommit will really be lost.\n\nIn this case, however, it is exactly what we want, but I'm still\nundecided about how to deal with this best. Possibly insane ideas for\nnow include:\n\n- parsing the remote.*.fetch lines to detect the leading + (just\n kidding ;)\n- using ORIG_HEAD, but then, how do we decide when it is valid to do\n so ?\n\nI'll now switch to some real-life activity to see if that helps to\nfind a better solution.\n\n-- \nYann Dirson |\nDebian-related: | Support Debian GNU/Linux:\n | Freedom, Power, Stability, Gratis\n http://ydirson.free.fr/ | Check \n"},{"id":"43870","messageId":"20070603134152.1681.82748.stgit@gandelf.nowhere.earth","threadId":"8410","inReplyTo":"20070603133006.1681.1742.stgit@gandelf.nowhere.earth","subject":"[PATCH 1/2] Changed rebasing safety check to look for reachability of stack base (gna bug #9181).","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-06-03T13:41:52Z","receivedAt":"2007-06-03T13:41:52Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"\n\n\nSigned-off-by: Yann Dirson \n---\n\n stgit/commands/common.py | 23 +++++++++++++++--------\n stgit/commands/pull.py | 12 ++++--------\n stgit/commands/rebase.py | 2 +-\n stgit/git.py | 6 ++++++\n stgit/stack.py | 1 -\n 5 files changed, 26 insertions(+), 18 deletions(-)\n\ndiff --git a/stgit/commands/common.py b/stgit/commands/common.py\nindex 22c78ae..a3df2a2 100644\n--- a/stgit/commands/common.py\n+++ b/stgit/commands/common.py\n@@ -318,13 +318,22 @@ def address_or_alias(addr_str):\n for addr in addr_str.split(',')]\n return ', '.join([addr for addr in addr_list if addr])\n \n-def prepare_rebase(real_rebase, force=None):\n+def prepare_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+ # Be sure we won't loose results of stg-commit by error.\n+ # Note: checking for refs/bases should not be necessary with\n+ # repo format version 2, but better safe than sorry.\n+ branchname = crt_series.get_branch()\n+ # references for anything but the current stack\n+ refs = [ref for ref in git.all_refs()\n+ if ref != 'refs/heads/'+branchname\n+ and ref != 'refs/bases/'+branchname\n+ and not re.match('^refs/patches/%s/'%branchname, ref)]\n+ stray_commits = git._output_lines(['git-rev-list',\n+ crt_series.get_base(),\n+ '--not'] + refs)\n+ if len(stray_commits) != 0:\n+ raise CmdException, 'Rebasing would make the following commits below the stack base unreachable: %s' % stray_commits\n \n # pop all patches\n applied = crt_series.get_applied()\n@@ -343,8 +352,6 @@ def rebase(target):\n out.done()\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/commands/pull.py b/stgit/commands/pull.py\nindex beaa7b5..bacd5fc 100644\n--- a/stgit/commands/pull.py\n+++ b/stgit/commands/pull.py\n@@ -77,16 +77,12 @@ def func(parser, options, args):\n check_conflicts()\n check_head_top_equal()\n \n- if policy == 'pull':\n- must_rebase = 0\n- elif policy == 'fetch-rebase':\n- must_rebase = 1\n- elif policy == 'rebase':\n- must_rebase = 1\n- else:\n+ if (policy != 'pull') \\\n+ and (policy != 'fetch-rebase') \\\n+ and (policy != 'rebase'):\n raise GitConfigException, 'Unsupported pull-policy \"%s\"' % policy\n \n- applied = prepare_rebase(real_rebase=must_rebase, force=options.force)\n+ applied = prepare_rebase(force=options.force)\n \n # pull the remote changes\n if policy == 'pull':\ndiff --git a/stgit/commands/rebase.py b/stgit/commands/rebase.py\nindex d132b60..13933d6 100644\n--- a/stgit/commands/rebase.py\n+++ b/stgit/commands/rebase.py\n@@ -52,7 +52,7 @@ def func(parser, options, args):\n check_conflicts()\n check_head_top_equal()\n \n- applied = prepare_rebase(real_rebase=True, force=options.force)\n+ applied = prepare_rebase(force=options.force)\n rebase(args[0])\n post_rebase(applied, options.nopush, options.merged)\n \ndiff --git a/stgit/git.py b/stgit/git.py\nindex 4bc41aa..bec691b 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -1070,3 +1070,9 @@ def fetch_head():\n \n # here we are sure to have a single fetch_head\n return fetch_head\n+\n+def all_refs():\n+ \"\"\"Return a list of all refs in the current repository.\n+ \"\"\"\n+\n+ return [line.split()[1] for line in _output_lines(['git-show-ref'])]\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex 7a06458..45fd98a 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -588,7 +588,6 @@ class Series(StgitObject):\n self.create_empty_field('applied')\n self.create_empty_field('unapplied')\n os.makedirs(self.__refs_dir)\n- self._set_field('orig-base', git.get_head())\n \n config.set(format_version_key(self.get_branch()), str(FORMAT_VERSION))\n \n"},{"id":"43871","messageId":"20070603134157.1681.70969.stgit@gandelf.nowhere.earth","threadId":"8410","inReplyTo":"20070603133006.1681.1742.stgit@gandelf.nowhere.earth","subject":"[PATCH 2/2] Test \"stg rebase\" after \"stg commit\"","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-06-03T13:41:57Z","receivedAt":"2007-06-03T13:41:57Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"\nTwo new tests for \"stg rebase\":\n\n 1. Try to rebase to a commit that is ahead of HEAD. This should\n work, and does.\n\n 2. Try to commit a patch, and then rebase. This doesn't work,\n because \"stg rebase\" aborts if orig-base != base, and \"stg\n commit\" doesn't update orig-base. (It does work if \"stg rebase\"\n is given the --force flag.)\n\nSigned-off-by: Karl Hasselström \n\nSigned-off-by: Karl Hasselström \n---\n\n t/t2200-rebase.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t2200-rebase.sh b/t/t2200-rebase.sh\nindex 52462dd..b48e513 100755\n--- a/t/t2200-rebase.sh\n+++ b/t/t2200-rebase.sh\n@@ -30,4 +30,20 @@ test_expect_success \\\n \ttest `stg id base@stack` = `git rev-parse master~1`\n \t'\n \n+test_expect_success \\\n+\t'Rebase to next commit' \\\n+\t'\n+\tstg rebase master &&\n+\ttest $(stg id base@stack) = $(git rev-parse master)\n+\t'\n+\n+test_expect_success \\\n+\t'Commit the patch and rebase again' \\\n+\t'\n+\tstg commit &&\n+\tgit tag committed-here &&\n+\tstg rebase master &&\n+\ttest $(stg id base@stack) = $(git rev-parse master)\n+\t'\n+\n test_done\n"},{"id":"43977","messageId":"878xazbzup.fsf@morpheus.local","threadId":"8410","inReplyTo":"20070603134152.1681.82748.stgit@gandelf.nowhere.earth","subject":"Re: [PATCH 1/2] Changed rebasing safety check to look for reachability of stack base (gna bug #9181).","fromName":"David Kågedal","fromEmail":"davidk@lysator.liu.se","sentAt":"2007-06-04T13:42:22Z","receivedAt":"2007-06-04T13:42:22Z","isPatch":true,"sender":{"key":"davidk@lysator.liu.se","avatar":"https://avatars.githubusercontent.com/u/60530?v=4"},"body":"Yann Dirson writes:\n\n> --- a/stgit/commands/pull.py\n> +++ b/stgit/commands/pull.py\n> @@ -77,16 +77,12 @@ def func(parser, options, args):\n> check_conflicts()\n> check_head_top_equal()\n> \n> - if policy == 'pull':\n> - must_rebase = 0\n> - elif policy == 'fetch-rebase':\n> - must_rebase = 1\n> - elif policy == 'rebase':\n> - must_rebase = 1\n> - else:\n> + if (policy != 'pull') \\\n> + and (policy != 'fetch-rebase') \\\n> + and (policy != 'rebase'):\n> raise GitConfigException, 'Unsupported pull-policy \"%s\"' % policy\n\nMinor nit: I think this is much more clearly written as\n\n if policy not in ('pull', 'fetch-rebase', 'rebase'):\n\n-- \nDavid Kågedal\n"},{"id":"44013","messageId":"b0943d9e0706041541i74f5dccct9f66a7dca1f1ffea@mail.gmail.com","threadId":"8410","inReplyTo":"20070603133006.1681.1742.stgit@gandelf.nowhere.earth","subject":"Re: [StGIT RFC PATCH 0/2] Fixing the rebase safeguard","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-06-04T22:41:36Z","receivedAt":"2007-06-04T22:41:36Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 03/06/07, Yann Dirson wrote:\n> This is an attempt at using a reachability test to replace the use of\n> orig-base, to decide when it is safe to accept a rebase.\n[...]\n> - using ORIG_HEAD, but then, how do we decide when it is valid to do\n> so ?\n\nI think using the ORIG_HEAD was the initial behaviour of StGIT. I\ndidn't have any problems as I am only following one tree (Linus'\nmainline kernel) but other users might have different scenarios.\n\n-- \nCatalin\n"}]}