Subject: Re: [PATCH 10/18] gitweb: Adding isBinaryAction() and isFeedAction() to determine the action type

From: Jakub Narebski <[email-masked]>
Date: Fri, 10 Dec 2010 13:25:15 +0100

On Fri, 10 Dec 2010, Jakub Narebski wrote:
> On Fri, 10 Dec 2010, J.H. wrote:
> 
>>>> This is fairly self explanatory, these are here just to centralize the checking
>>>> for these types of actions, as special things need to be done with regards to
>>>> them inside the caching engine.
>>>>
>>>> isBinaryAction() returns true if the action deals with creating binary files
>>>> (this needing :raw output)
>>> 
>>> Why do you need special case binary / :raw output?  It is not really
>>> necessary if it is done in right way, as shown in my rewrite.
>> 
>> Because that's not how my caching engine does it, and the reason for
>> that is I am mimicking how the rest of gitweb does it.
> 
> To shorten the explanation why treating binary (needing :raw) output in
> a special way is not necessary: with the way gitweb code is structured
> (with "binmode STDOUT, ':raw'" inside action subroutine), with the way
> capturing output is done (by redirecting STDOUT), and even with the way
> kernel.org caching code is structured the only thing that needs to be
> done to support both text (:utf8, as set at beginning of gitweb) and
> binary (:raw) output is to *dump cache to STDOUT in binary mode*:
> 
> 	binmode $cache_fh, ':raw';
> 	binmode STDOUT, ':raw';
> 	File::Copy::copy($fh, \*STDOUT);
> 
> Nothing more.
> 
> Just dump cache file to STDOUT in binary mode.

Note that special-casing binary output means that you would never be able
to replace custom caching engine with e.g. CHI with Memcached backend,
because that treating some actions in a special way interleaves gitweb
code with guts of caching code.

And memcached might be a way that kernel.org would have to go...
-- 
Jakub Narebski
Poland