Re: [PATCH 11/18] gitweb: add isDumbClient() check

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 11, 2010, 01:40 UTC
Message-ID: <201012110240.32854.jnareb@gmail.com>
In-Reply-To: <4D02D0C4.2020207@eaglescrag.net>

On Sat, 11 Dec 2010, J.H. wrote:

Show 27 quoted lines

> On 12/10/2010 04:15 PM, Jakub Narebski wrote:
>> Junio C Hamano wrote:
>>> "J.H." <warthog9@eaglescrag.net> writes:
>>>
>>>> My initial look indicated that perl-http-browserdetect wasn't available
>>>> for RHEL / CentOS 5 - it is however available in EPEL.
>>>>
>>>> However there are a couple of things to note about User Agents at all:
>>>> 	- They lie... a lot
>>>> 	- Robots lie even more
>>>>
>>>> Blacklisting is still the better option, by a lot.  I'll re-work this
>>>> some in v9, as I'm fine with the added dependency.
>>>
>>> Thanks, both.  I sense that we finally are going to get a single version
>>> of gitweb that can be used at larger sites ;-)
>> 
>> I wouldn't be so optimistic.  While we borrow features and ideas from
>> each other, the difference still remains that J.H. patches are bit hacky
>> but are tested, while my rewrite is IMHO cleaner but untested (well, 
>> untested on real life load).
> 
> At this point I'm not sure there is a way to rectify the two patch
> series, and while we may borrow ideas from each other it's becoming
> clear that we are both, generally speaking, heading in different
> directions for what we want and need out of gitweb.  Jakub's patches for
> the admin page are indicative of that.

Actually the cache administration page was just proof of concept. Perhaps a better solution would be to provide script that can be run to safely clean cache (or just heavily outdated entries).

What I want from caching series is a clean separation between capturing (so it can be replaced in the future e.g. by Capture::Tiny, or capturing to mmapped fragment for Cache::FastMmap-like cache, or simple capturing to memory for Memcached), caching engine (so it can be replaced by some good and tested caching engine, like Cache::Cache, Cache, Cache::Memcached, Cache::FastMmap, CHI and its drivers and options like cache levels), and caching output module. Modular build makes it easier to catch errors and allows for unit testing each component separately. And you can simply use 'require <Package>' instead of doing manual error handling and protecting against redefine errors / multiple include via 'do <file>'.

What I don't like is caching engine guts strewn all over the gitweb. I'd rather capturing engine was not tied too tightly with gitweb. The least controversial is "output caching" part...

Anyway I'd try to keep my rewrite feature-compatibile with J.H. series, including (from v7) also backward compatibility with cache config option names, including $cache_enable. (Grrr... API/ABI backwards compatibility).

Show 8 quoted lines

> 
>> Anyway the main issue that was discovered by PATCHv6 by me, and v8 by J.H.
>> is that die_error sucks... well, at least if background caching is enabled.
> 
> I'd agree with that, and as such I'm working on a complete re-work of
> error handling in gitweb for v9.  Things are looking pretty good so far,
> but to claim that it's a non-invasive patch would be akin to selling
> someone the Brooklyn bridge.

Hmmm... I am also thinking about changing the way error handling is done
in gitweb, but I don't think it would be very invasive: for a non-cached
case it would be simply one "eval" in run_request() or run(), and "die"
instead of "goto DONE_XXX" in die_error().
 
Now if only there were HTTP::Server::Simple::FCGI so I would be able to
test fastCGI support without need to install mod_fcgi / mod_fastcgi for
Apache... (local::lib and cpanm for the win!).

Show 6 quoted lines

> That said, the way Gitweb handles it's errors and things like exit are
> appalling and this has been something that's needed doing for a while
> anyway.  Guess now's the time to do it.  Might be a few days for me to
> get far enough for any of it to be worthwhile sharing, late next week
> maybe.  That said I hit vacation starting on the 20th so it might be
> next year before that is finalized.

I also don't think that output caching can be done before end of this year, sorry.

Hmmm... I guess that in shortened minimal version of my rewrite of output caching for gitweb (without zero-size check, adaptive cache lifetime, perhaps even without support for alternate caching engines) I should also include minimal improvement to die_error-handling. Just like there is "gitweb: Prepare for splitting gitweb" there.

-- 
Jakub Narebski
Poland