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

On Fri, Jul 10, 2009 at 7:04 PM, Linus
Torvalds<torvalds@linux-foundation.org> wrote:
>
> 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.

The smarter way is called the 'patches' ('patch' for single ones, e.g.
http://git.example.com/project.git/patch/master, whereas /patches/
would display the latest N with N configurable) view that just spouts
out the git format-patch output in a git-am friendly way. I'm pretty
sure my patches to implement that view are already upstream ... yes,
yes it is:

http://git.oblomov.eu/git/patches/9872cd..75bf2cb2

To understand the differences between this and commitdiff_plain, see

http://git.oblomov.eu/git/commitdiff_plain/9872cd..75bf2cb2

for comparison.

And yes, commitdiff is just TOTALLY not what you expect it to be,
ESPECIALLY with multiple commits, where it only displays the commit
message for the FIRST commit.

So tell the CIFS people to use the patch(es) view if they plan to use
gitweb for patch exchange (which I do all the time, and in fact was
the reason why I implemented this view in the first place).

-- 
Giuseppe "Oblomov" Bilotta

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

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-10 17:04 gitweb: buggy 'commitdiff_plain' output Linus Torvalds
2009-07-10 17:33 ` 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 [this message]

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=cb7bb73a0907101034v3e6d655btd28018d24ef6de63@mail.gmail.com \
    --to=giuseppe.bilotta@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /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).