git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>
Cc: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>,
	Jakub Narebski <jnareb@gmail.com>
Subject: gitweb: buggy 'commitdiff_plain' output
Date: Fri, 10 Jul 2009 10:04:52 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0907100941060.3352@localhost.localdomain> (raw)


I complained to the CIFS people about their crazy duplicated commit 
message headers: see for example

    [torvalds@nehalem linux]$ git show --stat aeaaf253c4dee7ff9af2f3f0595f3bb66964e944
    commit aeaaf253c4dee7ff9af2f3f0595f3bb66964e944
    Author: Jeff Layton <jlayton@redhat.com>
    Date:   Thu Jul 9 01:46:39 2009 -0400
    
        cifs: remove cifsInodeInfo->inUse counter
        
        cifs: remove cifsInodeInfo->inUse counter
        
        It was purported to be a refcounter of some sort, but was never
        used that way. It never served any purpose that wasn't served equally well
        by the I_NEW flag.
        
        Signed-off-by: Jeff Layton <jlayton@redhat.com>
        Reviewed-by: Christoph Hellwig <hch@lst.de>
        Signed-off-by: Steve French <sfrench@us.ibm.com>
    
     fs/cifs/cifsfs.c   |    1 -
     fs/cifs/cifsglob.h |    1 -
     2 files changed, 0 insertions(+), 2 deletions(-)

and note the duplicated "cifs: remove cifsInodeInfo->inUse counter" line.

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'.

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.

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.

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

---
 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') {

             reply	other threads:[~2009-07-10 17:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-10 17:04 Linus Torvalds [this message]
2009-07-10 17:33 ` gitweb: buggy 'commitdiff_plain' output Jakub Narebski
2009-07-10 17:38   ` Giuseppe Bilotta
2009-07-10 21:05   ` Linus Torvalds
2009-07-10 21:50     ` Jakub Narebski
2009-07-10 17:34 ` Giuseppe Bilotta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.01.0907100941060.3352@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=giuseppe.bilotta@gmail.com \
    --cc=jnareb@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).