Re: [PATCH v2 10/20] git-remote-testgit: fix error handling

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 19, 2011, 22:58 UTC
Message-ID: <20110619225810.GG23893@elie>
In-Reply-To: <1308496725-22329-11-git-send-email-srabbelier@gmail.com>

Hi,

Sverre Rabbelier wrote:

> If fast-export did not complete successfully the error handling code
> itself would error out.

That sounds like a problem indeed.  What error message does it produce?

> This was broken in commit 23b093ee0 (Brandon Casey, Wed Jun 9 2010,
> Remove python 2.5'isms). Revert that commit an introduce our own copy
> of check_call in util.py instead.

The "an" should probably be "and". More importantly, it's probably worth mentioning that this is only a partial revert or rephrasing some other way.

Have you checked if this still works with python 2.4? Cc-ing Brandon in case he has advice.

[...]

> +++ b/git_remote_helpers/git/exporter.py

[...]

Show 8 quoted lines

> @@ -53,6 +55,4 @@ class GitExporter(object):
>  
>          args = ["sed", "s_refs/heads/_" + self.repo.prefix + "_g"]
>  
> -        child = subprocess.Popen(args, stdin=p1.stdout)
> -        if child.wait() != 0:
> -            raise CalledProcessError
> +        check_call(args, stdin=p1.stdout)

[...]

Show 7 quoted lines

> +++ b/git_remote_helpers/util.py
> @@ -128,6 +128,38 @@ def run_command (args, cwd = None, shell = False, add_env = None,
>      return (exit_code, output, errors)
>  
>  
> +# from python2.7:subprocess.py
> +def call(*popenargs, **kwargs):

[...]

Show 5 quoted lines

> +    return subprocess.Popen(*popenargs, **kwargs).wait()
> +
> +
> +# from python2.7:subprocess.py
> +def check_call(*popenargs, **kwargs):

[...]

Show 7 quoted lines

> +    retcode = call(*popenargs, **kwargs)
> +    if retcode:
> +        cmd = kwargs.get("args")
> +        if cmd is None:
> +            cmd = popenargs[0]
> +        raise subprocess.CalledProcessError(retcode, cmd)
> +    return 0

So IIUC the existing code is not providing arguments to the CalledProcessError constructor, and your patch fixes that. Good.

Based on <http://pydoc.org/2.4.1/subprocess.html>, Python 2.4 doesn't seem to have a CalledProcessError type. Maybe these code paths weren't being exercised before.

Regards, Jonathan