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

From: Junio C Hamano <junkio@cox.net>
Date: Aug 6, 2006, 09:26 UTC
Message-ID: <7v8xm2b4g3.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200608060211.38604.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 8 quoted lines

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

You obviously would need to emulate the -L$from_name -L$to_name part by hand in the loop that reads from the diff output.

One worrysome pathological case is what happens if files being tracked happen to have names that consist of 40-byte hexdigits. I think "git-diff $from $to --" should do the right thing for gitweb use ($from and $to should not mistaken taken as pathnames with the explicit -- disambiguator) and "git-diff -- $from $to" also should do the right thing to take $from and $to as pathnames even if there are revisions with the same object names, but somebody may want to verify that (and send patches to fix if it is not the case).