Ok, I finished going through the code that changed between 0.3.2 and 0.3.2.1 and added comments to https://review.openstack.org/#/c/136281/ which should fix things.
Cheers, Mike ________________________________________ From: Clark Boylan [[email protected]] Sent: 25 November 2014 17:16 To: [email protected] Subject: Re: [OpenStack-Infra] [openstack-infra] [zuul] GitPython 0.3.2.1 fixes 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@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@TARDIS:~/code/zuul$ echo $? > 128 > [3] https://review.openstack.org/#/c/136281/ > _______________________________________________ > OpenStack-Infra mailing list > [email protected] > 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 _______________________________________________ OpenStack-Infra mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra _______________________________________________ OpenStack-Infra mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
