{"thread":{"id":"9660","subject":"[StGIT PATCH 1/4] Refactor output handling to break circular dependency","startedAt":"2007-08-26T20:33:21Z","lastAt":"2007-09-03T21:48:51Z","messageCount":14,"participants":["Karl Hasselström","Catalin Marinas"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"51590","messageId":"20070826202724.16265.85821.stgit@yoghurt","threadId":"9660","inReplyTo":null,"subject":"[StGIT PATCH 0/4] Clean up subprocess calling","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:33:21Z","receivedAt":"2007-08-26T20:33:21Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"We had nearly ten separate functions implementing about five different\nways of calling a subprocess. This series cleans it up. The aggregate\ndiffstat looks scary, but that's mostly because these functions were\ncalled in a bajillion different places.\n\nIf you choose to pull from my repository (which I highly recommend!),\nnote that this series is built on top of the misc fixes, so you get\nthose as well.\n\nThe following changes since commit 216a1524c4acbd9952ffaeec054e30cf14dde5fc:\n Karl Hasselström (1):\n Compile regexp just once\n\nare available in the git repository at:\n\n git://repo.or.cz/stgit/kha.git run\n\nKarl Hasselström (4):\n Refactor output handling to break circular dependency\n Refactor subprocess creation\n Assert that the argument to Run is a sequence of strings\n Add optional logging of subprocess execution\n\n stgit/basedir.py | 13 +-\n stgit/commands/applied.py | 1 +\n stgit/commands/assimilate.py | 1 +\n stgit/commands/branch.py | 1 +\n stgit/commands/clean.py | 1 +\n stgit/commands/commit.py | 1 +\n stgit/commands/common.py | 1 +\n stgit/commands/delete.py | 1 +\n stgit/commands/diff.py | 1 +\n stgit/commands/export.py | 10 +-\n stgit/commands/files.py | 1 +\n stgit/commands/fold.py | 1 +\n stgit/commands/hide.py | 1 +\n stgit/commands/id.py | 1 +\n stgit/commands/imprt.py | 1 +\n stgit/commands/log.py | 1 +\n stgit/commands/mail.py | 1 +\n stgit/commands/patches.py | 1 +\n stgit/commands/pick.py | 1 +\n stgit/commands/pull.py | 1 +\n stgit/commands/push.py | 1 +\n stgit/commands/refresh.py | 1 +\n stgit/commands/rename.py | 1 +\n stgit/commands/series.py | 1 +\n stgit/commands/sync.py | 1 +\n stgit/commands/top.py | 1 +\n stgit/commands/unapplied.py | 1 +\n stgit/commands/uncommit.py | 1 +\n stgit/commands/unhide.py | 1 +\n stgit/config.py | 56 ++-----\n stgit/git.py | 331 +++++++++++++++++-------------------------\n stgit/gitmergeonefile.py | 21 +--\n stgit/main.py | 2 +-\n stgit/out.py | 100 +++++++++++++\n stgit/run.py | 141 ++++++++++++++++++\n stgit/stack.py | 1 +\n stgit/utils.py | 81 +----------\n 37 files changed, 436 insertions(+), 347 deletions(-)\n create mode 100644 stgit/out.py\n create mode 100644 stgit/run.py\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"51589","messageId":"20070826203327.16265.62636.stgit@yoghurt","threadId":"9660","inReplyTo":"20070826202724.16265.85821.stgit@yoghurt","subject":"[StGIT PATCH 1/4] Refactor output handling to break circular dependency","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:33:27Z","receivedAt":"2007-08-26T20:33:27Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"I ran into problems when trying to use the fancy output handling in a\nnew module that was (recursively) imported by stgit.util -- I couldn't\nuse the out object because it wasn't defined yet.\n\nBreak out the output handler to its own module to break the circular\ndependency.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/commands/applied.py | 1 \n stgit/commands/assimilate.py | 1 \n stgit/commands/branch.py | 1 \n stgit/commands/clean.py | 1 \n stgit/commands/commit.py | 1 \n stgit/commands/common.py | 1 \n stgit/commands/delete.py | 1 \n stgit/commands/diff.py | 1 \n stgit/commands/export.py | 1 \n stgit/commands/files.py | 1 \n stgit/commands/fold.py | 1 \n stgit/commands/hide.py | 1 \n stgit/commands/id.py | 1 \n stgit/commands/imprt.py | 1 \n stgit/commands/log.py | 1 \n stgit/commands/mail.py | 1 \n stgit/commands/patches.py | 1 \n stgit/commands/pick.py | 1 \n stgit/commands/pull.py | 1 \n stgit/commands/push.py | 1 \n stgit/commands/refresh.py | 1 \n stgit/commands/rename.py | 1 \n stgit/commands/series.py | 1 \n stgit/commands/sync.py | 1 \n stgit/commands/top.py | 1 \n stgit/commands/unapplied.py | 1 \n stgit/commands/uncommit.py | 1 \n stgit/commands/unhide.py | 1 \n stgit/git.py | 1 \n stgit/gitmergeonefile.py | 3 +\n stgit/main.py | 2 -\n stgit/out.py | 100 ++++++++++++++++++++++++++++++++++++++++++\n stgit/stack.py | 1 \n stgit/utils.py | 81 ----------------------------------\n 34 files changed, 134 insertions(+), 82 deletions(-)\n\ndiff --git a/stgit/commands/applied.py b/stgit/commands/applied.py\nindex 0925de0..b9bb716 100644\n--- a/stgit/commands/applied.py\n+++ b/stgit/commands/applied.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/assimilate.py b/stgit/commands/assimilate.py\nindex 5134e34..c5f4340 100644\n--- a/stgit/commands/assimilate.py\n+++ b/stgit/commands/assimilate.py\n@@ -22,6 +22,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n help = 'StGIT-ify any GIT commits made on top of your StGIT stack'\ndiff --git a/stgit/commands/branch.py b/stgit/commands/branch.py\nindex 75a9046..2d491d5 100644\n--- a/stgit/commands/branch.py\n+++ b/stgit/commands/branch.py\n@@ -23,6 +23,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git, basedir\n \n \ndiff --git a/stgit/commands/clean.py b/stgit/commands/clean.py\nindex 7b57526..2e3b202 100644\n--- a/stgit/commands/clean.py\n+++ b/stgit/commands/clean.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/commit.py b/stgit/commands/commit.py\nindex 2b8d7ce..0b76c56 100644\n--- a/stgit/commands/commit.py\n+++ b/stgit/commands/commit.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n help = 'permanently store the applied patches into stack base'\ndiff --git a/stgit/commands/common.py b/stgit/commands/common.py\nindex dddee85..f3fa89d 100644\n--- a/stgit/commands/common.py\n+++ b/stgit/commands/common.py\n@@ -22,6 +22,7 @@ import sys, os, os.path, re\n from optparse import OptionParser, make_option\n \n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git, basedir\n from stgit.config import config, file_extensions\n \ndiff --git a/stgit/commands/delete.py b/stgit/commands/delete.py\nindex a9e2744..2121015 100644\n--- a/stgit/commands/delete.py\n+++ b/stgit/commands/delete.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/diff.py b/stgit/commands/diff.py\nindex f8b19f8..aeca4ab 100644\n--- a/stgit/commands/diff.py\n+++ b/stgit/commands/diff.py\n@@ -22,6 +22,7 @@ from pydoc import pager\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/export.py b/stgit/commands/export.py\nindex 8424f9d..5ca07d3 100644\n--- a/stgit/commands/export.py\n+++ b/stgit/commands/export.py\n@@ -23,6 +23,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git, templates\n \n \ndiff --git a/stgit/commands/files.py b/stgit/commands/files.py\nindex 659a82b..1a10023 100644\n--- a/stgit/commands/files.py\n+++ b/stgit/commands/files.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/fold.py b/stgit/commands/fold.py\nindex 297dfbf..d97185e 100644\n--- a/stgit/commands/fold.py\n+++ b/stgit/commands/fold.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/hide.py b/stgit/commands/hide.py\nindex 39cbd67..de19c19 100644\n--- a/stgit/commands/hide.py\n+++ b/stgit/commands/hide.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/id.py b/stgit/commands/id.py\nindex 8c717e8..f72d2f3 100644\n--- a/stgit/commands/id.py\n+++ b/stgit/commands/id.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py\nindex 98fe708..57bf2c8 100644\n--- a/stgit/commands/imprt.py\n+++ b/stgit/commands/imprt.py\n@@ -23,6 +23,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/log.py b/stgit/commands/log.py\nindex ebd8cab..4d6b022 100644\n--- a/stgit/commands/log.py\n+++ b/stgit/commands/log.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n from pydoc import pager\n from stgit.commands.common import *\n from stgit import stack, git\n+from stgit.out import *\n \n help = 'display the patch changelog'\n usage = \"\"\"%prog [options] [patch]\ndiff --git a/stgit/commands/mail.py b/stgit/commands/mail.py\nindex 71a6e4e..6202fc5 100644\n--- a/stgit/commands/mail.py\n+++ b/stgit/commands/mail.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git, version, templates\n from stgit.config import config\n \ndiff --git a/stgit/commands/patches.py b/stgit/commands/patches.py\nindex 23b3aa7..b3defb6 100644\n--- a/stgit/commands/patches.py\n+++ b/stgit/commands/patches.py\n@@ -21,6 +21,7 @@ from pydoc import pager\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/pick.py b/stgit/commands/pick.py\nindex 27fdf9c..1c3ef11 100644\n--- a/stgit/commands/pick.py\n+++ b/stgit/commands/pick.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n from stgit.stack import Series\n \ndiff --git a/stgit/commands/pull.py b/stgit/commands/pull.py\nindex fe3b67d..ad485a5 100644\n--- a/stgit/commands/pull.py\n+++ b/stgit/commands/pull.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit.config import GitConfigException\n from stgit import stack, git\n \ndiff --git a/stgit/commands/push.py b/stgit/commands/push.py\nindex 17b32f6..53cdb8f 100644\n--- a/stgit/commands/push.py\n+++ b/stgit/commands/push.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/refresh.py b/stgit/commands/refresh.py\nindex 8277388..218075b 100644\n--- a/stgit/commands/refresh.py\n+++ b/stgit/commands/refresh.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n from stgit.config import config\n \ndiff --git a/stgit/commands/rename.py b/stgit/commands/rename.py\nindex d6c53be..2830e72 100644\n--- a/stgit/commands/rename.py\n+++ b/stgit/commands/rename.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/series.py b/stgit/commands/series.py\nindex 9e0b0ff..00a3372 100644\n--- a/stgit/commands/series.py\n+++ b/stgit/commands/series.py\n@@ -22,6 +22,7 @@ from optparse import OptionParser, make_option\n import stgit.commands.common\n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/sync.py b/stgit/commands/sync.py\nindex 5e33324..580b5bd 100644\n--- a/stgit/commands/sync.py\n+++ b/stgit/commands/sync.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n import stgit.commands.common\n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/top.py b/stgit/commands/top.py\nindex 7cc92ca..1a9267a 100644\n--- a/stgit/commands/top.py\n+++ b/stgit/commands/top.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/unapplied.py b/stgit/commands/unapplied.py\nindex 0d330a1..c6408a3 100644\n--- a/stgit/commands/unapplied.py\n+++ b/stgit/commands/unapplied.py\n@@ -21,6 +21,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/commands/uncommit.py b/stgit/commands/uncommit.py\nindex f611d29..e3bf0d8 100644\n--- a/stgit/commands/uncommit.py\n+++ b/stgit/commands/uncommit.py\n@@ -22,6 +22,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n help = 'turn regular GIT commits into StGIT patches'\ndiff --git a/stgit/commands/unhide.py b/stgit/commands/unhide.py\nindex b6a2297..0a1dcaf 100644\n--- a/stgit/commands/unhide.py\n+++ b/stgit/commands/unhide.py\n@@ -20,6 +20,7 @@ from optparse import OptionParser, make_option\n \n from stgit.commands.common import *\n from stgit.utils import *\n+from stgit.out import *\n from stgit import stack, git\n \n \ndiff --git a/stgit/git.py b/stgit/git.py\nindex 14b4c81..827bd61 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -23,6 +23,7 @@ from shutil import copyfile\n \n from stgit import basedir\n from stgit.utils import *\n+from stgit.out import *\n from stgit.config import config\n from sets import Set\n \ndiff --git a/stgit/gitmergeonefile.py b/stgit/gitmergeonefile.py\nindex 5e51b8a..e76f9b1 100644\n--- a/stgit/gitmergeonefile.py\n+++ b/stgit/gitmergeonefile.py\n@@ -21,7 +21,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n import sys, os\n from stgit import basedir\n from stgit.config import config, file_extensions, ConfigOption\n-from stgit.utils import append_string, out\n+from stgit.utils import append_string\n+from stgit.out import *\n \n \n class GitMergeException(Exception):\ndiff --git a/stgit/main.py b/stgit/main.py\nindex 2390110..5b9d7c4 100644\n--- a/stgit/main.py\n+++ b/stgit/main.py\n@@ -22,7 +22,7 @@ import sys, os\n from optparse import OptionParser\n \n import stgit.commands\n-from stgit.utils import out\n+from stgit.out import *\n \n #\n # The commands map\ndiff --git a/stgit/out.py b/stgit/out.py\nnew file mode 100644\nindex 0000000..f80daf2\n--- /dev/null\n+++ b/stgit/out.py\n@@ -0,0 +1,100 @@\n+# -*- coding: utf-8 -*-\n+\n+__copyright__ = \"\"\"\n+Copyright (C) 2007, Karl Hasselström \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\n+\n+class MessagePrinter(object):\n+ def __init__(self):\n+ class Output(object):\n+ def __init__(self, write, flush):\n+ self.write = write\n+ self.flush = flush\n+ self.at_start_of_line = True\n+ self.level = 0\n+ def new_line(self):\n+ \"\"\"Ensure that we're at the beginning of a line.\"\"\"\n+ if not self.at_start_of_line:\n+ self.write('\\n')\n+ self.at_start_of_line = True\n+ def single_line(self, msg, print_newline = True,\n+ need_newline = True):\n+ \"\"\"Write a single line. Newline before and after are\n+ separately configurable.\"\"\"\n+ if need_newline:\n+ self.new_line()\n+ if self.at_start_of_line:\n+ self.write(' '*self.level)\n+ self.write(msg)\n+ if print_newline:\n+ self.write('\\n')\n+ self.at_start_of_line = True\n+ else:\n+ self.flush()\n+ self.at_start_of_line = False\n+ def tagged_lines(self, tag, lines):\n+ tag += ': '\n+ for line in lines:\n+ self.single_line(tag + line)\n+ tag = ' '*len(tag)\n+ def write_line(self, line):\n+ \"\"\"Write one line of text on a lines of its own, not\n+ indented.\"\"\"\n+ self.new_line()\n+ self.write('%s\\n' % line)\n+ self.at_start_of_line = True\n+ def write_raw(self, string):\n+ \"\"\"Write an arbitrary string, possibly containing\n+ newlines.\"\"\"\n+ self.new_line()\n+ self.write(string)\n+ self.at_start_of_line = string.endswith('\\n')\n+ self.__stdout = Output(sys.stdout.write, sys.stdout.flush)\n+ if sys.stdout.isatty():\n+ self.__out = self.__stdout\n+ else:\n+ self.__out = Output(lambda msg: None, lambda: None)\n+ def stdout(self, line):\n+ \"\"\"Write a line to stdout.\"\"\"\n+ self.__stdout.write_line(line)\n+ def stdout_raw(self, string):\n+ \"\"\"Write a string possibly containing newlines to stdout.\"\"\"\n+ self.__stdout.write_raw(string)\n+ def info(self, *msgs):\n+ for msg in msgs:\n+ self.__out.single_line(msg)\n+ def note(self, *msgs):\n+ self.__out.tagged_lines('Notice', msgs)\n+ def warn(self, *msgs):\n+ self.__out.tagged_lines('Warning', msgs)\n+ def error(self, *msgs):\n+ self.__out.tagged_lines('Error', msgs)\n+ def start(self, msg):\n+ \"\"\"Start a long-running operation.\"\"\"\n+ self.__out.single_line('%s ... ' % msg, print_newline = False)\n+ self.__out.level += 1\n+ def done(self, extramsg = None):\n+ \"\"\"Finish long-running operation.\"\"\"\n+ self.__out.level -= 1\n+ if extramsg:\n+ msg = 'done (%s)' % extramsg\n+ else:\n+ msg = 'done'\n+ self.__out.single_line(msg, need_newline = False)\n+\n+out = MessagePrinter()\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex 1ab10c5..12c5091 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -21,6 +21,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n import sys, os, re\n \n from stgit.utils import *\n+from stgit.out import *\n from stgit import git, basedir, templates\n from stgit.config import config\n from shutil import copyfile\ndiff --git a/stgit/utils.py b/stgit/utils.py\nindex 039b433..38dd474 100644\n--- a/stgit/utils.py\n+++ b/stgit/utils.py\n@@ -3,6 +3,7 @@\n \n import errno, os, os.path, re, sys\n from stgit.config import config\n+from stgit.out import *\n \n __copyright__ = \"\"\"\n Copyright (C) 2005, Catalin Marinas \n@@ -21,86 +22,6 @@ 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-class MessagePrinter(object):\n- def __init__(self):\n- class Output(object):\n- def __init__(self, write, flush):\n- self.write = write\n- self.flush = flush\n- self.at_start_of_line = True\n- self.level = 0\n- def new_line(self):\n- \"\"\"Ensure that we're at the beginning of a line.\"\"\"\n- if not self.at_start_of_line:\n- self.write('\\n')\n- self.at_start_of_line = True\n- def single_line(self, msg, print_newline = True,\n- need_newline = True):\n- \"\"\"Write a single line. Newline before and after are\n- separately configurable.\"\"\"\n- if need_newline:\n- self.new_line()\n- if self.at_start_of_line:\n- self.write(' '*self.level)\n- self.write(msg)\n- if print_newline:\n- self.write('\\n')\n- self.at_start_of_line = True\n- else:\n- self.flush()\n- self.at_start_of_line = False\n- def tagged_lines(self, tag, lines):\n- tag += ': '\n- for line in lines:\n- self.single_line(tag + line)\n- tag = ' '*len(tag)\n- def write_line(self, line):\n- \"\"\"Write one line of text on a lines of its own, not\n- indented.\"\"\"\n- self.new_line()\n- self.write('%s\\n' % line)\n- self.at_start_of_line = True\n- def write_raw(self, string):\n- \"\"\"Write an arbitrary string, possibly containing\n- newlines.\"\"\"\n- self.new_line()\n- self.write(string)\n- self.at_start_of_line = string.endswith('\\n')\n- self.__stdout = Output(sys.stdout.write, sys.stdout.flush)\n- if sys.stdout.isatty():\n- self.__out = self.__stdout\n- else:\n- self.__out = Output(lambda msg: None, lambda: None)\n- def stdout(self, line):\n- \"\"\"Write a line to stdout.\"\"\"\n- self.__stdout.write_line(line)\n- def stdout_raw(self, string):\n- \"\"\"Write a string possibly containing newlines to stdout.\"\"\"\n- self.__stdout.write_raw(string)\n- def info(self, *msgs):\n- for msg in msgs:\n- self.__out.single_line(msg)\n- def note(self, *msgs):\n- self.__out.tagged_lines('Notice', msgs)\n- def warn(self, *msgs):\n- self.__out.tagged_lines('Warning', msgs)\n- def error(self, *msgs):\n- self.__out.tagged_lines('Error', msgs)\n- def start(self, msg):\n- \"\"\"Start a long-running operation.\"\"\"\n- self.__out.single_line('%s ... ' % msg, print_newline = False)\n- self.__out.level += 1\n- def done(self, extramsg = None):\n- \"\"\"Finish long-running operation.\"\"\"\n- self.__out.level -= 1\n- if extramsg:\n- msg = 'done (%s)' % extramsg\n- else:\n- msg = 'done'\n- self.__out.single_line(msg, need_newline = False)\n-\n-out = MessagePrinter()\n-\n def mkdir_file(filename, mode):\n \"\"\"Opens filename with the given mode, creating the directory it's\n in if it doesn't already exist.\"\"\"\n"},{"id":"51593","messageId":"20070826203333.16265.63019.stgit@yoghurt","threadId":"9660","inReplyTo":"20070826202724.16265.85821.stgit@yoghurt","subject":"[StGIT PATCH 2/4] Refactor subprocess creation","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:33:33Z","receivedAt":"2007-08-26T20:33:33Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Instead of having a gazillion different little functions that wrap\nos.spawnvp and popen2.Popen3, make a single wrapper that's flexible\nenough to cater to everyone. Apart from making the code cleaner and\neasier to read, this has the added benefit of making it possible to\nadd little things such as extra safety checks and logging.\n\nIdeally, I'd have liked this wrapper to wrap subprocess.Popen, but\nthat's only available in Python 2.4 and up, so we'll have to make do\nwith os.spawnvp and popen2.Popen3 for now, even though they suck.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/basedir.py | 13 +-\n stgit/commands/export.py | 9 -\n stgit/config.py | 56 ++------\n stgit/git.py | 330 ++++++++++++++++++----------------------------\n stgit/gitmergeonefile.py | 18 +--\n stgit/run.py | 111 +++++++++++++++\n 6 files changed, 272 insertions(+), 265 deletions(-)\n\ndiff --git a/stgit/basedir.py b/stgit/basedir.py\nindex 81f2b40..30f45f9 100644\n--- a/stgit/basedir.py\n+++ b/stgit/basedir.py\n@@ -19,13 +19,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n \"\"\"\n \n import os\n-\n-def __output(cmd):\n- f = os.popen(cmd, 'r')\n- string = f.readline().rstrip()\n- if f.close():\n- return ''\n- return string\n+from stgit.run import *\n \n # GIT_DIR value cached\n __base_dir = None\n@@ -39,7 +33,10 @@ def get():\n if 'GIT_DIR' in os.environ:\n __base_dir = os.environ['GIT_DIR']\n else:\n- __base_dir = __output('git-rev-parse --git-dir 2> /dev/null')\n+ try:\n+ __base_dir = Run('git-rev-parse', '--git-dir').output_one_line()\n+ except RunException:\n+ __base_dir = ''\n \n return __base_dir\n \ndiff --git a/stgit/commands/export.py b/stgit/commands/export.py\nindex 5ca07d3..62be394 100644\n--- a/stgit/commands/export.py\n+++ b/stgit/commands/export.py\n@@ -175,13 +175,10 @@ def func(parser, options, args):\n print patch.get_name()\n print '-'*79\n \n- # write description\n f.write(descr)\n- # write the diff\n- git.diff(rev1 = patch.get_bottom(),\n- rev2 = patch.get_top(),\n- out_fd = f,\n- diff_flags = diff_flags )\n+ f.write(git.diff(rev1 = patch.get_bottom(),\n+ rev2 = patch.get_top(),\n+ diff_flags = diff_flags))\n if not options.stdout:\n f.close()\n patch_no += 1\ndiff --git a/stgit/config.py b/stgit/config.py\nindex 2fd1273..799e1d7 100644\n--- a/stgit/config.py\n+++ b/stgit/config.py\n@@ -20,6 +20,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n \n import os, re\n from stgit import basedir\n+from stgit.run import *\n \n class GitConfigException(Exception):\n pass\n@@ -43,50 +44,21 @@ class GitConfig:\n \n __cache={}\n \n- def __run(self, cmd, args=None):\n- \"\"\"__run: runs cmd using spawnvp.\n- \n- Runs cmd using spawnvp. The shell is avoided so it won't mess up\n- our arguments. If args is very large, the command is run multiple\n- times; args is split xargs style: cmd is passed on each\n- invocation. Unlike xargs, returns immediately if any non-zero\n- return code is received. \n- \"\"\"\n- \n- args_l=cmd.split()\n- if args is None:\n- args = []\n- for i in range(0, len(args)+1, 100):\n- r=os.spawnvp(os.P_WAIT, args_l[0], args_l + args[i:min(i+100, len(args))])\n- if r:\n- return r\n- return 0\n- \n def get(self, name):\n if self.__cache.has_key(name):\n return self.__cache[name]\n-\n- stream = os.popen('git repo-config --get %s' % name, 'r')\n- value = stream.readline().strip()\n- stream.close()\n- if len(value) > 0:\n- pass\n- elif (self.__defaults.has_key(name)):\n- value = self.__defaults[name]\n- else:\n- value = None\n-\n+ try:\n+ value = Run('git-repo-config', '--get', name).output_one_line()\n+ except RunException:\n+ value = self.__defaults.get(name, None)\n self.__cache[name] = value\n return value\n \n def getall(self, name):\n if self.__cache.has_key(name):\n return self.__cache[name]\n-\n- stream = os.popen('git repo-config --get-all %s' % name, 'r')\n- values = [line.strip() for line in stream]\n- stream.close()\n-\n+ values = Run('git-repo-config', '--get-all', name\n+ ).returns([0, 1]).output_lines()\n self.__cache[name] = values\n return values\n \n@@ -98,15 +70,18 @@ class GitConfig:\n raise GitConfigException, 'Value for \"%s\" is not an integer: \"%s\"' % (name, value)\n \n def rename_section(self, from_name, to_name):\n- self.__run('git-repo-config --rename-section', [from_name, to_name])\n+ \"\"\"Rename a section in the config file. Silently do nothing if\n+ the section doesn't exist.\"\"\"\n+ Run('git-repo-config', '--rename-section', from_name, to_name\n+ ).returns([0, 1]).run()\n self.__cache.clear()\n \n def set(self, name, value):\n- self.__run('git-repo-config', [name, value])\n+ Run('git-repo-config', name, value).run()\n self.__cache[name] = value\n \n def unset(self, name):\n- self.__run('git-repo-config --unset', [name])\n+ Run('git-repo-config', '--unset', name)\n self.__cache[name] = None\n \n def sections_matching(self, regexp):\n@@ -115,12 +90,11 @@ class GitConfig:\n group contents, for all variable names matching the regexp.\n \"\"\"\n result = []\n- stream = os.popen('git repo-config --get-regexp \"^%s$\"' % regexp, 'r')\n- for line in stream:\n+ for line in Run('git-repo-config', '--get-regexp', '\"^%s$\"' % regexp\n+ ).returns([0, 1]).output_lines():\n m = re.match('^%s ' % regexp, line)\n if m:\n result.append(m.group(1))\n- stream.close()\n return result\n \n config=GitConfig()\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 827bd61..7962cdb 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -18,12 +18,13 @@ 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, popen2, re, gitmergeonefile\n+import sys, os, re, gitmergeonefile\n from shutil import copyfile\n \n from stgit import basedir\n from stgit.utils import *\n from stgit.out import *\n+from stgit.run import *\n from stgit.config import config\n from sets import Set\n \n@@ -31,6 +32,12 @@ from sets import Set\n class GitException(Exception):\n pass\n \n+# When a subprocess has a problem, we want the exception to be a\n+# subclass of GitException.\n+class GitRunException(GitException):\n+ pass\n+class GRun(Run):\n+ exc = GitRunException\n \n \n #\n@@ -75,19 +82,21 @@ class Commit:\n def __init__(self, id_hash):\n self.__id_hash = id_hash\n \n- lines = _output_lines(['git-cat-file', 'commit', id_hash])\n+ lines = GRun('git-cat-file', 'commit', id_hash).output_lines()\n for i in range(len(lines)):\n line = lines[i]\n- if line == '\\n':\n- break\n- field = line.strip().split(' ', 1)\n- if field[0] == 'tree':\n- self.__tree = field[1]\n- if field[0] == 'author':\n- self.__author = field[1]\n- if field[0] == 'committer':\n- self.__committer = field[1]\n- self.__log = ''.join(lines[i+1:])\n+ if not line:\n+ break # we've seen all the header fields\n+ key, val = line.split(' ', 1)\n+ if key == 'tree':\n+ self.__tree = val\n+ elif key == 'author':\n+ self.__author = val\n+ elif key == 'committer':\n+ self.__committer = val\n+ else:\n+ pass # ignore other headers\n+ self.__log = '\\n'.join(lines[i+1:])\n \n def get_id_hash(self):\n return self.__id_hash\n@@ -103,8 +112,8 @@ class Commit:\n return None\n \n def get_parents(self):\n- return _output_lines(['git-rev-list', '--parents', '--max-count=1',\n- self.__id_hash])[0].split()[1:]\n+ return GRun('git-rev-list', '--parents', '--max-count=1', self.__id_hash\n+ ).output_one_line().split()[1:]\n \n def get_author(self):\n return self.__author\n@@ -150,73 +159,6 @@ def get_conflicts():\n else:\n return None\n \n-def _input(cmd, file_desc):\n- p = popen2.Popen3(cmd, True)\n- while True:\n- line = file_desc.readline()\n- if not line:\n- break\n- p.tochild.write(line)\n- p.tochild.close()\n- if p.wait():\n- raise GitException, '%s failed (%s)' % (' '.join(cmd),\n- p.childerr.read().strip())\n-\n-def _input_str(cmd, string):\n- p = popen2.Popen3(cmd, True)\n- p.tochild.write(string)\n- p.tochild.close()\n- if p.wait():\n- raise GitException, '%s failed (%s)' % (' '.join(cmd),\n- p.childerr.read().strip())\n-\n-def _output(cmd):\n- p=popen2.Popen3(cmd, True)\n- output = p.fromchild.read()\n- if p.wait():\n- raise GitException, '%s failed (%s)' % (' '.join(cmd),\n- p.childerr.read().strip())\n- return output\n-\n-def _output_one_line(cmd, file_desc = None):\n- p=popen2.Popen3(cmd, True)\n- if file_desc != None:\n- for line in file_desc:\n- p.tochild.write(line)\n- p.tochild.close()\n- output = p.fromchild.readline().strip()\n- if p.wait():\n- raise GitException, '%s failed (%s)' % (' '.join(cmd),\n- p.childerr.read().strip())\n- return output\n-\n-def _output_lines(cmd):\n- p=popen2.Popen3(cmd, True)\n- lines = p.fromchild.readlines()\n- if p.wait():\n- raise GitException, '%s failed (%s)' % (' '.join(cmd),\n- p.childerr.read().strip())\n- return lines\n-\n-def __run(cmd, args=None):\n- \"\"\"__run: runs cmd using spawnvp.\n-\n- Runs cmd using spawnvp. The shell is avoided so it won't mess up\n- our arguments. If args is very large, the command is run multiple\n- times; args is split xargs style: cmd is passed on each\n- invocation. Unlike xargs, returns immediately if any non-zero\n- return code is received. \n- \"\"\"\n- \n- args_l=cmd.split()\n- if args is None:\n- args = []\n- for i in range(0, len(args)+1, 100):\n- r=os.spawnvp(os.P_WAIT, args_l[0], args_l + args[i:min(i+100, len(args))])\n- if r:\n- return r\n- return 0\n-\n def exclude_files():\n files = [os.path.join(basedir.get(), 'info', 'exclude')]\n user_exclude = config.get('core.excludesfile')\n@@ -247,9 +189,9 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,\n + ['--exclude-per-directory=.gitignore']\n + ['--exclude-from=%s' % fn for fn in exclude_files()\n if os.path.exists(fn)])\n- lines = _output_lines(['git-ls-files', '--others', '--directory']\n- + exclude)\n- cache_files += [('?', line.strip()) for line in lines]\n+ lines = GRun('git-ls-files', '--others', '--directory', *exclude\n+ ).output_lines()\n+ cache_files += [('?', line) for line in lines]\n \n # conflicted files\n conflicts = get_conflicts()\n@@ -258,8 +200,8 @@ def tree_status(files = None, tree_id = 'HEAD', unknown = False,\n cache_files += [('C', filename) for filename in conflicts]\n \n # the rest\n- for line in _output_lines(['git-diff-index'] + diff_flags +\n- [ tree_id, '--'] + files):\n+ for line in GRun('git-diff-index', *(diff_flags + [tree_id, '--'] + files)\n+ ).output_lines():\n fs = tuple(line.rstrip().split(' ',4)[-1].split('\\t',1))\n if fs[1] not in conflicts:\n cache_files.append(fs)\n@@ -277,7 +219,7 @@ def local_changes(verbose = True):\n def get_heads():\n heads = []\n hr = re.compile(r'^[0-9a-f]{40} refs/heads/(.+)$')\n- for line in _output_lines(['git-show-ref', '--heads']):\n+ for line in GRun('git-show-ref', '--heads').output_lines():\n m = hr.match(line)\n heads.append(m.group(1))\n return heads\n@@ -298,7 +240,7 @@ def get_head_file():\n \"\"\"Returns the name of the file pointed to by the HEAD link\n \"\"\"\n return strip_prefix('refs/heads/',\n- _output_one_line(['git-symbolic-ref', 'HEAD']))\n+ GRun('git-symbolic-ref', 'HEAD').output_one_line())\n \n def set_head_file(ref):\n \"\"\"Resets HEAD to point to a new ref\n@@ -306,12 +248,16 @@ def set_head_file(ref):\n # head cache flushing is needed since we might have a different value\n # in the new head\n __clear_head_cache()\n- if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:\n+ try:\n+ GRun('git-symbolic-ref', 'HEAD', 'refs/heads/%s' % ref).run()\n+ except GitRunException:\n raise GitException, 'Could not set head to \"%s\"' % ref\n \n def set_ref(ref, val):\n \"\"\"Point ref at a new commit object.\"\"\"\n- if __run('git-update-ref', [ref, val]) != 0:\n+ try:\n+ GRun('git-update-ref', ref, val).run()\n+ except GitRunException:\n raise GitException, 'Could not update %s to \"%s\".' % (ref, val)\n \n def set_branch(branch, val):\n@@ -339,14 +285,14 @@ def __clear_head_cache():\n def refresh_index():\n \"\"\"Refresh index with stat() information from the working directory.\n \"\"\"\n- __run('git-update-index -q --unmerged --refresh')\n+ GRun('git-update-index', '-q', '--unmerged', '--refresh').run()\n \n def rev_parse(git_id):\n \"\"\"Parse the string and return a verified SHA1 id\n \"\"\"\n try:\n- return _output_one_line(['git-rev-parse', '--verify', git_id])\n- except GitException:\n+ return GRun('git-rev-parse', '--verify', git_id).output_one_line()\n+ except GitRunException:\n raise GitException, 'Unknown revision: %s' % git_id\n \n def ref_exists(ref):\n@@ -387,7 +333,9 @@ def switch_branch(new_branch):\n tree_id = rev_parse('refs/heads/%s^{commit}' % new_branch)\n if tree_id != get_head():\n refresh_index()\n- if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:\n+ try:\n+ GRun('git-read-tree', '-u', '-m', get_head(), tree_id).run()\n+ except GitRunException:\n raise GitException, 'git-read-tree failed (local changes maybe?)'\n __head = tree_id\n set_head_file(new_branch)\n@@ -398,8 +346,10 @@ def switch_branch(new_branch):\n def delete_ref(ref):\n if not ref_exists(ref):\n raise GitException, '%s does not exist' % ref\n- sha1 = _output_one_line(['git-show-ref', '-s', ref])\n- if __run('git-update-ref -d %s %s' % (ref, sha1)):\n+ sha1 = GRun('git-show-ref', '-s', ref).output_one_line()\n+ try:\n+ GRun('git-update-ref', '-d', ref, sha1).run()\n+ except GitRunException:\n raise GitException, 'Failed to delete ref %s' % ref\n \n def delete_branch(name):\n@@ -411,10 +361,14 @@ def rename_ref(from_ref, to_ref):\n if ref_exists(to_ref):\n raise GitException, '\"%s\" already exists' % to_ref\n \n- sha1 = _output_one_line(['git-show-ref', '-s', from_ref])\n- if __run('git-update-ref %s %s %s' % (to_ref, sha1, '0'*40)):\n+ sha1 = GRun('git-show-ref', '-s', from_ref).output_one_line()\n+ try:\n+ GRun('git-update-ref', to_ref, sha1, '0'*40).run()\n+ except GitRunException:\n raise GitException, 'Failed to create new ref %s' % to_ref\n- if __run('git-update-ref -d %s %s' % (from_ref, sha1)):\n+ try:\n+ GRun('git-update-ref', '-d', from_ref, sha1).run()\n+ except GitRunException:\n raise GitException, 'Failed to delete ref %s' % from_ref\n \n def rename_branch(from_name, to_name):\n@@ -448,14 +402,16 @@ def add(names):\n raise GitException, '%s is not a file or directory' % i\n \n if files:\n- if __run('git-update-index --add --', files):\n+ try:\n+ GRun('git-update-index', '--add', '--').xargs(files)\n+ except GitRunException:\n raise GitException, 'Unable to add file'\n \n def __copy_single(source, target, target2=''):\n \"\"\"Copy file or dir named 'source' to name target+target2\"\"\"\n \n # \"source\" (file or dir) must match one or more git-controlled file\n- realfiles = _output_lines(['git-ls-files', source])\n+ realfiles = GRun('git-ls-files', source).output_lines()\n if len(realfiles) == 0:\n raise GitException, '\"%s\" matches no git-controled files' % source\n \n@@ -523,10 +479,10 @@ def rm(files, force = False):\n if os.path.exists(f):\n raise GitException, '%s exists. Remove it first' %f\n if files:\n- __run('git-update-index --remove --', files)\n+ GRun('git-update-index', '--remove', '--').xargs(files)\n else:\n if files:\n- __run('git-update-index --force-remove --', files)\n+ GRun('git-update-index', '--force-remove', '--').xargs(files)\n \n # Persons caching\n __user = None\n@@ -601,12 +557,9 @@ def update_cache(files = None, force = False):\n rm_files = [x[1] for x in cache_files if x[0] in ['D']]\n m_files = [x[1] for x in cache_files if x[0] in ['M']]\n \n- if add_files and __run('git-update-index --add --', add_files) != 0:\n- raise GitException, 'Failed git-update-index --add'\n- if rm_files and __run('git-update-index --force-remove --', rm_files) != 0:\n- raise GitException, 'Failed git-update-index --rm'\n- if m_files and __run('git-update-index --', m_files) != 0:\n- raise GitException, 'Failed git-update-index'\n+ GRun('git-update-index', '--add', '--').xargs(add_files)\n+ GRun('git-update-index', '--force-remove', '--').xargs(rm_files)\n+ GRun('git-update-index', '--').xargs(m_files)\n \n return True\n \n@@ -635,28 +588,24 @@ def commit(message, files = None, parents = None, allowempty = False,\n \n # write the index to repository\n if tree_id == None:\n- tree_id = _output_one_line(['git-write-tree'])\n+ tree_id = GRun('git-write-tree').output_one_line()\n set_head = True\n \n # the commit\n- cmd = ['env']\n+ env = {}\n if author_name:\n- cmd += ['GIT_AUTHOR_NAME=%s' % author_name]\n+ env['GIT_AUTHOR_NAME'] = author_name\n if author_email:\n- cmd += ['GIT_AUTHOR_EMAIL=%s' % author_email]\n+ env['GIT_AUTHOR_EMAIL'] = author_email\n if author_date:\n- cmd += ['GIT_AUTHOR_DATE=%s' % author_date]\n+ env['GIT_AUTHOR_DATE'] = author_date\n if committer_name:\n- cmd += ['GIT_COMMITTER_NAME=%s' % committer_name]\n+ env['GIT_COMMITTER_NAME'] = committer_name\n if committer_email:\n- cmd += ['GIT_COMMITTER_EMAIL=%s' % committer_email]\n- cmd += ['git-commit-tree', tree_id]\n-\n- # get the parents\n- for p in parents:\n- cmd += ['-p', p]\n-\n- commit_id = _output_one_line(cmd, message)\n+ env['GIT_COMMITTER_EMAIL'] = committer_email\n+ commit_id = GRun('git-commit-tree', tree_id,\n+ *sum([['-p', p] for p in parents], [])\n+ ).env(env).raw_input(message).output_one_line()\n if set_head:\n __set_head(commit_id)\n \n@@ -679,8 +628,8 @@ def apply_diff(rev1, rev2, check_index = True, files = None):\n diff_str = diff(files, rev1, rev2)\n if diff_str:\n try:\n- _input_str(['git-apply'] + index_opt, diff_str)\n- except GitException:\n+ GRun('git-apply', *index_opt).raw_input(diff_str).no_output()\n+ except GitRunException:\n return False\n \n return True\n@@ -696,24 +645,28 @@ def merge(base, head1, head2, recursive = False):\n # this operation tracks renames but it is slower (used in\n # general when pushing or picking patches)\n try:\n- # use _output() to mask the verbose prints of the tool\n- _output(['git-merge-recursive', base, '--', head1, head2])\n- except GitException, ex:\n+ # discard output to mask the verbose prints of the tool\n+ GRun('git-merge-recursive', base, '--', head1, head2\n+ ).discard_output()\n+ except GitRunException, ex:\n err_output = str(ex)\n pass\n else:\n # the fast case where we don't track renames (used when the\n # distance between base and heads is small, i.e. folding or\n # synchronising patches)\n- if __run('git-read-tree -u -m --aggressive',\n- [base, head1, head2]) != 0:\n+ try:\n+ GRun('git-read-tree', '-u', '-m', '--aggressive',\n+ base, head1, head2).run()\n+ except GitRunException:\n raise GitException, 'git-read-tree failed (local changes maybe?)'\n \n # check the index for unmerged entries\n files = {}\n stages_re = re.compile('^([0-7]+) ([0-9a-f]{40}) ([1-3])\\t(.*)$', re.S)\n \n- for line in _output(['git-ls-files', '--unmerged', '--stage', '-z']).split('\\0'):\n+ for line in GRun('git-ls-files', '--unmerged', '--stage', '-z'\n+ ).raw_output().split('\\0'):\n if not line:\n continue\n \n@@ -788,98 +741,85 @@ def status(files = None, modified = False, new = False, deleted = False,\n else:\n out.stdout('%s' % fs[1])\n \n-def diff(files = None, rev1 = 'HEAD', rev2 = None, out_fd = None,\n- diff_flags = []):\n+def diff(files = None, rev1 = 'HEAD', rev2 = None, diff_flags = []):\n \"\"\"Show the diff between rev1 and rev2\n \"\"\"\n if not files:\n files = []\n \n if rev1 and rev2:\n- diff_str = _output(['git-diff-tree', '-p'] + diff_flags\n- + [rev1, rev2, '--'] + files)\n+ return GRun('git-diff-tree', '-p',\n+ *(diff_flags + [rev1, rev2, '--'] + files)).raw_output()\n elif rev1 or rev2:\n refresh_index()\n if rev2:\n- diff_str = _output(['git-diff-index', '-p', '-R']\n- + diff_flags + [rev2, '--'] + files)\n+ return GRun('git-diff-index', '-p', '-R',\n+ *(diff_flags + [rev2, '--'] + files)).raw_output()\n else:\n- diff_str = _output(['git-diff-index', '-p']\n- + diff_flags + [rev1, '--'] + files)\n+ return GRun('git-diff-index', '-p',\n+ *(diff_flags + [rev1, '--'] + files)).raw_output()\n else:\n- diff_str = ''\n-\n- if out_fd:\n- out_fd.write(diff_str)\n- else:\n- return diff_str\n+ return ''\n \n def diffstat(files = None, rev1 = 'HEAD', rev2 = None):\n- \"\"\"Return the diffstat between rev1 and rev2\n- \"\"\"\n- if not files:\n- files = []\n-\n- p=popen2.Popen3('git-apply --stat')\n- diff(files, rev1, rev2, p.tochild)\n- p.tochild.close()\n- diff_str = p.fromchild.read().rstrip()\n- if p.wait():\n- raise GitException, 'git.diffstat failed'\n- return diff_str\n+ \"\"\"Return the diffstat between rev1 and rev2.\"\"\"\n+ return GRun('git-apply', '--stat'\n+ ).raw_input(diff(files, rev1, rev2)).raw_output()\n \n def files(rev1, rev2, diff_flags = []):\n \"\"\"Return the files modified between rev1 and rev2\n \"\"\"\n \n- result = ''\n- for line in _output_lines(['git-diff-tree'] + diff_flags + ['-r', rev1, rev2]):\n- result += '%s %s\\n' % tuple(line.rstrip().split(' ',4)[-1].split('\\t',1))\n+ result = []\n+ for line in GRun('git-diff-tree', *(diff_flags + ['-r', rev1, rev2])\n+ ).output_lines():\n+ result.append('%s %s' % tuple(line.split(' ', 4)[-1].split('\\t', 1)))\n \n- return result.rstrip()\n+ return '\\n'.join(result)\n \n def barefiles(rev1, rev2):\n \"\"\"Return the files modified between rev1 and rev2, without status info\n \"\"\"\n \n- result = ''\n- for line in _output_lines(['git-diff-tree', '-r', rev1, rev2]):\n- result += '%s\\n' % line.rstrip().split(' ',4)[-1].split('\\t',1)[-1]\n+ result = []\n+ for line in GRun('git-diff-tree', '-r', rev1, rev2).output_lines():\n+ result.append(line.split(' ', 4)[-1].split('\\t', 1)[-1])\n \n- return result.rstrip()\n+ return '\\n'.join(result)\n \n def pretty_commit(commit_id = 'HEAD', diff_flags = []):\n \"\"\"Return a given commit (log + diff)\n \"\"\"\n- return _output(['git-diff-tree'] + diff_flags +\n- ['--cc', '--always', '--pretty', '-r', commit_id])\n+ return GRun('git-diff-tree',\n+ *(diff_flags\n+ + ['--cc', '--always', '--pretty', '-r', commit_id])\n+ ).raw_output()\n \n def checkout(files = None, tree_id = None, force = False):\n \"\"\"Check out the given or all files\n \"\"\"\n- if not files:\n- files = []\n-\n- if tree_id and __run('git-read-tree --reset', [tree_id]) != 0:\n- raise GitException, 'Failed git-read-tree --reset %s' % tree_id\n+ if tree_id:\n+ try:\n+ GRun('git-read-tree', '--reset', tree_id).run()\n+ except GitRunException:\n+ raise GitException, 'Failed git-read-tree --reset %s' % tree_id\n \n- checkout_cmd = 'git-checkout-index -q -u'\n+ cmd = ['git-checkout-index', '-q', '-u']\n if force:\n- checkout_cmd += ' -f'\n- if len(files) == 0:\n- checkout_cmd += ' -a'\n+ cmd.append('-f')\n+ if files:\n+ GRun(*(cmd + ['--'])).xargs(files)\n else:\n- checkout_cmd += ' --'\n-\n- if __run(checkout_cmd, files) != 0:\n- raise GitException, 'Failed git-checkout-index'\n+ GRun(*(cmd + ['-a'])).run()\n \n def switch(tree_id, keep = False):\n \"\"\"Switch the tree to the given id\n \"\"\"\n if not keep:\n refresh_index()\n- if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:\n+ try:\n+ GRun('git-read-tree', '-u', '-m', get_head(), tree_id).run()\n+ except GitRunException:\n raise GitException, 'git-read-tree failed (local changes maybe?)'\n \n __set_head(tree_id)\n@@ -917,8 +857,7 @@ def fetch(repository = 'origin', refspec = None):\n \n command = config.get('branch.%s.stgit.fetchcmd' % get_head_file()) or \\\n config.get('stgit.fetchcmd')\n- if __run(command, args) != 0:\n- raise GitException, 'Failed \"%s %s\"' % (command, repository)\n+ GRun(*(command.split() + args)).run()\n \n def pull(repository = 'origin', refspec = None):\n \"\"\"Fetches changes from the remote repository, using 'git-pull'\n@@ -933,13 +872,12 @@ def pull(repository = 'origin', refspec = None):\n \n command = config.get('branch.%s.stgit.pullcmd' % get_head_file()) or \\\n config.get('stgit.pullcmd')\n- if __run(command, args) != 0:\n- raise GitException, 'Failed \"%s %s\"' % (command, repository)\n+ GRun(*(command.split() + args)).run()\n \n def repack():\n \"\"\"Repack all objects into a single pack\n \"\"\"\n- __run('git-repack -a -d -f')\n+ GRun('git-repack', '-a', '-d', '-f').run()\n \n def apply_patch(filename = None, diff = None, base = None,\n fail_dump = True):\n@@ -962,8 +900,8 @@ def apply_patch(filename = None, diff = None, base = None,\n refresh_index()\n \n try:\n- _input_str(['git-apply', '--index'], diff)\n- except GitException:\n+ GRun('git-apply', '--index').raw_input(diff).no_output()\n+ except GitRunException:\n if base:\n switch(orig_head)\n if fail_dump:\n@@ -985,18 +923,12 @@ def clone(repository, local_dir):\n \"\"\"Clone a remote repository. At the moment, just use the\n 'git-clone' script\n \"\"\"\n- if __run('git-clone', [repository, local_dir]) != 0:\n- raise GitException, 'Failed \"git-clone %s %s\"' \\\n- % (repository, local_dir)\n+ GRun('git-clone', repository, local_dir).run()\n \n def modifying_revs(files, base_rev, head_rev):\n- \"\"\"Return the revisions from the list modifying the given files\n- \"\"\"\n- cmd = ['git-rev-list', '%s..%s' % (base_rev, head_rev), '--']\n- revs = [line.strip() for line in _output_lines(cmd + files)]\n-\n- return revs\n-\n+ \"\"\"Return the revisions from the list modifying the given files.\"\"\"\n+ return GRun('git-rev-list', '%s..%s' % (base_rev, head_rev), '--', *files\n+ ).output_lines()\n \n def refspec_localpart(refspec):\n m = re.match('^[^:]*:([^:]*)$', refspec)\n@@ -1091,4 +1023,4 @@ 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'])]\n+ return [line.split()[1] for line in GRun('git-show-ref').output_lines()]\ndiff --git a/stgit/gitmergeonefile.py b/stgit/gitmergeonefile.py\nindex e76f9b1..2aa5ef8 100644\n--- a/stgit/gitmergeonefile.py\n+++ b/stgit/gitmergeonefile.py\n@@ -23,7 +23,7 @@ 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-\n+from stgit.run import *\n \n class GitMergeException(Exception):\n pass\n@@ -44,12 +44,8 @@ def __str2none(x):\n else:\n return x\n \n-def __output(cmd):\n- f = os.popen(cmd, 'r')\n- string = f.readline().rstrip()\n- if f.close():\n- raise GitMergeException, 'Error: failed to execute \"%s\"' % cmd\n- return string\n+class MRun(Run):\n+ exc = GitMergeException # use a custom exception class on errors\n \n def __checkout_files(orig_hash, file1_hash, file2_hash,\n path,\n@@ -62,24 +58,24 @@ def __checkout_files(orig_hash, file1_hash, file2_hash,\n \n if orig_hash:\n orig = path + extensions['ancestor']\n- tmp = __output('git-unpack-file %s' % orig_hash)\n+ tmp = MRun('git-unpack-file', orig_hash).output_one_line()\n os.chmod(tmp, int(orig_mode, 8))\n os.renames(tmp, orig)\n if file1_hash:\n src1 = path + extensions['current']\n- tmp = __output('git-unpack-file %s' % file1_hash)\n+ tmp = MRun('git-unpack-file', file1_hash).output_one_line()\n os.chmod(tmp, int(file1_mode, 8))\n os.renames(tmp, src1)\n if file2_hash:\n src2 = path + extensions['patched']\n- tmp = __output('git-unpack-file %s' % file2_hash)\n+ tmp = MRun('git-unpack-file', file2_hash).output_one_line()\n os.chmod(tmp, int(file2_mode, 8))\n os.renames(tmp, src2)\n \n if file1_hash and not os.path.exists(path):\n # the current file might be removed by GIT when it is a new\n # file added in both branches. Just re-generate it\n- tmp = __output('git-unpack-file %s' % file1_hash)\n+ tmp = MRun('git-unpack-file', file1_hash).output_one_line()\n os.chmod(tmp, int(file1_mode, 8))\n os.renames(tmp, path)\n \ndiff --git a/stgit/run.py b/stgit/run.py\nnew file mode 100644\nindex 0000000..d925cce\n--- /dev/null\n+++ b/stgit/run.py\n@@ -0,0 +1,111 @@\n+# -*- coding: utf-8 -*-\n+\n+__copyright__ = \"\"\"\n+Copyright (C) 2007, Karl Hasselström \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+# popen2 and os.spawn* suck. We should really use subprocess instead,\n+# but that's only available in Python 2.4 and up, and we try our best\n+# to stay Python 2.3 compatible.\n+import popen2, os\n+\n+class RunException(Exception):\n+ \"\"\"Thrown when something bad happened when we tried to run the\n+ subprocess.\"\"\"\n+ pass\n+\n+class Run:\n+ exc = RunException\n+ def __init__(self, *cmd):\n+ self.__cmd = list(cmd)\n+ self.__good_retvals = [0]\n+ self.__env = None\n+ self.__indata = None\n+ def __run_io(self, cmd):\n+ \"\"\"Run with captured IO. Note: arguments are parsed by the\n+ shell. We single-quote them, so don't use anything with single\n+ quotes in it.\"\"\"\n+ if self.__env == None:\n+ ecmd = cmd\n+ else:\n+ ecmd = (['env'] + ['%s=%s' % (key, val)\n+ for key, val in self.__env.iteritems()]\n+ + cmd)\n+ p = popen2.Popen3(' '.join([\"'%s'\" % c for c in ecmd]), True)\n+ if self.__indata != None:\n+ p.tochild.write(self.__indata)\n+ p.tochild.close()\n+ outdata = p.fromchild.read()\n+ errdata = p.childerr.read()\n+ self.exitcode = p.wait() >> 8\n+ if errdata or self.exitcode not in self.__good_retvals:\n+ raise self.exc('%s failed with code %d:\\n%s'\n+ % (cmd[0], self.exitcode, errdata))\n+ return outdata\n+ def __run_noshell(self, cmd):\n+ \"\"\"Run without captured IO. Note: arguments are not parsed by\n+ the shell.\"\"\"\n+ assert self.__env == None\n+ assert self.__indata == None\n+ self.exitcode = os.spawnvp(os.P_WAIT, cmd[0], cmd)\n+ if not self.exitcode in self.__good_retvals:\n+ raise self.exc('%s failed with code %d'\n+ % (cmd[0], self.exitcode))\n+ def returns(self, retvals):\n+ self.__good_retvals = retvals\n+ return self\n+ def env(self, env):\n+ self.__env = env\n+ return self\n+ def raw_input(self, indata):\n+ self.__indata = indata\n+ return self\n+ def input_lines(self, lines):\n+ self.__indata = ''.join(['%s\\n' % line for line in lines])\n+ return self\n+ def no_output(self):\n+ outdata = self.__run_io(self.__cmd)\n+ if outdata:\n+ raise self.exc, '%s produced output' % self.__cmd[0]\n+ def discard_output(self):\n+ self.__run_io(self.__cmd)\n+ def raw_output(self):\n+ return self.__run_io(self.__cmd)\n+ def output_lines(self):\n+ outdata = self.__run_io(self.__cmd)\n+ if outdata.endswith('\\n'):\n+ outdata = outdata[:-1]\n+ if outdata:\n+ return outdata.split('\\n')\n+ else:\n+ return []\n+ def output_one_line(self):\n+ outlines = self.output_lines()\n+ if len(outlines) == 1:\n+ return outlines[0]\n+ else:\n+ raise self.exc('%s produced %d lines, expected 1'\n+ % (self.__cmd[0], len(outlines)))\n+ def run(self):\n+ \"\"\"Just run, with no IO redirection.\"\"\"\n+ self.__run_noshell(self.__cmd)\n+ def xargs(self, xargs):\n+ \"\"\"Just run, with no IO redirection. The extra arguments are\n+ appended to the command line a few at a time; the command is\n+ run as many times as needed to consume them all.\"\"\"\n+ step = 100\n+ for i in xrange(0, len(xargs), step):\n+ self.__run_noshell(self.__cmd + xargs[i:i+step])\n"},{"id":"51591","messageId":"20070826203339.16265.43048.stgit@yoghurt","threadId":"9660","inReplyTo":"20070826202724.16265.85821.stgit@yoghurt","subject":"[StGIT PATCH 3/4] Assert that the argument to Run is a sequence of strings","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:33:39Z","receivedAt":"2007-08-26T20:33:39Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"This runtime assertion makes bugs easier to find.\n\nIn most other languages, we'd have been able to check this at compile\ntime. But this is Python. Yay!\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/run.py | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/stgit/run.py b/stgit/run.py\nindex d925cce..1bc4759 100644\n--- a/stgit/run.py\n+++ b/stgit/run.py\n@@ -31,6 +31,9 @@ class Run:\n exc = RunException\n def __init__(self, *cmd):\n self.__cmd = list(cmd)\n+ for c in cmd:\n+ if type(c) != str:\n+ raise Exception, 'Bad command: %r' % cmd\n self.__good_retvals = [0]\n self.__env = None\n self.__indata = None\n"},{"id":"51592","messageId":"20070826203344.16265.66280.stgit@yoghurt","threadId":"9660","inReplyTo":"20070826202724.16265.85821.stgit@yoghurt","subject":"[StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-26T20:33:44Z","receivedAt":"2007-08-26T20:33:44Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Now that the subprocess calling has been refactored and is in a nice\nshape, it's quite simple to add some logging facilities. This patch\nadds two separate log modes, switched by the STG_SUBPROCESS_LOG\nenvironment variable:\n\n * Setting it to \"debug\" prints the executable name and all\n arguments, and the subprocess return value.\n\n * Setting it to \"profile\" prints just the executable name, and the\n (wallclock) time elapsed during the call.\n\n * Not setting it will disable logging, of course.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/run.py | 27 +++++++++++++++++++++++++++\n 1 files changed, 27 insertions(+), 0 deletions(-)\n\ndiff --git a/stgit/run.py b/stgit/run.py\nindex 1bc4759..719ecb7 100644\n--- a/stgit/run.py\n+++ b/stgit/run.py\n@@ -22,11 +22,22 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA\n # to stay Python 2.3 compatible.\n import popen2, os\n \n+import datetime\n+\n+from stgit.out import *\n+\n class RunException(Exception):\n \"\"\"Thrown when something bad happened when we tried to run the\n subprocess.\"\"\"\n pass\n \n+_all_log_modes = ['debug', 'profile']\n+_log_mode = os.environ.get('STG_SUBPROCESS_LOG', '')\n+if _log_mode and not _log_mode in _all_log_modes:\n+ out.warn(('Unknown log mode \"%s\" specified in $STG_SUBPROCESS_LOG.'\n+ % _log_mode),\n+ 'Valid values are: %s' % ', '.join(_all_log_modes))\n+\n class Run:\n exc = RunException\n def __init__(self, *cmd):\n@@ -37,6 +48,18 @@ class Run:\n self.__good_retvals = [0]\n self.__env = None\n self.__indata = None\n+ def __log_start(self, cmd):\n+ if _log_mode == 'debug':\n+ out.start('Running subprocess %s' % cmd)\n+ elif _log_mode == 'profile':\n+ out.start('Running subprocess %s' % cmd[0])\n+ self.__starttime = datetime.datetime.now()\n+ def __log_end(self, retcode):\n+ if _log_mode == 'debug':\n+ out.done('return code: %d' % retcode)\n+ elif _log_mode == 'profile':\n+ duration = datetime.datetime.now() - self.__starttime\n+ out.done('%1.3f s' % (duration.microseconds/1e6 + duration.seconds))\n def __run_io(self, cmd):\n \"\"\"Run with captured IO. Note: arguments are parsed by the\n shell. We single-quote them, so don't use anything with single\n@@ -47,6 +70,7 @@ class Run:\n ecmd = (['env'] + ['%s=%s' % (key, val)\n for key, val in self.__env.iteritems()]\n + cmd)\n+ self.__log_start(ecmd)\n p = popen2.Popen3(' '.join([\"'%s'\" % c for c in ecmd]), True)\n if self.__indata != None:\n p.tochild.write(self.__indata)\n@@ -54,6 +78,7 @@ class Run:\n outdata = p.fromchild.read()\n errdata = p.childerr.read()\n self.exitcode = p.wait() >> 8\n+ self.__log_end(self.exitcode)\n if errdata or self.exitcode not in self.__good_retvals:\n raise self.exc('%s failed with code %d:\\n%s'\n % (cmd[0], self.exitcode, errdata))\n@@ -63,7 +88,9 @@ class Run:\n the shell.\"\"\"\n assert self.__env == None\n assert self.__indata == None\n+ self.__log_start(cmd)\n self.exitcode = os.spawnvp(os.P_WAIT, cmd[0], cmd)\n+ self.__log_end(self.exitcode)\n if not self.exitcode in self.__good_retvals:\n raise self.exc('%s failed with code %d'\n % (cmd[0], self.exitcode))\n"},{"id":"51804","messageId":"b0943d9e0708290350rbadfe08g9bbab7888723980e@mail.gmail.com","threadId":"9660","inReplyTo":"20070826203344.16265.66280.stgit@yoghurt","subject":"Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-08-29T10:50:11Z","receivedAt":"2007-08-29T10:50:11Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 26/08/2007, Karl Hasselström wrote:\n> Now that the subprocess calling has been refactored and is in a nice\n> shape, it's quite simple to add some logging facilities. This patch\n> adds two separate log modes, switched by the STG_SUBPROCESS_LOG\n> environment variable:\n\nAny objection to calling this variable STGIT_SUBPROCESS_LOG? We\nalready have STGIT_DEBUG_LEVEL (used in stgit.main). I can do it in my\ntree before pushing as I already merged your branches.\n\nBTW, thanks for refactoring the subprocess calling.\n\n-- \nCatalin\n"},{"id":"51806","messageId":"20070829111102.GA29980@diana.vm.bytemark.co.uk","threadId":"9660","inReplyTo":"b0943d9e0708290350rbadfe08g9bbab7888723980e@mail.gmail.com","subject":"Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-29T11:11:02Z","receivedAt":"2007-08-29T11:11:02Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-29 11:50:11 +0100, Catalin Marinas wrote:\n\n> On 26/08/2007, Karl Hasselström wrote:\n>\n> > Now that the subprocess calling has been refactored and is in a\n> > nice shape, it's quite simple to add some logging facilities. This\n> > patch adds two separate log modes, switched by the\n> > STG_SUBPROCESS_LOG environment variable:\n>\n> Any objection to calling this variable STGIT_SUBPROCESS_LOG? We\n> already have STGIT_DEBUG_LEVEL (used in stgit.main). I can do it in\n> my tree before pushing as I already merged your branches.\n\nNo, no objection at all. I was simply too lazy to actually check what\nthe existing naming convention was, and misremembered.\n\n> BTW, thanks for refactoring the subprocess calling.\n\nIt needed doing, and was actually fun.\n\nAny chance we can drop Python 2.3 support any time soon, by the way?\nI've confined all the ickyness to one place, but it would still be\ngood to get rid of it (not to mention being able to use sets and\ngenerator expressions).\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"51840","messageId":"20070829171621.GA2214@diana.vm.bytemark.co.uk","threadId":"9660","inReplyTo":"20070829111102.GA29980@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-29T17:16:21Z","receivedAt":"2007-08-29T17:16:21Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-29 13:11:02 +0200, Karl Hasselström wrote:\n\n> On 2007-08-29 11:50:11 +0100, Catalin Marinas wrote:\n>\n> > Any objection to calling this variable STGIT_SUBPROCESS_LOG? We\n> > already have STGIT_DEBUG_LEVEL (used in stgit.main). I can do it\n> > in my tree before pushing as I already merged your branches.\n>\n> No, no objection at all. I was simply too lazy to actually check\n> what the existing naming convention was, and misremembered.\n\nYou changed it in the code, but forgot to change it in the commit\nmessage. :-P\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"52316","messageId":"b0943d9e0709030134g6cf9f69ctc688c27dc705df9@mail.gmail.com","threadId":"9660","inReplyTo":"20070829171621.GA2214@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-09-03T08:34:32Z","receivedAt":"2007-09-03T08:34:32Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 29/08/07, Karl Hasselström wrote:\n> On 2007-08-29 13:11:02 +0200, Karl Hasselström wrote:\n>\n> > On 2007-08-29 11:50:11 +0100, Catalin Marinas wrote:\n> >\n> > > Any objection to calling this variable STGIT_SUBPROCESS_LOG? We\n> > > already have STGIT_DEBUG_LEVEL (used in stgit.main). I can do it\n> > > in my tree before pushing as I already merged your branches.\n> >\n> > No, no objection at all. I was simply too lazy to actually check\n> > what the existing naming convention was, and misremembered.\n>\n> You changed it in the code, but forgot to change it in the commit\n> message. :-P\n\nI changed it on my office PC and the locale setup breaks your name\nencoding :-). I tried to avoid it.\n\n-- \nCatalin\n"},{"id":"52318","messageId":"b0943d9e0709030136y4f1f241cge28fb9607ec559c1@mail.gmail.com","threadId":"9660","inReplyTo":"20070829111102.GA29980@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-09-03T08:36:26Z","receivedAt":"2007-09-03T08:36:26Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 29/08/07, Karl Hasselström wrote:\n> Any chance we can drop Python 2.3 support any time soon, by the way?\n> I've confined all the ickyness to one place, but it would still be\n> good to get rid of it (not to mention being able to use sets and\n> generator expressions).\n\nUnless there is no objection, I'm actually OK with this. Feel free to\nbreak the 2.3 support from now on. I'll add a note stating the minimum\nversions for Python and GIT in the 0.14 release.\n\n-- \nCatalin\n"},{"id":"52328","messageId":"20070903090448.GB5051@diana.vm.bytemark.co.uk","threadId":"9660","inReplyTo":"b0943d9e0709030136y4f1f241cge28fb9607ec559c1@mail.gmail.com","subject":"Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-03T09:04:48Z","receivedAt":"2007-09-03T09:04:48Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-09-03 09:36:26 +0100, Catalin Marinas wrote:\n\n> On 29/08/07, Karl Hasselström wrote:\n>\n> > Any chance we can drop Python 2.3 support any time soon, by the\n> > way? I've confined all the ickyness to one place, but it would\n> > still be good to get rid of it (not to mention being able to use\n> > sets and generator expressions).\n>\n> Unless there is no objection, I'm actually OK with this. Feel free\n> to break the 2.3 support from now on. I'll add a note stating the\n> minimum versions for Python and GIT in the 0.14 release.\n\nGreat!\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"52377","messageId":"20070903214545.18057.79301.stgit@yoghurt","threadId":"9660","inReplyTo":"b0943d9e0709030136y4f1f241cge28fb9607ec559c1@mail.gmail.com","subject":"[StGit PATCH 0/2] Break Python 2.3 compatibility","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-03T21:48:38Z","receivedAt":"2007-09-03T21:48:38Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-09-03 09:36:26 +0100, Catalin Marinas wrote:\n\n> On 29/08/07, Karl Hasselström wrote:\n>\n> > Any chance we can drop Python 2.3 support any time soon, by the\n> > way? I've confined all the ickyness to one place, but it would\n> > still be good to get rid of it (not to mention being able to use\n> > sets and generator expressions).\n> \n> Unless there is no objection, I'm actually OK with this. Feel free\n> to break the 2.3 support from now on. I'll add a note stating the\n> minimum versions for Python and GIT in the 0.14 release.\n\nAnd here it is. We actually used sets in much fewer places than I\nthought -- but then again, there are a number of places where we ought\nto be using them but aren't.\n\n-- \nKarl Hasselström, kha@treskal.com\n www.treskal.com/kalle\n"},{"id":"52378","messageId":"20070903214845.18057.26023.stgit@yoghurt","threadId":"9660","inReplyTo":"20070903214545.18057.79301.stgit@yoghurt","subject":"[StGit PATCH 1/2] Use subprocess.Popen to call git executables","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-03T21:48:45Z","receivedAt":"2007-09-03T21:48:45Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Replace popen2.Popen3 and os.spawn* with the superior\nsubprocess.Popen. Now we can pass a modified environment to any\nsubprocess, redirect input and output as we please, and the shell is\nno longer involved, meaning we don't have to worry about argument\nquoting. (We could already do all of that, just not at the same time.)\n\nThis is a Python 2.4 library, so StGit now officially requires Python\n2.4 or later.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/run.py | 89 ++++++++++++++++++++++++++--------------------------------\n 1 files changed, 40 insertions(+), 49 deletions(-)\n\n\ndiff --git a/stgit/run.py b/stgit/run.py\nindex 94dd98e..29f8f71 100644\n--- a/stgit/run.py\n+++ b/stgit/run.py\n@@ -17,12 +17,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-# popen2 and os.spawn* suck. We should really use subprocess instead,\n-# but that's only available in Python 2.4 and up, and we try our best\n-# to stay Python 2.3 compatible.\n-import popen2, os\n-\n-import datetime\n+import datetime, os, subprocess\n \n from stgit.out import *\n \n@@ -48,11 +43,11 @@ class Run:\n self.__good_retvals = [0]\n self.__env = None\n self.__indata = None\n- def __log_start(self, cmd):\n+ def __log_start(self):\n if _log_mode == 'debug':\n- out.start('Running subprocess %s' % cmd)\n+ out.start('Running subprocess %s' % self.__cmd)\n elif _log_mode == 'profile':\n- out.start('Running subprocess %s' % cmd[0])\n+ out.start('Running subprocess %s' % self.__cmd[0])\n self.__starttime = datetime.datetime.now()\n def __log_end(self, retcode):\n if _log_mode == 'debug':\n@@ -60,48 +55,41 @@ class Run:\n elif _log_mode == 'profile':\n duration = datetime.datetime.now() - self.__starttime\n out.done('%1.3f s' % (duration.microseconds/1e6 + duration.seconds))\n- def __run_io(self, cmd):\n- \"\"\"Run with captured IO. Note: arguments are parsed by the\n- shell. We single-quote them, so don't use anything with single\n- quotes in it.\"\"\"\n- if self.__env == None:\n- ecmd = cmd\n- else:\n- ecmd = (['env'] + ['%s=%s' % (key, val)\n- for key, val in self.__env.iteritems()]\n- + cmd)\n- self.__log_start(ecmd)\n- p = popen2.Popen3(' '.join([\"'%s'\" % c for c in ecmd]), True)\n- if self.__indata != None:\n- p.tochild.write(self.__indata)\n- p.tochild.close()\n- outdata = p.fromchild.read()\n- errdata = p.childerr.read()\n- self.exitcode = p.wait() >> 8\n- self.__log_end(self.exitcode)\n+ def __check_exitcode(self):\n if self.exitcode not in self.__good_retvals:\n- raise self.exc('%s failed with code %d:\\n%s'\n- % (cmd[0], self.exitcode, errdata))\n- if errdata:\n- out.warn('call to %s succeeded, but generated a warning:' % cmd[0])\n- out.err_raw(errdata)\n+ raise self.exc('%s failed with code %d'\n+ % (self.__cmd[0], self.exitcode))\n+ def __run_io(self):\n+ \"\"\"Run with captured IO.\"\"\"\n+ self.__log_start()\n+ try:\n+ p = subprocess.Popen(self.__cmd, env = self.__env,\n+ stdin = subprocess.PIPE,\n+ stdout = subprocess.PIPE)\n+ outdata, errdata = p.communicate(self.__indata)\n+ self.exitcode = p.returncode\n+ except OSError, e:\n+ raise self.exc('%s failed: %s' % (self.__cmd[0], e))\n+ self.__log_end(self.exitcode)\n+ self.__check_exitcode()\n return outdata\n- def __run_noshell(self, cmd):\n- \"\"\"Run without captured IO. Note: arguments are not parsed by\n- the shell.\"\"\"\n- assert self.__env == None\n+ def __run_noio(self):\n+ \"\"\"Run without captured IO.\"\"\"\n assert self.__indata == None\n- self.__log_start(cmd)\n- self.exitcode = os.spawnvp(os.P_WAIT, cmd[0], cmd)\n+ self.__log_start()\n+ try:\n+ p = subprocess.Popen(self.__cmd, env = self.__env)\n+ self.exitcode = p.wait()\n+ except OSError, e:\n+ raise self.exc('%s failed: %s' % (self.__cmd[0], e))\n self.__log_end(self.exitcode)\n- if not self.exitcode in self.__good_retvals:\n- raise self.exc('%s failed with code %d'\n- % (cmd[0], self.exitcode))\n+ self.__check_exitcode()\n def returns(self, retvals):\n self.__good_retvals = retvals\n return self\n def env(self, env):\n- self.__env = env\n+ self.__env = dict(os.environ)\n+ self.__env.update(env)\n return self\n def raw_input(self, indata):\n self.__indata = indata\n@@ -110,15 +98,15 @@ class Run:\n self.__indata = ''.join(['%s\\n' % line for line in lines])\n return self\n def no_output(self):\n- outdata = self.__run_io(self.__cmd)\n+ outdata = self.__run_io()\n if outdata:\n raise self.exc, '%s produced output' % self.__cmd[0]\n def discard_output(self):\n- self.__run_io(self.__cmd)\n+ self.__run_io()\n def raw_output(self):\n- return self.__run_io(self.__cmd)\n+ return self.__run_io()\n def output_lines(self):\n- outdata = self.__run_io(self.__cmd)\n+ outdata = self.__run_io()\n if outdata.endswith('\\n'):\n outdata = outdata[:-1]\n if outdata:\n@@ -134,11 +122,14 @@ class Run:\n % (self.__cmd[0], len(outlines)))\n def run(self):\n \"\"\"Just run, with no IO redirection.\"\"\"\n- self.__run_noshell(self.__cmd)\n+ self.__run_noio()\n def xargs(self, xargs):\n \"\"\"Just run, with no IO redirection. The extra arguments are\n appended to the command line a few at a time; the command is\n run as many times as needed to consume them all.\"\"\"\n step = 100\n+ basecmd = self.__cmd\n for i in xrange(0, len(xargs), step):\n- self.__run_noshell(self.__cmd + xargs[i:i+step])\n+ self.__cmd = basecmd + xargs[i:i+step]\n+ self.__run_noio()\n+ self.__cmd = basecmd\n"},{"id":"52379","messageId":"20070903214851.18057.89136.stgit@yoghurt","threadId":"9660","inReplyTo":"20070903214545.18057.79301.stgit@yoghurt","subject":"[StGit PATCH 2/2] Use the builtin set() instead of sets.Set()","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-09-03T21:48:51Z","receivedAt":"2007-09-03T21:48:51Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"We can do that now that we're guaranteed to have Python 2.4 or later.\n\nSigned-off-by: Karl Hasselström \n\n---\n\n stgit/git.py | 8 +++-----\n 1 files changed, 3 insertions(+), 5 deletions(-)\n\n\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 8857209..4b4c626 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -26,7 +26,6 @@ from stgit.utils import *\n from stgit.out import *\n from stgit.run import *\n from stgit.config import config\n-from sets import Set\n \n # git exception class\n class GitException(Exception):\n@@ -959,10 +958,9 @@ def __remotes_from_dir(dir):\n def remotes_list():\n \"\"\"Return the list of remotes in the repository\n \"\"\"\n-\n- return Set(__remotes_from_config()) | \\\n- Set(__remotes_from_dir('remotes')) | \\\n- Set(__remotes_from_dir('branches'))\n+ return (set(__remotes_from_config())\n+ | set(__remotes_from_dir('remotes'))\n+ | set(__remotes_from_dir('branches')))\n \n def remotes_local_branches(remote):\n \"\"\"Returns the list of local branches fetched from given remote\n"}]}