Re: [PATCH 0/9] Gitweb caching v5

"John 'Warthog9' Hawley" [off-list ref] writes:

Afternoon everyone,
 
This is the latest incarnation of gitweb w/ caching.  This is
finally at the point where it should probably start either being
considered for inclusion or mainline, or I need to accept that this
will never get in and more perminantely fork (as is the case with
Fedora where this is going in as gitweb-caching as a parrallel rpm
package).

That said this brings the base up to mainline (again),

Could you tell us which commit is the base of this series (like in
git-request-pull output), i.e. which commit this series is rebased
against?

it updates a
number of elements in the caching engine, and this is a much cleaner
break-out of the tree vs. what I am currently developing against.

Is caching engine part changed since v2?

v5:
	- Missed a couple of things that were in my local tree, and
	  added them back in.

That doesn't tell us much.

	- Split up the die_error and the version matching patch
	- Set version matching to be on by default - otherwise this
	  really is code that will never get checked, or at best
	  enabled by default by distributions
	- Added a minor code cleanup with respect to $site_header
	  that was already in my tree
	- Applied against a more recent git tree vs. 1.6.6-rc2
	- Removed breakout patch for now (did that in v4 actually)
	  and will deal with that separately 

	http://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v5

 
----
Short comments about patches in this series; I will be sending
detailed comments for each patch individually.

John 'Warthog9' Hawley (9):
  gitweb: Load checking

Looks good.

  gitweb: change die_error to take "extra" argument for extended die
    information

Commit message could be better (summary should really be shorter), and
I think there is some indent typo, but otherwise looks good.

  gitweb: Add option to force version match

For me it needs to be disabled in gitweb test suite (t/gitweb-lib.sh),
if it is enabled by default.  I really like that I can test current
gitweb without need to recompile git.

Also it should have tests that it works as intended (both for matching
and non-matching versions) in t/t9501-gitweb-standalone-http-status.sh

  gitweb: Makefile improvements

Does it differ from my proposal (i.e. gitweb/Makefile doing the work),
based on your idea ("make gitweb" for Makefile and gitweb/Makefile)?

  gitweb: add a get function to compliment print_local_time
  gitweb: add a get function to compliment print_sort_th

Those two looks O.K. from what I seen.

  gitweb: cleanup error message produced by undefined $site_header

Shouldn't there be such protection for other such variables, like
$site_footer and $home_text (and a bit diferent protection against
undefined $projects_list)?  By the way, how did you arrived at
undefined $site_header: deafult build configuration leaves it empty,
but defined. 

  gitweb: Convert output to using indirect file handle

I have alternate solution, using shorter filehandle name (just $out)
in

  git://repo.or.cz/git/jnareb-git.git gitweb/cache-kernel
  http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/gitweb/cache-kernel

I would have to think a bit about separate handle for binary files;
I am not sure if it is really required.

  gitweb: File based caching layer (from git.kernel.org)

I am working (time permitting) in spliting this large code drop into
smaller commits, namely:

 - href(..., -path_info => 0)          (for cache key)
 - simple file based caching + tests
 - global expire time + tests
 - output caching in gitweb            (WIP)
 - adaptive expiration time            (planned)
 - tee output / cache write            (planned)
 - expire time variation from CHI      (planned)
 - locking for single writer           (planned)
 - server-side generating info         (planned)
 - AJAX-y generating info              (wishlist)

while ensuring that it pass all existing gitweb tests, and adding new
tests for new features.

-- 
Jakub Narebski
Poland
ShadeHawk on #git