Open Stack

On Tue, Nov 25, 2014, at 06:55 AM, Heald, Mike wrote:
> Hi all,
> 
> I've been investigating why the tests break with
> GitPython 0.3.2.1, and I've tracked the problem down
> to a commit in GitPython, but I'm not sure where
> the fix should go.
> 
> In GitPython, commit ff13922f [1], a call to finalize_process
> was added into the Repo class. finalize_process
> is there to make sure the call to the git binary is
> shut down cleanly, but it also picks up exit code 128
> and re-raises the GitCommandError exception if the
> binary exited with that return code, otherwise it suppresses
> the exception. The comments state that return code 128
> is used if there was a connection error, so it makes sense
> that we'd raise an exception then. However, 128 is also
> returned if you ask for a rev-list for a ref that does
> not exist locally [2], which we legitimately do in zuul (at least
> in the tests), but because of that added call, zuul
> acts as if there was a connection error. That shouldn't
> happen.
> 
> I can think of a few options to fix this:
> 
> 1) Fix zuul so that we handle GitCommandError when we're
> checking out refs. There's a review open [3] that already
> fixes the tests, and it seems that we already handle
> GitCommandError in the main code when we know it
> could happen.
> 
> 2) Fix finalize_process somehow to only re-raise the
> exception when git is dealing with a remote source - 
> this seems very tricky as we'd need to inspect the original
> command used to spawn the process...
> 
> 3) Deal with the GitCommandError in GitPython, handling
> GitCommandError when we know we're not dealing with a
> remote source, or it doesn't make sense to raise a connection
> related error (like, when you're checking for local untracked files).
> 
> 4) Do 1 and 3.
> 
> What looks best? I'm keen to get this fixed :)
> 
> Cheers,
> Mike
> 
> [1] https://github.com/gitpython-developers/GitPython/commit/ff13922f
> [2] :
> mikeh at TARDIS:~/code/zuul$ git rev-list refs/i/do/not/exist
> fatal: ambiguous argument 'refs/i/do/not/exist': unknown revision or path
> not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
> mikeh at TARDIS:~/code/zuul$ echo $?
> 128
> [3] https://review.openstack.org/#/c/136281/
> _______________________________________________
> OpenStack-Infra mailing list
> OpenStack-Infra at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra

I think we should move ahead with 1 and potentially try 3. I am not sure
how easy it will be to get the fix into GitPython but we can give it a
go and as long as we also do 1 and handle these exceptions in Zuul then
getting the fix into GitPython isn't critical. The other benefit to
handling these cases in Zuul is we will support a larger number of
GitPython releases (we can exclude those that we don't work with but not
doing that is even better).

Clark