Re: [PATCH] gitweb: Do not show 'patch' link in 'commit' view for merges

From: Jakub Narebski <jnareb@gmail.com>
Date: Oct 1, 2009, 07:36 UTC
Message-ID: <200910010936.24789.jnareb@gmail.com>
In-Reply-To: <20091001031140.GA30094@coredump.intra.peff.net>

Dnia czwartek 1. października 2009 05:11, Jeff King napisał:

Show 13 quoted lines

> On Wed, Sep 30, 2009 at 10:21:53PM +0200, Jakub Narebski wrote:
> 
> > Show 'patch' link in the 'commit' view only for commits which have
> > exactly one parent, otherwise call to git-format-patch would fail for
> > the hyperlinked 'patch' action.
> 
> Fail in what way? From my cursory reading of the code, it looks like the
> 'patch' action calls into git_commitdiff, which handles the multi-parent
> case.
> 
> I assume I'm reading wrong, since you obviously know gitweb much better
> than I do. :) But can you expand a little on the nature of the problem
> in the commit message?

Well, from the point of view of behavior, 'patch' link in 'commit' view
for a merge commit, e.g.
  gitweb.cgi/git.git/commit/833423ae071ffedb7fbca39789f14f9a45a3d1c4
leads to the 'patch' view
  git.git/patch/833423ae071ffedb7fbca39789f14f9a45a3d1c4
which leads to 'text/plain' output with the following contents:

  Reading git-format-patch failed

From the point of view of code, 'patch' view is handled by git_patch() subroutine, which in turn calls git_commitdiff(-format => 'patch', -single=> 1); git_commitdiff checks if 'patch' view is enabled feature, and then composes and calls the following command (I have skipped --git-dir=...):

  git format-patch --encoding=utf8 --stdout -1 --root <commit-id>

And git-format-patch produces no output for merge commit. Then git_commitdiff dumps output of git-format-patch

	local $/ = undef;
	print <$fd>;

and somehow fails on closing filehandle

	close $fd
		or print "Reading git-format-patch failed\n";

Even if 'patch' view didn't fail, it is not a good idea to have link to an empty page (or page with only error message). Though probably git_commitdiff could check if it is used for a merge commit...

-- 
Jakub Narebski
Poland