{"thread":{"id":"8376","subject":"git-p4import.py robustness changes","startedAt":"2007-05-31T16:47:51Z","lastAt":"2007-06-15T05:30:39Z","messageCount":26,"participants":["Scott Lamb","Junio C Hamano","Simon Hausmann","Shawn O. Pearce","Dana How","Marius Storm-Olsen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"43689","messageId":"4ACE2ABC-8D73-4097-87AC-F3B27EDA97DE@slamb.org","threadId":"8376","inReplyTo":null,"subject":"git-p4import.py robustness changes","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-05-31T16:47:51Z","receivedAt":"2007-05-31T16:47:51Z","isPatch":false,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"I'm trying out git-p4import.py (and git itself) for the first time.  \nI'm frustrated with its error behavior. For example, it's saying this:\n\n     $ git-p4import.py //my/path/... master\n     Setting perforce to  //my/path/...\n     Already up to date...\n\nwhen it should be saying this:\n\n     $ git-p4import.py //my/path/... master\n     Setting perforce to  //my/path/...\n     git-p4import fatal error: p4 changes //my/path/...@1,#head:  \nRequest too large (over 150000); see 'p4 help maxresults'.\n\nThere's a logfile option, but that's a poor excuse for no error  \nhandling. I'd like to fix it. A couple questions, though:\n\n\nFirst, is it acceptable to switch from os.popen to the subprocess  \nmodule? I ask because the latter was only introduced with Python 2.4  \non. The subprocess module does work with earlier versions of Python  \n(definitely 2.3) and is GPL-compatible, so maybe it could be thrown  \ninto the distribution if desired.\n\nI could make do with popen2.Popen3, but subprocess is actually  \npleasant to use:\n\n         git = subprocess.Popen(cmdlist,\n                                stdin=subprocess.PIPE,\n                                stdout=subprocess.PIPE,\n                                stderr=subprocess.PIPE)\n         stdout, stderr = git.communicate(stdin)\n         if git.wait() != 0:\n             raise GitException(\"'git %s' failed: %s\" % (cmd, stderr))\n\nvs. the popen2 way, which is longer and uglier. It'd probably involve  \ntempfiles rather than reimplementing subprocess.Popen.communicate().\n\n\nSecond, this crowd seems to want sequences of tiny patches. How does  \nthis sound?\n\n* patch 1 - use subprocess to make git_command.git() and p4_command.p4 \n() throw properly-typed exceptions on error, fix caller exception  \nhandling to match.\n\n* patch 2 - remove the use of the shell and pipelines (fix some  \nescaping problems).\n\n* patch 3 - use lists instead of space separation for the commandline  \narguments (fix more escaping problems).\n\n* patch 4 - allow grabbing partial history (make my error go away).\n\n\nCheers,\nScott\n\n-- \nScott Lamb <http://www.slamb.org/>\n"},{"id":"43707","messageId":"7vbqg01reo.fsf@assigned-by-dhcp.cox.net","threadId":"8376","inReplyTo":"4ACE2ABC-8D73-4097-87AC-F3B27EDA97DE@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-31T23:53:35Z","receivedAt":"2007-05-31T23:53:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Scott Lamb <slamb@slamb.org> writes:\n\n> There's a logfile option, but that's a poor excuse for no error\n> handling. I'd like to fix it. A couple questions, though:\n\nGood to have somebody who has access to p4.\n\n> First, is it acceptable to switch from os.popen to the subprocess\n> module? I ask because the latter was only introduced with Python 2.4\n> on. The subprocess module does work with earlier versions of Python\n> (definitely 2.3) and is GPL-compatible, so maybe it could be thrown\n> into the distribution if desired.\n\nWe actually did ship with our own copy after clearing the\nlicensing situation with Python people, although we removed it\nwhen it lost the last script that used it.  I do not think\nresurrecting it is a problem.\n\n> Second, this crowd seems to want sequences of tiny patches. How does\n> this sound?\n>\n> * patch 1 - use subprocess to make git_command.git() and p4_command.p4\n> () throw properly-typed exceptions on error, fix caller exception\n> handling to match.\n>\n> * patch 2 - remove the use of the shell and pipelines (fix some\n> escaping problems).\n>\n> * patch 3 - use lists instead of space separation for the commandline\n> arguments (fix more escaping problems).\n>\n> * patch 4 - allow grabbing partial history (make my error go away).\n\nActually, my preference is to have a \"patch 0\" before all of the\nabove, that demotes git-p4import to contrib/ hierarchy.  Having\nno access to p4 managed repositories (nor much inclination to\nget one), I can never test nor maintain it myself, so it is just\ncrazy for me to be the maintainer for it.\n\nBut I do read Python and speak it passably -- the above 4 step\noutline sounds sane to me.\n"},{"id":"43827","messageId":"0EDF1E14-3682-4B1E-A7D2-F82388F752AA@slamb.org","threadId":"8376","inReplyTo":"7vbqg01reo.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-p4import.py robustness changes","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-02T20:41:55Z","receivedAt":"2007-06-02T20:41:55Z","isPatch":false,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"\nOn May 31, 2007, at 4:53 PM, Junio C Hamano wrote:\n\n> Actually, my preference is to have a \"patch 0\" before all of the\n> above, that demotes git-p4import to contrib/ hierarchy.  Having\n> no access to p4 managed repositories (nor much inclination to\n> get one), I can never test nor maintain it myself, so it is just\n> crazy for me to be the maintainer for it.\n\nWill do. What does that mean for Documentation/git-p4import.txt and  \nthe git-p4 rpm (defined in git.spec.in)? Should I move them with it?  \n(Seems nothing else in the main tree references contrib.) If so,  \nmaybe I should set up a common \"Documentation/asciidoc.mak\" or  \nsomething for building the man/html pages rather than duplicating all  \nthat Makefile logic.\n\n-- \nScott Lamb <http://www.slamb.org/>\n"},{"id":"43830","messageId":"7vzm3inisa.fsf@assigned-by-dhcp.cox.net","threadId":"8376","inReplyTo":"0EDF1E14-3682-4B1E-A7D2-F82388F752AA@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-06-02T21:33:25Z","receivedAt":"2007-06-02T21:33:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Scott Lamb <slamb@slamb.org> writes:\n\n> On May 31, 2007, at 4:53 PM, Junio C Hamano wrote:\n>\n>> Actually, my preference is to have a \"patch 0\" before all of the\n>> above, that demotes git-p4import to contrib/ hierarchy.  Having\n>> no access to p4 managed repositories (nor much inclination to\n>> get one), I can never test nor maintain it myself, so it is just\n>> crazy for me to be the maintainer for it.\n>\n> Will do. What does that mean for Documentation/git-p4import.txt and\n> the git-p4 rpm (defined in git.spec.in)? Should I move them with it?\n> (Seems nothing else in the main tree references contrib.) If so,\n> maybe I should set up a common \"Documentation/asciidoc.mak\" or\n> something for building the man/html pages rather than duplicating all\n> that Makefile logic.\n\nA much more preferable alternative is for you to say \"Hey, don't\nsay you want to demote it.  I'll keep it maintained, I regularly\nuse p4 and have a strong incentive to keep it working\".  Then we\ndo not have to do the \"patch 0\" ;-)\n"},{"id":"43834","messageId":"87F9A283-C51F-49FB-9A13-40E850AC0474@slamb.org","threadId":"8376","inReplyTo":"7vzm3inisa.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-p4import.py robustness changes","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-02T23:21:34Z","receivedAt":"2007-06-02T23:21:34Z","isPatch":false,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"\nOn Jun 2, 2007, at 2:33 PM, Junio C Hamano wrote:\n\n> A much more preferable alternative is for you to say \"Hey, don't\n> say you want to demote it.  I'll keep it maintained, I regularly\n> use p4 and have a strong incentive to keep it working\".  Then we\n> do not have to do the \"patch 0\" ;-)\n\nHmm. I'd like to say that, but keep in mind that I'd never even used  \ngit before Wednesday, and I'm not sure yet how well git-p4import.py  \nwill work out for me.\n\nIt'd be a huge leap from git-p4import.py to something that could  \nremove my need to use p4 commands daily. First, I'd need something  \nthat could follow all upstream branches with merge history. Then I'd  \neither need to convince my team to ditch p4 entirely (not easy, and  \nthen I wouldn't use/maintain git-p4import.py afterward anyway) or a  \nway to robustly generate \"p4 integrate\", \"p4 resolve\", \"p4 submit\"  \ncommand sequences to merge between upstream branches.\n\nWe're attempting to address more modest needs, like those of our off- \nsite contractors who should only be sending patches anyway. (Another  \nguy wrote a script that pulls changes into an svn mirror basically by  \n\"svn ci -m 'changed some stuff'\" every half hour, but I made fun of  \nit and now have to replace it. ;)\n\nI'll at least finish up the other patches first and see how it goes.\n\n-- \nScott Lamb <http://www.slamb.org/>\n"},{"id":"43835","messageId":"7vr6otoqw5.fsf@assigned-by-dhcp.cox.net","threadId":"8376","inReplyTo":"87F9A283-C51F-49FB-9A13-40E850AC0474@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-06-02T23:52:58Z","receivedAt":"2007-06-02T23:52:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Scott Lamb <slamb@slamb.org> writes:\n\n> On Jun 2, 2007, at 2:33 PM, Junio C Hamano wrote:\n>\n>> A much more preferable alternative is for you to say \"Hey, don't\n>> say you want to demote it.  I'll keep it maintained, I regularly\n>> use p4 and have a strong incentive to keep it working\".  Then we\n>> do not have to do the \"patch 0\" ;-)\n>\n> Hmm. I'd like to say that, but keep in mind that I'd never even used\n> git before Wednesday, and I'm not sure yet how well git-p4import.py\n> will work out for me.\n\nOh, you do not have to worry so much.  There certainly are\npeople who are familiar enough with git on this list to help\nimproving p4import from the git side.\n\nThe thing to me personally is that I am p4 illiterate.  However,\nas you might be aware, a few people posted their own version of\n\"better than p4import\" scripts to the list in the past few\nmonths, so there should also be enough people with p4 expertise\nand motivation to help you with it.\n\nIt _might_ turn out that one of their scripts might be better\nthan the p4import we have in-tree and we would end up replacing\nit with it.  I won't be able to judge that myself, but if people\nwho need to interoperate with p4 on the list can join forces it\nwould be good.\n"},{"id":"43845","messageId":"1180843126948-git-send-email-slamb@slamb.org","threadId":"8376","inReplyTo":"4ACE2ABC-8D73-4097-87AC-F3B27EDA97DE@slamb.org","subject":"[PATCH 1/4] git-p4import: fix subcommand error handling","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-03T03:58:43Z","receivedAt":"2007-06-03T03:58:43Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"Use the Python \"subcommand\" module to properly handle the subcommand\npipeline and raise exceptions on error.\n\nSigned-off-by: Scott Lamb <slamb@slamb.org>\n---\nI folded the elimination of the shell pipelines into this patch - the error\nhandling couldn't work otherwise.\n\nThe difference between exceptions paths with \"die\" and ones with an exception\nthrown to the top is somewhat arbitrary; I tried to use it to distinguish\nbetween user error and bugs.\n\n git-p4import.py |  166 +++++++++++++++++++++++++++++++++----------------------\n 1 files changed, 99 insertions(+), 67 deletions(-)\n\ndiff --git a/git-p4import.py b/git-p4import.py\nindex 60a758b..002f8d8 100644\n--- a/git-p4import.py\n+++ b/git-p4import.py\n@@ -13,6 +13,9 @@ import os\n import sys\n import time\n import getopt\n+import subprocess\n+import re\n+import errno\n \n from signal import signal, \\\n    SIGPIPE, SIGINT, SIG_DFL, \\\n@@ -34,7 +37,7 @@ def usage():\n     sys.exit(1)\n \n verbosity = 1\n-logfile = \"/dev/null\"\n+logfile = file(\"/dev/null\", \"a\")\n ignore_warnings = False\n stitch = 0\n tagall = True\n@@ -44,59 +47,70 @@ def report(level, msg, *args):\n     global logfile\n     for a in args:\n         msg = \"%s %s\" % (msg, a)\n-    fd = open(logfile, \"a\")\n-    fd.writelines(msg)\n-    fd.close()\n+    logfile.writelines(msg)\n     if level <= verbosity:\n         print msg\n \n+class P4Exception(Exception):\n+    def __init__(self, cmd, errmsg):\n+        Exception.__init__(self, '%r: %s' % (cmd, errmsg))\n+        self.cmd = cmd\n+        self.errmsg = errmsg\n+\n class p4_command:\n     def __init__(self, _repopath):\n-        try:\n-            global logfile\n-            self.userlist = {}\n-            if _repopath[-1] == '/':\n-                self.repopath = _repopath[:-1]\n-            else:\n-                self.repopath = _repopath\n-            if self.repopath[-4:] != \"/...\":\n-                self.repopath= \"%s/...\" % self.repopath\n-            f=os.popen('p4 -V 2>>%s'%logfile, 'rb')\n-            a = f.readlines()\n-            if f.close():\n-                raise\n-        except:\n-                die(\"Could not find the \\\"p4\\\" command\")\n+        self.userlist = {}\n+        if _repopath[-1] == '/':\n+            self.repopath = _repopath[:-1]\n+        else:\n+            self.repopath = _repopath\n+        if self.repopath[-4:] != \"/...\":\n+            self.repopath= \"%s/...\" % self.repopath\n+        p4 = subprocess.Popen(['p4', '-V'],\n+                              stdout=file('/dev/null', 'a'),\n+                              stderr=subprocess.PIPE)\n+        err = p4.stderr.read()\n+        if p4.wait() != 0:\n+            die(\"Could not run the \\\"p4\\\" command: %r\" % (err,))\n \n     def p4(self, cmd, *args):\n         global logfile\n         cmd = \"%s %s\" % (cmd, ' '.join(args))\n+        cmdlist = ['p4', '-G'] + cmd.split(' ')\n         report(2, \"P4:\", cmd)\n-        f=os.popen('p4 -G %s 2>>%s' % (cmd,logfile), 'rb')\n+        p4 = subprocess.Popen(cmdlist,\n+                              stdout=subprocess.PIPE,\n+                              stderr=logfile)\n         list = []\n         while 1:\n            try:\n-                list.append(marshal.load(f))\n+                elem = marshal.load(p4.stdout)\n            except EOFError:\n                 break\n-        self.ret = f.close()\n+           if elem['code'] == 'error':\n+                raise P4Exception(cmd, elem['data'])\n+           list.append(elem)\n+        if p4.wait() != 0:\n+            raise Exception(\"'p4 %s' failed\" % (cmd,))\n         return list\n \n     def sync(self, id, force=False, trick=False, test=False):\n-        if force:\n-            ret = self.p4(\"sync -f %s@%s\"%(self.repopath, id))[0]\n-        elif trick:\n-            ret = self.p4(\"sync -k %s@%s\"%(self.repopath, id))[0]\n-        elif test:\n-            ret = self.p4(\"sync -n %s@%s\"%(self.repopath, id))[0]\n-        else:\n-            ret = self.p4(\"sync    %s@%s\"%(self.repopath, id))[0]\n-        if ret['code'] == \"error\":\n-             data = ret['data'].upper()\n-             if data.find('VIEW') > 0:\n-                 die(\"Perforce reports %s is not in client view\"% self.repopath)\n-             elif data.find('UP-TO-DATE') < 0:\n-                 die(\"Could not sync files from perforce\", self.repopath)\n+        try:\n+            if force:\n+                self.p4(\"sync -f %s@%s\"%(self.repopath, id))\n+            elif trick:\n+                self.p4(\"sync -k %s@%s\"%(self.repopath, id))\n+            elif test:\n+                self.p4(\"sync -n %s@%s\"%(self.repopath, id))\n+            else:\n+                self.p4(\"sync    %s@%s\"%(self.repopath, id))\n+        except P4Exception, e:\n+            data = e.errmsg.upper()\n+            if data.find('VIEW') > 0:\n+                die(\"Perforce reports %s is not in client view: %s\"\n+                    % (self.repopath, e))\n+            elif data.find('UP-TO-DATE') < 0:\n+                die(e)\n \n     def changes(self, since=0):\n         try:\n@@ -105,8 +119,8 @@ class p4_command:\n                 list.append(rec['change'])\n             list.reverse()\n             return list\n-        except:\n-            return []\n+        except P4Exception, e:\n+            die(e)\n \n     def authors(self, filename):\n         f=open(filename)\n@@ -122,7 +136,8 @@ class p4_command:\n             try:\n                 user = self.p4(\"users\", id)[0]\n                 self.userlist[id] = (user['FullName'], user['Email'])\n-            except:\n+            except P4Exception, e:\n+                report(2, \"P4: missing user %s\" % (id,))\n                 self.userlist[id] = (id, \"\")\n         return self.userlist[id]\n \n@@ -143,7 +158,7 @@ class p4_command:\n     def where(self):\n         try:\n             return self.p4(\"where %s\" % self.repopath)[-1]['path']\n-        except:\n+        except P4Exception, e:\n             return \"\"\n \n     def describe(self, num):\n@@ -153,16 +168,18 @@ class p4_command:\n         self.date = self._format_date(time.localtime(long(desc['time'])))\n         return self\n \n+class GitException(Exception): pass\n+\n class git_command:\n     def __init__(self):\n         try:\n             self.version = self.git(\"--version\")[0][12:].rstrip()\n-        except:\n-            die(\"Could not find the \\\"git\\\" command\")\n+        except GitException, e:\n+            die(e)\n         try:\n             self.gitdir = self.get_single(\"rev-parse --git-dir\")\n             report(2, \"gdir:\", self.gitdir)\n-        except:\n+        except GitException, e:\n             die(\"Not a git repository... did you forget to \\\"git init\\\" ?\")\n         try:\n             self.cdup = self.get_single(\"rev-parse --show-cdup\")\n@@ -170,38 +187,47 @@ class git_command:\n                 os.chdir(self.cdup)\n             self.topdir = os.getcwd()\n             report(2, \"topdir:\", self.topdir)\n-        except:\n+        except GitException, e:\n             die(\"Could not find top git directory\")\n \n-    def git(self, cmd):\n-        global logfile\n+    def git(self, cmd, stdin=None):\n         report(2, \"GIT:\", cmd)\n-        f=os.popen('git %s 2>>%s' % (cmd,logfile), 'rb')\n-        r=f.readlines()\n-        self.ret = f.close()\n-        return r\n+        cmdlist = ['git'] + cmd.split(' ')\n+        git = subprocess.Popen(cmdlist,\n+                               stdin=subprocess.PIPE,\n+                               stdout=subprocess.PIPE,\n+                               stderr=subprocess.PIPE)\n+        stdout, stderr = git.communicate(stdin)\n+        if git.wait() != 0:\n+            raise GitException(\"'git %s' failed: %s\" % (cmd, stderr))\n+        if stderr != '':\n+            report(2, stderr)\n+        return re.findall(r'.*\\n', stdout)\n \n     def get_single(self, cmd):\n-        return self.git(cmd)[0].rstrip()\n+        list = self.git(cmd)\n+        if len(list) != 1:\n+            raise GitException(\"%r returned %r\" % (cmd, list))\n+        return list[0].rstrip()\n \n     def current_branch(self):\n         try:\n             testit = self.git(\"rev-parse --verify HEAD\")[0]\n             return self.git(\"symbolic-ref HEAD\")[0][11:].rstrip()\n-        except:\n+        except GitException, e:\n             return None\n \n     def get_config(self, variable):\n         try:\n             return self.git(\"config --get %s\" % variable)[0].rstrip()\n-        except:\n+        except GitException, e:\n             return None\n \n     def set_config(self, variable, value):\n         try:\n             self.git(\"config %s %s\"%(variable, value) )\n-        except:\n-            die(\"Could not set %s to \" % variable, value)\n+        except GitException, e:\n+            die(e)\n \n     def make_tag(self, name, head):\n         self.git(\"tag -f %s %s\"%(name,head))\n@@ -217,7 +243,8 @@ class git_command:\n             return 0\n \n     def update_index(self):\n-        self.git(\"ls-files -m -d -o -z | git update-index --add --remove -z --stdin\")\n+        files = self.git(\"ls-files -m -d -o -z\")\n+        self.git(\"update-index --add --remove -z --stdin\", stdin=files)\n \n     def checkout(self, branch):\n         self.git(\"checkout %s\" % branch)\n@@ -226,15 +253,21 @@ class git_command:\n         self.git(\"symbolic-ref HEAD refs/heads/%s\" % branch)\n \n     def remove_files(self):\n-        self.git(\"ls-files | xargs rm\")\n+        files = self.git(\"ls-files\")\n+        for file in files:\n+            os.unlink(file)\n \n     def clean_directories(self):\n         self.git(\"clean -d\")\n \n     def fresh_branch(self, branch):\n         report(1, \"Creating new branch\", branch)\n-        self.git(\"ls-files | xargs rm\")\n-        os.remove(\".git/index\")\n+        self.remove_files()\n+        try:\n+            os.remove(\".git/index\")\n+        except OSError, e:\n+            if e.errno != errno.ENOENT:\n+                raise\n         self.repoint_head(branch)\n         self.git(\"clean -d\")\n \n@@ -243,21 +276,17 @@ class git_command:\n \n     def commit(self, author, email, date, msg, id):\n         self.update_index()\n-        fd=open(\".msg\", \"w\")\n-        fd.writelines(msg)\n-        fd.close()\n         try:\n                 current = self.get_single(\"rev-parse --verify HEAD\")\n                 head = \"-p HEAD\"\n-        except:\n+        except GitException, e:\n                 current = \"\"\n                 head = \"\"\n         tree = self.get_single(\"write-tree\")\n         for r,l in [('DATE',date),('NAME',author),('EMAIL',email)]:\n             os.environ['GIT_AUTHOR_%s'%r] = l\n             os.environ['GIT_COMMITTER_%s'%r] = l\n-        commit = self.get_single(\"commit-tree %s %s < .msg\" % (tree,head))\n-        os.remove(\".msg\")\n+        commit = self.get_single(\"commit-tree %s %s\" % (tree,head), stdin=msg)\n         self.make_tag(\"p4/%s\"%id, commit)\n         self.git(\"update-ref HEAD %s %s\" % (commit, current) )\n \n@@ -273,7 +302,7 @@ for o, a in opts:\n     if o == \"-v\":\n         verbosity += 1\n     if o in (\"--log\"):\n-        logfile = a\n+        logfile = file(a, \"a\")\n     if o in (\"--notags\"):\n         tagall = False\n     if o in (\"-h\", \"--help\"):\n@@ -293,7 +322,10 @@ for o, a in opts:\n \n if len(args) == 2:\n     branch = args[1]\n-    git.checkout(branch)\n+    try:\n+        git.checkout(branch)\n+    except GitException, e:\n+        pass\n     if branch == git.current_branch():\n         die(\"Branch %s already exists!\" % branch)\n     report(1, \"Setting perforce to \", args[0])\n-- \n1.5.2\n"},{"id":"43846","messageId":"11808431291938-git-send-email-slamb@slamb.org","threadId":"8376","inReplyTo":"1180843126948-git-send-email-slamb@slamb.org","subject":"[PATCH 2/4] git-p4import: use lists of subcommand arguments","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-03T03:58:44Z","receivedAt":"2007-06-03T03:58:44Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"This fixes problems with spaces in filenames.\n\nSigned-off-by: Scott Lamb <slamb@slamb.org>\n---\n git-p4import.py |   84 +++++++++++++++++++++++++++++-------------------------\n 1 files changed, 45 insertions(+), 39 deletions(-)\n\ndiff --git a/git-p4import.py b/git-p4import.py\nindex 002f8d8..54e5e9e 100644\n--- a/git-p4import.py\n+++ b/git-p4import.py\n@@ -73,11 +73,12 @@ class p4_command:\n         if p4.wait() != 0:\n             die(\"Could not run the \\\"p4\\\" command: %r\" % (err,))\n \n-    def p4(self, cmd, *args):\n+    def p4(self, args):\n         global logfile\n-        cmd = \"%s %s\" % (cmd, ' '.join(args))\n-        cmdlist = ['p4', '-G'] + cmd.split(' ')\n+        cmd = ' '.join(args)\n         report(2, \"P4:\", cmd)\n+        cmdlist = ['p4', '-G']\n+        cmdlist.extend(args)\n         p4 = subprocess.Popen(cmdlist,\n                               stdout=subprocess.PIPE,\n                               stderr=logfile)\n@@ -97,13 +98,14 @@ class p4_command:\n     def sync(self, id, force=False, trick=False, test=False):\n         try:\n             if force:\n-                self.p4(\"sync -f %s@%s\"%(self.repopath, id))\n+                extra = [\"-f\"]\n             elif trick:\n-                self.p4(\"sync -k %s@%s\"%(self.repopath, id))\n+                extra = [\"-k\"]\n             elif test:\n-                self.p4(\"sync -n %s@%s\"%(self.repopath, id))\n+                extra = [\"-n\"]\n             else:\n-                self.p4(\"sync    %s@%s\"%(self.repopath, id))\n+                extra = []\n+            self.p4([\"sync\"] + extra + [\"%s@%s\"%(self.repopath, id)])\n         except P4Exception, e:\n             data = e.errmsg.upper()\n             if data.find('VIEW') > 0:\n@@ -115,7 +117,8 @@ class p4_command:\n     def changes(self, since=0):\n         try:\n             list = []\n-            for rec in self.p4(\"changes %s@%s,#head\" % (self.repopath, since+1)):\n+            for rec in self.p4([\"changes\", \"%s@%s,#head\"\n+                               % (self.repopath, since+1)]):\n                 list.append(rec['change'])\n             list.reverse()\n             return list\n@@ -134,7 +137,7 @@ class p4_command:\n     def _get_user(self, id):\n         if not self.userlist.has_key(id):\n             try:\n-                user = self.p4(\"users\", id)[0]\n+                user = self.p4([\"users\", id])[0]\n                 self.userlist[id] = (user['FullName'], user['Email'])\n             except P4Exception, e:\n                 report(2, \"P4: missing user %s\" % (id,))\n@@ -157,12 +160,12 @@ class p4_command:\n \n     def where(self):\n         try:\n-            return self.p4(\"where %s\" % self.repopath)[-1]['path']\n+            return self.p4([\"where\", self.repopath])[-1]['path']\n         except P4Exception, e:\n             return \"\"\n \n     def describe(self, num):\n-        desc = self.p4(\"describe -s\", num)[0]\n+        desc = self.p4([\"describe\", \"-s\", num])[0]\n         self.msg = desc['desc']\n         self.author, self.email = self._get_user(desc['user'])\n         self.date = self._format_date(time.localtime(long(desc['time'])))\n@@ -173,16 +176,16 @@ class GitException(Exception): pass\n class git_command:\n     def __init__(self):\n         try:\n-            self.version = self.git(\"--version\")[0][12:].rstrip()\n+            self.version = self.git([\"--version\"])[0][12:].rstrip()\n         except GitException, e:\n             die(e)\n         try:\n-            self.gitdir = self.get_single(\"rev-parse --git-dir\")\n+            self.gitdir = self.get_single([\"rev-parse\", \"--git-dir\"])\n             report(2, \"gdir:\", self.gitdir)\n         except GitException, e:\n             die(\"Not a git repository... did you forget to \\\"git init\\\" ?\")\n         try:\n-            self.cdup = self.get_single(\"rev-parse --show-cdup\")\n+            self.cdup = self.get_single([\"rev-parse\", \"--show-cdup\"])\n             if self.cdup != \"\":\n                 os.chdir(self.cdup)\n             self.topdir = os.getcwd()\n@@ -190,9 +193,11 @@ class git_command:\n         except GitException, e:\n             die(\"Could not find top git directory\")\n \n-    def git(self, cmd, stdin=None):\n+    def git(self, args, stdin=None):\n+        cmd = ' '.join(args)\n         report(2, \"GIT:\", cmd)\n-        cmdlist = ['git'] + cmd.split(' ')\n+        cmdlist = ['git']\n+        cmdlist.extend(args)\n         git = subprocess.Popen(cmdlist,\n                                stdin=subprocess.PIPE,\n                                stdout=subprocess.PIPE,\n@@ -204,37 +209,37 @@ class git_command:\n             report(2, stderr)\n         return re.findall(r'.*\\n', stdout)\n \n-    def get_single(self, cmd):\n-        list = self.git(cmd)\n+    def get_single(self, args, stdin=None):\n+        list = self.git(args, stdin=stdin)\n         if len(list) != 1:\n-            raise GitException(\"%r returned %r\" % (cmd, list))\n+            raise GitException(\"%r returned %r\" % (' '.join(args), list))\n         return list[0].rstrip()\n \n     def current_branch(self):\n         try:\n-            testit = self.git(\"rev-parse --verify HEAD\")[0]\n-            return self.git(\"symbolic-ref HEAD\")[0][11:].rstrip()\n+            testit = self.git([\"rev-parse\", \"--verify\", \"HEAD\"])[0]\n+            return self.git([\"symbolic-ref\", \"HEAD\"])[0][11:].rstrip()\n         except GitException, e:\n             return None\n \n     def get_config(self, variable):\n         try:\n-            return self.git(\"config --get %s\" % variable)[0].rstrip()\n+            return self.git([\"config\", \"--get\", variable])[0].rstrip()\n         except GitException, e:\n             return None\n \n     def set_config(self, variable, value):\n         try:\n-            self.git(\"config %s %s\"%(variable, value) )\n+            self.git([\"config\", variable, value])\n         except GitException, e:\n             die(e)\n \n     def make_tag(self, name, head):\n-        self.git(\"tag -f %s %s\"%(name,head))\n+        self.git([\"tag\", \"-f\", name, head])\n \n     def top_change(self, branch):\n         try:\n-            a=self.get_single(\"name-rev --tags refs/heads/%s\" % branch)\n+            a=self.get_single([\"name-rev\", \"--tags\", \"refs/heads/%s\" % branch])\n             loc = a.find(' tags/') + 6\n             if a[loc:loc+3] != \"p4/\":\n                 raise\n@@ -243,22 +248,23 @@ class git_command:\n             return 0\n \n     def update_index(self):\n-        files = self.git(\"ls-files -m -d -o -z\")\n-        self.git(\"update-index --add --remove -z --stdin\", stdin=files)\n+        files = self.git(\"ls-files -m -d -o -z\".split(\" \"))\n+        self.git(\"update-index --add --remove -z --stdin\".split(\" \"),\n+                 stdin=files)\n \n     def checkout(self, branch):\n-        self.git(\"checkout %s\" % branch)\n+        self.git([\"checkout\", branch])\n \n     def repoint_head(self, branch):\n-        self.git(\"symbolic-ref HEAD refs/heads/%s\" % branch)\n+        self.git([\"symbolic-ref\", \"HEAD\", \"refs/heads/%s\" % branch])\n \n     def remove_files(self):\n-        files = self.git(\"ls-files\")\n+        files = self.git([\"ls-files\"])\n         for file in files:\n             os.unlink(file)\n \n     def clean_directories(self):\n-        self.git(\"clean -d\")\n+        self.git([\"clean\", \"-d\"])\n \n     def fresh_branch(self, branch):\n         report(1, \"Creating new branch\", branch)\n@@ -269,7 +275,7 @@ class git_command:\n             if e.errno != errno.ENOENT:\n                 raise\n         self.repoint_head(branch)\n-        self.git(\"clean -d\")\n+        self.clean_directories()\n \n     def basedir(self):\n         return self.topdir\n@@ -277,18 +283,18 @@ class git_command:\n     def commit(self, author, email, date, msg, id):\n         self.update_index()\n         try:\n-                current = self.get_single(\"rev-parse --verify HEAD\")\n-                head = \"-p HEAD\"\n+                current = [self.get_single([\"rev-parse\", \"--verify\", \"HEAD\"])]\n+                head = [\"-p\", \"HEAD\"]\n         except GitException, e:\n-                current = \"\"\n-                head = \"\"\n-        tree = self.get_single(\"write-tree\")\n+                current = []\n+                head = []\n+        tree = self.get_single([\"write-tree\"])\n         for r,l in [('DATE',date),('NAME',author),('EMAIL',email)]:\n             os.environ['GIT_AUTHOR_%s'%r] = l\n             os.environ['GIT_COMMITTER_%s'%r] = l\n-        commit = self.get_single(\"commit-tree %s %s\" % (tree,head), stdin=msg)\n+        commit = self.get_single([\"commit-tree\", tree] + head, stdin=msg)\n         self.make_tag(\"p4/%s\"%id, commit)\n-        self.git(\"update-ref HEAD %s %s\" % (commit, current) )\n+        self.git([\"update-ref\", \"HEAD\", commit] + current)\n \n try:\n     opts, args = getopt.getopt(sys.argv[1:], \"qhvt:\",\n-- \n1.5.2\n"},{"id":"43847","messageId":"11808431364066-git-send-email-slamb@slamb.org","threadId":"8376","inReplyTo":"11808431291938-git-send-email-slamb@slamb.org","subject":"[PATCH 3/4] git-p4import: resume on correct p4 changeset","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-03T03:58:45Z","receivedAt":"2007-06-03T03:58:45Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"This had been resuming on change 222 rather than 22283.\n\ntop_change's removal of the last two characters must have predated the use\nof rstrip() in get_single(). A regexp should be less fragile, or at least\nmore obvious when it breaks.\n\nSigned-off-by: Scott Lamb <slamb@slamb.org>\n---\n git-p4import.py |   13 +++++++------\n 1 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/git-p4import.py b/git-p4import.py\nindex 54e5e9e..e7a52b3 100644\n--- a/git-p4import.py\n+++ b/git-p4import.py\n@@ -237,15 +237,16 @@ class git_command:\n     def make_tag(self, name, head):\n         self.git([\"tag\", \"-f\", name, head])\n \n+    _tag_re = re.compile(r'tags/p4/(\\d+)')\n     def top_change(self, branch):\n         try:\n             a=self.get_single([\"name-rev\", \"--tags\", \"refs/heads/%s\" % branch])\n-            loc = a.find(' tags/') + 6\n-            if a[loc:loc+3] != \"p4/\":\n-                raise\n-            return int(a[loc+3:][:-2])\n-        except:\n-            return 0\n+        except GitException, e:\n+            return 0 # fresh repository\n+        m = self._tag_re.search(a)\n+        if m is None:\n+            raise Exception('unable to parse: %r' % (a,))\n+        return int(m.group(1))\n \n     def update_index(self):\n         files = self.git(\"ls-files -m -d -o -z\".split(\" \"))\n-- \n1.5.2\n"},{"id":"43848","messageId":"11808431401213-git-send-email-slamb@slamb.org","threadId":"8376","inReplyTo":"11808431364066-git-send-email-slamb@slamb.org","subject":"[PATCH 4/4] git-p4import: partial history","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-03T03:58:46Z","receivedAt":"2007-06-03T03:58:46Z","isPatch":true,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"Allow importing partial history, which is quicker and may be necessary with\na low Perforce MaxScanRows limit.\n\nSigned-off-by: Scott Lamb <slamb@slamb.org>\n---\n Documentation/git-p4import.txt |    6 ++++++\n git-p4import.py                |   14 ++++++++++++--\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-p4import.txt b/Documentation/git-p4import.txt\nindex 714abbe..bf40b5a 100644\n--- a/Documentation/git-p4import.txt\n+++ b/Documentation/git-p4import.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n --------\n [verse]\n `git-p4import` [-q|-v] [--notags] [--authors <file>] [-t <timezone>]\n+               [--start-with <change>]\n                <//p4repo/path> <branch>\n `git-p4import` --stitch <//p4repo/path>\n `git-p4import`\n@@ -59,6 +60,11 @@ OPTIONS\n \tetc.  You only need to specify this once, it will be saved in\n \tthe git config file for the repository.\n \n+\\--start-with::\n+\tStart the import with the given Perforce change. A partial history can\n+\tbe much faster to generate and is possible even with a low MaxScanRows\n+\tlimit.\n+\n <//p4repo/path>::\n \tThe Perforce path that will be imported into the specified branch.\n \ndiff --git a/git-p4import.py b/git-p4import.py\nindex e7a52b3..c7a2033 100644\n--- a/git-p4import.py\n+++ b/git-p4import.py\n@@ -33,7 +33,12 @@ def die(msg, *args):\n     sys.exit(1)\n \n def usage():\n-    print \"USAGE: git-p4import [-q|-v]  [--authors=<file>]  [-t <timezone>]  [//p4repo/path <branch>]\"\n+    print \"usage:\"\n+    print \"  git-p4import [-q|-v] [--notags] [--authors <file>] [-t <timezone>]\"\n+    print \"               [--start-with <change>]\"\n+    print \"               <//p4repo/path> <branch>\"\n+    print \"  git-p4import --stitch <//p4repo/path>\"\n+    print \"  git-p4import\"\n     sys.exit(1)\n \n verbosity = 1\n@@ -41,6 +46,7 @@ logfile = file(\"/dev/null\", \"a\")\n ignore_warnings = False\n stitch = 0\n tagall = True\n+start_with = 0\n \n def report(level, msg, *args):\n     global verbosity\n@@ -299,7 +305,8 @@ class git_command:\n \n try:\n     opts, args = getopt.getopt(sys.argv[1:], \"qhvt:\",\n-            [\"authors=\",\"help\",\"stitch=\",\"timezone=\",\"log=\",\"ignore\",\"notags\"])\n+            [\"authors=\",\"help\",\"stitch=\",\"timezone=\",\"log=\",\"ignore\",\"notags\",\n+             \"start-with=\"])\n except getopt.GetoptError:\n     usage()\n \n@@ -316,6 +323,8 @@ for o, a in opts:\n         usage()\n     if o in (\"--ignore\"):\n         ignore_warnings = True\n+    if o in (\"--start-with\"):\n+        start_with = int(a)\n \n git = git_command()\n branch=git.current_branch()\n@@ -361,6 +370,7 @@ if stitch == 0:\n     top = git.top_change(branch)\n else:\n     top = 0\n+top = max(top, start_with)\n changes = p4.changes(top)\n count = len(changes)\n if count == 0:\n-- \n1.5.2\n"},{"id":"43864","messageId":"200706031511.31157.simon@lst.de","threadId":"8376","inReplyTo":"7vzm3inisa.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-p4import.py robustness changes","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-06-03T13:11:27Z","receivedAt":"2007-06-03T13:11:27Z","isPatch":false,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Saturday 02 June 2007 23:33:25 Junio C Hamano wrote:\n> Scott Lamb <slamb@slamb.org> writes:\n> > On May 31, 2007, at 4:53 PM, Junio C Hamano wrote:\n> >> Actually, my preference is to have a \"patch 0\" before all of the\n> >> above, that demotes git-p4import to contrib/ hierarchy.  Having\n> >> no access to p4 managed repositories (nor much inclination to\n> >> get one), I can never test nor maintain it myself, so it is just\n> >> crazy for me to be the maintainer for it.\n> >\n> > Will do. What does that mean for Documentation/git-p4import.txt and\n> > the git-p4 rpm (defined in git.spec.in)? Should I move them with it?\n> > (Seems nothing else in the main tree references contrib.) If so,\n> > maybe I should set up a common \"Documentation/asciidoc.mak\" or\n> > something for building the man/html pages rather than duplicating all\n> > that Makefile logic.\n>\n> A much more preferable alternative is for you to say \"Hey, don't\n> say you want to demote it.  I'll keep it maintained, I regularly\n> use p4 and have a strong incentive to keep it working\".  Then we\n> do not have to do the \"patch 0\" ;-)\n\nOn the topic of git integration with perforce, what are the chances of getting \ngit-p4 ( http://repo.or.cz/w/fast-export.git ) into git's contrib/fast-export \narea? :)\n\ngit-p4 can do everything git-p4import can do plus a lot more (it can track \nmultiple branches, it's a hell of a lot faster, it can export back to p4 and \nit also works on Windows!).\n\n\nSimon\n"},{"id":"43903","messageId":"839AEF71-ED29-4A79-BE97-C79EAFEDC466@slamb.org","threadId":"8376","inReplyTo":"200706031511.31157.simon@lst.de","subject":"Re: git-p4import.py robustness changes","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-03T20:12:17Z","receivedAt":"2007-06-03T20:12:17Z","isPatch":false,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"\nOn Jun 3, 2007, at 6:11 AM, Simon Hausmann wrote:\n\n> On the topic of git integration with perforce, what are the chances  \n> of getting\n> git-p4 ( http://repo.or.cz/w/fast-export.git ) into git's contrib/ \n> fast-export\n> area? :)\n>\n> git-p4 can do everything git-p4import can do plus a lot more (it  \n> can track\n> multiple branches, it's a hell of a lot faster, it can export back  \n> to p4 and\n> it also works on Windows!).\n\nI missed that one...I just saw Tailor and the Perl script someone  \nelse had written.\n\nErgh. git-p4 imports both \"subprocess\" and \"popen2\" and also uses  \n\"system\" and \"os.popen\". Why use four different modules to launch git  \nand p4?\n\nThe branch support's interesting. Have you considered tracking  \nintegration history? I was pondering it and am not sure if it's  \nfeasible. Perforce doesn't seem to have an efficient way of  \ndisplaying it (just \"p4 integrates\" that will fetch *all* revisions  \neven if you want incremental results and \"p4 filelog\" which would  \nhave to be done on each file). Also, I think there's some mismatch  \nbetween the Perforce and git models.\n\ngit-p4import.py should work fine on Windows, too - the binary mode on  \nthe pipe should be all handled by \"subprocess\", and git-p4's  \ndata.replace(\"\\r\\n\", \"\\n\") is not necessary if you use \"LineEnd:  \nunix\" or \"share\" in the Perforce client specification.\n\nAs for performance...hmm. Looks like git-p4import.py runs these  \ncommands for each Perforce revision:\n\n     realtime  operation\n         3.4%  p4 describe -s N\n        66.6%  p4 sync ...@N\n    [*] 10.2%  git ls-files -m -d -o -z | git update-index --add -- \nremove -z --stdin\n         2.6%  git rev-parse --verify HEAD\n         4.2%  git write-tree\n         2.8%  git commit-tree xxxxxx\n         7.5%  git tag -f p4/N xxxxxx\n         2.7%  git update-ref HEAD xxxxxx\n\nThat's with Perforce running over the network. Are you running locally?\n\ngit-p4 seems to use \"git fast-import\". I guess the big performance  \nimprovement there is removing the ls-files operation? So we're  \ntalking about a 0-10% speedup, right? Plus some fork()/exec() overhead.\n\n[*] - Note that I just discovered a big performance regression in my  \npatches. Reading the ls-files into Python, through a regexp, and back  \nout through update-index was a horrible idea. The times above are  \nwith that fixed.\n\n-- \nScott Lamb <http://www.slamb.org/>\n"},{"id":"43937","messageId":"20070604055433.GD4507@spearce.org","threadId":"8376","inReplyTo":"839AEF71-ED29-4A79-BE97-C79EAFEDC466@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-04T05:54:33Z","receivedAt":"2007-06-04T05:54:33Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Scott Lamb <slamb@slamb.org> wrote:\n> On Jun 3, 2007, at 6:11 AM, Simon Hausmann wrote:\n> >On the topic of git integration with perforce, what are the chances  \n> >of getting\n> >git-p4 ( http://repo.or.cz/w/fast-export.git ) into git's contrib/ \n> >fast-export\n> >area? :)\n> \n> I missed that one...I just saw Tailor and the Perl script someone  \n> else had written.\n\nPerhaps why it should be in contrib/fast-import?  ;-)\n \n> As for performance...hmm. Looks like git-p4import.py runs these  \n> commands for each Perforce revision:\n> \n>     realtime  operation\n>         3.4%  p4 describe -s N\n>        66.6%  p4 sync ...@N\n>    [*] 10.2%  git ls-files -m -d -o -z | git update-index --add -- \n> remove -z --stdin\n>         2.6%  git rev-parse --verify HEAD\n>         4.2%  git write-tree\n>         2.8%  git commit-tree xxxxxx\n>         7.5%  git tag -f p4/N xxxxxx\n>         2.7%  git update-ref HEAD xxxxxx\n...\n> git-p4 seems to use \"git fast-import\". I guess the big performance  \n> improvement there is removing the ls-files operation? So we're  \n> talking about a 0-10% speedup, right? Plus some fork()/exec() overhead.\n\nfast-import folds all of the git commands you list above behind\na single engine that is *fast*.  So its actually a 0-30% gain\nthat is available by using the fast-import backend, with a single\nfork()/exec() for the *entire import*.  The local object IO performed\nby Git is also minimized, so large imports have much better IO\nbehavior from the Git perspective.  Its not something to sneeze at.\n\nfast-import also can run in parallel with the frontend process,\nallowing you to use a dual-core system, to the extent that your\ndisk(s) and network can keep up.  Generally p4 is going to be\nthe bottleneck.\n\nI think writing data to fast-import is much easier than running\nthe raw Git commands, especially when you are talking about an\nimport engine where you need to set all of the special environment\nvariables for git-commit-tree or git-tag to do its job properly.\nIts a good tool that simply doesn't get enough use, partly because\nnobody is using it...\n\n-- \nShawn.\n"},{"id":"43939","messageId":"20070604055600.GE4507@spearce.org","threadId":"8376","inReplyTo":"200706031511.31157.simon@lst.de","subject":"Re: git-p4import.py robustness changes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-04T05:56:00Z","receivedAt":"2007-06-04T05:56:00Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Simon Hausmann <simon@lst.de> wrote:\n> On the topic of git integration with perforce, what are the chances of getting \n> git-p4 ( http://repo.or.cz/w/fast-export.git ) into git's contrib/fast-export \n> area? :)\n> \n> git-p4 can do everything git-p4import can do plus a lot more (it can track \n> multiple branches, it's a hell of a lot faster, it can export back to p4 and \n> it also works on Windows!).\n\nI was sort of hoping we could fold the fast-export Git repository\non repo.or.cz into core Git at some point.  Right now the only\nthing in contrib/fast-export is the import-tars.perl script that\nI maintain in my fastimport repository...  ;-)\n\nLike Junio I don't use Perforce, and can't test against it, but\nif you can maintain git-p4 (and I think the history on repo.or.cz\nshows that you do) then it may be a good idea to add it to core Git.\n\nSend a patch to add it.  Worst that happens is both Junio and I\ndecide not to apply it.  Or I apply it, but Junio refuses to pull\nfrom me afterwards.  ;-)\n\n-- \nShawn.\n"},{"id":"43943","messageId":"56b7f5510706032309w4aee791dnd3bf5d46974bdaba@mail.gmail.com","threadId":"8376","inReplyTo":"20070604055433.GD4507@spearce.org","subject":"Re: git-p4import.py robustness changes","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-06-04T06:09:12Z","receivedAt":"2007-06-04T06:09:12Z","isPatch":false,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 6/3/07, Shawn O. Pearce <spearce@spearce.org> wrote:\n> I think writing data to fast-import is much easier than running\n> the raw Git commands, especially when you are talking about an\n> import engine where you need to set all of the special environment\n> variables for git-commit-tree or git-tag to do its job properly.\n> Its a good tool that simply doesn't get enough use, partly because\n> nobody is using it...\n\nWell,  perhaps they use it *once*,  in that they write a wrapper script for\nit and then forget about it.  At least that's what I did.  And the _only_\nannoyance was the trailing NL requirement on the delimited \"data\" statement,\nso you don't get much noise/complaints when people use it.\n\nThanks,\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"},{"id":"43944","messageId":"20070604061859.GG4507@spearce.org","threadId":"8376","inReplyTo":"56b7f5510706032309w4aee791dnd3bf5d46974bdaba@mail.gmail.com","subject":"Re: git-p4import.py robustness changes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-04T06:18:59Z","receivedAt":"2007-06-04T06:18:59Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Dana How <danahow@gmail.com> wrote:\n> On 6/3/07, Shawn O. Pearce <spearce@spearce.org> wrote:\n> >I think writing data to fast-import is much easier than running\n> >the raw Git commands, especially when you are talking about an\n> >import engine where you need to set all of the special environment\n> >variables for git-commit-tree or git-tag to do its job properly.\n> >Its a good tool that simply doesn't get enough use, partly because\n> >nobody is using it...\n> \n> Well,  perhaps they use it *once*,  in that they write a wrapper script for\n> it and then forget about it.  At least that's what I did.  And the _only_\n> annoyance was the trailing NL requirement on the delimited \"data\" statement,\n> so you don't get much noise/complaints when people use it.\n\nTrue.  I did try to make fast-import take a simple enough format\nthat you could write throwaway code against it, run it, and never\nlook back...\n\nThe trailing NL after data was because of cvs2svn.  The SVN dump\nfile format apparently does something like this, and the version\nof cvs2svn that Jon Smirl was working on output that trailing NL.\nAccepting it in fast-import was easier than fixing cvs2svn to not\ncreate it.\n\nI'll admit the error handling in fast-import could probably\nbe easier, and that NL after data probably could be optional.\nI don't think the input stream parser needs it to understand what\nis going on.  Its just sheer laziness on my part that the code\nrequires it there.\n\n-- \nShawn.\n"},{"id":"43948","messageId":"99C09A45-EACF-43C0-8EF6-85450B109BF6@slamb.org","threadId":"8376","inReplyTo":"20070604055433.GD4507@spearce.org","subject":"Re: git-p4import.py robustness changes","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-04T07:19:56Z","receivedAt":"2007-06-04T07:19:56Z","isPatch":false,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"\nOn Jun 3, 2007, at 10:54 PM, Shawn O. Pearce wrote:\n\n> I think writing data to fast-import is much easier than running\n> the raw Git commands, especially when you are talking about an\n> import engine where you need to set all of the special environment\n> variables for git-commit-tree or git-tag to do its job properly.\n> Its a good tool that simply doesn't get enough use, partly because\n> nobody is using it...\n\nYeah, I'm sold. I read git-p4 more thoroughly and tried it out...it's  \npretty nice. The P4Sync command has a simpler, more trustworthy flow  \nthan git-p4import.py.\n\nOn the Perforce side, I particularly like the use of \"p4 print\" to  \ngrab the files instead of \"p4 sync\". It avoids playing weird games  \nwith the client - I think nothing good can come of git-p4import.py's  \n\"p4 sync -k\" and symlinks to map multiple branches into the same  \ndirectory, which is not the Perforce way. Makes me nervous that  \nwhat's submitted to git won't be the same as what's in the Perforce  \ndepot.\n\nI would have thought launching a \"p4 print\" on each file would be  \nhorribly slow with the network latency of each request, but...well,  \napparently not.\n\nMaybe I'll work up git-p4 patches for subcommand error handling, like  \nmy git-p4import.py ones. And fix some style - seriously, who puts  \nsemicolons at the end of Python commands? *grumble*\n\nBest regards,\nScott\n\n-- \nScott Lamb <http://www.slamb.org/>\n"},{"id":"43959","messageId":"4663D03E.5040601@trolltech.com","threadId":"8376","inReplyTo":"839AEF71-ED29-4A79-BE97-C79EAFEDC466@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Marius Storm-Olsen","fromEmail":"marius@trolltech.com","sentAt":"2007-06-04T08:41:34Z","receivedAt":"2007-06-04T08:41:34Z","isPatch":false,"sender":{"key":"marius@trolltech.com","avatar":"https://gravatar.com/avatar/a40071d8f651862c6ab10bd7996f0ad84d94f06c0399de9e3fa4f06beb390a71?d=mp&s=160"},"body":"> git-p4import.py should work fine on Windows, too - the binary mode on  \n> the pipe should be all handled by \"subprocess\", and git-p4's  \n> data.replace(\"\\r\\n\", \"\\n\") is not necessary if you use \"LineEnd:  \n> unix\" or \"share\" in the Perforce client specification.\n\nThe problem is that you cannot set the LineEnd when using the 'p4 \nprint' command, since it doesn't use the client spec; so Perforce the \nuses the platform default when printing the file.\n\n> git-p4 seems to use \"git fast-import\". I guess the big performance\n> improvement there is removing the ls-files operation? So we're \n> talking about a 0-10% speedup, right? Plus some fork()/exec()\n> overhead.\n\nWith git-p4 the performance bottleneck is from what we can see the \nPerforce server, on non-Windows machines.\n\n-- \n.marius\n\n"},{"id":"44044","messageId":"200706050922.01431.simon@lst.de","threadId":"8376","inReplyTo":"99C09A45-EACF-43C0-8EF6-85450B109BF6@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-06-05T07:21:56Z","receivedAt":"2007-06-05T07:21:56Z","isPatch":false,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Monday 04 June 2007 09:19:56 Scott Lamb wrote:\n> On Jun 3, 2007, at 10:54 PM, Shawn O. Pearce wrote:\n> > I think writing data to fast-import is much easier than running\n> > the raw Git commands, especially when you are talking about an\n> > import engine where you need to set all of the special environment\n> > variables for git-commit-tree or git-tag to do its job properly.\n> > Its a good tool that simply doesn't get enough use, partly because\n> > nobody is using it...\n>\n> Yeah, I'm sold. I read git-p4 more thoroughly and tried it out...it's\n> pretty nice. The P4Sync command has a simpler, more trustworthy flow\n> than git-p4import.py.\n>\n> On the Perforce side, I particularly like the use of \"p4 print\" to\n> grab the files instead of \"p4 sync\". It avoids playing weird games\n> with the client - I think nothing good can come of git-p4import.py's\n> \"p4 sync -k\" and symlinks to map multiple branches into the same\n> directory, which is not the Perforce way. Makes me nervous that\n> what's submitted to git won't be the same as what's in the Perforce\n> depot.\n>\n> I would have thought launching a \"p4 print\" on each file would be\n> horribly slow with the network latency of each request, but...well,\n> apparently not.\n\nI've found it to be fast enough for \"standard software development\". When \nimporting big changes like integrations of an entire branch then it naturally \nslows down. The workaround me and my colleague have come up with is to \ncombine git-p4 usage with the regular git protocol:\n\nFor imports of simple projects from perforce the direct use of git-p4 clone \nand sync/rebase is good enough.\n\nFor big projects we have set up a dedicated (recycled old) machine that \ncontinuously imports from the perforce server. That makes the initial clone \nvery fast thanks to the use of the git protocol, it still allows imports from \nperforce afterwards and when the developer syncs the chances are very high \nthat the dedicated machine already imported the necessary changes/objects \nfrom the perforce server and the faster git protocol instead of \"p4 print\" on \na lot of files can be used.\n\nIn order to avoid that machine constantly polling the p4 server we've come up \nwith a neat little trick by adding a change-commit trigger on the p4 server \nthat consists of a little perl script that just sends a single udp packet \nwith the latest change number as notification to the git machine, which upon \nreception imports then.\n\nThat is why git-p4 sync/rebase call \"git fetch\" by default (configurable \nthrough config key) if there is an origin remote present.\n\n> Maybe I'll work up git-p4 patches for subcommand error handling, like\n> my git-p4import.py ones. And fix some style - seriously, who puts\n> semicolons at the end of Python commands? *grumble*\n\nI'd be more than happy to apply style patches. I'm not a very experienced \npython programmer and I admit that I certainly lack the style there :)\n\nSimon\n"},{"id":"44875","messageId":"200706122347.00696.simon@lst.de","threadId":"8376","inReplyTo":"20070604055600.GE4507@spearce.org","subject":"Re: git-p4import.py robustness changes","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-06-12T21:46:56Z","receivedAt":"2007-06-12T21:46:56Z","isPatch":false,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Monday 04 June 2007 07:56:00 Shawn O. Pearce wrote:\n> Simon Hausmann <simon@lst.de> wrote:\n> > On the topic of git integration with perforce, what are the chances of\n> > getting git-p4 ( http://repo.or.cz/w/fast-export.git ) into git's\n> > contrib/fast-export area? :)\n> >\n> > git-p4 can do everything git-p4import can do plus a lot more (it can\n> > track multiple branches, it's a hell of a lot faster, it can export back\n> > to p4 and it also works on Windows!).\n>\n> I was sort of hoping we could fold the fast-export Git repository\n> on repo.or.cz into core Git at some point.  Right now the only\n> thing in contrib/fast-export is the import-tars.perl script that\n> I maintain in my fastimport repository...  ;-)\n>\n> Like Junio I don't use Perforce, and can't test against it, but\n> if you can maintain git-p4 (and I think the history on repo.or.cz\n> shows that you do) then it may be a good idea to add it to core Git.\n>\n> Send a patch to add it.  Worst that happens is both Junio and I\n> decide not to apply it.  Or I apply it, but Junio refuses to pull\n> from me afterwards.  ;-)\n\nOk, I'll give it a try :)\n\nI've used git-filter-branch to rewrite the history in fast-export to include \nonly changes relevant to git-p4 and at the same time move all files into \ncontrib/fast-import. The result is available as separate branch at\n\n\tgit://repo.or.cz/fast-export.git git-p4\n\nand technically merges fine into git.git's contrib/fast-import directory with \nthree files (git-p4, git-p4.txt and git-p4.bat for windows convenience).\n\nPlease let me know if there's anything missing or if you prefer a different \nformat or so. I also realized that I haven't really used the 'Signed-off-by' \ntags in the past but I'd be happy to adopt it for git inclusion if you prefer \nthat :)\n\n\n_If_ one of you decides to pull then my plan is to discontinue the git-p4 \nbranch in the fast-export repository and instead work in a git.git fork on \nrepo.or.cz (similar to the fastimport repository).\n\n\nSimon\n"},{"id":"45003","messageId":"46705C69.8000500@slamb.org","threadId":"8376","inReplyTo":"200706122347.00696.simon@lst.de","subject":"Re: git-p4import.py robustness changes","fromName":"Scott Lamb","fromEmail":"slamb@slamb.org","sentAt":"2007-06-13T21:06:49Z","receivedAt":"2007-06-13T21:06:49Z","isPatch":false,"sender":{"key":"slamb@slamb.org","avatar":null},"body":"Simon Hausmann wrote:\n> _If_ one of you decides to pull then my plan is to discontinue the git-p4 \n> branch in the fast-export repository and instead work in a git.git fork on \n> repo.or.cz (similar to the fastimport repository).\n\nSo you'll continue maintain this code and others should submit changes \nthrough you? What is the best way to do so? (Not sure what it was \nbefore, or if it would change under this plan.) Email a format-patch To: \nyou? Cc: this list? some other list? no list?\n\n-- \nScott Lamb <http://www.slamb.org/>\n"},{"id":"45011","messageId":"200706140034.45968.simon@lst.de","threadId":"8376","inReplyTo":"46705C69.8000500@slamb.org","subject":"Re: git-p4import.py robustness changes","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-06-13T22:34:42Z","receivedAt":"2007-06-13T22:34:42Z","isPatch":false,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Wednesday 13 June 2007 23:06:49 Scott Lamb wrote:\n> Simon Hausmann wrote:\n> > _If_ one of you decides to pull then my plan is to discontinue the git-p4\n> > branch in the fast-export repository and instead work in a git.git fork\n> > on repo.or.cz (similar to the fastimport repository).\n>\n> So you'll continue maintain this code and others should submit changes\n> through you? What is the best way to do so? (Not sure what it was\n> before, or if it would change under this plan.) Email a format-patch To:\n> you? Cc: this list? some other list? no list?\n\nI would say whichever you prefer :)\n\nFor the fast-export repository multiple people have access and for example \nafter Han-Wen made a lot of patches I asked Chris Lee (owner of the module on \nrepo.or.cz) to add Han-Wen to the list of people with push access and he \npushed his changes directly. I actually like working that way for a project \nthat is as simple as that, I don't mind if somebody pushes simple changes \ndirectly as much as I like discussing bigger plans if they potentially clash \nwith somebody else's work or use-case.\n\nSo if git-p4 continues to live in fast-export I'll continue to encourage Chris \nLee to give git-p4 contributors push access, if it's in a git.git fork I'd be \nhappy to do so myself (give access).\n\nIf git-p4 also ends up in git/contrib/fastimport and somebody likes to send \npatches to Junio or somebody else and CC this list that's fine with me, too.\n\nIt's just a few lines of python code after all ;-)\n\nSimon\n"},{"id":"45040","messageId":"20070614053538.GA6073@spearce.org","threadId":"8376","inReplyTo":"200706122347.00696.simon@lst.de","subject":"Re: git-p4import.py robustness changes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-14T05:35:38Z","receivedAt":"2007-06-14T05:35:38Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Simon Hausmann <simon@lst.de> wrote:\n> I've used git-filter-branch to rewrite the history in fast-export to include \n> only changes relevant to git-p4 and at the same time move all files into \n> contrib/fast-import. The result is available as separate branch at\n> \n> \tgit://repo.or.cz/fast-export.git git-p4\n> \n> and technically merges fine into git.git's contrib/fast-import directory with \n> three files (git-p4, git-p4.txt and git-p4.bat for windows convenience).\n> \n> Please let me know if there's anything missing or if you prefer a different \n> format or so. I also realized that I haven't really used the 'Signed-off-by' \n> tags in the past but I'd be happy to adopt it for git inclusion if you prefer \n> that :)\n\nYes.  The SBO line is your assertion that you own the rights to the\ncode and can release it under the license you are offering it under.\nOne of the issues I have with this git-p4 history you have built\nis the lack of the SBO line on all 255 commits.\n\nOf course an SBO line doesn't carry that much weight, its just a line\nafter all, but according to Git's project standards it should be there\nif you are agreeing to release it.  See Documentation/SubmittingPatches\nfor details.\n\nMy other problem with this history is a commit like b79112 \"a\nlittle bit more convenience\" (and there are many such commits).\nThis message is insanely short, doesn't really talk at all about\nwhat a little bit is, how it is more convenient, or who it is more\nconvenient for.\n\nThink about how that oneline (and the others) would look in Junio's\n\"What's new in git.git\" emails, or in gitweb.  There is not enough\ndetail here to be of any value to the reader.  Expanding out to the\nfull message offers nothing additional either, because that is all\nthere is in the entire commit message body.\n\nI do appreciate you taking the time to use filter-branch to try to\ncleanup this history a bit.  I really had originally planned on\npulling your tree through to my fastimport tree and then talking\nJunio into merging with me.  But after reading through this history I\ndon't want do that, because of the oneline summaries I just pointed\nout above, and because of the missing SBO.\n \n-- \nShawn.\n"},{"id":"45094","messageId":"200706142344.29089.simon@lst.de","threadId":"8376","inReplyTo":"20070614053538.GA6073@spearce.org","subject":"Re: git-p4import.py robustness changes","fromName":"Simon Hausmann","fromEmail":"simon@lst.de","sentAt":"2007-06-14T21:44:25Z","receivedAt":"2007-06-14T21:44:25Z","isPatch":false,"sender":{"key":"hausmann@kde.org","avatar":"https://gravatar.com/avatar/bc9aad4fb31dce17eb66e690e7b51fe980c62da3c225c785da35dd806b8da778?d=mp&s=160"},"body":"On Thursday 14 June 2007 07:35:38 Shawn O. Pearce wrote:\n> Simon Hausmann <simon@lst.de> wrote:\n> > I've used git-filter-branch to rewrite the history in fast-export to\n> > include only changes relevant to git-p4 and at the same time move all\n> > files into contrib/fast-import. The result is available as separate\n> > branch at\n> >\n> > \tgit://repo.or.cz/fast-export.git git-p4\n> >\n> > and technically merges fine into git.git's contrib/fast-import directory\n> > with three files (git-p4, git-p4.txt and git-p4.bat for windows\n> > convenience).\n> >\n> > Please let me know if there's anything missing or if you prefer a\n> > different format or so. I also realized that I haven't really used the\n> > 'Signed-off-by' tags in the past but I'd be happy to adopt it for git\n> > inclusion if you prefer that :)\n>\n> Yes.  The SBO line is your assertion that you own the rights to the\n> code and can release it under the license you are offering it under.\n> One of the issues I have with this git-p4 history you have built\n> is the lack of the SBO line on all 255 commits.\n>\n> Of course an SBO line doesn't carry that much weight, its just a line\n> after all, but according to Git's project standards it should be there\n> if you are agreeing to release it.  See Documentation/SubmittingPatches\n> for details.\n>\n> My other problem with this history is a commit like b79112 \"a\n> little bit more convenience\" (and there are many such commits).\n> This message is insanely short, doesn't really talk at all about\n> what a little bit is, how it is more convenient, or who it is more\n> convenient for.\n>\n> Think about how that oneline (and the others) would look in Junio's\n> \"What's new in git.git\" emails, or in gitweb.  There is not enough\n> detail here to be of any value to the reader.  Expanding out to the\n> full message offers nothing additional either, because that is all\n> there is in the entire commit message body.\n>\n> I do appreciate you taking the time to use filter-branch to try to\n> cleanup this history a bit.  I really had originally planned on\n> pulling your tree through to my fastimport tree and then talking\n> Junio into merging with me.  But after reading through this history I\n> don't want do that, because of the oneline summaries I just pointed\n> out above, and because of the missing SBO.\n\nFirst of all thanks for looking at the branch. I agree with your concerns and \nI do admit that I've been a bit too sloppy with the log messages.\n\nI have started cleaning up the history even more by reworking the log messages \nof my commits (git-p4-enhanced-logs branch in fast-export, starting at the \nlast page). Once that is done (I expect that to take a few days) I'll add the \nmissing SOB lines with git-filter-branch and see if I can get an agreement \nfrom Han-Wen and Marius for doing the same with their commits (adding the \nmissing lines).\n\nWould you be willing to reevaluate the situation regarding a merge once that's \ndone?\n\n\nSimon\n"},{"id":"45105","messageId":"20070615031338.GB18491@spearce.org","threadId":"8376","inReplyTo":"200706142344.29089.simon@lst.de","subject":"Re: git-p4import.py robustness changes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-06-15T03:13:39Z","receivedAt":"2007-06-15T03:13:39Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Simon Hausmann <simon@lst.de> wrote:\n> On Thursday 14 June 2007 07:35:38 Shawn O. Pearce wrote:\n> > I do appreciate you taking the time to use filter-branch to try to\n> > cleanup this history a bit.  I really had originally planned on\n> > pulling your tree through to my fastimport tree and then talking\n> > Junio into merging with me.  But after reading through this history I\n> > don't want do that, because of the oneline summaries I just pointed\n> > out above, and because of the missing SBO.\n...\n> I have started cleaning up the history even more by reworking the log messages \n> of my commits (git-p4-enhanced-logs branch in fast-export, starting at the \n> last page). Once that is done (I expect that to take a few days) I'll add the \n> missing SOB lines with git-filter-branch and see if I can get an agreement \n> from Han-Wen and Marius for doing the same with their commits (adding the \n> missing lines).\n\nOK.\n \n> Would you be willing to reevaluate the situation regarding a merge once that's \n> done?\n\nAbsolutely.  I would like to see the git-p4 work in the main tree,\nso it is more readily available to users, even though I'm not a p4\nuser myself.  ;-)\n\n-- \nShawn.\n"},{"id":"45120","messageId":"467223FF.6010701@storm-olsen.com","threadId":"8376","inReplyTo":"200706142344.29089.simon@lst.de","subject":"Re: git-p4import.py robustness changes","fromName":"Marius Storm-Olsen","fromEmail":"marius@storm-olsen.com","sentAt":"2007-06-15T05:30:39Z","receivedAt":"2007-06-15T05:30:39Z","isPatch":false,"sender":{"key":"marius@storm-olsen.com","avatar":"https://avatars.githubusercontent.com/u/1500?v=4"},"body":"Simon Hausmann said the following on 14.06.2007 23:44:\n> First of all thanks for looking at the branch. I agree with your\n> concerns and I do admit that I've been a bit too sloppy with the\n> log messages.\n> \n> I have started cleaning up the history even more by reworking the\n> log messages of my commits (git-p4-enhanced-logs branch in\n> fast-export, starting at the last page). Once that is done (I\n> expect that to take a few days) I'll add the missing SOB lines with\n> git-filter-branch and see if I can get an agreement from Han-Wen\n> and Marius for doing the same with their commits (adding the \n> missing lines).\n\nSimon,\n\nOf course! Go right ahead and add the SOB for my commits while you're \nat it.\n\n-- \n.marius\n\n"}]}