Re: [RFC] Git Perl bindings, and OO interface

From: Tom Lanyon <tom@netspot.com.au>
Date: Jul 10, 2009, 02:08 UTC
Message-ID: <E6AB02D1-CF72-4611-91B0-DA524081A2EE@netspot.com.au>
In-Reply-To: <200811270258.50898.jnareb@gmail.com>

On 27/11/2008, at 12:28 PM, Jakub Narebski wrote:

Show 10 quoted lines

> 0. One of points of disagreement between Git.pm and new Git::Repo was
>   using Error module for frontend error handling.  While the
>   explanation in http://www.perl.com/pub/a/2002/11/14/exception.html
>   is compelling, it is not standard Perl technique.  Additionally
>   adding "cmd_git_try { CODE } ERRORMSG" syntactic sugar was not very
>   good idea.
>
>   So the first thing I'd like to discuss: to use Error and try/catch,
>   or not in Perl interface (bindings) to Git?  I would really like to
>   hear from Perl experts / Perl hackers here...

Sorry to bring up an old thread - but there was no further discussion on this and I've recently run into some grief with Git.pm.

I'm new to Git, but not new to Perl and recently attempted to perform some simple operations over Git repositories from a Perl application (it needs to clone, push, checkout, merge and that's about it) and found the Error.pm style handling of errors unintuitive and annoying. It is currently fairly simple to capture errors into the application by wrapping git_cmd_try { CODE } ERROR into an eval {} block but this really only provides you with the command's exit status and no meaningful error messages to display to your users; not to mention it's fairly ugly.

A long standing Perl motto is 'There Is More Than One Way To Do It' and the use of Error.pm here forces developers down a specific path for error handling - some may like this, some may not, but there's not a lot they can do about it. I would suggest that the Perl way for Git.pm to handle errors is for its methods to return the standard 1 or 0 for success or failure and perhaps store some meaningful error messages in an accessor or variable. The module should also not die() if there's an error - leave this up to the users of the module to handle errors how they prefer - if it dies, we must wrap the methods in eval{} blocks or handle with $SIG{__DIE__}, making for some messy and ugly code.

I would love to be able to:

	my $repo = Git->repository( directory => '/some/repo' )
		or die "Unable to load git repo /some/repo: $Git::errstr";

	$repo->command( 'push', [ 'some-remote' ] )
		or die "Unable to push to origin: $Git::errstr";

... or similar, and have $Git::errstr set to something meaningful like the "fatal: 'some-remote': unable to chdir or not a git archive" returned by git-push. This also leads into some discussion around git commands printing to STDERR when there is no error -- example: if everything is fine and up to date, I don't need git-push to tell me "Everything up-to-date" in STDERR...

Hope this helps.

Regards, Tom