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

[Jonathan Nieder <jrnieder@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
[...]
> @@ -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)
[...]
> +++ 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):
[...]
> +    return subprocess.Popen(*popenargs, **kwargs).wait()
> +
> +
> +# from python2.7:subprocess.py
> +def check_call(*popenargs, **kwargs):
[...]
> +    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