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

On Fri, Jul 18, 2008 at 07:05:05PM +0200, Jakub Narebski wrote:

Dnia piątek 18. lipca 2008 18:48, Petr Baudis napisał:

quoted

On Tue, Jul 15, 2008 at 12:19:31AM +0200, Lea Wiemann wrote:

quoted

quoted

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