Re: [PATCH 2/6] gitweb: Simplify git_diff_print

[Jakub Narebski <jnareb@gmail.com>]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Copy to temporaty file more directly, not using temporary variable @file.
>> Use list form of open for diff invocation (we cannot use git-diff because
>> first it doesn't support -L/--label option, and we cannot generate diff
>> between /dev/null and blob given by it's sha1 identifier). 
>>
>> Use "local $/ = undef;" for (temporary) slurp mode.
>>
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> 
> I think this part of the gitweb code predates the git-diff
> support by the core level.  If I recall correctly, back then we
> did not even have "git-diff-{tree,files,index}" when this part
> was written.
> 
> I think you could use "git-diff $from $to" instead of using
> temporary files and running /usr/bin/diff on it with today's
> git.  Which is a big win from both security and ease of
> administration point of view.  Not having to write anything into
> temporary file from gitweb.cgi means you do not have to worry
> about stray gitweb process leaving temporary files dangling
> behind, for example.

If I remember gitweb code correctly, diff to /dev/null is called
_only_ in commitdiff, where we can use git-diff -p to generate patch
directly, instead of generating difftree and calling diff on entries.

Still it would be nice if git-diff could diff between blob and arbitrary
file on filesystem (including /dev/null), and acquired -L/--label option.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git