Re: git-p4import.py robustness changes

From: Junio C Hamano <junkio@cox.net>
Date: May 31, 2007, 23:53 UTC
Message-ID: <7vbqg01reo.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <4ACE2ABC-8D73-4097-87AC-F3B27EDA97DE@slamb.org>

Scott Lamb <slamb@slamb.org> writes:

> There's a logfile option, but that's a poor excuse for no error
> handling. I'd like to fix it. A couple questions, though:

Good to have somebody who has access to p4.

Show 5 quoted lines

> First, is it acceptable to switch from os.popen to the subprocess
> module? I ask because the latter was only introduced with Python 2.4
> on. The subprocess module does work with earlier versions of Python
> (definitely 2.3) and is GPL-compatible, so maybe it could be thrown
> into the distribution if desired.

We actually did ship with our own copy after clearing the licensing situation with Python people, although we removed it when it lost the last script that used it. I do not think resurrecting it is a problem.

Show 14 quoted lines

> Second, this crowd seems to want sequences of tiny patches. How does
> this sound?
>
> * patch 1 - use subprocess to make git_command.git() and p4_command.p4
> () throw properly-typed exceptions on error, fix caller exception
> handling to match.
>
> * patch 2 - remove the use of the shell and pipelines (fix some
> escaping problems).
>
> * patch 3 - use lists instead of space separation for the commandline
> arguments (fix more escaping problems).
>
> * patch 4 - allow grabbing partial history (make my error go away).

Actually, my preference is to have a "patch 0" before all of the above, that demotes git-p4import to contrib/ hierarchy. Having no access to p4 managed repositories (nor much inclination to get one), I can never test nor maintain it myself, so it is just crazy for me to be the maintainer for it.

But I do read Python and speak it passably -- the above 4 step outline sounds sane to me.