--- On Tue, 6/3/08, Jakub Narebski <jnareb@xxxxxxxxx> wrote: > From: Jakub Narebski <jnareb@xxxxxxxxx> > Subject: Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame > To: "Rafael Garcia-Suarez" <rgarciasuarez@xxxxxxxxx> > Cc: git@xxxxxxxxxxxxxxx, "Luben Tuikov" <ltuikov@xxxxxxxxx> > Date: Tuesday, June 3, 2008, 4:43 AM > Cc-ed Luben Tuikov, author of this part. Thanks guys! :-) > Rafael Garcia-Suarez <rgarciasuarez@xxxxxxxxx> > writes: > > > git-rev-parse will abort with an error when passed a > non-existent > > revision spec, such as "deadbeef^" where > deadbeef has no parent. Yes, I've known about this ever since I coded this. The reasoning was that the value of parsing up the "tree" of parent changes was a lot more (valuable) than the value of detecting that "deadbeef^" had no parent -- which would've been logically apparent to the coder/reviewer/user of "blame2". > > Using the --revs-only parameter makes this error go > away, while > > retaining functionality, keeping the web server error > log nice > > and clean. Ok, that's fine, as long as indeed the functionality is preserved. I leave it up to Jakub and Junio to make sure that indeed the functionality is preserved. (I.e. saving us yet another patch from anyone of us.) > Thanks. This error wasn't detected earlier probably > because > 'blame' view is rarely enabled; and repo.or.cz > gitweb which has > 'blame' enabled IIRC use gitweb which is modified > there, allowing > incremental blame using AJAX. I'm a heavy user of "blame2", often exploring the course of "evolution" of the code and/or code lines and segments. I value this in gitweb (being easier/more visual to use as opposed to command line). > > > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl > > index 55fb100..f3b4b24 100755 > > --- a/gitweb/gitweb.perl > > +++ b/gitweb/gitweb.perl > > @@ -4226,9 +4226,9 @@ git_blame2 > > esc_html($rev)); > > print "</td>\n"; > > } > > - open (my $dd, "-|", git_cmd(), > "rev-parse", "$full_rev^") > > + open (my $dd, "-|", git_cmd(), > "rev-parse", '--revs-only', > "$full_rev^") > > or die_error(undef, "Open git-rev-parse > failed"); > > - my $parent_commit = <$dd>; > > + my $parent_commit = <$dd> || ''; > > close $dd; > > chomp($parent_commit); > > my $blamed = href(action => 'blame', > > I'd rather remove this, correct it, or make it optional > (this is very > fork-heavy). > > But this patch is good as it is now... Yes, I agree it is good. Just you guys make sure that it doesn't change the value of the functionality of the code. Thanks everyone! Luben -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
