Re: gitweb: buggy 'commitdiff_plain' output
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Jul 10, 2009, 17:33 UTC
- Message-ID
- <200907101933.39424.jnareb@gmail.com>
- In-Reply-To
- <alpine.LFD.2.01.0907100941060.3352@localhost.localdomain>
On Fri, 10 July 2009, Linus Torvalds wrote:
> > I complained to the CIFS people about their crazy duplicated commit > message headers: see for example
[...]
> It turns out that that duplication is because they use gitweb as a strange > patch distribution system (rather than emailing each other patches), and > download the 'commitdiff_plain' version of the diff and then apply it with > 'git am -s'.
First question: do they use gitweb from git.git repository, or a custom fork of gitweb (like git.kernel.org gitweb, which has caching, but IIRC does not have all new gitweb features)?
Show 6 quoted lines
> Ok, so it's a really odd way of doing things, but hey, that gitweb feature > clearly does try to support that exact workflow, or why would that > commitdiff_plain output try to look like an email? > > But gitweb is a totally buggy piece of trash when it comes to exporting > commits that way.
Actually the problem is deeper; both 'commitdiff' and 'commitdiff_plain' try to do two things at once: an equivalent of "git show <commit>" _and_ equivalent of "git diff <commit 1> <commit 2>"... and they fail.
> > Why? Because it first has a 'Subject:' line, and then the "body" of the > email repeats the raw commit message output. So _of_course_ you get the > header duplicated.
Yes, this is bug in 'commitdiff_plain' output. Perhaps it should mimic '--pretty=email' format better. Perhaps it shouldn't mimic email format at all, but be more like "git show <commit>".
THE SOLUTION is to use 'patch' or 'patches' view, introduced by Giuseppe Bilotta on 18 Dec 2008 in series of commits from 9872cd6f to 75bf2cb2. It uses git-format-patch, and can be fed directly to git-am. Problem solved.
Show 14 quoted lines
> Now, I asked Steve to not use gitweb (or edit the result some way), but > this really is a gitweb bug. And since I don't do perl, I can't fix it, > even though I can pinpoint exactly where the bug is (lines 5732 - 5752 in > gitweb/gitweb.perl). > > I totally untested patch written by a monkey who doesn't actually do perl > is appended as a purely theoretical pointer in the right direction. But I > really have no clue about perl, so what the heck do I know? This is like > my tcl programming - pattern matching rather than any real understanding. > > I'm sure there are smarter ways to do this with some simple mapping > function or whatever. > > Linus
I'd have to examine closer what git_commitdiff_plain does, and what parse_commit_text does to extract subject from commit message. With the caveat that perhaps it would be better to get rid of mail-like 'commitdiff_plain' and mimic git-show output...
Show 30 quoted lines
> ---
> gitweb/gitweb.perl | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 6a1b5b5..a809768 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5729,6 +5729,7 @@ sub git_commitdiff {
> }
>
> } elsif ($format eq 'plain') {
> + my $show = false;
> my $refs = git_get_references("tags");
> my $tagname = git_get_rev_name_tags($hash);
> my $filename = basename($project) . "-$hash.patch";
> @@ -5747,7 +5748,11 @@ sub git_commitdiff {
> print "X-Git-Url: " . $cgi->self_url() . "\n\n";
>
> foreach my $line (@{$co{'comment'}}) {
> - print to_utf8($line) . "\n";
> + if ($show) {
> + print to_utf8($line) . "\n";
> + } else if ($line ne "") {
> + $show = true;
> + }
> }
> print "---\n\n";
> } elsif ($format eq 'patch') {
> -- Jakub Narebski Poland