Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)

On Mon, 25 Jan 2010, J.H. wrote:

On 01/25/2010 05:48 AM, Jakub Narebski wrote:

quoted

On Mon, Jan 25, 2010, Petr Baudis wrote:

quoted

On Mon, Jan 25, 2010 at 12:46:39PM +0100, Jakub Narebski wrote:

quoted

On Sun, 24 Jan 2010, Petr Baudis wrote:

quoted

quoted

quoted

  I have stupid question, common to both the original patch and this
RFC.

quoted

[RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
regenerating cache (WIP)

  Just why is a "Generating..." page appropriate?

  I have to admit I hate it; can you please at least make it
configurable? Why is it needed at all? It [...] confuses
non-interactive HTTP clients [...]

quoted

quoted

Second, gitweb can always check User-Agent header, and serve 
"Generating..." page only to web browsers:

  unless (defined $cgi->user_agent() &&
          $cgi->user_agent() =~ /\b(Mozilla|Opera)\b/i) {
  	return;
  }

or something like that.

I'm not too happy with this. What about Safari? Opera? ELinks? There's a
lot of web browsers.

The "Mozilla" part would catch all "Mozilla compatibile" web browsers,
including Firefox (and other Gecko-based web browsers), Internet Explorer,
WebKit based browsers including Safari and Chrome and Konqueror.
The "Opera" part would catch Opera.
http://www.nczonline.net/blog/2010/01/12/history-of-the-user-agent-string/

As to other web browsers like Elinks, Lynx, w3m, Dillo, etc.: the issue
is whether they honor '<meta http-equiv="refresh" content="0" />'.  
I think it is better to stay on the safe side; it is not disaster if web
browser is not shown "Generating..." page where it could (but see 
explanation below).

Most of them do, that particular tag has been around for a long time and
since it doesn't require Javascript to do the page refresh it's pretty
much universal.

For this method to work there is additional assumption about web browser,
namely that it would follow refresh only after it gets page in full (only
when connection closes).  But perhaps that is true for all web browsers
following http-equiv refresh.

The problem is going to be with things like wget when someone wants to
snag a binary file.  This works fine if the file is already cached, but
the user doesn't get what they are expecting if they get a blob that
isn't the final file, but the html contents of the page.  I don't know
of any hint that things like wget would send to the server that you
could switch based on, but it would be more or less the non-background
caching state.

Well, the above solution would work as long as you don't run wget with
--user-agent.

[...]

quoted

quoted

But if it will be possible to turn this off eventually, it's all your
call whether to bother implementing it. :-)

In my implementation it is (or rather would be) as simple as just
not passing 'generating_info' => \&git_generating_data_html in the
GitwebCache::SimpleFileCache constructor.

At least in mine it was don't allow background caching.  It would force
everyone to wait on the one process that was actually generating
content.  But it means a few blank pages with a spinning working icon
until the cache releases it's exclusive lock.

Theoretically at least having "Generating..." page is orthogonal to using
locking, to background serving and to serving stale data.  In current 
implementation those features are dependent, serving stale data on locking,
and "Generating..." also on locking.

I think that serving stale data should depend on background caching, as
otherwise we introduce inequality in treating process which regenerates
cache (and must wait without background caching) and rest of processes
(which are served stale data).

Certainly in final version of patch all this should be made configurable.

 
P.S. With background caching there is a question whether parent should
generate and child display stale or "Generating...", or whether parent
should display stale/activity and child (re)generate cache entry.  Also
there is an issue whether parent should waitpid for child, would fiddling
with SIGCHLD handler be enough, or is it not needed.

-- 
Jakub Narebski
Poland