git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "John 'Warthog9' Hawley" <warthog9@kernel.org>
Subject: Re: What's cooking in git.git (Nov 2010, #02; Wed, 17)
Date: Sat, 20 Nov 2010 01:42:24 +0100	[thread overview]
Message-ID: <201011200142.26522.jnareb@gmail.com> (raw)
In-Reply-To: <7v1v6icyb0.fsf@alter.siamese.dyndns.org>

On Thu, 18 Nov 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Sidenote: recently sent
>>
>>   gitweb: selectable configurations that change with each request
>>
>> practically reverts
>>
>>   gitweb: Move call to evaluate_git_version after evaluate_gitweb_config
>>
>> Just FYI.
> 
> Hmph, will have to look at it again.

A bit of history:

* c2394fe (gitweb: Put all per-connection code in run() subroutine, 2010-05-07)
  a0446e7 (gitweb: Add support for FastCGI, using CGI::Fast, 2010-05-07)
  those added run() and run_request(), separating "main" code of gitweb from
  code that is run once per requect (once per connection).

* 869d588 (gitweb: Move evaluate_gitweb_config out of run_request, 2010-07-05)
  among others moved evaluate_gitweb_config out of run_request() to run(),
  because I thought that gitweb config that changes per-request is vanishingly
  rare: it isn't (e.g. gitolite has parts that are per-request).

  evaluate_git_version was moved *together* with evaluate_gitweb_config.

* 7f425db (gitweb: allow configurations that change with each request, 2010-07-30)
  partially reverted commit 869d58... but forgot to move evaluate_git_version
  when moving evaluate_gitweb_config.  This didn't matter in most cases...
  except for test suite.

* 8ff76f4 (gitweb: Move call to evaluate_git_version after evaluate_gitweb_config,
  2010-09-26) fixed that issue, in a simplest way, by moving evaluate_git_version
  after evaluate_gitweb_config.


+ 0ac5f64 (gitweb: selectable configurations that change with each request,
  2010-11-18) in my 'gitweb/web' branch solves the issue in other way:
  evaluate_gitweb_config is run first time outside of run_request(), and on 
  subsequent requests in run_request() (by default).  So evaluate_gitweb_config
  is back before evaluate_git_version.  Hence there is no need to move 
  evaluate_git_version.

HTH.

>>> * jn/gitweb-time-hires-comes-with-5.8 (2010-11-09) 1 commit
>>>  - gitweb: Time::HiRes is in core for Perl 5.8
>>> 
>>> Looked reasonable.  Will merge to next.
>>
>> Thanks. With or without improvement to commit message?
> 
> I think what I pushed out has already been reworded.  Please check.

I have checked the version in 'pu' and it looks good.  Thanks.
 
>>> * jh/gitweb-caching (2010-11-01) 4 commits
>>>  . gitweb: Minimal testing of gitweb caching
>>>  . gitweb: File based caching layer (from git.kernel.org)
>>>  . gitweb: add output buffering and associated functions
>>>  . gitweb: Prepare for splitting gitweb
>>> 
>>> Temporarily ejected while I shuffled jn/gitweb-testing; will queue the
>>> latest back in pu or perhaps in next.
>>
>> The advantage of 'gitweb: File based caching layer (from git.kernel.org)'
>> is that it is tested in real-life on heavy load (assuming that 
>> git.kernel.org uses the same version as is/would be in pu/next).
>>
>> The disadvantage is that it is seriously messy code.  Something that I
>> wanted to improve in my rewrite.  This is only minimal fixup.
> 
> Which is exactly what we want at this point (I want to release 1.7.4 by
> the end-of-year holidays, which means a feature-freeze will have to start
> soon).  My understanding is that the serious messiness does not come from
> the caching layer.

Well... the capturing, caching and actual gitweb operations are quite
intermixed in J.H. patch.  I wanted to separate those issues, and have
them modularized in my rewrite[1], making it easy to use other caching engine
(tested with Cache::FileCache from Cache::Cache).  On the other hand 
intermixing allows capturing directly to cache file (although it is only
since v7), something that would be more difficult in my rewrite.

The Perl code of J.H. patch does not follow style of other parts of gitweb,
doesn't follow best practices (like e.g. using lexical filehandles instead
of globs), does include some code repetitions... all of which made me
attempt rewrite rather than fix it.

[1] http://repo.or.cz/w/git/jnareb-git.git
    http://github.com/jnareb/git
    'gitweb/cache-kernel-pu' branch

BTW. I didn't get any responses to "[RFC] Implementing gitweb output caching
- issues to solve".

-- 
Jakub Narebski
Poland

  parent reply	other threads:[~2010-11-20  0:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-18  0:56 What's cooking in git.git (Nov 2010, #02; Wed, 17) Junio C Hamano
2010-11-18 14:00 ` Jakub Narebski
2010-11-18 17:54   ` Junio C Hamano
2010-11-18 21:33     ` J.H.
2010-11-20  0:42     ` Jakub Narebski [this message]
2010-11-24  1:45       ` J.H.
2010-11-24 17:13         ` Junio C Hamano
2010-11-24 21:30           ` Jakub Narebski
2010-11-24 21:26         ` Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201011200142.26522.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=warthog9@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).