{"thread":{"id":"12769","subject":"[StGit PATCH 4/6] Make sure that we only uncommit commits with exactly one parent","startedAt":"2008-03-20T00:31:28Z","lastAt":"2008-03-25T15:27:03Z","messageCount":14,"participants":["Karl Hasselström","Catalin Marinas"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"72457","messageId":"20080320002604.13102.53757.stgit@yoghurt","threadId":"12769","inReplyTo":null,"subject":"[StGit PATCH 0/6] Two bugfixes","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:31:28Z","receivedAt":"2008-03-20T00:31:28Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"This series fixes one rather benign bug (4/6) and one that caused\npatches to become empty, which is rather worse (6/6). (The patch\ncontents could still be recovered via the patch log or reflog, but\nit's still a major inconvenience.)\n\nBoth bugs were discovered by Erik Sandberg.\n\nThese patches are also available in the kha/safe branch.\n\n---\n\nKarl Hasselström (6):\n Handle failed pushes differently depending on cause\n New test: conflicting push in dirty worktree\n Make sure that we only uncommit commits with exactly one parent\n Try uncommitting a commit with not exactly one parent\n Make sure patches with no parents have an empty list of parents\n Use a special exit code for bugs\n\n\n stgit/commands/uncommit.py | 14 ++++++++++++--\n stgit/lib/git.py | 23 +++++++++++++++--------\n stgit/lib/transaction.py | 4 +++-\n stgit/main.py | 11 +++++++----\n stgit/utils.py | 1 +\n t/t1300-uncommit.sh | 5 +++++\n t/t3000-dirty-merge.sh | 35 +++++++++++++++++++++++++++++++++++\n 7 files changed, 78 insertions(+), 15 deletions(-)\n create mode 100755 t/t3000-dirty-merge.sh\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"72452","messageId":"20080320003134.13102.73631.stgit@yoghurt","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"[StGit PATCH 1/6] Use a special exit code for bugs","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:31:34Z","receivedAt":"2008-03-20T00:31:34Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Use a special exit code (4) for any error condition that indicates a\nbug in StGit. This will be useful in the test suite.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/main.py | 11 +++++++----\n stgit/utils.py | 1 +\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\n\ndiff --git a/stgit/main.py b/stgit/main.py\nindex 79044b0..663fdec 100644\n--- a/stgit/main.py\n+++ b/stgit/main.py\n@@ -18,7 +18,7 @@ along with this program; if not, write to the Free Software\n Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n \"\"\"\n \n-import sys, os\n+import sys, os, traceback\n from optparse import OptionParser\n \n import stgit.commands\n@@ -279,10 +279,13 @@ def main():\n except (StgException, IOError, ParsingError, NoSectionError), err:\n out.error(str(err), title = '%s %s' % (prog, cmd))\n if debug_level > 0:\n- raise\n- else:\n- sys.exit(utils.STGIT_COMMAND_ERROR)\n+ traceback.print_exc()\n+ sys.exit(utils.STGIT_COMMAND_ERROR)\n except KeyboardInterrupt:\n sys.exit(utils.STGIT_GENERAL_ERROR)\n+ except:\n+ out.error('Unhandled exception:')\n+ traceback.print_exc()\n+ sys.exit(utils.STGIT_BUG_ERROR)\n \n sys.exit(ret or utils.STGIT_SUCCESS)\ndiff --git a/stgit/utils.py b/stgit/utils.py\nindex b75c3b4..cd52382 100644\n--- a/stgit/utils.py\n+++ b/stgit/utils.py\n@@ -327,6 +327,7 @@ STGIT_SUCCESS = 0 # everything's OK\n STGIT_GENERAL_ERROR = 1 # seems to be non-command-specific error\n STGIT_COMMAND_ERROR = 2 # seems to be a command that failed\n STGIT_CONFLICT = 3 # merge conflict, otherwise OK\n+STGIT_BUG_ERROR = 4 # a bug in StGit\n \n def strip_leading(prefix, s):\n \"\"\"Strip leading prefix from a string. Blow up if the prefix isn't\n"},{"id":"72454","messageId":"20080320003140.13102.91209.stgit@yoghurt","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"[StGit PATCH 2/6] Make sure patches with no parents have an empty list of parents","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:31:40Z","receivedAt":"2008-03-20T00:31:40Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"They used to have None instead of an empty list, which was\ninconsistent. (It went undetected for quite a while because StGit\nseldom needs to handle initial commits.)\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/lib/git.py | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\n\ndiff --git a/stgit/lib/git.py b/stgit/lib/git.py\nindex 50dc4f1..cdfe885 100644\n--- a/stgit/lib/git.py\n+++ b/stgit/lib/git.py\n@@ -204,7 +204,7 @@ class Commitdata(Repr):\n ) % (tree, parents, self.author, self.committer, self.message)\n @classmethod\n def parse(cls, repository, s):\n- cd = cls()\n+ cd = cls(parents = [])\n lines = list(s.splitlines(True))\n for i in xrange(len(lines)):\n line = lines[i].strip()\n"},{"id":"72456","messageId":"20080320003145.13102.12700.stgit@yoghurt","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"[StGit PATCH 3/6] Try uncommitting a commit with not exactly one parent","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:31:45Z","receivedAt":"2008-03-20T00:31:45Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"This should fail cleanly -- and in fact it does. Except for printing\nan assertion backtrace instead of a nice error message. (This is a\nregression introduced by the conversion of \"stg uncommit\" to the new\ninfrastructure.)\n\nFound by Erik Sandberg .\n\nSigned-off-by: Karl Hasselström \n\n---\n\n t/t1300-uncommit.sh | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\n\ndiff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh\nindex d86e579..0d952a7 100755\n--- a/t/t1300-uncommit.sh\n+++ b/t/t1300-uncommit.sh\n@@ -78,4 +78,9 @@ test_expect_success \\\n stg commit --all\n '\n \n+test_expect_failure 'Uncommit a commit with not precisely one parent' '\n+ stg uncommit -n 5 ; [ $? = 2 ] &&\n+ [ \"$(echo $(stg series))\" = \"\" ]\n+'\n+\n test_done\n"},{"id":"72451","messageId":"20080320003151.13102.44813.stgit@yoghurt","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"[StGit PATCH 4/6] Make sure that we only uncommit commits with exactly one parent","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:31:51Z","receivedAt":"2008-03-20T00:31:51Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"If we encounter a commit with 0, or 2 or more parents, fail with a\nnice error message instead of crashing.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/commands/uncommit.py | 14 ++++++++++++--\n t/t1300-uncommit.sh | 2 +-\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\n\ndiff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py\nindex 933ec60..272c5db 100644\n--- a/stgit/commands/uncommit.py\n+++ b/stgit/commands/uncommit.py\n@@ -85,13 +85,23 @@ def func(parser, options, args):\n patchnames = args\n patch_nr = len(patchnames)\n \n+ def get_parent(c):\n+ next = c.data.parents\n+ try:\n+ [next] = next\n+ except ValueError:\n+ raise common.CmdException(\n+ 'Trying to uncommit %s, which does not have exactly one parent'\n+ % c.sha1)\n+ return next\n+\n commits = []\n next_commit = stack.base\n if patch_nr:\n out.start('Uncommitting %d patches' % patch_nr)\n for i in xrange(patch_nr):\n commits.append(next_commit)\n- next_commit = next_commit.data.parent\n+ next_commit = get_parent(next_commit)\n else:\n if options.exclusive:\n out.start('Uncommitting to %s (exclusive)' % to_commit)\n@@ -103,7 +113,7 @@ def func(parser, options, args):\n commits.append(next_commit)\n break\n commits.append(next_commit)\n- next_commit = next_commit.data.parent\n+ next_commit = get_parent(next_commit)\n patch_nr = len(commits)\n \n taken_names = set(stack.patchorder.applied + stack.patchorder.unapplied)\ndiff --git a/t/t1300-uncommit.sh b/t/t1300-uncommit.sh\nindex 0d952a7..a906d13 100755\n--- a/t/t1300-uncommit.sh\n+++ b/t/t1300-uncommit.sh\n@@ -78,7 +78,7 @@ test_expect_success \\\n stg commit --all\n '\n \n-test_expect_failure 'Uncommit a commit with not precisely one parent' '\n+test_expect_success 'Uncommit a commit with not precisely one parent' '\n stg uncommit -n 5 ; [ $? = 2 ] &&\n [ \"$(echo $(stg series))\" = \"\" ]\n '\n"},{"id":"72453","messageId":"20080320003157.13102.10350.stgit@yoghurt","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"[StGit PATCH 5/6] New test: conflicting push in dirty worktree","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:31:57Z","receivedAt":"2008-03-20T00:31:57Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"When the result of a conflicting push can't be represented in the\nworktree because the worktree is dirty, the push should be aborted.\nSimilarly, the push should be aborted if we have to do the merge in\nthe worktree, but can't because the worktree is dirty.\n\nAdd a new test that tests for this. It currently fails, in a bad way:\nthe contents of the pushed patch is lost.\n\n(The test uses goto instead of push, because push doesn't use the new\ninfrastructure yet. And old-infrastructure commands never have this\nbug, because they refuse to run with a dirty worktree.)\n\nThis bug was found by Erik Sandberg , who also\ncame up with the minimal test case that I turned into this new test.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n t/t3000-dirty-merge.sh | 35 +++++++++++++++++++++++++++++++++++\n 1 files changed, 35 insertions(+), 0 deletions(-)\n create mode 100755 t/t3000-dirty-merge.sh\n\n\ndiff --git a/t/t3000-dirty-merge.sh b/t/t3000-dirty-merge.sh\nnew file mode 100755\nindex 0000000..dd81c9e\n--- /dev/null\n+++ b/t/t3000-dirty-merge.sh\n@@ -0,0 +1,35 @@\n+#!/bin/sh\n+\n+test_description='Try a push that requires merging a file that is dirty'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'Initialize StGit stack with two patches' '\n+ stg init &&\n+ touch a &&\n+ git add a &&\n+ git commit -m a &&\n+ echo 1 > a &&\n+ git commit -a -m p1 &&\n+ echo 2 > a &&\n+ git commit -a -m p2 &&\n+ stg uncommit -n 2\n+'\n+\n+test_expect_success 'Pop one patch and update the other' '\n+ stg goto p1 &&\n+ echo 3 > a &&\n+ stg refresh\n+'\n+\n+test_expect_failure 'Push with dirty worktree' '\n+ echo 4 > a &&\n+ [ \"$(echo $(stg applied))\" = \"p1\" ] &&\n+ [ \"$(echo $(stg unapplied))\" = \"p2\" ] &&\n+ ! stg goto p2 &&\n+ [ \"$(echo $(stg applied))\" = \"p1\" ] &&\n+ [ \"$(echo $(stg unapplied))\" = \"p2\" ] &&\n+ [ \"$(echo $(cat a))\" = \"4\" ]\n+'\n+\n+test_done\n"},{"id":"72455","messageId":"20080320003202.13102.54792.stgit@yoghurt","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"[StGit PATCH 6/6] Handle failed pushes differently depending on cause","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-20T00:32:02Z","receivedAt":"2008-03-20T00:32:02Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Specifically, if the push failed because of a merge conflict, the\npatch should be applied but empty; and if it fails for any other\nreason (such as a too-dirty worktree), the patch should not be\napplied.\n\nThis fixes the data loss bug tested for by t3000.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/lib/git.py | 21 ++++++++++++++-------\n stgit/lib/transaction.py | 4 +++-\n t/t3000-dirty-merge.sh | 2 +-\n 3 files changed, 18 insertions(+), 9 deletions(-)\n\n\ndiff --git a/stgit/lib/git.py b/stgit/lib/git.py\nindex cdfe885..35b9bbf 100644\n--- a/stgit/lib/git.py\n+++ b/stgit/lib/git.py\n@@ -433,6 +433,9 @@ class Repository(RunWithEnv):\n class MergeException(exception.StgException):\n pass\n \n+class MergeConflictException(MergeException):\n+ pass\n+\n class Index(RunWithEnv):\n def __init__(self, repository, filename):\n self.__repository = repository\n@@ -517,14 +520,18 @@ class IndexAndWorktree(RunWithEnv):\n assert isinstance(ours, Tree)\n assert isinstance(theirs, Tree)\n try:\n- self.run(['git', 'merge-recursive', base.sha1, '--', ours.sha1,\n- theirs.sha1],\n- env = { 'GITHEAD_%s' % base.sha1: 'ancestor',\n- 'GITHEAD_%s' % ours.sha1: 'current',\n- 'GITHEAD_%s' % theirs.sha1: 'patched'}\n- ).cwd(self.__worktree.directory).discard_output()\n+ r = self.run(['git', 'merge-recursive', base.sha1, '--', ours.sha1,\n+ theirs.sha1],\n+ env = { 'GITHEAD_%s' % base.sha1: 'ancestor',\n+ 'GITHEAD_%s' % ours.sha1: 'current',\n+ 'GITHEAD_%s' % theirs.sha1: 'patched'}\n+ ).cwd(self.__worktree.directory)\n+ r.discard_output()\n except run.RunException, e:\n- raise MergeException('Index/worktree dirty')\n+ if r.exitcode == 1:\n+ raise MergeConflictException()\n+ else:\n+ raise MergeException('Index/worktree dirty')\n def changed_files(self):\n return self.run(['git', 'diff-files', '--name-only']).output_lines()\n def update_index(self, files):\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 3613b15..2946a67 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -197,10 +197,12 @@ class StackTransaction(object):\n tree = iw.index.write_tree()\n self.__current_tree = tree\n s = ' (modified)'\n- except git.MergeException:\n+ except git.MergeConflictException:\n tree = ours\n merge_conflict = True\n s = ' (conflict)'\n+ except git.MergeException, e:\n+ self.__halt(str(e))\n cd = cd.set_tree(tree)\n self.patches[pn] = self.__stack.repository.commit(cd)\n del self.unapplied[self.unapplied.index(pn)]\ndiff --git a/t/t3000-dirty-merge.sh b/t/t3000-dirty-merge.sh\nindex dd81c9e..d87bba1 100755\n--- a/t/t3000-dirty-merge.sh\n+++ b/t/t3000-dirty-merge.sh\n@@ -22,7 +22,7 @@ test_expect_success 'Pop one patch and update the other' '\n stg refresh\n '\n \n-test_expect_failure 'Push with dirty worktree' '\n+test_expect_success 'Push with dirty worktree' '\n echo 4 > a &&\n [ \"$(echo $(stg applied))\" = \"p1\" ] &&\n [ \"$(echo $(stg unapplied))\" = \"p2\" ] &&\n"},{"id":"72518","messageId":"b0943d9e0803200819k7300fd1fn8a21896c7ad2a1@mail.gmail.com","threadId":"12769","inReplyTo":"20080320002604.13102.53757.stgit@yoghurt","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-03-20T15:19:12Z","receivedAt":"2008-03-20T15:19:12Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 20/03/2008, Karl Hasselström wrote:\n> This series fixes one rather benign bug (4/6) and one that caused\n> patches to become empty, which is rather worse (6/6). (The patch\n> contents could still be recovered via the patch log or reflog, but\n> it's still a major inconvenience.)\n>\n> Both bugs were discovered by Erik Sandberg.\n\nThanks to both Erik and Karl for finding and fixing this. As I wrote\non the patch system, I'd like to put back the explicit --keep option\nin goto.\n\nBTW, I have about 5 patches that apply to the stable and master\nbranch, mainly UI updates I needed recently (like picking multiple\npatches at once). Since the master branch still needs some work (which\nI'll try to help with), maybe it's worth releasing a 0.14.2, together\nwith some of the bugs reported so far.\n\n-- \nCatalin\n"},{"id":"72863","messageId":"20080324083550.GB23337@diana.vm.bytemark.co.uk","threadId":"12769","inReplyTo":"b0943d9e0803200819k7300fd1fn8a21896c7ad2a1@mail.gmail.com","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-24T08:35:50Z","receivedAt":"2008-03-24T08:35:50Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-03-20 15:19:12 +0000, Catalin Marinas wrote:\n\n> BTW, I have about 5 patches that apply to the stable and master\n> branch, mainly UI updates I needed recently (like picking multiple\n> patches at once). Since the master branch still needs some work\n> (which I'll try to help with), maybe it's worth releasing a 0.14.2,\n> together with some of the bugs reported so far.\n\nAbsolutely. I think it would be worthwhile to treat \"stable\" much like\nJunio handles his \"maint\" -- apply minor and/or important fixes there,\nand release minor releases from it somewhat frequently. And merge it\nto master often, so that master always has everything stable has.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"72866","messageId":"b0943d9e0803240216p2000dc23i41580ecc15102cd5@mail.gmail.com","threadId":"12769","inReplyTo":"20080324083550.GB23337@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-03-24T09:16:42Z","receivedAt":"2008-03-24T09:16:42Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 24/03/2008, Karl Hasselström wrote:\n> On 2008-03-20 15:19:12 +0000, Catalin Marinas wrote:\n>\n> > BTW, I have about 5 patches that apply to the stable and master\n> > branch, mainly UI updates I needed recently (like picking multiple\n> > patches at once). Since the master branch still needs some work\n> > (which I'll try to help with), maybe it's worth releasing a 0.14.2,\n> > together with some of the bugs reported so far.\n>\n>\n> Absolutely. I think it would be worthwhile to treat \"stable\" much like\n> Junio handles his \"maint\" -- apply minor and/or important fixes there,\n> and release minor releases from it somewhat frequently. And merge it\n> to master often, so that master always has everything stable has.\n\nI'll try to release 0.14.2 today or tomorrow as I'll be on holiday\nafterwards for 2.5 weeks.\n\nBTW, on the master branch, I noticed that\nStackTransaction.push_patch() checks whether the patch is empty before\npushing it. The behaviour on stable is that it reports whether it's\nempty after push (just like \"modified\", so that you know that a patch\nno longer has data as a result of the merge).\n\nOn the merging side during push, we have to add the plain patch\napplying first followed by a three-way merge. Do we ever need the\nIndex.merge() function (implemented with git-read-tree) or should we\nshould always uses the git-recursive-merge in\nIndexAndWorktree.merge()?\n\n-- \nCatalin\n"},{"id":"72923","messageId":"20080324181225.GC23337@diana.vm.bytemark.co.uk","threadId":"12769","inReplyTo":"b0943d9e0803200819k7300fd1fn8a21896c7ad2a1@mail.gmail.com","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-24T18:12:25Z","receivedAt":"2008-03-24T18:12:25Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-03-20 15:19:12 +0000, Catalin Marinas wrote:\n\n> As I wrote on the patch system, I'd like to put back the explicit\n> --keep option in goto.\n\nThere are three possible values of \"keepiness\":\n\n 1. Make sure there are _no_ local changes. (Default for old\n infrastructure.)\n\n 2. Make sure there are no local changes in the files we need to\n touch. (Default for new infrastructure.)\n\n 3. Bring along local changes by means of a merge. (What the --keep\n option does.)\n\ngit defaults to doing (2), and optionally does (3). (1) is\nsignificantly slower than (2); I don't know how slow (3) is.\n\nThere are two questions: what subset of these options do we support,\nand which of the supported modes should be the default?\n\nI think that (2) should be the default, because it's faster, it's what\ngit does, and I don't really see the point in complaining about local\nchanges in a file we won't need to touch anyway. Having an option for\n(3) might be handy, though.\n\nBut I gather you want (1) to be the default (with (3) as an option).\nCorrect?\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"73028","messageId":"b0943d9e0803250346h7405c37egc9ba170a6dcc06bf@mail.gmail.com","threadId":"12769","inReplyTo":"20080324181225.GC23337@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-03-25T10:46:00Z","receivedAt":"2008-03-25T10:46:00Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 24/03/2008, Karl Hasselström wrote:\n> On 2008-03-20 15:19:12 +0000, Catalin Marinas wrote:\n>\n>\n> > As I wrote on the patch system, I'd like to put back the explicit\n> > --keep option in goto.\n>\n>\n> There are three possible values of \"keepiness\":\n>\n> 1. Make sure there are _no_ local changes. (Default for old\n> infrastructure.)\n>\n> 2. Make sure there are no local changes in the files we need to\n> touch. (Default for new infrastructure.)\n>\n> 3. Bring along local changes by means of a merge. (What the --keep\n> option does.)\n>\n> git defaults to doing (2), and optionally does (3). (1) is\n> significantly slower than (2); I don't know how slow (3) is.\n\nWith the current functionality (see below), I prefer (1) to be the\ndefault and (3) as optional (on stable, it does a reverse-apply of the\ndiff and it's kind of combination between 2 and 3 but without the\nconflicts you could get with only 3).\n\nMy reason for this is that usually you \"goto\" a patch to do some\nchanges followed by \"refresh\". If there were local changes, they would\nbe included in the refreshed patch since this is the default StGIT\nbehaviour. To avoid this, I find myself running \"status\" before any\n\"goto\".\n\n> I think that (2) should be the default, because it's faster, it's what\n> git does, and I don't really see the point in complaining about local\n> changes in a file we won't need to touch anyway.\n\nBut in git, for committing, you usually need to run \"git add\" on the\nfiles or specify \"commit -a\" explicitly. We would need to change the\n\"refresh\" behaviour in the same way and, in this case, I would be OK\nwith (2) as the default.\n\nI personally prefer the current \"refresh\" way but maybe because I'm\nused to it. It would be useful to get other users' opinion on this UI\nchange. Might not be a bad change since git does this already, quilt\nneeds an explicit \"add\" (anyone knows about guilt?).\n\n-- \nCatalin\n"},{"id":"73032","messageId":"20080325110559.GA10333@diana.vm.bytemark.co.uk","threadId":"12769","inReplyTo":"b0943d9e0803250346h7405c37egc9ba170a6dcc06bf@mail.gmail.com","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2008-03-25T11:05:59Z","receivedAt":"2008-03-25T11:05:59Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2008-03-25 10:46:00 +0000, Catalin Marinas wrote:\n\n> My reason for this is that usually you \"goto\" a patch to do some\n> changes followed by \"refresh\". If there were local changes, they\n> would be included in the refreshed patch since this is the default\n> StGIT behaviour. To avoid this, I find myself running \"status\"\n> before any \"goto\".\n\nYeah, I can see how that would be irritating. Especially since the\nconversion to the new infrastructure is only half done, so that not\nall commands behave the same.\n\n> But in git, for committing, you usually need to run \"git add\" on the\n> files or specify \"commit -a\" explicitly. We would need to change the\n> \"refresh\" behaviour in the same way and, in this case, I would be OK\n> with (2) as the default.\n>\n> I personally prefer the current \"refresh\" way but maybe because I'm\n> used to it. It would be useful to get other users' opinion on this\n> UI change. Might not be a bad change since git does this already,\n> quilt needs an explicit \"add\" (anyone knows about guilt?).\n\nI think my preference would be to to what git does: let just \"stg\nrefresh\" commit what's in the index, and have \"stg refresh -a\" or\nsomething to automatically update the index first. (This should be\neasy to do, btw -- refresh already has an --index flag.)\n\nIn general, I think it's a bad idea to do things differently from git\nwithout a good reason.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"73044","messageId":"b0943d9e0803250827i4474fd8dx80b18b0b33542ec8@mail.gmail.com","threadId":"12769","inReplyTo":"20080325110559.GA10333@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 0/6] Two bugfixes","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2008-03-25T15:27:03Z","receivedAt":"2008-03-25T15:27:03Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 25/03/2008, Karl Hasselström wrote:\n> On 2008-03-25 10:46:00 +0000, Catalin Marinas wrote:\n> > I personally prefer the current \"refresh\" way but maybe because I'm\n> > used to it. It would be useful to get other users' opinion on this\n> > UI change. Might not be a bad change since git does this already,\n> > quilt needs an explicit \"add\" (anyone knows about guilt?).\n>\n>\n> I think my preference would be to to what git does: let just \"stg\n> refresh\" commit what's in the index, and have \"stg refresh -a\" or\n> something to automatically update the index first. (This should be\n> easy to do, btw -- refresh already has an --index flag.)\n>\n> In general, I think it's a bad idea to do things differently from git\n> without a good reason.\n\nWe can give this a try on the master branch and see how it goes.\n\n-- \nCatalin\n"}]}