{"thread":{"id":"18285","subject":"[StGit PATCH 0/5] Various StGit patches","startedAt":"2009-03-12T12:08:50Z","lastAt":"2009-03-17T13:36:12Z","messageCount":19,"participants":["Catalin Marinas","Karl Hasselström"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"107810","messageId":"20090312120426.2992.35213.stgit@pc1117.cambridge.arm.com","threadId":"18285","inReplyTo":null,"subject":"[StGit PATCH 0/5] Various StGit patches","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2009-03-12T12:08:50Z","receivedAt":"2009-03-12T12:08:50Z","isPatch":true,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"Some of these patches were posted before and there were some suggestions\nwhich I added. The \"Check for local changes with goto\" patch was\nslightly improved so that the \"stgit.keep\" option could be set to \"yes\"\nif this is preferred as a default behaviour.\n\nI plan to implement transition as many commands as possible to the new\ninfrastructure. For some of them it should be trivial. The \"status\"\ncommand needs some support in stgit.lib.\n\n\nCatalin Marinas (5):\n Convert \"float\" to the lib infrastructure\n Convert \"sink\" to the new infrastructure\n Add automatic git-mergetool invocation to the new infrastructure\n Add mergetool support to the classic StGit infrastructure\n Check for local changes with \"goto\"\n\n\n examples/gitconfig | 24 +------\n stgit/argparse.py | 5 +\n stgit/commands/edit.py | 2 -\n stgit/commands/float.py | 75 +++++++++-------------\n stgit/commands/goto.py | 9 ++-\n stgit/commands/resolved.py | 5 -\n stgit/commands/sink.py | 86 +++++++++++--------------\n stgit/config.py | 1 \n stgit/git.py | 33 ++++++----\n stgit/gitmergeonefile.py | 150 --------------------------------------------\n stgit/lib/git.py | 35 +++++++++-\n stgit/lib/transaction.py | 17 ++++-\n t/t0002-status.sh | 3 -\n t/t1501-sink.sh | 2 -\n t/t2300-refresh-subdir.sh | 2 -\n t/t2800-goto-subdir.sh | 4 +\n t/t3000-dirty-merge.sh | 2 -\n 17 files changed, 153 insertions(+), 302 deletions(-)\n delete mode 100644 stgit/gitmergeonefile.py\n\n-- \nCatalin\n"},{"id":"107813","messageId":"20090312120856.2992.48548.stgit@pc1117.cambridge.arm.com","threadId":"18285","inReplyTo":"20090312120426.2992.35213.stgit@pc1117.cambridge.arm.com","subject":"[StGit PATCH 1/5] Check for local changes with \"goto\"","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2009-03-12T12:08:56Z","receivedAt":"2009-03-12T12:08:56Z","isPatch":true,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"This is done by default, unless the --keep option is passed, for\nconsistency with the \"pop\" command. The index is checked in the\nTransaction.run() function so that other commands could benefit from\nthis feature (off by default).\n\nThis behaviour can be overridden by setting the stgit.autokeep option.\n\nSigned-off-by: Catalin Marinas \n---\n examples/gitconfig | 3 +++\n stgit/argparse.py | 5 +++++\n stgit/commands/goto.py | 7 +++++--\n stgit/lib/git.py | 14 ++++++++++++--\n stgit/lib/transaction.py | 10 +++++++++-\n t/t2300-refresh-subdir.sh | 2 +-\n t/t2800-goto-subdir.sh | 4 ++--\n t/t3000-dirty-merge.sh | 2 +-\n 8 files changed, 38 insertions(+), 9 deletions(-)\n\ndiff --git a/examples/gitconfig b/examples/gitconfig\nindex 9efc089..2fc5f52 100644\n--- a/examples/gitconfig\n+++ b/examples/gitconfig\n@@ -103,6 +103,9 @@\n \t# -O/--diff-opts). For example, -M turns on rename detection.\n \t#diff-opts = -M\n \n+\t# Behave as if the --keep option is always passed\n+\t#autokeep = no\n+\n [mail \"alias\"]\n \t# E-mail aliases used with the 'mail' command\n \tgit = git@vger.kernel.org\ndiff --git a/stgit/argparse.py b/stgit/argparse.py\nindex 418a506..85ee6e3 100644\n--- a/stgit/argparse.py\n+++ b/stgit/argparse.py\n@@ -220,6 +220,11 @@ def _person_opts(person, short):\n def author_options():\n return _person_opts('author', 'auth')\n \n+def keep_option():\n+ return [opt('-k', '--keep', action = 'store_true',\n+ short = 'Keep the local changes',\n+ default = config.get('stgit.autokeep') == 'yes')]\n+\n class CompgenBase(object):\n def actions(self, var): return set()\n def words(self, var): return set()\ndiff --git a/stgit/commands/goto.py b/stgit/commands/goto.py\nindex 60a917e..0f67314 100644\n--- a/stgit/commands/goto.py\n+++ b/stgit/commands/goto.py\n@@ -18,6 +18,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n from stgit.commands import common\n from stgit.lib import transaction\n from stgit import argparse\n+from stgit.argparse import opt\n \n help = 'Push or pop patches to the given one'\n kind = 'stack'\n@@ -27,7 +28,7 @@ Push/pop patches to/from the stack until the one given on the command\n line becomes current.\"\"\"\n \n args = [argparse.other_applied_patches, argparse.unapplied_patches]\n-options = []\n+options = argparse.keep_option()\n \n directory = common.DirectoryHasRepositoryLib()\n \n@@ -38,7 +39,9 @@ def func(parser, options, args):\n \n stack = directory.repository.current_stack\n iw = stack.repository.default_iw\n- trans = transaction.StackTransaction(stack, 'goto')\n+ clean_iw = not options.keep and iw or None\n+ trans = transaction.StackTransaction(stack, 'goto',\n+ check_clean_iw = clean_iw)\n if patch in trans.applied:\n to_pop = set(trans.applied[trans.applied.index(patch)+1:])\n assert not trans.pop_patches(lambda pn: pn in to_pop)\ndiff --git a/stgit/lib/git.py b/stgit/lib/git.py\nindex e2b4266..07079b8 100644\n--- a/stgit/lib/git.py\n+++ b/stgit/lib/git.py\n@@ -706,9 +706,11 @@ class Index(RunWithEnv):\n ).output_one_line())\n except run.RunException:\n raise MergeException('Conflicting merge')\n- def is_clean(self):\n+ def is_clean(self, tree):\n+ \"\"\"Check whether the index is clean relative to the given treeish.\"\"\"\n try:\n- self.run(['git', 'update-index', '--refresh']).discard_output()\n+ self.run(['git', 'diff-index', '--quiet', '--cached', tree.sha1]\n+ ).discard_output()\n except run.RunException:\n return False\n else:\n@@ -858,6 +860,14 @@ class IndexAndWorktree(RunWithEnvCwd):\n cmd = ['git', 'update-index', '--remove']\n self.run(cmd + ['-z', '--stdin']\n ).input_nulterm(paths).discard_output()\n+ def worktree_clean(self):\n+ \"\"\"Check whether the worktree is clean relative to index.\"\"\"\n+ try:\n+ self.run(['git', 'update-index', '--refresh']).discard_output()\n+ except run.RunException:\n+ return False\n+ else:\n+ return True\n \n class Branch(object):\n \"\"\"Represents a Git branch.\"\"\"\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 54de127..5a81f9d 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -75,7 +75,8 @@ class StackTransaction(object):\n your refs and index+worktree, or fail without having done\n anything.\"\"\"\n def __init__(self, stack, msg, discard_changes = False,\n- allow_conflicts = False, allow_bad_head = False):\n+ allow_conflicts = False, allow_bad_head = False,\n+ check_clean_iw = None):\n \"\"\"Create a new L{StackTransaction}.\n \n @param discard_changes: Discard any changes in index+worktree\n@@ -102,6 +103,8 @@ class StackTransaction(object):\n self.__temp_index = self.temp_index_tree = None\n if not allow_bad_head:\n self.__assert_head_top_equal()\n+ if check_clean_iw:\n+ self.__assert_index_worktree_clean(check_clean_iw)\n stack = property(lambda self: self.__stack)\n patches = property(lambda self: self.__patches)\n def __set_applied(self, val):\n@@ -147,6 +150,11 @@ class StackTransaction(object):\n 'This can happen if you modify a branch with git.',\n '\"stg repair --help\" explains more about what to do next.')\n self.__abort()\n+ def __assert_index_worktree_clean(self, iw):\n+ if not iw.worktree_clean() or \\\n+ not iw.index.is_clean(self.stack.head):\n+ self.__halt('Repository not clean. Use \"refresh\" or '\n+ '\"status --reset\"')\n def __checkout(self, tree, iw, allow_bad_head):\n if not allow_bad_head:\n self.__assert_head_top_equal()\ndiff --git a/t/t2300-refresh-subdir.sh b/t/t2300-refresh-subdir.sh\nindex d731a11..89c95db 100755\n--- a/t/t2300-refresh-subdir.sh\n+++ b/t/t2300-refresh-subdir.sh\n@@ -65,7 +65,7 @@ test_expect_success 'refresh -u -p ' '\n \n test_expect_success 'refresh an unapplied patch' '\n stg refresh -u &&\n- stg goto p0 &&\n+ stg goto --keep p0 &&\n test \"$(stg status)\" = \"M foo.txt\" &&\n stg refresh -p p1 &&\n test \"$(stg status)\" = \"\" &&\ndiff --git a/t/t2800-goto-subdir.sh b/t/t2800-goto-subdir.sh\nindex 28b8292..855972b 100755\n--- a/t/t2800-goto-subdir.sh\n+++ b/t/t2800-goto-subdir.sh\n@@ -25,7 +25,7 @@ cat > expected2.txt < actual.txt &&\n test_cmp expected1.txt actual.txt &&\n ls foo > actual.txt &&\n@@ -48,7 +48,7 @@ cat > expected2.txt < actual.txt &&\n test_cmp expected1.txt actual.txt &&\ndiff --git a/t/t3000-dirty-merge.sh b/t/t3000-dirty-merge.sh\nindex f0f79d5..419d86e 100755\n--- a/t/t3000-dirty-merge.sh\n+++ b/t/t3000-dirty-merge.sh\n@@ -26,7 +26,7 @@ test_expect_success 'Push with dirty worktree' '\n echo 4 > a &&\n [ \"$(echo $(stg series --applied --noprefix))\" = \"p1\" ] &&\n [ \"$(echo $(stg series --unapplied --noprefix))\" = \"p2\" ] &&\n- conflict stg goto p2 &&\n+ conflict stg goto --keep p2 &&\n [ \"$(echo $(stg series --applied --noprefix))\" = \"p1\" ] &&\n [ \"$(echo $(stg series --unapplied --noprefix))\" = \"p2\" ] &&\n [ \"$(echo $(cat a))\" = \"4\" ]\n"},{"id":"107811","messageId":"20090312120902.2992.25192.stgit@pc1117.cambridge.arm.com","threadId":"18285","inReplyTo":"20090312120426.2992.35213.stgit@pc1117.cambridge.arm.com","subject":"[StGit PATCH 2/5] Add mergetool support to the classic StGit infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2009-03-12T12:09:02Z","receivedAt":"2009-03-12T12:09:02Z","isPatch":true,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"Since Git already has a tool for interactively solving conflicts which\nis highly customisable, there is no need to duplicate this feature via\nthe i3merge and i2merge configuration options. The user-visible change\nis that now mergetool is invoked rather than the previously customised\ninteractive merging tool.\n\nThe stgit.keeporig option is no longer available to be more consistent\nwith the Git behaviour.\n\nSigned-off-by: Catalin Marinas \n---\n examples/gitconfig | 21 +-----\n stgit/commands/resolved.py | 5 -\n stgit/config.py | 1 \n stgit/git.py | 33 ++++++----\n stgit/gitmergeonefile.py | 150 --------------------------------------------\n t/t0002-status.sh | 3 -\n 6 files changed, 22 insertions(+), 191 deletions(-)\n delete mode 100644 stgit/gitmergeonefile.py\n\ndiff --git a/examples/gitconfig b/examples/gitconfig\nindex 2fc5f52..f6e3a79 100644\n--- a/examples/gitconfig\n+++ b/examples/gitconfig\n@@ -64,27 +64,10 @@\n \t# To support local parent branches:\n \t#pull-policy = rebase\n \n-\t# Interactive two/three-way merge tool. It is executed by the\n-\t# 'resolved --interactive' command\n-\t#i3merge = xxdiff --title1 current --title2 ancestor --title3 patched \\\n-\t#\t--show-merged-pane -m -E -O -X -M \\\"%(output)s\\\" \\\n-\t#\t\\\"%(branch1)s\\\" \\\"%(ancestor)s\\\" \\\"%(branch2)s\\\"\n-\t#i2merge = xxdiff --title1 current --title2 patched \\\n-\t#\t--show-merged-pane -m -E -O -X -M \\\"%(output)s\\\" \\\n-\t#\t\\\"%(branch1)s\\\" \\\"%(branch2)s\\\"\n-\t#i3merge = emacs --eval '(ediff-merge-files-with-ancestor \\\n-\t#\t\\\"%(branch1)s\\\" \\\"%(branch2)s\\\" \\\"%(ancestor)s\\\" nil \\\n-\t#\t\\\"%(output)s\\\")'\n-\t#i2merge = emacs --eval '(ediff-merge-files \\\n-\t#\t\\\"%(branch1)s\\\" \\\"%(branch2)s\\\" nil \\\"%(output)s\\\")'\n-\n-\t# Automatically invoke the interactive merger in case of conflicts\n+\t# Automatically invoke the interactive merger (git mergetool) in case\n+\t# of conflicts\n \t#autoimerge = no\n \n-\t# Leave the original files in the working tree in case of a\n-\t# merge conflict\n-\t#keeporig = yes\n-\n \t# Optimize (repack) the object store after every pull\n \t#keepoptimized = yes\n \ndiff --git a/stgit/commands/resolved.py b/stgit/commands/resolved.py\nindex 2ce7ec3..eba778d 100644\n--- a/stgit/commands/resolved.py\n+++ b/stgit/commands/resolved.py\n@@ -22,7 +22,6 @@ from stgit.commands.common import *\n from stgit.utils import *\n from stgit import argparse, stack, git, basedir\n from stgit.config import config, file_extensions\n-from stgit.gitmergeonefile import interactive_merge\n \n help = 'Mark a file conflict as solved'\n kind = 'wc'\n@@ -78,8 +77,6 @@ def func(parser, options, args):\n \n # resolved\n if options.interactive:\n- for filename in files:\n- interactive_merge(filename)\n- git.resolved([filename])\n+ git.mergetool(files)\n else:\n git.resolved(files, options.reset)\ndiff --git a/stgit/config.py b/stgit/config.py\nindex 05ef624..efce097 100644\n--- a/stgit/config.py\n+++ b/stgit/config.py\n@@ -35,7 +35,6 @@ class GitConfig:\n 'stgit.fetchcmd':\t'git fetch',\n 'stgit.pull-policy':\t'pull',\n 'stgit.autoimerge':\t'no',\n- 'stgit.keeporig':\t'yes',\n 'stgit.keepoptimized':\t'no',\n 'stgit.extensions':\t'.ancestor .current .patched',\n 'stgit.shortnr':\t '5'\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 4d01fc2..50e3621 100644\n--- a/stgit/git.py\n+++ b/stgit/git.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, re, gitmergeonefile\n+import sys, os, re\n from shutil import copyfile\n \n from stgit.exception import *\n@@ -632,19 +632,25 @@ def merge_recursive(base, head1, head2):\n output = p.output_lines()\n if p.exitcode:\n # There were conflicts\n- conflicts = [l.strip() for l in output if l.startswith('CONFLICT')]\n+ if config.get('stgit.autoimerge') == 'yes':\n+ mergetool()\n+ else:\n+ conflicts = [l for l in output if l.startswith('CONFLICT')]\n+ out.info(*conflicts)\n+ raise GitException, \"%d conflict(s)\" % len(conflicts)\n+\n+def mergetool(files = ()):\n+ \"\"\"Invoke 'git mergetool' to resolve any outstanding conflicts. If 'not\n+ files', all the files in an unmerged state will be processed.\"\"\"\n+ err = os.system('git mergetool %s' % ' '.join(files))\n+ # check for unmerged entries (prepend 'CONFLICT ' for consistency with\n+ # merge_recursive())\n+ conflicts = ['CONFLICT ' + f for f in get_conflicts()]\n+ if conflicts:\n out.info(*conflicts)\n-\n- # try the interactive merge or stage checkout (if enabled)\n- for filename in get_conflicts():\n- if (gitmergeonefile.merge(filename)):\n- # interactive merge succeeded\n- resolved([filename])\n-\n- # any conflicts left unsolved?\n- cn = len(get_conflicts())\n- if cn:\n- raise GitException, \"%d conflict(s)\" % cn\n+ raise GitException, \"%d conflict(s)\" % len(conflicts)\n+ elif err:\n+ raise GitException('\"git mergetool\" failed, exit code: %d' % err)\n \n def diff(files = None, rev1 = 'HEAD', rev2 = None, diff_flags = [],\n binary = True):\n@@ -754,7 +760,6 @@ def resolved(filenames, reset = None):\n '--stdin', '-z').input_nulterm(filenames).no_output()\n GRun('update-index', '--add', '--').xargs(filenames)\n for filename in filenames:\n- gitmergeonefile.clean_up(filename)\n # update the access and modificatied times\n os.utime(filename, None)\n \ndiff --git a/stgit/gitmergeonefile.py b/stgit/gitmergeonefile.py\ndeleted file mode 100644\nindex 1fe226e..0000000\n--- a/stgit/gitmergeonefile.py\n+++ /dev/null\n@@ -1,150 +0,0 @@\n-\"\"\"Performs a 3-way merge for GIT files\n-\"\"\"\n-\n-__copyright__ = \"\"\"\n-Copyright (C) 2006, Catalin Marinas \n-\n-This program is free software; you can redistribute it and/or modify\n-it under the terms of the GNU General Public License version 2 as\n-published by the Free Software Foundation.\n-\n-This program is distributed in the hope that it will be useful,\n-but WITHOUT ANY WARRANTY; without even the implied warranty of\n-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n-GNU General Public License for more details.\n-\n-You should have received a copy of the GNU General Public License\n-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-from stgit.exception import *\n-from stgit import basedir\n-from stgit.config import config, file_extensions, ConfigOption\n-from stgit.utils import append_string\n-from stgit.out import *\n-from stgit.run import *\n-\n-class GitMergeException(StgException):\n- pass\n-\n-\n-#\n-# Options\n-#\n-autoimerge = ConfigOption('stgit', 'autoimerge')\n-keeporig = ConfigOption('stgit', 'keeporig')\n-\n-#\n-# Utility functions\n-#\n-def __str2none(x):\n- if x == '':\n- return None\n- else:\n- return x\n-\n-class MRun(Run):\n- exc = GitMergeException # use a custom exception class on errors\n-\n-def __checkout_stages(filename):\n- \"\"\"Check-out the merge stages in the index for the give file\n- \"\"\"\n- extensions = file_extensions()\n- line = MRun('git', 'checkout-index', '--stage=all', '--', filename\n- ).output_one_line()\n- stages, path = line.split('\\t')\n- stages = dict(zip(['ancestor', 'current', 'patched'],\n- stages.split(' ')))\n-\n- for stage, fn in stages.iteritems():\n- if stages[stage] == '.':\n- stages[stage] = None\n- else:\n- newname = filename + extensions[stage]\n- if os.path.exists(newname):\n- # remove the stage if it is already checked out\n- os.remove(newname)\n- os.rename(stages[stage], newname)\n- stages[stage] = newname\n-\n- return stages\n-\n-def __remove_stages(filename):\n- \"\"\"Remove the merge stages from the working directory\n- \"\"\"\n- extensions = file_extensions()\n- for ext in extensions.itervalues():\n- fn = filename + ext\n- if os.path.isfile(fn):\n- os.remove(fn)\n-\n-def interactive_merge(filename):\n- \"\"\"Run the interactive merger on the given file. Stages will be\n- removed according to stgit.keeporig. If successful and stages\n- kept, they will be removed via git.resolved().\n- \"\"\"\n- stages = __checkout_stages(filename)\n-\n- try:\n- # Check whether we have all the files for the merge.\n- if not (stages['current'] and stages['patched']):\n- raise GitMergeException('Cannot run the interactive merge')\n-\n- if stages['ancestor']:\n- three_way = True\n- files_dict = {'branch1': stages['current'],\n- 'ancestor': stages['ancestor'],\n- 'branch2': stages['patched'],\n- 'output': filename}\n- imerger = config.get('stgit.i3merge')\n- else:\n- three_way = False\n- files_dict = {'branch1': stages['current'],\n- 'branch2': stages['patched'],\n- 'output': filename}\n- imerger = config.get('stgit.i2merge')\n-\n- if not imerger:\n- raise GitMergeException, 'No interactive merge command configured'\n-\n- mtime = os.path.getmtime(filename)\n-\n- out.start('Trying the interactive %s merge'\n- % (three_way and 'three-way' or 'two-way'))\n- err = os.system(imerger % files_dict)\n- out.done()\n- if err != 0:\n- raise GitMergeException, 'The interactive merge failed'\n- if not os.path.isfile(filename):\n- raise GitMergeException, 'The \"%s\" file is missing' % filename\n- if mtime == os.path.getmtime(filename):\n- raise GitMergeException, 'The \"%s\" file was not modified' % filename\n- finally:\n- # keep the merge stages?\n- if str(keeporig) != 'yes':\n- __remove_stages(filename)\n-\n-def clean_up(filename):\n- \"\"\"Remove merge conflict stages if they were generated.\n- \"\"\"\n- if str(keeporig) == 'yes':\n- __remove_stages(filename)\n-\n-def merge(filename):\n- \"\"\"Merge one file if interactive is allowed or check out the stages\n- if keeporig is set.\n- \"\"\"\n- if str(autoimerge) == 'yes':\n- try:\n- interactive_merge(filename)\n- except GitMergeException, ex:\n- out.error(str(ex))\n- return False\n- return True\n-\n- if str(keeporig) == 'yes':\n- __checkout_stages(filename)\n-\n- return False\ndiff --git a/t/t0002-status.sh b/t/t0002-status.sh\nindex ac92aa8..d95a83b 100755\n--- a/t/t0002-status.sh\n+++ b/t/t0002-status.sh\n@@ -107,9 +107,6 @@ test_expect_success 'Make a conflicting patch' '\n '\n \n cat > expected.txt <\n---\n stgit/commands/edit.py | 2 +-\n stgit/commands/goto.py | 2 +-\n stgit/lib/git.py | 21 ++++++++++++++++++---\n stgit/lib/transaction.py | 7 +++++--\n 4 files changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/stgit/commands/edit.py b/stgit/commands/edit.py\nindex ed785aa..42eb792 100644\n--- a/stgit/commands/edit.py\n+++ b/stgit/commands/edit.py\n@@ -128,7 +128,7 @@ def func(parser, options, args):\n trans.patches[patchname] = stack.repository.commit(cd)\n try:\n for pn in popped:\n- trans.push_patch(pn, iw)\n+ trans.push_patch(pn, iw, allow_interactive = True)\n except transaction.TransactionHalted:\n pass\n try:\ndiff --git a/stgit/commands/goto.py b/stgit/commands/goto.py\nindex 0f67314..56c09da 100644\n--- a/stgit/commands/goto.py\n+++ b/stgit/commands/goto.py\n@@ -48,7 +48,7 @@ def func(parser, options, args):\n elif patch in trans.unapplied:\n try:\n for pn in trans.unapplied[:trans.unapplied.index(patch)+1]:\n- trans.push_patch(pn, iw)\n+ trans.push_patch(pn, iw, allow_interactive = True)\n except transaction.TransactionHalted:\n pass\n elif patch in trans.hidden:\ndiff --git a/stgit/lib/git.py b/stgit/lib/git.py\nindex 07079b8..d83b475 100644\n--- a/stgit/lib/git.py\n+++ b/stgit/lib/git.py\n@@ -824,7 +824,7 @@ class IndexAndWorktree(RunWithEnvCwd):\n ).discard_output()\n except run.RunException:\n raise CheckoutException('Index/workdir dirty')\n- def merge(self, base, ours, theirs):\n+ def merge(self, base, ours, theirs, interactive = False):\n assert isinstance(base, Tree)\n assert isinstance(ours, Tree)\n assert isinstance(theirs, Tree)\n@@ -838,10 +838,25 @@ class IndexAndWorktree(RunWithEnvCwd):\n output = r.output_lines()\n if r.exitcode:\n # There were conflicts\n- conflicts = [l for l in output if l.startswith('CONFLICT')]\n- raise MergeConflictException(conflicts)\n+ if interactive:\n+ self.mergetool()\n+ else:\n+ conflicts = [l for l in output if l.startswith('CONFLICT')]\n+ raise MergeConflictException(conflicts)\n except run.RunException, e:\n raise MergeException('Index/worktree dirty')\n+ def mergetool(self, files = ()):\n+ \"\"\"Invoke 'git mergetool' on the current IndexAndWorktree to resolve\n+ any outstanding conflicts. If 'not files', all the files in an\n+ unmerged state will be processed.\"\"\"\n+ err = os.system('git mergetool %s' % ' '.join(files))\n+ # check for unmerged entries (prepend 'CONFLICT ' for consistency with\n+ # merge())\n+ conflicts = ['CONFLICT ' + f for f in self.index.conflicts()]\n+ if conflicts:\n+ raise MergeConflictException(conflicts)\n+ elif err:\n+ raise MergeException('\"git mergetool\" failed, exit code: %d' % err)\n def changed_files(self, tree, pathlimits = []):\n \"\"\"Return the set of files in the worktree that have changed with\n respect to C{tree}. The listing is optionally restricted to\ndiff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py\nindex 5a81f9d..a918a66 100644\n--- a/stgit/lib/transaction.py\n+++ b/stgit/lib/transaction.py\n@@ -8,6 +8,7 @@ from stgit import exception, utils\n from stgit.utils import any, all\n from stgit.out import *\n from stgit.lib import git, log\n+from stgit.config import config\n \n class TransactionException(exception.StgException):\n \"\"\"Exception raised when something goes wrong with a\n@@ -296,7 +297,7 @@ class StackTransaction(object):\n out.info('Deleted %s%s' % (pn, s))\n return popped\n \n- def push_patch(self, pn, iw = None):\n+ def push_patch(self, pn, iw = None, allow_interactive = False):\n \"\"\"Attempt to push the named patch. If this results in conflicts,\n halts the transaction. If index+worktree are given, spill any\n conflicts to them.\"\"\"\n@@ -319,7 +320,9 @@ class StackTransaction(object):\n except git.CheckoutException:\n self.__halt('Index/worktree dirty')\n try:\n- iw.merge(base, ours, theirs)\n+ interactive = allow_interactive and \\\n+ config.get('stgit.autoimerge') == 'yes'\n+ iw.merge(base, ours, theirs, interactive = interactive)\n tree = iw.index.write_tree()\n self.__current_tree = tree\n s = ' (modified)'\n"},{"id":"107815","messageId":"20090312120913.2992.21403.stgit@pc1117.cambridge.arm.com","threadId":"18285","inReplyTo":"20090312120426.2992.35213.stgit@pc1117.cambridge.arm.com","subject":"[StGit PATCH 4/5] Convert \"sink\" to the new infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2009-03-12T12:09:13Z","receivedAt":"2009-03-12T12:09:13Z","isPatch":true,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"Signed-off-by: Catalin Marinas \n---\n stgit/commands/sink.py | 86 +++++++++++++++++++++---------------------------\n t/t1501-sink.sh | 2 +\n 2 files changed, 39 insertions(+), 49 deletions(-)\n\ndiff --git a/stgit/commands/sink.py b/stgit/commands/sink.py\nindex d4561ed..8b17fd1 100644\n--- a/stgit/commands/sink.py\n+++ b/stgit/commands/sink.py\n@@ -16,11 +16,10 @@ 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 from stgit.argparse import opt\n-from stgit.commands.common import *\n-from stgit.utils import *\n-from stgit import argparse, stack, git\n+from stgit.commands import common\n+from stgit.lib import transaction\n+from stgit import argparse\n \n help = 'Send patches deeper down the stack'\n kind = 'stack'\n@@ -51,57 +50,48 @@ options = [\n opt('-t', '--to', metavar = 'TARGET', args = [argparse.applied_patches],\n short = 'Sink patches below the TARGET patch', long = \"\"\"\n Specify a target patch to place the patches below, instead of\n- sinking them to the bottom of the stack.\"\"\")]\n+ sinking them to the bottom of the stack.\"\"\")\n+ ] + argparse.keep_option()\n \n-directory = DirectoryGotoToplevel(log = True)\n+directory = common.DirectoryHasRepositoryLib()\n \n def func(parser, options, args):\n \"\"\"Sink patches down the stack.\n \"\"\"\n+ stack = directory.repository.current_stack\n \n- check_local_changes()\n- check_conflicts()\n- check_head_top_equal(crt_series)\n+ if options.to and not options.to in stack.patchorder.applied:\n+ raise common.CmdException('Cannot sink below %s since it is not applied'\n+ % options.to)\n \n- oldapplied = crt_series.get_applied()\n- unapplied = crt_series.get_unapplied()\n- all = oldapplied + unapplied\n+ if len(args) > 0:\n+ patches = common.parse_patches(args, stack.patchorder.all)\n+ else:\n+ # current patch\n+ patches = list(stack.patchorder.applied[-1:])\n \n- if options.to and not options.to in oldapplied:\n- raise CmdException('Cannot sink below %s, since it is not applied'\n- % options.to)\n+ if not patches:\n+ raise common.CmdException('No patches to sink')\n+ if options.to and options.to in patches:\n+ raise common.CmdException('Cannot have a sinked patch as target')\n \n- if len(args) > 0:\n- patches = parse_patches(args, all)\n+ applied = [p for p in stack.patchorder.applied if p not in patches]\n+ if options.to:\n+ insert_idx = applied.index(options.to)\n else:\n- current = crt_series.get_current()\n- if not current:\n- raise CmdException('No patch applied')\n- patches = [current]\n-\n- before_patches = after_patches = []\n-\n- # pop necessary patches\n- if oldapplied:\n- if options.to:\n- pop_idx = oldapplied.index(options.to)\n- else:\n- pop_idx = 0\n- after_patches = [p for p in oldapplied[pop_idx:] if p not in patches]\n-\n- # find the deepest patch to pop\n- sink_applied = [p for p in oldapplied if p in patches]\n- if sink_applied:\n- sinked_idx = oldapplied.index(sink_applied[0])\n- if sinked_idx < pop_idx:\n- # this is the case where sink brings patches forward\n- before_patches = [p for p in oldapplied[sinked_idx:pop_idx]\n- if p not in patches]\n- pop_idx = sinked_idx\n-\n- crt_series.pop_patch(oldapplied[pop_idx])\n-\n- push_patches(crt_series, before_patches)\n- push_patches(crt_series, patches)\n- if not options.nopush:\n- push_patches(crt_series, after_patches)\n+ insert_idx = 0\n+ applied = applied[:insert_idx] + patches + applied[insert_idx:]\n+\n+ unapplied = [p for p in stack.patchorder.unapplied if p not in patches]\n+ hidden = list(stack.patchorder.hidden)\n+\n+ iw = stack.repository.default_iw\n+ clean_iw = not options.keep and iw or None\n+ trans = transaction.StackTransaction(stack, 'sink',\n+ check_clean_iw = clean_iw)\n+\n+ try:\n+ trans.reorder_patches(applied, unapplied, hidden, iw)\n+ except transaction.TransactionHalted:\n+ pass\n+ return trans.run(iw)\ndiff --git a/t/t1501-sink.sh b/t/t1501-sink.sh\nindex 516aa44..a0769dd 100755\n--- a/t/t1501-sink.sh\n+++ b/t/t1501-sink.sh\n@@ -58,7 +58,7 @@ test_expect_success 'sink specified patch below a target' '\n '\n \n test_expect_success 'sink with conflict' '\n- conflict_old stg sink --to=p2 p22 &&\n+ conflict stg sink --to=p2 p22 &&\n test \"$(echo $(stg series --applied --noprefix))\" = \"p1 p22\" &&\n test \"$(echo $(stg status -c))\" = \"f2\"\n '\n"},{"id":"107814","messageId":"20090312120918.2992.82713.stgit@pc1117.cambridge.arm.com","threadId":"18285","inReplyTo":"20090312120426.2992.35213.stgit@pc1117.cambridge.arm.com","subject":"[StGit PATCH 5/5] Convert \"float\" to the lib infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2009-03-12T12:09:18Z","receivedAt":"2009-03-12T12:09:18Z","isPatch":true,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"Signed-off-by: Catalin Marinas \n---\n stgit/commands/float.py | 75 ++++++++++++++++++-----------------------------\n 1 files changed, 29 insertions(+), 46 deletions(-)\n\ndiff --git a/stgit/commands/float.py b/stgit/commands/float.py\nindex 7c3dcdf..f94de88 100644\n--- a/stgit/commands/float.py\n+++ b/stgit/commands/float.py\n@@ -16,11 +16,11 @@ 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\n from stgit.argparse import opt\n-from stgit.commands.common import *\n-from stgit.utils import *\n-from stgit import argparse, stack, git\n+from stgit.commands import common\n+from stgit.lib import transaction\n+from stgit import argparse\n \n help = 'Push patches to the top, even if applied'\n kind = 'stack'\n@@ -36,25 +36,20 @@ args = [argparse.patch_range(argparse.applied_patches,\n argparse.unapplied_patches)]\n options = [\n opt('-s', '--series', action = 'store_true',\n- short = 'Rearrange according to a series file')]\n+ short = 'Rearrange according to a series file')\n+ ] + argparse.keep_option()\n \n-directory = DirectoryGotoToplevel(log = True)\n+directory = common.DirectoryHasRepositoryLib()\n \n def func(parser, options, args):\n- \"\"\"Pops and pushed to make the named patch the topmost patch\n+ \"\"\"Reorder patches to make the named patch the topmost one.\n \"\"\"\n args_nr = len(args)\n if (options.series and args_nr > 1) \\\n or (not options.series and args_nr == 0):\n parser.error('incorrect number of arguments')\n \n- check_local_changes()\n- check_conflicts()\n- check_head_top_equal(crt_series)\n-\n- unapplied = crt_series.get_unapplied()\n- applied = crt_series.get_applied()\n- all = unapplied + applied\n+ stack = directory.repository.current_stack\n \n if options.series:\n if args_nr:\n@@ -68,35 +63,23 @@ def func(parser, options, args):\n if patch:\n patches.append(patch)\n else:\n- patches = parse_patches(args, all)\n-\n- # working with \"topush\" patches in reverse order might be a bit\n- # more efficient for large series but the main reason is for the\n- # \"topop != topush\" comparison to work\n- patches.reverse()\n-\n- topush = []\n- topop = []\n-\n- for p in patches:\n- while p in applied:\n- top = applied.pop()\n- if not top in patches:\n- topush.append(top)\n- topop.append(top)\n- topush = patches + topush\n-\n- # remove common patches to avoid unnecessary pop/push\n- while topush and topop:\n- if topush[-1] != topop[-1]:\n- break\n- topush.pop()\n- topop.pop()\n-\n- # check whether the operation is really needed\n- if topop != topush:\n- if topop:\n- pop_patches(crt_series, topop)\n- if topush:\n- topush.reverse()\n- push_patches(crt_series, topush)\n+ patches = common.parse_patches(args, stack.patchorder.all)\n+\n+ if not patches:\n+ raise common.CmdException('No patches to float')\n+\n+ applied = [p for p in stack.patchorder.applied if p not in patches] + \\\n+ patches\n+ unapplied = [p for p in stack.patchorder.unapplied if p not in patches]\n+ hidden = list(stack.patchorder.hidden)\n+\n+ iw = stack.repository.default_iw\n+ clean_iw = not options.keep and iw or None\n+ trans = transaction.StackTransaction(stack, 'sink',\n+ check_clean_iw = clean_iw)\n+\n+ try:\n+ trans.reorder_patches(applied, unapplied, hidden, iw)\n+ except transaction.TransactionHalted:\n+ pass\n+ return trans.run(iw)\n"},{"id":"107888","messageId":"20090313015755.GA15393@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"20090312120856.2992.48548.stgit@pc1117.cambridge.arm.com","subject":"Re: [StGit PATCH 1/5] Check for local changes with \"goto\"","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-13T01:57:55Z","receivedAt":"2009-03-13T01:57:55Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-12 12:08:56 +0000, Catalin Marinas wrote:\n\n> This is done by default, unless the --keep option is passed, for\n> consistency with the \"pop\" command. The index is checked in the\n> Transaction.run() function so that other commands could benefit from\n> this feature (off by default).\n>\n> This behaviour can be overridden by setting the stgit.autokeep option.\n\nTwo small nits; otherwise,\n\n Acked-by: Karl Hasselström \n\n> - trans = transaction.StackTransaction(stack, 'goto')\n> + clean_iw = not options.keep and iw or None\n\nAdd some parentheses here, please! I know that \"and\" binds tighter\nthan \"or\", but I have to think for too long to remember which is\nwhich, and I'll bet I'm not alone ...\n\n> + def __assert_index_worktree_clean(self, iw):\n> + if not iw.worktree_clean() or \\\n> + not iw.index.is_clean(self.stack.head):\n> + self.__halt('Repository not clean. Use \"refresh\" or '\n> + '\"status --reset\"')\n\n\"Repository\" is misleading here. Maybe something like\n\n ix_c = iw.index.is_clean(self.stack.head)\n wt_c = iw.worktree_clean()\n if not ix_c or not wt_c:\n self.__halt('%s not clean. Use \"refresh\" or \"status --reset\"'\n % { (False, True): 'Index', (True, False): 'Worktree',\n (False, False): 'Index and worktree' }[(ix_c, wt_c)])\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"107889","messageId":"20090313020243.GB15393@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"20090312120902.2992.25192.stgit@pc1117.cambridge.arm.com","subject":"Re: [StGit PATCH 2/5] Add mergetool support to the classic StGit infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-13T02:02:43Z","receivedAt":"2009-03-13T02:02:43Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-12 12:09:02 +0000, Catalin Marinas wrote:\n\n> Since Git already has a tool for interactively solving conflicts\n> which is highly customisable, there is no need to duplicate this\n> feature via the i3merge and i2merge configuration options. The\n> user-visible change is that now mergetool is invoked rather than the\n> previously customised interactive merging tool.\n\nI agree wholeheartedly with the idea. Just one issue:\n\n> +def mergetool(files = ()):\n> + \"\"\"Invoke 'git mergetool' to resolve any outstanding conflicts. If 'not\n> + files', all the files in an unmerged state will be processed.\"\"\"\n> + err = os.system('git mergetool %s' % ' '.join(files))\n> + # check for unmerged entries (prepend 'CONFLICT ' for consistency with\n> + # merge_recursive())\n> + conflicts = ['CONFLICT ' + f for f in get_conflicts()]\n> + if conflicts:\n\nMmm, os.system()? That'll break things as soon as we have a file name\nwith a space in it. I'm pretty sure there's something in stgit.run\nthat you could use.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"107896","messageId":"20090313021751.GC15393@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"20090312120907.2992.81035.stgit@pc1117.cambridge.arm.com","subject":"Re: [StGit PATCH 3/5] Add automatic git-mergetool invocation to the new infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-13T02:17:51Z","receivedAt":"2009-03-13T02:17:51Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-12 12:09:07 +0000, Catalin Marinas wrote:\n\n> This patch adds the IndexAndWorktree.mergetool() function responsible\n> for calling 'git mergetool' to interactively solve conflicts. The\n> function may also be called from IndexAndWorktree.merge() if the\n> standard 'git merge-recursive' fails and 'interactive == True'. The\n> 'allow_interactive' parameter is passed to Transaction.push_patch() from\n> the functions allowing interactive merging.\n\nNicely done with the \"interactive\" and \"allow_interactive\" arguments;\nthe policy and the implementation end up at the right levels.\n\n> # There were conflicts\n> - conflicts = [l for l in output if l.startswith('CONFLICT')]\n> - raise MergeConflictException(conflicts)\n> + if interactive:\n> + self.mergetool()\n> + else:\n> + conflicts = [l for l in output if l.startswith('CONFLICT')]\n> + raise MergeConflictException(conflicts)\n\nDoes the merge tool always resolve all conflicts? If it doesn't, the\ntwo lines in the \"else\" branch should probably be run unconditionally.\n\n> except run.RunException, e:\n> raise MergeException('Index/worktree dirty')\n> + def mergetool(self, files = ()):\n> + \"\"\"Invoke 'git mergetool' on the current IndexAndWorktree to resolve\n> + any outstanding conflicts. If 'not files', all the files in an\n> + unmerged state will be processed.\"\"\"\n> + err = os.system('git mergetool %s' % ' '.join(files))\n\nLook at how the surrounding code calls git. os.system() will do nasty\nthings with filenames that require quoting, such as for example\n\"; rm -rf ~/\".\n\n> + # check for unmerged entries (prepend 'CONFLICT ' for consistency with\n> + # merge())\n> + conflicts = ['CONFLICT ' + f for f in self.index.conflicts()]\n> + if conflicts:\n> + raise MergeConflictException(conflicts)\n> + elif err:\n> + raise MergeException('\"git mergetool\" failed, exit code: %d' % err)\n\nAh, you take care of conflicts here too. Hmm. I guess that's fine too,\nthough there is some code duplication. Maybe a helper function that\ntakes output as a parameter, and raises MergeConflictException if\nnecessary?\n\n> + interactive = allow_interactive and \\\n> + config.get('stgit.autoimerge') == 'yes'\n\nSmall style nit: backslash line continuations are ugly. :-)\n\nIf you put parentheses around the expression, you can break the line\nwithout a backslash.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"107897","messageId":"20090313022751.GD15393@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"20090312120913.2992.21403.stgit@pc1117.cambridge.arm.com","subject":"Re: [StGit PATCH 4/5] Convert \"sink\" to the new infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-13T02:27:51Z","receivedAt":"2009-03-13T02:27:51Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Nicely done.\n\n Acked-by: Karl Hasselström \n\nOn 2009-03-12 12:09:13 +0000, Catalin Marinas wrote:\n\n> + applied = applied[:insert_idx] + patches + applied[insert_idx:]\n> +\n> + unapplied = [p for p in stack.patchorder.unapplied if p not in patches]\n> + hidden = list(stack.patchorder.hidden)\n> +\n> + iw = stack.repository.default_iw\n> + clean_iw = not options.keep and iw or None\n> + trans = transaction.StackTransaction(stack, 'sink',\n> + check_clean_iw = clean_iw)\n> +\n> + try:\n> + trans.reorder_patches(applied, unapplied, hidden, iw)\n\nHmm. We should maybe have a default value for hidden: the current list\nof patches. Not changing the hidden patches is a common operation.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"107898","messageId":"20090313024137.GE15393@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"20090312120918.2992.82713.stgit@pc1117.cambridge.arm.com","subject":"Re: [StGit PATCH 5/5] Convert \"float\" to the lib infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-13T02:41:37Z","receivedAt":"2009-03-13T02:41:37Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Nice job here too.\n\nOn 2009-03-12 12:09:18 +0000, Catalin Marinas wrote:\n\n> options = [\n> opt('-s', '--series', action = 'store_true',\n> - short = 'Rearrange according to a series file')]\n> + short = 'Rearrange according to a series file')\n> + ] + argparse.keep_option()\n\nThis flag should take the filename as a parameter, both because it's\nthe right thing to do and because it'll make the tab completion work\nright (as is, it'll complete on patch names after the -s flag).\n\nSomething like\n\n opt('-s', '--series', type = 'string')\n\nought to do it.\n\n> + applied = [p for p in stack.patchorder.applied if p not in patches] + \\\n> + patches\n> + unapplied = [p for p in stack.patchorder.unapplied if p not in patches]\n\nIt may be just me, but I find \"not p in patches\" more readable than \"p\nnot in patches\". Oh, and the backslash.\n\nFeel free to ignore, of course. :-)\n\n> + hidden = list(stack.patchorder.hidden)\n> +\n> + iw = stack.repository.default_iw\n> + clean_iw = not options.keep and iw or None\n> + trans = transaction.StackTransaction(stack, 'sink',\n> + check_clean_iw = clean_iw)\n> +\n> + try:\n> + trans.reorder_patches(applied, unapplied, hidden, iw)\n\nThat default value for hidden would've come in handy here too!\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"108087","messageId":"b0943d9e0903160756g79ac3464i4bb6f7f61ba0555a@mail.gmail.com","threadId":"18285","inReplyTo":"20090313015755.GA15393@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 1/5] Check for local changes with \"goto\"","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-03-16T14:56:11Z","receivedAt":"2009-03-16T14:56:11Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/3/13 Karl Hasselström :\n> On 2009-03-12 12:08:56 +0000, Catalin Marinas wrote:\n>> +    def __assert_index_worktree_clean(self, iw):\n>> +        if not iw.worktree_clean() or \\\n>> +           not iw.index.is_clean(self.stack.head):\n>> +            self.__halt('Repository not clean. Use \"refresh\" or '\n>> +                        '\"status --reset\"')\n>\n> \"Repository\" is misleading here. Maybe something like\n>\n>   ix_c = iw.index.is_clean(self.stack.head)\n>   wt_c = iw.worktree_clean()\n>   if not ix_c or not wt_c:\n>       self.__halt('%s not clean. Use \"refresh\" or \"status --reset\"'\n>                   % { (False, True): 'Index', (True, False): 'Worktree',\n>                       (False, False): 'Index and worktree' }[(ix_c, wt_c)])\n\nI added two separate if's as I don't find the above readable :-)\n\nif not iw.worktree_clean():\n self.__halt('Worktree not clean. Use \"refresh\" or \"status --reset\"')\nif not iw.index.is_clean(self.stack.head):\n self.__halt('Index not clean. Use \"refresh\" or \"status --reset\"')\ndef __checkout(self, tree, iw, allow_bad_head):\n\n-- \nCatalin\n"},{"id":"108088","messageId":"b0943d9e0903160803y25eb3f45y96d68e734b22e5f9@mail.gmail.com","threadId":"18285","inReplyTo":"20090313021751.GC15393@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 3/5] Add automatic git-mergetool invocation to the new infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-03-16T15:03:09Z","receivedAt":"2009-03-16T15:03:09Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/3/13 Karl Hasselström :\n> On 2009-03-12 12:09:07 +0000, Catalin Marinas wrote:\n>>                  # There were conflicts\n>> -                conflicts = [l for l in output if l.startswith('CONFLICT')]\n>> -                raise MergeConflictException(conflicts)\n>> +                if interactive:\n>> +                    self.mergetool()\n>> +                else:\n>> +                    conflicts = [l for l in output if l.startswith('CONFLICT')]\n>> +                    raise MergeConflictException(conflicts)\n>\n> Does the merge tool always resolve all conflicts? If it doesn't, the\n> two lines in the \"else\" branch should probably be run unconditionally.\n[...]\n>> +        # check for unmerged entries (prepend 'CONFLICT ' for consistency with\n>> +        # merge())\n>> +        conflicts = ['CONFLICT ' + f for f in self.index.conflicts()]\n>> +        if conflicts:\n>> +            raise MergeConflictException(conflicts)\n>> +        elif err:\n>> +            raise MergeException('\"git mergetool\" failed, exit code: %d' % err)\n>\n> Ah, you take care of conflicts here too. Hmm. I guess that's fine too,\n> though there is some code duplication. Maybe a helper function that\n> takes output as a parameter, and raises MergeConflictException if\n> necessary?\n\nThe non-interactive path assumes that there are conflicts if \"git\nmerge-recursive\" returned an error and it simply splits the output if\nthis command. The mergetool path has to run \"git ls-files --unmerged\"\nto check if there were any left conflicts. I wouldn't call \"git\nls-files\" in the first case as we already have the information.\n\n-- \nCatalin\n"},{"id":"108098","messageId":"b0943d9e0903160936m1971fbaft928a495eaab4fa20@mail.gmail.com","threadId":"18285","inReplyTo":"20090313024137.GE15393@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 5/5] Convert \"float\" to the lib infrastructure","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-03-16T16:36:43Z","receivedAt":"2009-03-16T16:36:43Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/3/13 Karl Hasselström :\n> On 2009-03-12 12:09:18 +0000, Catalin Marinas wrote:\n>>  options = [\n>>      opt('-s', '--series', action = 'store_true',\n>> -        short = 'Rearrange according to a series file')]\n>> +        short = 'Rearrange according to a series file')\n>> +    ] + argparse.keep_option()\n>\n> This flag should take the filename as a parameter, both because it's\n> the right thing to do and because it'll make the tab completion work\n> right (as is, it'll complete on patch names after the -s flag).\n>\n> Something like\n>\n>  opt('-s', '--series', type = 'string')\n>\n> ought to do it.\n\nThis command was accepting series via the stdin as well (maybe for\neasier use in other scripts or from stgit.el). Anyway, it doesn't seem\nto make any difference with the bash completion. It still tries to\ncomplete patches but when this fails bash lists files if the prefix\nmatches some.\n\n-- \nCatalin\n"},{"id":"108182","messageId":"20090317070654.GA3716@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"b0943d9e0903160756g79ac3464i4bb6f7f61ba0555a@mail.gmail.com","subject":"Re: [StGit PATCH 1/5] Check for local changes with \"goto\"","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-17T07:06:54Z","receivedAt":"2009-03-17T07:06:54Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-16 14:56:11 +0000, Catalin Marinas wrote:\n\n> 2009/3/13 Karl Hasselström :\n>\n> > \"Repository\" is misleading here. Maybe something like\n> >\n> >   ix_c = iw.index.is_clean(self.stack.head)\n> >   wt_c = iw.worktree_clean()\n> >   if not ix_c or not wt_c:\n> >       self.__halt('%s not clean. Use \"refresh\" or \"status --reset\"'\n> >                   % { (False, True): 'Index', (True, False): 'Worktree',\n> >                       (False, False): 'Index and worktree' }[(ix_c, wt_c)])\n>\n> I added two separate if's as I don't find the above readable :-)\n>\n> if not iw.worktree_clean():\n> self.__halt('Worktree not clean. Use \"refresh\" or \"status --reset\"')\n> if not iw.index.is_clean(self.stack.head):\n> self.__halt('Index not clean. Use \"refresh\" or \"status --reset\"')\n\nIt's actually quite nice once you get used to the idea of ephemeral\ndictionaries just for selection inside an expression ... :-)\n\nYour version doesn't generate the \"Your index and worktree are both\ndirty\" warning, but I guess that's OK.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"108184","messageId":"20090317071246.GB3716@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"b0943d9e0903160803y25eb3f45y96d68e734b22e5f9@mail.gmail.com","subject":"Re: [StGit PATCH 3/5] Add automatic git-mergetool invocation to the new infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-17T07:12:46Z","receivedAt":"2009-03-17T07:12:46Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-16 15:03:09 +0000, Catalin Marinas wrote:\n\n> 2009/3/13 Karl Hasselström :\n>\n> > On 2009-03-12 12:09:07 +0000, Catalin Marinas wrote:\n> >\n> > >                  # There were conflicts\n> > > -                conflicts = [l for l in output if l.startswith('CONFLICT')]\n> > > -                raise MergeConflictException(conflicts)\n> > > +                if interactive:\n> > > +                    self.mergetool()\n> > > +                else:\n> > > +                    conflicts = [l for l in output if l.startswith('CONFLICT')]\n> > > +                    raise MergeConflictException(conflicts)\n> >\n> > Does the merge tool always resolve all conflicts? If it doesn't,\n> > the two lines in the \"else\" branch should probably be run\n> > unconditionally.\n> [...]\n> > > +        # check for unmerged entries (prepend 'CONFLICT ' for consistency with\n> > > +        # merge())\n> > > +        conflicts = ['CONFLICT ' + f for f in self.index.conflicts()]\n> > > +        if conflicts:\n> > > +            raise MergeConflictException(conflicts)\n> > > +        elif err:\n> > > +            raise MergeException('\"git mergetool\" failed, exit code: %d' % err)\n> >\n> > Ah, you take care of conflicts here too. Hmm. I guess that's fine\n> > too, though there is some code duplication. Maybe a helper\n> > function that takes output as a parameter, and raises\n> > MergeConflictException if necessary?\n>\n> The non-interactive path assumes that there are conflicts if \"git\n> merge-recursive\" returned an error and it simply splits the output\n> if this command. The mergetool path has to run \"git ls-files\n> --unmerged\" to check if there were any left conflicts. I wouldn't\n> call \"git ls-files\" in the first case as we already have the\n> information.\n\nI was thinking of a function you'd call with either the output of the\nmerge operation or ls-files depending on the code path. But maybe it's\nnot worth it.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"108185","messageId":"20090317071645.GC3716@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"b0943d9e0903160936m1971fbaft928a495eaab4fa20@mail.gmail.com","subject":"Re: [StGit PATCH 5/5] Convert \"float\" to the lib infrastructure","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-17T07:16:45Z","receivedAt":"2009-03-17T07:16:45Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-16 16:36:43 +0000, Catalin Marinas wrote:\n\n> 2009/3/13 Karl Hasselström :\n>\n> > On 2009-03-12 12:09:18 +0000, Catalin Marinas wrote:\n> >\n> > >  options = [\n> > >      opt('-s', '--series', action = 'store_true',\n> > > -        short = 'Rearrange according to a series file')]\n> > > +        short = 'Rearrange according to a series file')\n> > > +    ] + argparse.keep_option()\n> >\n> > This flag should take the filename as a parameter, both because\n> > it's the right thing to do and because it'll make the tab\n> > completion work right (as is, it'll complete on patch names after\n> > the -s flag).\n> >\n> > Something like\n> >\n> >  opt('-s', '--series', type = 'string')\n> >\n> > ought to do it.\n>\n> This command was accepting series via the stdin as well (maybe for\n> easier use in other scripts or from stgit.el).\n\nAh. Hmm, I'd prefer if it used \"-\" for that, for consistency. And\nbecause I don't know how to make flags with optional parameters. :-/\n\n> Anyway, it doesn't seem to make any difference with the bash\n> completion. It still tries to complete patches but when this fails\n> bash lists files if the prefix matches some.\n\nHmm, that's not right. But I can have a look at it if you like.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"108213","messageId":"b0943d9e0903170351p319bad36icf8252610e5c4f05@mail.gmail.com","threadId":"18285","inReplyTo":"20090317070654.GA3716@diana.vm.bytemark.co.uk","subject":"Re: [StGit PATCH 1/5] Check for local changes with \"goto\"","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2009-03-17T10:51:08Z","receivedAt":"2009-03-17T10:51:08Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"2009/3/17 Karl Hasselström :\n> On 2009-03-16 14:56:11 +0000, Catalin Marinas wrote:\n>> if not iw.worktree_clean():\n>>     self.__halt('Worktree not clean. Use \"refresh\" or \"status --reset\"')\n>> if not iw.index.is_clean(self.stack.head):\n>>     self.__halt('Index not clean. Use \"refresh\" or \"status --reset\"')\n[...]\n> Your version doesn't generate the \"Your index and worktree are both\n> dirty\" warning, but I guess that's OK.\n\nThe iw.worktree_clean() only checks whether the worktree is clean\nrelative to the index (I just tried \"git update-index --refresh\" after\n\"git add \" and it returns 0).\n\n-- \nCatalin\n"},{"id":"108225","messageId":"20090317133612.GA10001@diana.vm.bytemark.co.uk","threadId":"18285","inReplyTo":"b0943d9e0903170351p319bad36icf8252610e5c4f05@mail.gmail.com","subject":"Re: [StGit PATCH 1/5] Check for local changes with \"goto\"","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2009-03-17T13:36:12Z","receivedAt":"2009-03-17T13:36:12Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2009-03-17 10:51:08 +0000, Catalin Marinas wrote:\n\n> 2009/3/17 Karl Hasselström :\n>\n> > On 2009-03-16 14:56:11 +0000, Catalin Marinas wrote:\n> >\n> > > if not iw.worktree_clean():\n> > >     self.__halt('Worktree not clean. Use \"refresh\" or \"status --reset\"')\n> > > if not iw.index.is_clean(self.stack.head):\n> > >     self.__halt('Index not clean. Use \"refresh\" or \"status --reset\"')\n> [...]\n> > Your version doesn't generate the \"Your index and worktree are\n> > both dirty\" warning, but I guess that's OK.\n>\n> The iw.worktree_clean() only checks whether the worktree is clean\n> relative to the index (I just tried \"git update-index --refresh\"\n> after \"git add \" and it returns 0).\n\nYes, I know. The point I was trying to make was that your code doesn't\nmake a difference between\n\n (iw.worktree_clean(), iw.index.is_clean(self.stack.head)) == (False, True)\n\nand\n\n (iw.worktree_clean(), iw.index.is_clean(self.stack.head)) == (False, False)\n\nBut as I said, it's not really important.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"}]}