[PATCH 1/4] git-p4import: fix subcommand error handling
- From
- Scott Lamb <slamb@slamb.org>
- Date
- Jun 3, 2007, 03:58 UTC
- Message-ID
- <1180843126948-git-send-email-slamb@slamb.org>
- In-Reply-To
- <4ACE2ABC-8D73-4097-87AC-F3B27EDA97DE@slamb.org>
Use the Python "subcommand" module to properly handle the subcommand pipeline and raise exceptions on error.
Signed-off-by: Scott Lamb <slamb@slamb.org> --- I folded the elimination of the shell pipelines into this patch - the error handling couldn't work otherwise.
The difference between exceptions paths with "die" and ones with an exception thrown to the top is somewhat arbitrary; I tried to use it to distinguish between user error and bugs.
git-p4import.py | 166 +++++++++++++++++++++++++++++++++---------------------- 1 files changed, 99 insertions(+), 67 deletions(-)
diff --git a/git-p4import.py b/git-p4import.py index 60a758b..002f8d8 100644 --- a/git-p4import.py +++ b/git-p4import.py @@ -13,6 +13,9 @@ import os import sys import time import getopt +import subprocess +import re +import errno from signal import signal, \ SIGPIPE, SIGINT, SIG_DFL, \ @@ -34,7 +37,7 @@ def usage(): sys.exit(1) verbosity = 1 -logfile = "/dev/null" +logfile = file("/dev/null", "a") ignore_warnings = False stitch = 0 tagall = True @@ -44,59 +47,70 @@ def report(level, msg, *args): global logfile for a in args: msg = "%s %s" % (msg, a) - fd = open(logfile, "a") - fd.writelines(msg) - fd.close() + logfile.writelines(msg) if level <= verbosity: print msg +class P4Exception(Exception): + def __init__(self, cmd, errmsg): + Exception.__init__(self, '%r: %s' % (cmd, errmsg)) + self.cmd = cmd + self.errmsg = errmsg + class p4_command: def __init__(self, _repopath): - try: - global logfile - self.userlist = {} - if _repopath[-1] == '/': - self.repopath = _repopath[:-1] - else: - self.repopath = _repopath - if self.repopath[-4:] != "/...": - self.repopath= "%s/..." % self.repopath - f=os.popen('p4 -V 2>>%s'%logfile, 'rb') - a = f.readlines() - if f.close(): - raise - except: - die("Could not find the \"p4\" command") + self.userlist = {} + if _repopath[-1] == '/': + self.repopath = _repopath[:-1] + else: + self.repopath = _repopath + if self.repopath[-4:] != "/...": + self.repopath= "%s/..." % self.repopath + p4 = subprocess.Popen(['p4', '-V'], + stdout=file('/dev/null', 'a'), + stderr=subprocess.PIPE) + err = p4.stderr.read() + if p4.wait() != 0: + die("Could not run the \"p4\" command: %r" % (err,)) def p4(self, cmd, *args): global logfile cmd = "%s %s" % (cmd, ' '.join(args)) + cmdlist = ['p4', '-G'] + cmd.split(' ') report(2, "P4:", cmd) - f=os.popen('p4 -G %s 2>>%s' % (cmd,logfile), 'rb') + p4 = subprocess.Popen(cmdlist, + stdout=subprocess.PIPE, + stderr=logfile) list = [] while 1: try: - list.append(marshal.load(f)) + elem = marshal.load(p4.stdout) except EOFError: break - self.ret = f.close() + if elem['code'] == 'error': + raise P4Exception(cmd, elem['data']) + list.append(elem) + if p4.wait() != 0: + raise Exception("'p4 %s' failed" % (cmd,)) return list def sync(self, id, force=False, trick=False, test=False): - if force: - ret = self.p4("sync -f %s@%s"%(self.repopath, id))[0] - elif trick: - ret = self.p4("sync -k %s@%s"%(self.repopath, id))[0] - elif test: - ret = self.p4("sync -n %s@%s"%(self.repopath, id))[0] - else: - ret = self.p4("sync %s@%s"%(self.repopath, id))[0] - if ret['code'] == "error": - data = ret['data'].upper() - if data.find('VIEW') > 0: - die("Perforce reports %s is not in client view"% self.repopath) - elif data.find('UP-TO-DATE') < 0: - die("Could not sync files from perforce", self.repopath) + try: + if force: + self.p4("sync -f %s@%s"%(self.repopath, id)) + elif trick: + self.p4("sync -k %s@%s"%(self.repopath, id)) + elif test: + self.p4("sync -n %s@%s"%(self.repopath, id)) + else: + self.p4("sync %s@%s"%(self.repopath, id)) + except P4Exception, e: + data = e.errmsg.upper() + if data.find('VIEW') > 0: + die("Perforce reports %s is not in client view: %s" + % (self.repopath, e)) + elif data.find('UP-TO-DATE') < 0: + die(e) def changes(self, since=0): try: @@ -105,8 +119,8 @@ class p4_command: list.append(rec['change']) list.reverse() return list - except: - return [] + except P4Exception, e: + die(e) def authors(self, filename): f=open(filename) @@ -122,7 +136,8 @@ class p4_command: try: user = self.p4("users", id)[0] self.userlist[id] = (user['FullName'], user['Email']) - except: + except P4Exception, e: + report(2, "P4: missing user %s" % (id,)) self.userlist[id] = (id, "") return self.userlist[id] @@ -143,7 +158,7 @@ class p4_command: def where(self): try: return self.p4("where %s" % self.repopath)[-1]['path'] - except: + except P4Exception, e: return "" def describe(self, num): @@ -153,16 +168,18 @@ class p4_command: self.date = self._format_date(time.localtime(long(desc['time']))) return self +class GitException(Exception): pass + class git_command: def __init__(self): try: self.version = self.git("--version")[0][12:].rstrip() - except: - die("Could not find the \"git\" command") + except GitException, e: + die(e) try: self.gitdir = self.get_single("rev-parse --git-dir") report(2, "gdir:", self.gitdir) - except: + except GitException, e: die("Not a git repository... did you forget to \"git init\" ?") try: self.cdup = self.get_single("rev-parse --show-cdup") @@ -170,38 +187,47 @@ class git_command: os.chdir(self.cdup) self.topdir = os.getcwd() report(2, "topdir:", self.topdir) - except: + except GitException, e: die("Could not find top git directory") - def git(self, cmd): - global logfile + def git(self, cmd, stdin=None): report(2, "GIT:", cmd) - f=os.popen('git %s 2>>%s' % (cmd,logfile), 'rb') - r=f.readlines() - self.ret = f.close() - return r + cmdlist = ['git'] + cmd.split(' ') + git = subprocess.Popen(cmdlist, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdout, stderr = git.communicate(stdin) + if git.wait() != 0: + raise GitException("'git %s' failed: %s" % (cmd, stderr)) + if stderr != '': + report(2, stderr) + return re.findall(r'.*\n', stdout) def get_single(self, cmd): - return self.git(cmd)[0].rstrip() + list = self.git(cmd) + if len(list) != 1: + raise GitException("%r returned %r" % (cmd, list)) + return list[0].rstrip() def current_branch(self): try: testit = self.git("rev-parse --verify HEAD")[0] return self.git("symbolic-ref HEAD")[0][11:].rstrip() - except: + except GitException, e: return None def get_config(self, variable): try: return self.git("config --get %s" % variable)[0].rstrip() - except: + except GitException, e: return None def set_config(self, variable, value): try: self.git("config %s %s"%(variable, value) ) - except: - die("Could not set %s to " % variable, value) + except GitException, e: + die(e) def make_tag(self, name, head): self.git("tag -f %s %s"%(name,head)) @@ -217,7 +243,8 @@ class git_command: return 0 def update_index(self): - self.git("ls-files -m -d -o -z | git update-index --add --remove -z --stdin") + files = self.git("ls-files -m -d -o -z") + self.git("update-index --add --remove -z --stdin", stdin=files) def checkout(self, branch): self.git("checkout %s" % branch) @@ -226,15 +253,21 @@ class git_command: self.git("symbolic-ref HEAD refs/heads/%s" % branch) def remove_files(self): - self.git("ls-files | xargs rm") + files = self.git("ls-files") + for file in files: + os.unlink(file) def clean_directories(self): self.git("clean -d") def fresh_branch(self, branch): report(1, "Creating new branch", branch) - self.git("ls-files | xargs rm") - os.remove(".git/index") + self.remove_files() + try: + os.remove(".git/index") + except OSError, e: + if e.errno != errno.ENOENT: + raise self.repoint_head(branch) self.git("clean -d") @@ -243,21 +276,17 @@ class git_command: def commit(self, author, email, date, msg, id): self.update_index() - fd=open(".msg", "w") - fd.writelines(msg) - fd.close() try: current = self.get_single("rev-parse --verify HEAD") head = "-p HEAD" - except: + except GitException, e: current = "" head = "" tree = self.get_single("write-tree") for r,l in [('DATE',date),('NAME',author),('EMAIL',email)]: os.environ['GIT_AUTHOR_%s'%r] = l os.environ['GIT_COMMITTER_%s'%r] = l - commit = self.get_single("commit-tree %s %s < .msg" % (tree,head)) - os.remove(".msg") + commit = self.get_single("commit-tree %s %s" % (tree,head), stdin=msg) self.make_tag("p4/%s"%id, commit) self.git("update-ref HEAD %s %s" % (commit, current) ) @@ -273,7 +302,7 @@ for o, a in opts: if o == "-v": verbosity += 1 if o in ("--log"): - logfile = a + logfile = file(a, "a") if o in ("--notags"): tagall = False if o in ("-h", "--help"): @@ -293,7 +322,10 @@ for o, a in opts: if len(args) == 2: branch = args[1] - git.checkout(branch) + try: + git.checkout(branch) + except GitException, e: + pass if branch == git.current_branch(): die("Branch %s already exists!" % branch) report(1, "Setting perforce to ", args[0])
-- 1.5.2