Subject: Re: [PATCH 2/3] add new Git::Repo API

From: Petr Baudis <[email-masked]>
Date: Fri, 18 Jul 2008 19:17:33 +0200

On Fri, Jul 18, 2008 at 07:05:05PM +0200, Jakub Narebski wrote:
> Dnia piątek 18. lipca 2008 18:48, Petr Baudis napisał:
> > On Tue, Jul 15, 2008 at 12:19:31AM +0200, Lea Wiemann wrote:
> 
> > > No, you should never pass in an invalid SHA1 in the first place.  The
> > > above piece of documentation is just a warning that bugs will show up
> > > delayed.  IOW, this is not the right place to have your error handling.
> > > 
> > > If you're getting a SHA1 through the user-interface, check its existence
> > > with get_sha1 before passing it to the constructor.
> > 
> > But that's an expensive operation, you need extra Git exec for this,
> > while all the Git commands can do the checks for you, if you give them
> > the chance.
> > 
> > I was doing pretty much this thing in Cogito (initially out of
> > necessity) and it made it ungodly slow for any kind of batch operations.
> 
> Lea probably would point out that thanks to '--batch-check' option
> to git-cat-file, and "reuse => 1" option to ->get_bidi_pipe you would
> need only one extra Git exec...  BUT it is one extra Git exec per
> entire Perl script (one per request, for example, for gitweb).
> And it wouldn't help batching Perl script commands.

Even so, I don't like this restriction simply since it makes the usage
more complicated - in my commandline tool, I will be forced to write

	my $csha1 = $repo->get_sha1($ARGV[0]);
	$csha1 or die "a random inconsistent error message";
	$repo->get_commit($csha1)

instead of

	$repo->get_commit($ARGV[0]) # die if unresolved

and I don't understand why. Me and 80% of the scripts don't *care* about
some more graceful error handling, and if gitweb and the other 20%
(or less) do, *they* should do the extra work, not me.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie