* Re: [PATCH 00/18] Gitweb caching v8
From: J.H. @ 2010-12-10 0:43 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3bp4u34vj.fsf@localhost.localdomain>
On 12/09/2010 03:26 PM, Jakub Narebski wrote:
> John, could you please in the future Cc me? I am interested in gitweb
> output caching development. Thanks in advance.
Apologies, apparently screwed up on my git send-email line. I'll get
that right one of these eons.
> Could you please rebase it on top of v7.2 version? The v7.2 patch
> series contained a few bugs that needs to be corrected.
I assume you mean 7.4, as opposed to 7.2... otherwise already done!
> I guess that in the final submission (i.e. the one that is to be
> merged in into git.git repository) those changes would be squashed in,
> isn't it?
I have no objections to squashing the reversions into a single patch,
just figured it was easier to break them out for the time being.
>> This still differs, by two patches, from whats in production on kernel.org.
>> It's missing the index page git:// link, and kernel.org and kernel.org also
>
>>
>> has the forced version matching. As a note I'll probably let this stew
>> another day or so on kernel.org and then I'll push it into the Fedora update
>> stream, as there's a couple of things in this patch series that would be
>> good for them to have.
>
> There was some discussion about git:// link in the past; nevertheless
> this issue is independent on gitweb caching and can (and should) be
> sent as a aeparate patch.
>
> IIRC we agreed that because of backward compatibility forced versions
> match is quite useless (in general)...
The former wasn't submitted as that is a separate issue, the later was
not agreed on really but mostly me retracting the patches as they
weren't making any headway.
I mention the patches at all as clarification of what's actually running
on kernel.org, and eventually what will be in the gitweb-caching
packages that are part of Fedora and EPEL.
>> There is one additional script I've written that the Fedora folks are using,
>> and that might be useful to include, which is an 'offline' cache file generator.
>> It basically wraps gitweb.cgi and at the end moves the cache file into the right
>> place. The Fedora folks were finding it took hours to generate their front
>> page, and that doing a background generation almost never completed (due to
>> process death). This was a simple way to handle that. If people would like
>> I can add it in as an additional patch.
>
> Are you detaching the background process?
No, in fact I completely turn off forking (using the $cacheDoFork variable.)
> It would be nice to have it as separate patch.
I can add it easily enough.
>> v8:
>> - Reverting several changes from Jakub's change set that make no sense
>> - is_cacheable changed to always return true - nothing special about
>> blame or blame_incremental as far as the caching engine is concerned
>
> 'blame_incremental' is just another version of 'blame' view. I have
> disabled it when caching is enabled in my rewrite (you instead disabled
> caching for 'blame_incremental' in your v7 and mine v7.x) because I
> couldn't get it to work together with caching. Did you check that it
> works?
blame works fine, blame_incremental generates but doesn't..... ohhhh
someone added ajaxy kinda stuff and doesn't mention it anywhere.
Exciting.
blame_data needs to not get a 'generating...' page in all likelihood,
generating a blame_incremental page, letting it load and then refreshing
the whole thing gets me what I'm expecting.
Is enough to mask.
Guess I'm looking at a v9 now.
> Besides, withou "tee"-ing, i.e. printing output as it is captured,
> cached 'blame_data' means that 'blame_incremental' is not incremental,
> and therefore it vanishes its advantage over 'blame'.
There are only 2 ways to get to a blame_incremental page
1) By going to a blame page and clicking on the incremental link in the nav
2) By enabling it by default so when you click 'blame' it goes to
incremental first.
> In the case data is in cache, then 'blame_inremental' doesn't have
> advantage over 'blame' either.
Agreed, though it's easy enough to support in the caching engine,
basically don't return 'Generating...' and wait for that data to cache.
Not really an advantage except that your not waiting for the whole
generation to get a page back at all.
>> - Reverted change to reset_output as
>> open STDOUT, ">&", \*STDOUT_REAL;
>> causes assertion failures:
>> Assertion !((((s->var)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVLV)) failed: file "scalar.xs", line 49 at gitweb.cgi line 1221.
>> if we encounter an error *BEFORE* we've ever changed the output.
>
> Which Perl version are you using? Because I think you found error in Perl.
> Well, at least I have not happen on this bug.
This is perl, v5.10.0 built for x86_64-linux-thread-multi
> I have nothing againts using
>
> open STDOUT, ">&STDOUT_REAL";
>
> though I really prefer that you used lexical filehandles, instead of
> "globs" which are global variables.
>
> The following works:
>
> open STDOUT, '>&', fileno($fh);
>
> Note that fileno(Symbol::qualify_to_ref($fh)) might be needed...
I see 0 advantage to shifting around STDOUT and STDERR to a lexical
filehandle vs. a glob in this case. STDOUT_REAL retains all the
properties of STDOUT should it be needed elsewhere, including what it
was going and what it was doing.
I have no objection to shifting the file handles I'm using to lexical
variables, if nothing else the argument about them closing when falling
out of scope is worth it, but for STDOUT, STDERR, etc I don't think
switching to lexicals makes a lot of sense
>> - Cleanups there were indirectly mentioned by Jakub
>> - Elimination of anything even remotely looking like duplicate code
>> - Creation of isBinaryAction() and isFeedAction()
>
> Could you please do not use mixedCase names?
I'm fine with renaming those if you wish.
> First, that is what %actions_info from
>
> [PATCH 16/24] gitweb: Introduce %actions_info, gathering information about actions
> http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163038
> http://repo.or.cz/w/git/jnareb-git.git/commitdiff/305a10339b33d56b4a50708d71e8f42453c8cb1f
>
> I have invented for.
I have not based any of my caching engine, right now, on anything you've
done for your rewrite.
> Second, why 'isBinaryAction()'? there isn't something inherently
> different between binary (':raw') and text (':utf8') output, as I have
> repeatedly said before.
It's a binary action in that you are shoving something down the pipe
with the intention of sending the bits completely raw. You read the
data raw, and write the data raw. There is no interpretation of the
data as being anything but straight raw.
Right now, in gitweb already, there are two places that treat output
completely differently:
- snapshot
- blob_plain
The only reason isBinaryAction() (or any other function name or process
you want to grant it) exists is so that I can figure out if it's one of
those actions so I can deal with the cache and output handling
differently for each.
Yes, I could flip the entire caching engine over to following the same
mantra for everything and thus there is no need to care, but gitweb
itself isn't really setup to handle that separation cleanly right now,
and I'm trying to make as few bigger changes right now as is.
>> - Added better error handling
>> - Creation of .err file in the cache directory
>> - Trap STDERR output into $output_err as this was spewing data prior
>> to any header information being sent
>
> Why it is needed? We capture output of "die" via CGI::Util::set_message,
> and "warn" output is captured to web server logs... unless you explicitely
> use "print STDERR <sth>" -- don't do that instead.
I have seen, in several instances, a case where git itself will generate
an error, it shoves it to STDERR which makes it to the client before
anything else, thus causing 500 level errors.
Added this so that STDERR got trapped and those messages didn't make it out.
>> - Added hidden field in footer for url & hash of url, which is extremely useful
>> for debugging
>
> Nice idea, I'll see it. Can it be disabled (information leakage)?
There's not really any information leakage per-se, unless you call
md5suming the url information leakage.
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: [PATCH 00/18] Gitweb caching v8
From: Junio C Hamano @ 2010-12-10 0:39 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
It seems that t950X tests do not like me. I am getting these in gitweb.body:
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www/w3.porg/TR/html4/strict.dtd">
<!-- git web w/caching interface version current, (C) 2006-2010, John 'Warthog9' Hawley <warthog9@kernel.org> -->
<!-- git core binaries version 1.7.3.3.494.gcf92e -->
<head>
<meta http-equiv="content-type" content="; charset=utf-8"/>
<meta name="generator" content="gitweb/current git/1.7.3.3.494.gcf92e"/>
<meta name="robots" content="index, nofollow"/>
<meta http-equiv="refresh" content="10"/>
<title></title>
</head>
<body>
<p><strong>*** Warning ***:</strong> Caching enabled but cache directory did not exsist. (cache)<br/>/<span class="cntrl">\n</span>Cache directory created successfully<span class="cntrl">\n</span><p>
</body>
</html>
^ permalink raw reply
* Re: [PATCH 14/18] gitweb: add print_transient_header() function for central header printing
From: Jakub Narebski @ 2010-12-10 0:36 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-15-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> There are a few things I would like to reuse the transient header
> information I'm using, currently this is only the 'Generating...'
> page, but there is at least one additional warning page I would
> like to use this on.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/lib/cache.pl | 47 ++++++++++++++++++++++++++---------------------
> 1 files changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index 63dbe9e..723ae9b 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -94,6 +94,31 @@ sub cache_fetch {
> #$actions{$action}->();
> }
>
> +sub print_transient_header {
> + print $::cgi->header(
Why you use $::cgi->header() instead of equivalent $cgi->header()?
Note that $::cgi->header() is $main::cgi->header(), and is not
CGI::header().
> + -type=>'text/html',
> + -charset => 'utf-8',
> + -status=> 200,
> + -expires => 'now',
> + # HTTP/1.0
> + -Pragma => 'no-cache',
> + # HTTP/1.1
> + -Cache_Control => join(
> + ', ',
> + qw(
> + private
> + no-cache
> + no-store
> + must-revalidate
> + max-age=0
> + pre-check=0
> + post-check=0
> + )
> + )
> + );
> + return;
> +}
Why not use
our %no_cache = (
# HTTP/1.0
-Pragma => 'no-cache',
# HTTP/1.1
-Cache_Control => join(', ', qw(private no-cache no-store must-revalidate
max-age=0 pre-check=0 post-check=0)),
);
(or something like that). This way you can reuse it even if content
type is different (e.g. 'text/plain').
But that is just a proposal.
> +
> sub isBinaryAction {
> my ($action) = @_;
>
> @@ -292,27 +317,7 @@ sub cacheWaitForUpdate {
>
> $| = 1;
>
> - print $::cgi->header(
> - -type=>'text/html',
> - -charset => 'utf-8',
> - -status=> 200,
> - -expires => 'now',
> - # HTTP/1.0
> - -Pragma => 'no-cache',
> - # HTTP/1.1
> - -Cache_Control => join(
> - ', ',
> - qw(
> - private
> - no-cache
> - no-store
> - must-revalidate
> - max-age=0
> - pre-check=0
> - post-check=0
> - )
> - )
> - );
> + print_transient_header();
>
> print <<EOF;
> <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www/w3.porg/TR/html4/strict.dtd">
> --
> 1.7.2.3
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 12/18] gitweb: Change file handles (in caching) to lexical variables as opposed to globs
From: Junio C Hamano @ 2010-12-10 0:32 UTC (permalink / raw)
To: Jakub Narebski; +Cc: John 'Warthog9' Hawley, git
In-Reply-To: <m3d3pa1o0j.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
>> +# Global declarations
>> +our $cacheFile;
>> +our $cacheFileBG;
>> +our $cacheFileBinWT;
>> +our $cacheFileBin;
>
> You are trading globs for global (well, package) variables. They are
> not lexical filehandles... though I'm not sure if it would be possible
> without restructuring code; note that if variable holding filehandle
> falls out of scope, then file would be automatically closed.
Hmm. why is it a bad idea, when you need to access these from practically
everywhere, to use global variables to begin with? To a certain degree,
it sounds like an unnecessary burden without much gain to me.
^ permalink raw reply
* Re: [PATCH 13/18] gitweb: Add commented url & url hash to page footer
From: Jakub Narebski @ 2010-12-10 0:26 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-14-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> This is mostly a debugging tool, but it adds a small bit of information
> to the footer:
>
> <!--
> Full URL: |http://localhost/gitweb-caching/gitweb.cgi?p=/project.git;a=summary|
> URL Hash: |7a31cfb8a43f5643679eec88aa9d7981|
> -->
Nice idea. It helps with debugging and doesn't introduce information
leakage.
Note that in my rewrite there would be *three* pieces of information,
not two. Namely:
Full URL: |http://localhost/gitweb-caching/gitweb.cgi/project.git|
Key: |http://localhost/gitweb-caching/gitweb.cgi?p=/project.git;a=summary|
Key hash: |7a31cfb8a43f5643679eec88aa9d7981|
>
> The first bit tells you what the url that generated the page actually was, the second is
> the hash used to store the file with the first two characters being used as the directory:
>
> <cachedir>/7a/31cfb8a43f5643679eec88aa9d7981
Isn't it
<cachedir>/7a/7a31cfb8a43f5643679eec88aa9d7981
in your series?
>
> Also useful for greping through the existing cache and finding files with unique paths that
> you may want to explicitly flush.
Though probably better 'cache_admin' page would be ultimately best
solution, see proof of concept in
[RFC PATCHv6 24/24] gitweb: Add beginnings of cache administration page (proof of concept)
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163051
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/aa9fd77ff206eae8838fdde626d2afea563f9f75
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/gitweb.perl | 7 +++++++
> gitweb/lib/cache.pl | 4 ++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e8c028b..7f8292e 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -303,6 +303,9 @@ our $fullhashpath = *STDOUT;
> our $fullhashbinpath = *STDOUT;
> our $fullhashbinpathfinal = *STDOUT;
>
> +our $full_url;
> +our $urlhash;
> +
> # configuration for 'highlight' (http://www.andre-simon.de/)
> # match by basename
> our %highlight_basename = (
> @@ -3663,6 +3666,10 @@ sub git_footer_html {
>
> print "<div class=\"page_footer\">\n";
> print "<div class=\"cachetime\">Cache Last Updated: ". gmtime( time ) ." GMT</div>\n";
> + print "<!--\n".
> + " Full URL: |$full_url|\n".
> + " URL Hash: |$urlhash|\n".
> + "-->\n" if ($cache_enable);
Don't you need to esc_html on it? $full_url can contain ' -->', and
what you would do then?
> if (defined $project) {
> my $descr = git_get_project_description($project);
> if (defined $descr) {
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index fafc028..63dbe9e 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -30,8 +30,8 @@ sub cache_fetch {
> print "Cache directory created successfully\n";
> }
>
> - our $full_url = "$my_url?". $ENV{'QUERY_STRING'};
Note that $my_url is $cgi->url(), which does not include path_info.
> - our $urlhash = md5_hex($full_url);
> + $full_url = "$my_url?". $ENV{'QUERY_STRING'};
> + $urlhash = md5_hex($full_url);
> our $fullhashdir = "$cachedir/". substr( $urlhash, 0, 2) ."/";
>
> eval { mkpath( $fullhashdir, 0, 0777 ) };
> --
> 1.7.2.3
Looks quite nice.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 12/18] gitweb: Change file handles (in caching) to lexical variables as opposed to globs
From: Jakub Narebski @ 2010-12-10 0:16 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-13-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> This isn't a huge change, it just adds global variables for the file handles,
> an additional cleanup to localize the variable a bit more which should alleviate
> the issues that Jakub had with my original approach.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/lib/cache.pl | 114 +++++++++++++++++++++++++++++++-------------------
> 1 files changed, 71 insertions(+), 43 deletions(-)
>
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index 5182a94..fafc028 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -14,6 +14,12 @@ use Digest::MD5 qw(md5 md5_hex md5_base64);
> use Fcntl ':flock';
> use File::Copy;
>
> +# Global declarations
> +our $cacheFile;
> +our $cacheFileBG;
> +our $cacheFileBinWT;
> +our $cacheFileBin;
You are trading globs for global (well, package) variables. They are
not lexical filehandles... though I'm not sure if it would be possible
without restructuring code; note that if variable holding filehandle
falls out of scope, then file would be automatically closed.
BTW. Do you really need all those types/variables?
> +
> sub cache_fetch {
> my ($action) = @_;
> my $cacheTime = 0;
> @@ -49,9 +55,9 @@ sub cache_fetch {
> }else{
> #if cache is out dated, update
> #else displayCache();
> - open(cacheFile, '<', "$fullhashpath");
> - stat(cacheFile);
> - close(cacheFile);
> + open($cacheFile, '<', "$fullhashpath");
> + stat($cacheFile);
> + close($cacheFile);
[...]
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 11/18] gitweb: add isDumbClient() check
From: Jakub Narebski @ 2010-12-10 0:12 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-12-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> Basic check for the claimed Agent string, if it matches a known
> blacklist (wget and curl currently) don't display the 'Generating...'
> page.
>
> Jakub has mentioned a couple of other possible ways to handle
> this, so if a better way comes along this should be used as a
> wrapper to any better way we can find to deal with this.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/lib/cache.pl | 30 ++++++++++++++++++++++++++++++
> 1 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index d55b572..5182a94 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -116,6 +116,34 @@ sub isFeedAction {
> return 0; # False
> }
>
> +# There have been a number of requests that things like "dumb" clients, I.E. wget
> +# lynx, links, etc (things that just download, but don't parse the html) actually
> +# work without getting the wonkiness that is the "Generating..." page.
> +#
> +# There's only one good way to deal with this, and that's to read the browser User
> +# Agent string and do matching based on that. This has a whole slew of error cases
> +# and mess, but there's no other way to determine if the "Generating..." page
> +# will break things.
> +#
> +# This assumes the client is not dumb, thus the default behavior is to return
> +# "false" (0) (and eventually the "Generating..." page). If it is a dumb client
> +# return "true" (1)
> +sub isDumbClient {
Please don't use mixedCase, but underline_separated words,
e.g. browser_is_robot(), or client_is_dumb().
> + my($user_agent) = $ENV{'HTTP_USER_AGENT'};
What if $ENV{'HTTP_USER_AGENT'} is unset / undef, e.g. because we are
runing gitweb as a script... which includes running gitweb tests?
> +
> + if(
> + # wget case
> + $user_agent =~ /^Wget/i
> + ||
> + # curl should be excluded I think, probably better safe than sorry
> + $user_agent =~ /^curl/i
> + ){
> + return 1; # True
> + }
> +
> + return 0;
> +}
Compare (note: handcrafted solution is to whitelist, not blacklist):
+sub browser_is_robot {
+ return 1 if !exists $ENV{'HTTP_USER_AGENT'}; # gitweb run as script
+ if (eval { require HTTP::BrowserDetect; }) {
+ my $browser = HTTP::BrowserDetect->new();
+ return $browser->robot();
+ }
+ # fallback on detecting known web browsers
+ return 0 if ($ENV{'HTTP_USER_AGENT'} =~ /\b(?:Mozilla|Opera|Safari|IE)\b/);
+ # be conservative; if not sure, assume non-interactive
+ return 1;
+}
from
"[PATCHv6 17/24] gitweb: Show appropriate "Generating..." page when regenerating cache"
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163040
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/48679f7985ccda16dc54fda97790841bab4a0ba2
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] t9143: do not fail when unhandled.log.gz is not created
From: Junio C Hamano @ 2010-12-10 0:11 UTC (permalink / raw)
To: Eric Wong; +Cc: Steven Walter, git, Torsten Bögershausen
In-Reply-To: <20101209213123.GA25684@dcvr.yhbt.net>
Eric Wong <normalperson@yhbt.net> writes:
> It's pushed out to git://git.bogomips.org/git-svn along with an
> older patch from Steven:
>
> Steven Walter (1):
> git-svn: allow the mergeinfo property to be set
>
> Torsten Bögershausen (1):
> t9143: do not fail when unhandled.log.gz is not created
Thanks; will pull.
^ permalink raw reply
* Re: [PATCH 10/18] gitweb: Adding isBinaryAction() and isFeedAction() to determine the action type
From: Jakub Narebski @ 2010-12-10 0:06 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-11-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> This is fairly self explanitory, 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.
> isFeedAction() returns true if the action deals with a news feed of some sort,
> basically used to bypass the 'Generating...' message should it be a news reader
> as those will explode badly on that page.
Why blacklisting 'feed', instead of whitelisting HTML-output?
BTW., please don't use mixedCase names, but underline_separated.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/lib/cache.pl | 69 ++++++++++++++++++++++++++-------------------------
> 1 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index a8ee99e..d55b572 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -88,6 +88,34 @@ sub cache_fetch {
> #$actions{$action}->();
> }
>
> +sub isBinaryAction {
> + my ($action) = @_;
> +
> + if(
> + $action eq "snapshot"
> + ||
> + $action eq "blob_plain"
> + ){
> + return 1; # True
> + }
> +
> + return 0; # False
> +}
> +
> +sub isFeedAction {
> + if(
> + $action eq "atom"
> + ||
> + $action eq "rss"
> + ||
> + $action eq "opml"
> + ){
> + return 1; # True
> + }
> +
> + return 0; # False
> +}
Compare to:
+our %actions_info = ();
+sub evaluate_actions_info {
+ our %actions_info;
+ our (%actions);
+
+ # unless explicitely stated otherwise, default output format is html
+ foreach my $action (keys %actions) {
+ $actions_info{$action}{'output_format'} = 'html';
+ }
+ # list all exceptions; undef means variable (no definite format)
+ map { $actions_info{$_}{'output_format'} = 'text' }
+ qw(commitdiff_plain patch patches project_index blame_data);
+ map { $actions_info{$_}{'output_format'} = 'xml' }
+ qw(rss atom opml); # there are different types (document formats) of XML
+ map { $actions_info{$_}{'output_format'} = undef }
+ qw(blob_plain object);
+ $actions_info{'snapshot'}{'output_format'} = 'binary';
+}
Instead of 'xml' you can use 'feed'.
Then e.g.:
+sub action_outputs_html {
+ my $action = shift;
+ return $actions_info{$action}{'output_format'} eq 'html';
+}
See
"gitweb: Introduce %actions_info, gathering information about actions"
"gitweb: Show appropriate "Generating..." page when regenerating cache"
http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/origin..refs/heads/gitweb/cache-kernel-v6
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 09/18] gitweb: Revert reset_output() back to original code
From: Jakub Narebski @ 2010-12-09 23:58 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-10-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> Reverted change to reset_output as
>
> open STDOUT, ">&", \*STDOUT_REAL;
For somebody not following our discussion the above would be very,
very cryptic... though I suppose this would be squashed in final
(ready to be merged in) version of the code.
> causes assertion failures:
>
> Assertion !((((s->var)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVLV)) failed: file "scalar.xs", line 49 at gitweb.cgi line 1221.
It looks like bug in Perl, because it should give some kind of Perl
error, not failed assertion from within guts of Perl C code.
Which Perl version are you using?
> if we encounter an error *BEFORE* we've ever changed the output.
And how to reproduce this error (i.e. how did you found it)?
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/gitweb.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1d8bc74..e8c028b 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1225,7 +1225,7 @@ sub change_output {
>
> sub reset_output {
> # This basically takes STDOUT_REAL and puts it back as STDOUT
> - open STDOUT, ">&", \*STDOUT_REAL;
> + open(STDOUT,">&STDOUT_REAL");
Hmmm... how to silence spurious warning then:
gitweb.perl: Name "main::STDOUT_REAL" used only once: possible typo
at ../gitweb/gitweb.perl line 1130.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 08/18] gitweb: Change is_cacheable() to return true always
From: Jakub Narebski @ 2010-12-09 23:46 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-9-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> is_cacheable() was set to return false for blame or blame_incremental
> which both use unique urls so there's no reason this shouldn't pass
> through the caching engine.
I have disabled caching 'blame_incremental' (and its workhorse
'blame_data'), in slightly different way (by disabling these views
rather than making them un-cacheable), because last time when I was
chaing this it simply didn't work with caching. Did you check that it
works?
Besides with caching (without "tee"-ing captre) 'blame_incremental'
view doesn't offer any advantage over 'blame' view, so it should be
IMHO disabled.
> Leaving the function in place for now should something actually arrise
> that we can't use caching for (think ajaxy kinda things likely).
Sidenote: I use it for 'cache' and for 'cache_clear' action in
"[RFC PATCHv6 24/24] gitweb: Add beginnings of cache administration page (proof of concept)"
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163051
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/aa9fd77ff206eae8838fdde626d2afea563f9f75
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/gitweb.perl | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 5eb0309..1d8bc74 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -798,7 +798,8 @@ our %actions = (
> );
> sub is_cacheable {
> my $action = shift;
> - return !($action eq 'blame_data' || $action eq 'blame_incremental');
> + # There are no known actions that do no involve a unique URL that shouldn't be cached.
> + return 1;
> }
>
> # finally, we have the hash of allowed extra_options for the commands that
> --
> 1.7.2.3
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 07/18] gitweb: Revert back to $cache_enable vs. $caching_enabled
From: Jakub Narebski @ 2010-12-09 23:38 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git
In-Reply-To: <1291931844-28454-8-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> Simple enough, $cache_enable (along with all caching variables) are
> already in production in multiple places and doing a small semantic
> change without backwards compatibility is pointless breakage.
Formally, there is no backward compatibility with any released code.
Using out-of-tree patches is on one's own risk.
But even discarding that, I'd rather use the same solution as in
[PATCHv6/RFC 22/24] gitweb: Support legacy options used by kernel.org caching engine
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163058
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/27ec67ad90ecd56ac3d05f6a9ea49b6faabf7d0a
i.e.
our $cache_enable;
[...]
# somewhere just before call to cache_fetch()
$caching_enabled = !!$cache_enable if defined $cache_enable;
>
> This reverts back to the previous variable to enable / disable caching
[...]
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -258,7 +258,7 @@ our $maxload = 300;
> # that the cache directory be periodically completely deleted, and this is safe to perform.
> # Suggested mechanism
> # mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush
> -our $caching_enabled = 0;
> +our $cache_enable = 0;
>
> # Used to set the minimum cache timeout for the dynamic caching algorithm. Basically
> # if we calculate the cache to be under this number of seconds we set the cache timeout
> @@ -1138,7 +1138,7 @@ sub dispatch {
> !$project) {
> die_error(400, "Project needed");
> }
> - if ($caching_enabled && is_cacheable($action)) {
> + if ($cache_enable && is_cacheable($action)) {
> cache_fetch($action);
> } else {
> $actions{$action}->();
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 05/18] gitweb: Regression fix concerning binary output of files
From: Jakub Narebski @ 2010-12-09 23:33 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-6-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> This solves the regression introduced with v7.2 of the gitweb-caching code,
> fix proposed by Jakub in his e-mail.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
> gitweb/gitweb.perl | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 3c3ff08..f2ef3da 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5664,7 +5664,7 @@ sub git_blob_plain {
> if ($caching_enabled) {
> open BINOUT, '>', $fullhashbinpath or die_error(500, "Could not open bin dump file");
> }else{
> - open BINOUT, '>', \$fullhashbinpath or die_error(500, "Could not open bin dump file");
> + open BINOUT, '>&', \$fullhashbinpath or die_error(500, "Could not open bin dump file");
> }
> binmode BINOUT, ':raw';
> print BINOUT <$fd>;
I'd rather you rebase on top of v7.4, where this issue was fixed in
different way... well, at least in easier to undertstand way (in the
solution used above one must know that if caching is disabled,
$fullhashbinpath is *STDOUT - and has nothing to do with any _path_).
This probably should be squashed, if using v7.4 is not chosen.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 01/18] gitweb: Prepare for splitting gitweb
From: Jakub Narebski @ 2010-12-09 23:30 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-2-git-send-email-warthog9@eaglescrag.net>
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> From: Jakub Narebski <jnareb@gmail.com>
> +install-modules:
> + install_dirs="$(sort $(dir $(GITWEB_MODULES)))" && \
> + for dir in $$install_dirs; do \
> + test -d '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir' || \
> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/$$dir'; \
This should be
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)'/$$dir; \
or even
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)'/"$$dir"; \
Shell variables should be not inside single quotes (as oposed to make
variables, where it does not matter).
Please rebase on top of v7.4, where it was fixed.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 00/18] Gitweb caching v8
From: Jakub Narebski @ 2010-12-09 23:26 UTC (permalink / raw)
To: John 'Warthog9' Hawley; +Cc: git, Jakub Narebski
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
John, could you please in the future Cc me? I am interested in gitweb
output caching development. Thanks in advance.
"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> Afternoon everyone,
>
> (Afternoon is like morning, right?)
>
> This is the latest incarnation of gitweb w/ caching. Per the general
> consensus and requests from the recent GitTogether I'm re-submitting
> my patches.
>
> Bunch of re-works in the code, and several requested features. Sadly the
> patch series has balloned as I've been adding things. It was 3-4 patches,
> it's now 18. This is based on top of Jakub's v7.2 patch series, but
> it should be more or less clean now.
Could you please rebase it on top of v7.2 version? The v7.2 patch
series contained a few bugs that needs to be corrected.
>
> As such there was a bunch of changes that I needed to do to Jakub's tree
> which are indicated in the series. Why did I do them up as separate things?
> Mainly there's a bunch of history that's getting lost right now between
> going back and forth, and I wanted to have clear patches to discuss
> should further discussion be needed.
I guess that in the final submission (i.e. the one that is to be
merged in into git.git repository) those changes would be squashed in,
isn't it?
>
> This still differs, by two patches, from whats in production on kernel.org.
> It's missing the index page git:// link, and kernel.org and kernel.org also
> has the forced version matching. As a note I'll probably let this stew
> another day or so on kernel.org and then I'll push it into the Fedora update
> stream, as there's a couple of things in this patch series that would be
> good for them to have.
There was some discussion about git:// link in the past; nevertheless
this issue is independent on gitweb caching and can (and should) be
sent as a aeparate patch.
IIRC we agreed that because of backward compatibility forced versions
match is quite useless (in general)...
>
> There is one additional script I've written that the Fedora folks are using,
> and that might be useful to include, which is an 'offline' cache file generator.
> It basically wraps gitweb.cgi and at the end moves the cache file into the right
> place. The Fedora folks were finding it took hours to generate their front
> page, and that doing a background generation almost never completed (due to
> process death). This was a simple way to handle that. If people would like
> I can add it in as an additional patch.
Are you detaching the background process?
It would be nice to have it as separate patch.
>
> v8:
> - Reverting several changes from Jakub's change set that make no sense
> - is_cacheable changed to always return true - nothing special about
> blame or blame_incremental as far as the caching engine is concerned
'blame_incremental' is just another version of 'blame' view. I have
disabled it when caching is enabled in my rewrite (you instead disabled
caching for 'blame_incremental' in your v7 and mine v7.x) because I
couldn't get it to work together with caching. Did you check that it
works?
Besides, withou "tee"-ing, i.e. printing output as it is captured,
cached 'blame_data' means that 'blame_incremental' is not incremental,
and therefore it vanishes its advantage over 'blame'.
In the case data is in cache, then 'blame_inremental' doesn't have
advantage over 'blame' either.
> - Reverted config file change "caching_enabled" back to "cache_enable" as this
> config file option is already in the wild in production code, as are all
> current gitweb-caching configuration variables.
[Explitive deleted.] I dislike strongly this $cache_enable. I think it
would be better for backward compatibility (should we keep backward
compatibility with out-of-tree patches?) to use the same mechanism as
provided in
[PATCHv6/RFC 22/24] gitweb: Support legacy options used by kernel.org caching engine
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163058
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/27ec67ad90ecd56ac3d05f6a9ea49b6faabf7d0a
in my rewrite. Just set $caching_enabled to true if $cache_enable is
defined and true.
> - Reverted change to reset_output as
> open STDOUT, ">&", \*STDOUT_REAL;
> causes assertion failures:
> Assertion !((((s->var)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVLV)) failed: file "scalar.xs", line 49 at gitweb.cgi line 1221.
> if we encounter an error *BEFORE* we've ever changed the output.
Which Perl version are you using? Because I think you found error in Perl.
Well, at least I have not happen on this bug.
I have nothing againts using
open STDOUT, ">&STDOUT_REAL";
though I really prefer that you used lexical filehandles, instead of
"globs" which are global variables.
The following works:
open STDOUT, '>&', fileno($fh);
Note that fileno(Symbol::qualify_to_ref($fh)) might be needed...
> - Cleanups there were indirectly mentioned by Jakub
> - Elimination of anything even remotely looking like duplicate code
> - Creation of isBinaryAction() and isFeedAction()
Could you please do not use mixedCase names?
First, that is what %actions_info from
[PATCH 16/24] gitweb: Introduce %actions_info, gathering information about actions
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163038
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/305a10339b33d56b4a50708d71e8f42453c8cb1f
I have invented for.
Second, why 'isBinaryAction()'? there isn't something inherently
different between binary (':raw') and text (':utf8') output, as I have
repeatedly said before. See my rewrite: there is no special case for
binary output (or perhaps binary output as in the case of 'blob_plain'
action).
> - Adding in blacklist of "dumb" clients for purposes of downloading content
> - Added more explicit disablement of "Generating..." page
Good, I'll check this.
> - Added better error handling
> - Creation of .err file in the cache directory
> - Trap STDERR output into $output_err as this was spewing data prior
> to any header information being sent
Why it is needed? We capture output of "die" via CGI::Util::set_message,
and "warn" output is captured to web server logs... unless you explicitely
use "print STDERR <sth>" -- don't do that instead.
> - Added hidden field in footer for url & hash of url, which is extremely useful
> for debugging
Nice idea, I'll see it. Can it be disabled (information leakage)?
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH v4 1/3] fetch/pull: Add the --recurse-submodules option
From: Jens Lehmann @ 2010-12-09 23:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Git Mailing List, Kevin Ballard, Jon Seymour,
Chris Packham, Marc Branchaud
In-Reply-To: <7vei9qfxzy.fsf@alter.siamese.dyndns.org>
Am 09.12.2010 22:16, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> Until now you had to call "git submodule update" (without -N|--no-fetch
>> option) or something like "git submodule foreach git fetch" to fetch
>> new commits in populated submodules from their remote.
>> ...
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index d35f000..db3fba3 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> ...
>> @@ -784,28 +790,36 @@ static int add_remote_or_group(const char *name, struct string_list *list)
>> return 1;
>> }
>>
>> -static int fetch_multiple(struct string_list *list)
>> +static void add_options_to_argv(int *argc, const char **argv)
>> {
>> - int i, result = 0;
>> - const char *argv[11] = { "fetch", "--append" };
>> -...
>> +static int fetch_multiple(struct string_list *list)
>> +{
>> + int i, result = 0;
>> + const char *argv[12] = { "fetch", "--append" };
>
> This used to be 11; are we adding something new? Ahh, possibly
> "--recurse_submodules".
Yup, that's why! (see a few lines down between "--keep" and "-v").
>> diff --git a/submodule.c b/submodule.c
>> index 91a4758..4d9b774 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -63,7 +63,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
>> }
>> }
>>
>> -static int submodule_config(const char *var, const char *value, void *cb)
>> +int submodule_config(const char *var, const char *value, void *cb)
>> {
>> if (!prefixcmp(var, "submodule."))
>> return parse_submodule_config_option(var, value);
>> @@ -229,6 +229,70 @@ void show_submodule_summary(FILE *f, const char *path,
>> strbuf_release(&sb);
>> }
>>
>> +int fetch_populated_submodules(int num_options, const char **options,
>> + const char *prefix, int quiet)
>> +{
>> + int i, result = 0, argc = 0;
>> + struct child_process cp;
>> + const char **argv;
>> + struct string_list_item *name_for_path;
>> + const char *work_tree = get_git_work_tree();
>> + if (!work_tree)
>> + return 0;
>> +
>> + if (!the_index.initialized)
>> + if (read_cache() < 0)
>> + die("index file corrupt");
>> +
>> + argv = xcalloc(num_options + 5, sizeof(const char *));
>
> Where is this '5' coming from? "fetch" "--submodule-prefix", the prefix,
> and the terminating NULL? What did I miss?
No, you didn't miss anything but I have been off by one ... '4' is
sufficient here.
^ permalink raw reply
* Re: [RFC] Implementing gitweb output caching - issues to solve
From: Jonathan Nieder @ 2010-12-09 22:52 UTC (permalink / raw)
To: Jakub Narebski
Cc: J.H., git, John 'Warthog9' Hawley, Junio C Hamano,
Petr Baudis, admin, olaf
In-Reply-To: <201012092330.06688.jnareb@gmail.com>
Jakub Narebski wrote:
> In my rewrite
>
> [PATCHv6 17/24] gitweb: Show appropriate "Generating..." page when regenerating cache
> http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163040
> http://repo.or.cz/w/git/jnareb-git.git/commitdiff/48679f7985ccda16dc54fda97790841bab4a0ba2#patch1
>
> (see the browser_is_robot() subroutine:
>
> http://repo.or.cz/w/git/jnareb-git.git/blob/48679f7985ccda16dc54fda97790841bab4a0ba2:/gitweb/gitweb.perl#l870
>
> I use HTTP::BrowserDetect package if available and it's ->robot() method.
>
> The fallback is to use *whitelist*, assuming that it would be better to
> not show "Generating..." page rather than download the wrong thing.
> I also guess that most (all?) web browsers use "Mozilla compatibile"
> somewhere in their User-Agent string, thus matching 'Mozilla'.
Interesting. http://www.user-agents.org/ seems to suggest that many
robots do use Mozilla (though I don't think it's worth bending over
backwards to help them see the page correctly).
HTTP::BrowserDetect uses a blacklist as far as I can tell. Maybe in
the long term it would be nice to add a whitelist ->human() method.
Cc-ing Olaf Alders for ideas.
^ permalink raw reply
* Re: [RFC] Implementing gitweb output caching - issues to solve
From: Jakub Narebski @ 2010-12-09 22:30 UTC (permalink / raw)
To: J.H.
Cc: git, John 'Warthog9' Hawley, Junio C Hamano, Petr Baudis,
admin
In-Reply-To: <4D00316F.9000305@eaglescrag.net>
On Thu, 9 Dec 2010, J.H. wrote:
> > 1. Error handling
> [...]
> > Note that in my opinion cache should not be initialized if caching is
> > disabled.
>
> Well it wasn't getting initialized in my code if caching was turned off.
> It might have been buffering, but it wasn't mucking with the directory
> creation and such...
What I wanted to say here was that, in my opinion, adding support for
caching to gitweb shouldn't change gitweb behavior in the case when
caching is enabled (and should modify gitweb code only minimally).
The original v7 series ran capture_fetch also when caching was disabled,
and in such case captured gitweb to in-memory file to print it at end.
This was fixed (well, fixed if we consider this a bug/misfeature, which
I do) in v7.1, and finally done without errors in v7.4
> > The modern CHI caching interface provides a way to change how runtime
> > errors are handled via 'on_get_error' and 'on_set_error' parameters to
> > constructor. The default for CHI is to log error, or ignore if no
> > logger is set. You can provide a coderef (callback).
> >
> >
> > For errors during getting data from cache (case B), we can ignore error
> > which means generating data as if cache was disabled/entry did not
> > exist in cache, or erroring out and showing error to user. Such errors
> > should be rare, so erroring out might be appropriate reaction. Note
> > that we have special case of zero sized cache entry file, in which case
> > we treat it as non-existent entry, already.
> >
> >
> > For errors during setting (saving) data to cache (case C), we can
> > ignore error and not re-generate cache (not even calling callback if
> > error is detected early), or error out. Such errors can happen for
> > example if filesystem gets full. It might be better to ignore such
> > errore, at least in the case of ENOSPC / ENOMEM.
>
> The potential, at least for the deployed sites who are already using the
> gitweb caching, for disaster if caching isn't running is pretty high. I
> would agree that erroring out with a 500 that explains the 'why' behind
> the failure is the only real option right now.
>
> The callback mechanism in CHI is nice, but I think for the time being is
> a little overkill for what we've got right now. I also don't see that
> many, typical, sysadmins adding in that many error handling hooks and
> such into gitweb and/or their infrastructure. I could be wrong, I just
> don't see that as being a common occurrence.
Well, we would of course provide default callback that uses die_error,
see for example
[PATCHv6/RFC 21/24] gitweb: Wrap die_error to use as error handler for caching engine
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163043
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/9e8e92f318c6b776486ab19509326f0b470d491d
What I want to avoid is using die_error in caching module; this entangles
caching module code with gitweb code, and makes it much more difficult to
do unit tests / test caching code in isolation (die_error would have to be
mocked). This would also prevent making caching engine a proper Perl
module, I think...
There is also another issue: with error handler passed as callback we
can for example use CHI as caching engine (with any of its drivers:
File, FastMmap, Memcached) with our own/gitweb error reporting.
> > 2. Progress info and not caching error pages
> >
> > J.H. find out or invented very nice hack to generate progress info
> > page without Ajax (and therefore without need for JavaScript), to make
> > user aware that data is being regenerated, and that he/she should be
> > patient instead of refreshing or complaining.
>
> Not so much found or invented, it's something that's been around since I
> started doing any web development back in 1995. It's a bit old-school,
> I'll admit, but it gets the job done still.
Nice idea, anyway. I do wonder though if all web browsers follow this
behavior, and is this behavior documented / required...
> [...]
> > JH> There are still a few known "issues" with respect to this:
> > JH> - Code needs to be added to be "browser" aware so
> > JH> that clients like wget that are trying to get a
> > JH> binary blob don't obtain a "Generating..." page
>
> I've solved this in v8, there is now an explicit blacklist for clients
> that we assume are only there for whatever is getting generated. Right
> now the list is only wget and curl as those are the only two I can think
> of immediately that wouldn't be able to parse the meta-equiv refresh.
In my rewrite
[PATCHv6 17/24] gitweb: Show appropriate "Generating..." page when regenerating cache
http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163040
http://repo.or.cz/w/git/jnareb-git.git/commitdiff/48679f7985ccda16dc54fda97790841bab4a0ba2#patch1
(see the browser_is_robot() subroutine:
http://repo.or.cz/w/git/jnareb-git.git/blob/48679f7985ccda16dc54fda97790841bab4a0ba2:/gitweb/gitweb.perl#l870
I use HTTP::BrowserDetect package if available and it's ->robot() method.
The fallback is to use *whitelist*, assuming that it would be better to
not show "Generating..." page rather than download the wrong thing.
I also guess that most (all?) web browsers use "Mozilla compatibile"
somewhere in their User-Agent string, thus matching 'Mozilla'.
>
> There is already a special case for feeds to not get this, so those
> shouldn't be affected as it is already.
Yes, it is true that you limit actions that are affected. I also
use whitelist in my rewrite, allowing only actions which output HTML
to use "Generating..." page. Perhaps that's too strict.
> Downside is it's a blacklist, and inherits all the problems of a
> blacklist. Though there really isn't a better way to handle that, given
> there's no way to really query a browser and ask what it's capabilities
> really are.
Well, we can always use whitelist instead, see above.
> > there is additional problem, at least in my rewrite. (I didn't follow
> > the code flow in J.H. v7 patch series to check if it also affected).
> >
> >
> > Let's assume that 'progress_info' feature is turned on, and that cache
> > is generated in background. Let's assume that error pages (die_error)
> > are not cached.
> >
> > Now client tries to access page which would/will result in an error.
> > With caching it acquires exclusive (writers) lock, check that there
> > are no stale data (error pages are never cached), checks that it is
> > provided with 'generating_info' subroutine, so it forks a process to
> > generate data in background.
> >
> > Background process detaches, tries to generate data to cache,
> > die_error is called. die_error turns off capturing, and prints to
> > STDOUT. At the end of die_error there is jump to outer scope to
> > DONE_GITWEB; therefore $lock_fh goes out of scope and lockfile is
> > closed, and lock released. Background process finishes.
> >
> > Parent process runs 'generating_info' subroutine. Now if it waits a
> > bit like in my rewrite before employing trick mentioned above, and
> > does not print anything if lockfile is released before startup delay,
> > _and_ die_error finishes within this delay, then everything is all
> > right: the error message is sent to client.
> >
> > If die_error doesn't finish before git_generating_data_html prints
> > meta refresh, or there is no startup delay, then error pages would get
> > infinite redirects (remember: there is never anything in cache for
> > error pages). This is a bad thing.
> >
> >
> > One possible solution for this problem (beside employing startup
> > delay) is to have tee output, i.e. print it as it is being captured.
> > Streamed (partial) response would serve as progress indicator for
> > process (re)generating cache; only parallel processes waiting for
> > cache would show 'generating_info'.
> >
> > I think that in current implementation of capturing it would be as
> > simple as not closing STDOUT, but I'd have to check that.
>
> Actually in reading through this I thought of a better way to get the
> error message cleanly passed from the backend process to the frontend
> waiting processes.
>
> It's implemented in v8, but it basically boils down to, strangely
> enough, caching the error. (I've got a hammer, it clearly solves all
> problems!). It's not a full cache (like what I'm doing with the rest of
> the cache) but it basically generates a file that can be used to short
> circuit things a bit.
Hmmm... I'll take a look at your solution then. Looks like it can be
interesting. I can think of capturing die_error output to in-memory
file (doing caching of its output in-memory), but I don't see how you
can change "goto DONE_GITWEB" (or in patch[1] sent to git mailing list
"goto DONE_REQUEST") to exit action subroutine but not exit cache_fetch
so gitweb would be able to display catched output.
[1]: http://permalink.gmane.org/gmane.comp.version-control.git/162156
"[PATCH/RFC] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error"
> > 3. Using generic cache engine and memory consumption
> >
> > Most Perl caching interfaces support only $cache->set($key, $data),
> > where $data is a Perl variable, and $data = $cache->get($key), or
> > their equivalents. Even for file-based cache drivers you save from
> > memory and read into memory.
> >
> > The only exception I know of is Cache interface with Cache::File
> > driver, that provides $cache->handle($key [, $mode]), where optional
> > $mode argument can be any of mode strings that can be used in 'open'
> > function, or one of fopen(3) modes. (Of course for memory-based or
> > network-based (like memcached) caches it might not make sense to
> > provide such interface).
> >
> > The most often recommended capture module, Capture::Tiny, allows only
> > capturing into scalar (into memory)
> >
> > $stdout = capture \&code;
> >
> > or (using its prototype)
> >
> > $stdout = capture { <code> };
> >
> > Well, it is *::Tiny, so it supports minimal API. From the list of
> > different capture modules, that allow capturing of Perl code output,
> > with different interfaces (API), in "See also" section of
> > Capture::Tiny documentation[3], only IO::CaptureOutput allow capturing
> > into specified file:
> >
> > capture \&code, \$stdout, undef, $outfile;
> >
> > [3] http://p3rl.org/Capture::Tiny
> >
> >
> > J.H.'s gitweb output caching v7 captures output directly ito cache
> > files. The problem with doing it in my rewrite is to allow capturing
> > directly into cache entry file without losing ability to select
> > different caching engine, which might be not file-based (like
> > e.g. memcached-based).
>
> One of the big things that my caching engine now stops is the stampeding
> herd problem, basically by locking everyone out of the cache for that
> entry till it's prepared. Assuming that can be preserved with any
> arbitrary caching engine, then capturing directly to the file isn't that
> big of a deal.
Well, the recommended mechanism that one can use to prevent 'cache miss
stampede' problem in CHI[2] caching interface (though not the more generic
'stampeding herd' problem) is to use "expires_variance" option. From
CHI manpage:
expires_variance [FLOAT]
Controls the variable expiration feature, which allows items to expire
a little earlier than the stated expiration time to help prevent cache
miss stampedes.
Value is between 0.0 and 1.0, with 0.0 meaning that items expire
exactly when specified (feature is disabled), and 1.0 meaning that
items might expire anytime from now til the stated expiration time.
The default is 0.0. A setting of 0.10 to 0.25 would introduce a small
amount of variation without interfering too much with intended
expiration times.
The probability of expiration increases as a function of how far along
we are in the potential expiration window, with the probability being
near 0 at the beginning of the window and approaching 1 at the end.
[...]
[2]: http://p3rl.org/CHI
Though with locking we can for example lock whole cache instead of single
entry when average load gets too high (but lower than $maxload, when gitweb
returns '503 - The load average on the server is too high' (custom version
of '503 Service Unavailable' HTTP error code/status)).
I think that CHI can be extended (via roles perhaps) to use locking:
flock for 'File' driver, IPC::Lock::Memcached or similar for 'Memcached'
driver, get_and_set from Cache::FastMmap which IIUC uses fcntl to lock
fragments of mmapped file for 'FastMmap' driver.
Cache::File driver from Cache package (http://p3rl.org/Cache) supports
locking by itself:
lock_level
Specify the level of locking to be used. There are three different
levels available:
Cache::File::LOCK_NONE()
No locking is performed. Useful when you can guarantee only one
process will be accessing the cache at a time.
Cache::File::LOCK_LOCAL()
Locking is performed, but it is not suitable for use over NFS
filesystems. However it is more efficient.
Cache::File::LOCK_NFS()
Locking is performed in a way that is suitable for use on NFS
filesystems.
[...]
locking efficiency
Currently LOCK_LOCAL is not implemented (if uses the same code as
LOCK_NFS).
There are two points of locking in Cache::File, index locking and
entry locking. The index locking is always exclusive and the lock
is required briefly during most operations. The entry locking is
either shared or exclusive and is also required during most operations.
When locking is enabled, File::NFSLock is used to provide the locking
for both situations. This is not overly efficient, especially as the
entry lock is only ever grabbed whilst the index lock is held.
> If that can be preserved in your full re-write, then it doesn't really
> matter how or when the data makes it into the caching system be it
> memory, disk, /dev/random or collapsing quantum wave form
In my rewrite I use the same flock-based locking mechanism... just
rewritten. From the cover letter:
JN> The main ideas in lifted from J.H. patches are the following
JN> (features in common with "Gitweb caching v7" series by John Hawley):
JN>
JN> * caching captured output of gitweb in flat files, without any
JN> serialization (caching raw data)
JN>
JN> * using global (per-cache, not per-entry) expiration time, and
JN> using difference between mtime of cache file and current time
JN> for expiration
JN>
JN> * using file locking (flock) to prevent 'cache miss stampede'
JN> problem, i.e. to ensure that only one process is (re)generating
JN> cache entry
JN>
JN> * serving stale but not too old version, and regenerating data
JN> in background, to avoid waiting for data to be regenerated
JN>
JN> * progress info indicator based on http-equiv refresh trick
JN> (described in more detail how it works in the commit message)
JN>
JN> * capturing gitweb output by redirecting STDOUT to cache entry file
> I'm going to try and get v8 out tomorrow morning, and I'm going to let
> this stew on kernel.org overnight. It's looking like 18 patches (which
> includes Jakub's v7.2, since I was building straight on top of that).
Hmmm... I was thinking about doing rewrite of rewrite, keeping only
parts which are actually used. Instead of 22 patches it would be
4-9 patches:
* capturing output to file or filehandle
* caching with locking, using 'compute_fh' interface
* module implementing cache_output subroutine, joining those two
* adding caching support to gitweb
where the following patches might be extracted as additional patches
* adaptive cache lifetime
* zero-size file check
* "Generating..." page for cache and for gitweb
perhaps also with separating
* serving stale data
* background cache generation
> In poking around, I'll admit, I keep finding things I'd love to fix /
> cleanup throughout the entirety of the code base. I'm kinda hoping to
> finally get caching dealt with and merged so I can start scratching
> those other itches. Mind you this has been 4+ years in the making
> already...
I am thinking about adding support for "tee" in addition to "capture",
i.e. printing when capturing, and using it in cache...
> - John 'Warthog9' Hawley
>
> P.S. Most of this e-mail was written a couple of weeks ago, I found it
> in my drafts folder and wanted to get it out along with a note that I've
> got v8 percolating. -JH
This email was written before v8 appeared on git mailing list...
P.S. Did you had a chance to read mine rerwite of gitweb output caching
"[PATCHv6/RFC 00/24] gitweb: Simple file based output caching"?
--
Jakub Narebski
Poland
^ permalink raw reply
* git-cvsimport with cvsps output in commit msg breaks imports
From: Thomas Adam @ 2010-12-09 22:03 UTC (permalink / raw)
To: git; +Cc: cvsps, smurf
Hi all,
[ I've Cced both the cvsps maintainer and the author listed for
git-cvsimport in case it's more relevant to either tool. ]
I am wondering if anyone here is able to shed some light on a problem I've
encountered with git-cvsimport. For ages now, I've had an automatic
conversion of a CVS repository to a Git one, using git-cvsimport to update a
repository as commits happen in CVS.
The repository in question is here:
https://github.com/ThomasAdam/tmux
Everything is on the Master branch.
More specifically, the commit which I think introduced the problem, and all
subsequent commits thereafter is here:
https://github.com/ThomasAdam/tmux/commit/f0220a10b01a764e0dc52ea1b2407f58600a30eb
Note that from this commit onwards, the commit *message* has a bunch of
cvsps output in it. I can only surmise that this somehow causes problems
for cvsimport.
But I can't say for sure.
But if you look at the commit after f0220a10b0:
https://github.com/ThomasAdam/tmux/commit/a7e5b474af93e36e314170e2db18e544e34cf7b3
The file cvsimport has imported has actually *reverted* the file in
question. Confer:
-/* $Id: tmux.h,v 1.582 2010/12/06 21:48:56 nicm Exp $ */
+/* $Id: tmux.h,v 1.246 2009/01/21 19:38:51 nicm Exp $ */
Is anyone able to shed some light on this? Needless to say I am a little
perplexed that content in the commit message (if that's what it is) would
cause a cvsimport to fail.
The net result of this is I can no longer compile the master branch at all
because of this. Oh, and should anyone be thinking it, compiling the CVS
repository which the git-cvsimport command is referencing, compiles fine.
So it's definitely a problem with the import.
Any questions or further bits of information I can provide, do shout.
TIA.
-- Thomas Adam
--
"Deep in my heart I wish I was wrong. But deep in my heart I know I am
not." -- Morrissey ("Girl Least Likely To" -- off of Viva Hate.)
^ permalink raw reply
* [PATCH 02/18] gitweb: add output buffering and associated functions
From: John 'Warthog9' Hawley @ 2010-12-09 21:57 UTC (permalink / raw)
To: git
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
This adds output buffering for gitweb, mainly in preparation for
caching support. This is a dramatic change to how caching was being
done, mainly in passing around the variable manually and such.
This centrally flips the entire STDOUT to a variable, which after the
completion of the run, flips it back and does a print on the resulting
data.
This should save on the previous 10K line patch (or so) that adds more
explicit output passing.
[jn: modified reset_output to silence 'gitweb.perl: Name "main::STDOUT_REAL"
used only once: possible typo at ../gitweb/gitweb.perl line 1130.' warning]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cfa511c..cae0e34 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -39,6 +39,9 @@ BEGIN {
our $version = "++GIT_VERSION++";
+# Output buffer variable
+our $output = "";
+
our ($my_url, $my_uri, $base_url, $path_info, $home_link);
sub evaluate_uri {
our $cgi;
@@ -1134,6 +1137,25 @@ sub evaluate_argv {
);
}
+sub change_output {
+ our $output;
+
+ # Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such
+ open(STDOUT_REAL,">&STDOUT") or die "Unable to capture STDOUT $!\n";
+
+ # Close STDOUT, so that it isn't being used anymore.
+ close STDOUT;
+
+ # Trap STDOUT to the $output variable, which is what I was using in the original
+ # patch anyway.
+ open(STDOUT,">", \$output) || die "Unable to open STDOUT: $!"; #open STDOUT handle to use $var
+}
+
+sub reset_output {
+ # This basically takes STDOUT_REAL and puts it back as STDOUT
+ open STDOUT, ">&", \*STDOUT_REAL;
+}
+
sub run {
evaluate_argv();
@@ -1145,7 +1167,10 @@ sub run {
$pre_dispatch_hook->()
if $pre_dispatch_hook;
+ change_output();
run_request();
+ reset_output();
+ print $output;
$post_dispatch_hook->()
if $post_dispatch_hook;
@@ -3655,6 +3680,10 @@ sub die_error {
500 => '500 Internal Server Error',
503 => '503 Service Unavailable',
);
+ # Reset the output so that we are actually going to STDOUT as opposed
+ # to buffering the output.
+ reset_output();
+
git_header_html($http_responses{$status}, undef, %opts);
print <<EOF;
<div class="page_body">
--
1.7.2.3
^ permalink raw reply related
* [PATCH 03/18] gitweb: File based caching layer (from git.kernel.org)
From: John 'Warthog9' Hawley @ 2010-12-09 21:57 UTC (permalink / raw)
To: git
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
This is a relatively large patch that implements the file based
caching layer that is quite similar to the one used on such large
sites as kernel.org and soon git.fedoraproject.org. This provides
a simple, and straight forward caching mechanism that scales
dramatically better than Gitweb by itself.
The caching layer basically buffers the output that Gitweb would
normally return, and saves that output to a cache file on the local
disk. When the file is requested it attempts to gain a shared lock
on the cache file and cat it out to the client. Should an exclusive
lock be on a file (it's being updated) the code has a choice to either
update in the background and go ahead and show the stale page while
update is being performed, or stall the client(s) until the page
is generated.
There are two forms of stalling involved here, background building
and non-background building, both of which are discussed in the
configuration page.
There are still a few known "issues" with respect to this:
- Code needs to be added to be "browser" aware so
that clients like wget that are trying to get a
binary blob don't obtain a "Generating..." page
Caching is disabled by default. You can turn it on by setting
$caching_enabled variable to true to enable file based caching.
[jn: added error checking to loading 'cache.pl'; moved check
for $caching_enabled outside out of cache_fetch, which required
update to die_error()]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitweb/Makefile | 3 +
gitweb/gitweb.perl | 105 ++++++++++++--
gitweb/lib/cache.pl | 348 ++++++++++++++++++++++++++++++++++++++++++++++
gitweb/static/gitweb.css | 6 +
4 files changed, 450 insertions(+), 12 deletions(-)
create mode 100644 gitweb/lib/cache.pl
diff --git a/gitweb/Makefile b/gitweb/Makefile
index f9e32eb..6ddd4f1 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -113,6 +113,9 @@ endif
GITWEB_FILES += static/git-logo.png static/git-favicon.png
+# Gitweb caching
+GITWEB_MODULES += cache.pl
+
GITWEB_REPLACE = \
-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
-e 's|++GIT_BINDIR++|$(bindir)|g' \
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cae0e34..3c3ff08 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -250,6 +250,53 @@ our %avatar_size = (
# Leave it undefined (or set to 'undef') to turn off load checking.
our $maxload = 300;
+# This enables/disables the caching layer in gitweb. This currently only supports the
+# 'dumb' file based caching layer, primarily used on git.kernel.org. this is reasonably
+# effective but it has the downside of requiring a huge amount of disk space if there
+# are a number of repositories involved. It is not uncommon for git.kernel.org to have
+# on the order of 80G - 120G accumulate over the course of a few months. It is recommended
+# that the cache directory be periodically completely deleted, and this is safe to perform.
+# Suggested mechanism
+# mv $cacheidr $cachedir.flush;mkdir $cachedir;rm -rf $cachedir.flush
+our $caching_enabled = 0;
+
+# Used to set the minimum cache timeout for the dynamic caching algorithm. Basically
+# if we calculate the cache to be under this number of seconds we set the cache timeout
+# to this minimum.
+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour
+our $minCacheTime = 20;
+
+# Used to set the maximum cache timeout for the dynamic caching algorithm. Basically
+# if we calculate the cache to exceed this number of seconds we set the cache timeout
+# to this maximum.
+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour
+our $maxCacheTime = 1200;
+
+# If you need to change the location of the caching directory, override this
+# otherwise this will probably do fine for you
+our $cachedir = 'cache';
+
+# If this is set (to 1) cache will do it's best to always display something instead
+# of making someone wait for the cache to update. This will launch the cacheUpdate
+# into the background and it will lock a <file>.bg file and will only lock the
+# actual cache file when it needs to write into it. In theory this will make
+# gitweb seem more responsive at the price of possibly stale data.
+our $backgroundCache = 1;
+
+# Used to set the maximum cache file life. If a cache files last modify time exceeds
+# this value, it will assume that the data is just too old, and HAS to be regenerated
+# instead of trying to display the existing cache data.
+# Value is in seconds. 1 = 1 seconds, 60 = 1 minute, 600 = 10 minutes, 3600 = 1 hour
+# 18000 = 5 hours
+our $maxCacheLife = 18000;
+
+# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes
+our $cacheDoFork = 1;
+
+our $fullhashpath = *STDOUT;
+our $fullhashbinpath = *STDOUT;
+our $fullhashbinpathfinal = *STDOUT;
+
# configuration for 'highlight' (http://www.andre-simon.de/)
# match by basename
our %highlight_basename = (
@@ -506,6 +553,15 @@ our %feature = (
'default' => [0]},
);
+#
+# Includes
+#
+if (!exists $INC{'cache.pl'}) {
+ my $return = do 'cache.pl';
+ die $@ if $@;
+ die "Couldn't read 'cache.pl': $!" if (!defined $return);
+}
+
sub gitweb_get_feature {
my ($name) = @_;
return unless exists $feature{$name};
@@ -734,6 +790,10 @@ our %actions = (
"project_list" => \&git_project_list,
"project_index" => \&git_project_index,
);
+sub is_cacheable {
+ my $action = shift;
+ return !($action eq 'blame_data' || $action eq 'blame_incremental');
+}
# finally, we have the hash of allowed extra_options for the commands that
# allow them
@@ -1072,7 +1132,11 @@ sub dispatch {
!$project) {
die_error(400, "Project needed");
}
- $actions{$action}->();
+ if ($caching_enabled && is_cacheable($action)) {
+ cache_fetch($action);
+ } else {
+ $actions{$action}->();
+ }
}
sub reset_timer {
@@ -1142,6 +1206,7 @@ sub change_output {
# Trap the 'proper' STDOUT to STDOUT_REAL for things like error messages and such
open(STDOUT_REAL,">&STDOUT") or die "Unable to capture STDOUT $!\n";
+ print STDOUT_REAL "";
# Close STDOUT, so that it isn't being used anymore.
close STDOUT;
@@ -1167,10 +1232,7 @@ sub run {
$pre_dispatch_hook->()
if $pre_dispatch_hook;
- change_output();
run_request();
- reset_output();
- print $output;
$post_dispatch_hook->()
if $post_dispatch_hook;
@@ -3447,7 +3509,8 @@ sub git_header_html {
# support xhtml+xml but choking when it gets what it asked for.
if (defined $cgi->http('HTTP_ACCEPT') &&
$cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
- $cgi->Accept('application/xhtml+xml') != 0) {
+ $cgi->Accept('application/xhtml+xml') != 0 &&
+ !$caching_enabled) {
$content_type = 'application/xhtml+xml';
} else {
$content_type = 'text/html';
@@ -3592,6 +3655,7 @@ sub git_footer_html {
my $feed_class = 'rss_logo';
print "<div class=\"page_footer\">\n";
+ print "<div class=\"cachetime\">Cache Last Updated: ". gmtime( time ) ." GMT</div>\n";
if (defined $project) {
my $descr = git_get_project_description($project);
if (defined $descr) {
@@ -3680,9 +3744,14 @@ sub die_error {
500 => '500 Internal Server Error',
503 => '503 Service Unavailable',
);
+ # The output handlers for die_error need to be reset to STDOUT
+ # so that half the message isn't being output to random and
+ # half to STDOUT as expected. This is mainly for the benefit
+ # of using git_header_html() and git_footer_html() since
+ #
# Reset the output so that we are actually going to STDOUT as opposed
# to buffering the output.
- reset_output();
+ reset_output() if ($caching_enabled);
git_header_html($http_responses{$status}, undef, %opts);
print <<EOF;
@@ -5592,9 +5661,15 @@ sub git_blob_plain {
($sandbox ? 'attachment' : 'inline')
. '; filename="' . $save_as . '"');
local $/ = undef;
- binmode STDOUT, ':raw';
- print <$fd>;
- binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ if ($caching_enabled) {
+ open BINOUT, '>', $fullhashbinpath or die_error(500, "Could not open bin dump file");
+ }else{
+ open BINOUT, '>', \$fullhashbinpath or die_error(500, "Could not open bin dump file");
+ }
+ binmode BINOUT, ':raw';
+ print BINOUT <$fd>;
+ binmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ close BINOUT;
close $fd;
}
@@ -5879,9 +5954,15 @@ sub git_snapshot {
open my $fd, "-|", $cmd
or die_error(500, "Execute git-archive failed");
- binmode STDOUT, ':raw';
- print <$fd>;
- binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ if ($caching_enabled) {
+ open BINOUT, '>', $fullhashbinpath or die_error(500, "Could not open bin dump file");
+ }else{
+ open BINOUT, '>', \$fullhashbinpath or die_error(500, "Could not open bin dump file");
+ }
+ binmode BINOUT, ':raw';
+ print BINOUT <$fd>;
+ binmode BINOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ close BINOUT;
close $fd;
}
diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
new file mode 100644
index 0000000..dd14bfb
--- /dev/null
+++ b/gitweb/lib/cache.pl
@@ -0,0 +1,348 @@
+# gitweb - simple web interface to track changes in git repositories
+#
+# (C) 2006, John 'Warthog9' Hawley <warthog19@eaglescrag.net>
+#
+# This program is licensed under the GPLv2
+
+#
+# Gitweb caching engine
+#
+
+#use File::Path qw(make_path remove_tree);
+use File::Path qw(mkpath rmtree); # Used for compatability reasons
+use Digest::MD5 qw(md5 md5_hex md5_base64);
+use Fcntl ':flock';
+use File::Copy;
+
+sub cache_fetch {
+ my ($action) = @_;
+ my $cacheTime = 0;
+
+ if(! -d $cachedir){
+ print "*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\n";
+ mkdir ("cache", 0755) || die "Cannot create cache dir - you will need to manually create";
+ print "Cache directory created successfully\n";
+ }
+
+ our $full_url = "$my_url?". $ENV{'QUERY_STRING'};
+ our $urlhash = md5_hex($full_url);
+ our $fullhashdir = "$cachedir/". substr( $urlhash, 0, 2) ."/";
+
+ eval { mkpath( $fullhashdir, 0, 0777 ) };
+ if ($@) {
+ die_error(500, "Internal Server Error", "Could not create cache directory: $@");
+ }
+ $fullhashpath = "$fullhashdir/". substr( $urlhash, 2 );
+ $fullhashbinpath = "$fullhashpath.bin.wt";
+ $fullhashbinpathfinal = "$fullhashpath.bin";
+
+ if(! -e "$fullhashpath" ){
+ if(! $cacheDoFork || ! defined(my $childPid = fork()) ){
+ cacheUpdate($action,0);
+ cacheDisplay($action);
+ } elsif ( $childPid == 0 ){
+ #run the updater
+ cacheUpdate($action,1);
+ }else{
+ cacheWaitForUpdate($action);
+ }
+ }else{
+ #if cache is out dated, update
+ #else displayCache();
+ open(cacheFile, '<', "$fullhashpath");
+ stat(cacheFile);
+ close(cacheFile);
+ my $stat_time = (stat(_))[9];
+ my $stat_size = (stat(_))[7];
+
+ $cacheTime = get_loadavg() * 60;
+ if( $cacheTime > $maxCacheTime ){
+ $cacheTime = $maxCacheTime;
+ }
+ if( $cacheTime < $minCacheTime ){
+ $cacheTime = $minCacheTime;
+ }
+ if( $stat_time < (time - $cacheTime) || $stat_size == 0 ){
+ if( ! $cacheDoFork || ! defined(my $childPid = fork()) ){
+ cacheUpdate($action,0);
+ cacheDisplay($action);
+ } elsif ( $childPid == 0 ){
+ #run the updater
+ #print "Running updater\n";
+ cacheUpdate($action,1);
+ }else{
+ #print "Waiting for update\n";
+ cacheWaitForUpdate($action);
+ }
+ } else {
+ cacheDisplay($action);
+ }
+
+
+ }
+
+ #
+ # If all of the caching failes - lets go ahead and press on without it and fall back to 'default'
+ # non-caching behavior. This is the softest of the failure conditions.
+ #
+ #$actions{$action}->();
+}
+
+sub cacheUpdate {
+ my ($action,$areForked) = @_;
+ my $lockingStatus;
+ my $fileData = "";
+
+ if($backgroundCache){
+ open(cacheFileBG, '>:utf8', "$fullhashpath.bg");
+ my $lockStatBG = flock(cacheFileBG,LOCK_EX|LOCK_NB);
+
+ $lockStatus = $lockStatBG;
+ }else{
+ open(cacheFile, '>:utf8', \$fullhashpath);
+ my $lockStat = flock(cacheFile,LOCK_EX|LOCK_NB);
+
+ $lockStatus = $lockStat;
+ }
+ #print "lock status: $lockStat\n";
+
+
+ if (! $lockStatus ){
+ if ( $areForked ){
+ exit(0);
+ }else{
+ return;
+ }
+ }
+
+ if(
+ $action eq "snapshot"
+ ||
+ $action eq "blob_plain"
+ ){
+ my $openstat = open(cacheFileBinWT, '>>:utf8', "$fullhashbinpath");
+ my $lockStatBin = flock(cacheFileBinWT,LOCK_EX|LOCK_NB);
+ }
+
+ # Trap all output from the action
+ change_output();
+
+ $actions{$action}->();
+
+ # Reset the outputs as we should be fine now
+ reset_output();
+
+
+ if($backgroundCache){
+ open(cacheFile, '>:utf8', "$fullhashpath");
+ $lockStat = flock(cacheFile,LOCK_EX);
+
+ if (! $lockStat ){
+ if ( $areForked ){
+ exit(0);
+ }else{
+ return;
+ }
+ }
+ }
+
+ if(
+ $action eq "snapshot"
+ ||
+ $action eq "blob_plain"
+ ){
+ my $openstat = open(cacheFileBinFINAL, '>:utf8', "$fullhashbinpathfinal");
+ $lockStatBIN = flock(cacheFileBinFINAL,LOCK_EX);
+
+ if (! $lockStatBIN ){
+ if ( $areForked ){
+ exit(0);
+ }else{
+ return;
+ }
+ }
+ }
+
+ # Actually dump the output to the proper file handler
+ local $/ = undef;
+ $|++;
+ print cacheFile "$output";
+ $|--;
+ if(
+ $action eq "snapshot"
+ ||
+ $action eq "blob_plain"
+ ){
+ move("$fullhashbinpath", "$fullhashbinpathfinal") or die "Binary Cache file could not be updated: $!";
+
+ flock(cacheFileBinFINAL,LOCK_UN);
+ close(cacheFileBinFINAL);
+
+ flock(cacheFileBinWT,LOCK_UN);
+ close(cacheFileBinWT);
+ }
+
+ flock(cacheFile,LOCK_UN);
+ close(cacheFile);
+
+ if($backgroundCache){
+ flock(cacheFileBG,LOCK_UN);
+ close(cacheFileBG);
+ }
+
+ if ( $areForked ){
+ exit(0);
+ } else {
+ return;
+ }
+}
+
+
+sub cacheWaitForUpdate {
+ my ($action) = @_;
+ my $x = 0;
+ my $max = 10;
+ my $lockStat = 0;
+
+ if( $backgroundCache ){
+ if( -e "$fullhashpath" ){
+ open(cacheFile, '<:utf8', "$fullhashpath");
+ $lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);
+ stat(cacheFile);
+ close(cacheFile);
+
+ if( $lockStat && ( (stat(_))[9] > (time - $maxCacheLife) ) ){
+ cacheDisplay($action);
+ return;
+ }
+ }
+ }
+
+ if(
+ $action eq "atom"
+ ||
+ $action eq "rss"
+ ||
+ $action eq "opml"
+ ){
+ do {
+ sleep 2 if $x > 0;
+ open(cacheFile, '<:utf8', "$fullhashpath");
+ $lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);
+ close(cacheFile);
+ $x++;
+ $combinedLockStat = $lockStat;
+ } while ((! $combinedLockStat) && ($x < $max));
+
+ if( $x != $max ){
+ cacheDisplay($action);
+ }
+ return;
+ }
+
+ $| = 1;
+
+ print $::cgi->header(
+ -type=>'text/html',
+ -charset => 'utf-8',
+ -status=> 200,
+ -expires => 'now',
+ # HTTP/1.0
+ -Pragma => 'no-cache',
+ # HTTP/1.1
+ -Cache_Control => join(
+ ', ',
+ qw(
+ private
+ no-cache
+ no-store
+ must-revalidate
+ max-age=0
+ pre-check=0
+ post-check=0
+ )
+ )
+ );
+
+ print <<EOF;
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www/w3.porg/TR/html4/strict.dtd">
+<!-- git web w/caching interface version $version, (C) 2006-2010, John 'Warthog9' Hawley <warthog9\@kernel.org> -->
+<!-- git core binaries version $git_version -->
+<head>
+<meta http-equiv="content-type" content="$content_type; charset=utf-8"/>
+<meta name="generator" content="gitweb/$version git/$git_version"/>
+<meta name="robots" content="index, nofollow"/>
+<meta http-equiv="refresh" content="0"/>
+<title>$title</title>
+</head>
+<body>
+EOF
+
+ print "Generating..";
+ do {
+ print ".";
+ sleep 2 if $x > 0;
+ open(cacheFile, '<:utf8', "$fullhashpath");
+ $lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);
+ close(cacheFile);
+ $x++;
+ $combinedLockStat = $lockStat;
+ } while ((! $combinedLockStat) && ($x < $max));
+ print <<EOF;
+</body>
+</html>
+EOF
+ return;
+}
+
+sub cacheDisplay {
+ local $/ = undef;
+ $|++;
+
+ my ($action) = @_;
+ open(cacheFile, '<:utf8', "$fullhashpath");
+ $lockStat = flock(cacheFile,LOCK_SH|LOCK_NB);
+
+ if (! $lockStat ){
+ close(cacheFile);
+ cacheWaitForUpdate($action);
+ }
+
+ if(
+ (
+ $action eq "snapshot"
+ ||
+ $action eq "blob_plain"
+ )
+ ){
+ my $openstat = open(cacheFileBin, '<', "$fullhashbinpathfinal");
+ $lockStatBIN = flock(cacheFileBin,LOCK_SH|LOCK_NB);
+ if (! $lockStatBIN ){
+ system ("echo 'cacheDisplay - bailing due to binary lock failure' >> /tmp/gitweb.log");
+ close(cacheFile);
+ close(cacheFileBin);
+ cacheWaitForUpdate($action);
+ }
+
+ my $binfilesize = -s "$fullhashbinpathfinal";
+ print "Content-Length: $binfilesize";
+ }
+ while( <cacheFile> ){
+ print $_;
+ }
+ if(
+ $action eq "snapshot"
+ ||
+ $action eq "blob_plain"
+ ){
+ binmode STDOUT, ':raw';
+ print <cacheFileBin>;
+ binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+ close(cacheFileBin);
+ }
+ close(cacheFile);
+ $|--;
+}
+
+1;
+__END__
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 4132aab..972d32e 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -67,6 +67,12 @@ div.page_path {
border-width: 0px 0px 1px;
}
+div.cachetime {
+ float: left;
+ margin-right: 10px;
+ color: #555555;
+}
+
div.page_footer {
height: 17px;
padding: 4px 8px;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 06/18] gitweb: Add more explicit means of disabling 'Generating...' page
From: John 'Warthog9' Hawley @ 2010-12-09 21:57 UTC (permalink / raw)
To: git
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
As requested this adds $cacheGenStatus variable, default 1 (on).
If caching is enabled it will explicitly disble the display of the
'Generating...' page and just force the user to stall indefinately.
Also adding it to gitweb's test code as I'm sure the 'Generating...'
page isn't that useful there.
Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
---
gitweb/gitweb.perl | 6 ++++++
gitweb/lib/cache.pl | 2 ++
t/gitweb-lib.sh | 1 +
3 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f2ef3da..05e7ba6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -293,6 +293,12 @@ our $maxCacheLife = 18000;
# Used to enable or disable background forking of the gitweb caching. Mainly here for debugging purposes
our $cacheDoFork = 1;
+# Used to enable or disable the foreground "Generating..." page. This is here to be more explicit should
+# people want to disable it.
+# Default: 1 (True - Enabled)
+# To disable set to 0
+our $cacheGenStatus = 1;
+
our $fullhashpath = *STDOUT;
our $fullhashbinpath = *STDOUT;
our $fullhashbinpathfinal = *STDOUT;
diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
index dd14bfb..a8ee99e 100644
--- a/gitweb/lib/cache.pl
+++ b/gitweb/lib/cache.pl
@@ -224,6 +224,8 @@ sub cacheWaitForUpdate {
$action eq "rss"
||
$action eq "opml"
+ ||
+ ! $cacheGenStatus
){
do {
sleep 2 if $x > 0;
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 16ce811..10c4a3d 100755
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -26,6 +26,7 @@ our \$projects_list = '';
our \$export_ok = '';
our \$strict_export = '';
our \$maxload = undef;
+our \$cacheGenStatus = 0;
EOF
--
1.7.2.3
^ permalink raw reply related
* [PATCH 00/18] Gitweb caching v8
From: John 'Warthog9' Hawley @ 2010-12-09 21:57 UTC (permalink / raw)
To: git
Afternoon everyone,
(Afternoon is like morning, right?)
This is the latest incarnation of gitweb w/ caching. Per the general
consensus and requests from the recent Gittogether I'm re-submitting
my patches.
Bunch of re-works in the code, and several requested features. Sadly the
patch series has balloned as I've been adding things. It was 3-4 patches,
it's now 18. This is based on top of Jakub's v7.2 patch series, but
it should be more or less clean now.
As such there was a bunch of changes that I needed to do to Jakub's tree
which are indicated in the series. Why did I do them up as separate things?
Mainly there's a bunch of history that's getting lost right now between
going back and forth, and I wanted to have clear patches to discuss
should further discussion be needed.
This still differs, by two patches, from whats in production on kernel.org.
It's missing the index page git:// link, and kernel.org and kernel.org also
has the forced version matching. As a note I'll probably let this stew
another day or so on kernel.org and then I'll push it into the Fedora update
stream, as there's a couple of things in this patch series that would be
good for them to have.
There is one additional script I've written that the Fedora folks are using,
and that might be useful to include, which is an 'offline' cache file generator.
It basically wraps gitweb.cgi and at the end moves the cache file into the right
place. The Fedora folks were finding it took hours to generate their front
page, and that doing a background generation almost never completed (due to
process death). This was a simple way to handle that. If people would like
I can add it in as an additional patch.
v8:
- Reverting several changes from Jakub's change set that make no sense
- is_cacheable changed to always return true - nothing special about
blame or blame_incremental as far as the caching engine is concerned
- Reverted config file change "caching_enabled" back to "cache_enable" as this
config file option is already in the wild in production code, as are all
current gitweb-caching configuration variables.
- Reverted change to reset_output as
open STDOUT, ">&", \*STDOUT_REAL;
causes assertion failures:
Assertion !((((s->var)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVLV)) failed: file "scalar.xs", line 49 at gitweb.cgi line 1221.
if we encounter an error *BEFORE* we've ever changed the output.
- Cleanups there were indirectly mentioned by Jakub
- Elimination of anything even remotely looking like duplicate code
- Creation of isBinaryAction() and isFeedAction()
- Adding in blacklist of "dumb" clients for purposes of downloading content
- Added more explicit disablement of "Generating..." page
- Added better error handling
- Creation of .err file in the cache directory
- Trap STDERR output into $output_err as this was spewing data prior to any header information being sent
- Added hidden field in footer for url & hash of url, which is extremely useful for debugging
v7:
- Rework output system, now central STDOUT redirect
- Various fixes to caching brought in from existing
running system
v6:
- Never saw the light of day
- Various testing, and reworks.
v5:
- Missed a couple of things that were in my local tree, and
added them back in.
- 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
v4:
- major re-working of the caching layer to use file handle
redirection instead of buffering output
- other minor improvements
http://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v4
v3:
- various minor re-works based on mailing list feedback,
this series was not sent to the mailing list.
v2:
- Better breakout
- You can actually disable the cache now
- John 'Warthog9' Hawley
Jakub Narebski (2):
gitweb: Prepare for splitting gitweb
gitweb: Minimal testing of gitweb caching
John 'Warthog9' Hawley (16):
gitweb: add output buffering and associated functions
gitweb: File based caching layer (from git.kernel.org)
gitweb: Regression fix concerning binary output of files
gitweb: Add more explicit means of disabling 'Generating...' page
gitweb: Revert back to $cache_enable vs. $caching_enabled
gitweb: Change is_cacheable() to return true always
gitweb: Revert reset_output() back to original code
gitweb: Adding isBinaryAction() and isFeedAction() to determine the
action type
gitweb: add isDumbClient() check
gitweb: Change file handles (in caching) to lexical variables as
opposed to globs
gitweb: Add commented url & url hash to page footer
gitweb: add print_transient_header() function for central header
printing
gitweb: Add show_warning() to display an immediate warning, with
refresh
gitweb: When changing output (STDOUT) change STDERR as well
gitweb: Prepare for cached error pages & better error page handling
gitweb: Add better error handling for gitweb caching
gitweb/Makefile | 20 +-
gitweb/gitweb.perl | 176 ++++++++++-
gitweb/lib/cache.pl | 488 +++++++++++++++++++++++++++++
gitweb/static/gitweb.css | 6 +
t/gitweb-lib.sh | 16 +
t/t9500-gitweb-standalone-no-errors.sh | 20 ++
t/t9501-gitweb-standalone-http-status.sh | 13 +
t/t9502-gitweb-standalone-parse-output.sh | 33 ++
8 files changed, 762 insertions(+), 10 deletions(-)
create mode 100644 gitweb/lib/cache.pl
mode change 100644 => 100755 t/gitweb-lib.sh
--
1.7.2.3
^ permalink raw reply
* [PATCH 13/18] gitweb: Add commented url & url hash to page footer
From: John 'Warthog9' Hawley @ 2010-12-09 21:57 UTC (permalink / raw)
To: git
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
This is mostly a debugging tool, but it adds a small bit of information
to the footer:
<!--
Full URL: |http://localhost/gitweb-caching/gitweb.cgi?p=/project.git;a=summary|
URL Hash: |7a31cfb8a43f5643679eec88aa9d7981|
-->
The first bit tells you what the url that generated the page actually was, the second is
the hash used to store the file with the first two characters being used as the directory:
<cachedir>/7a/31cfb8a43f5643679eec88aa9d7981
Also useful for greping through the existing cache and finding files with unique paths that
you may want to explicitly flush.
Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
---
gitweb/gitweb.perl | 7 +++++++
gitweb/lib/cache.pl | 4 ++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e8c028b..7f8292e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -303,6 +303,9 @@ our $fullhashpath = *STDOUT;
our $fullhashbinpath = *STDOUT;
our $fullhashbinpathfinal = *STDOUT;
+our $full_url;
+our $urlhash;
+
# configuration for 'highlight' (http://www.andre-simon.de/)
# match by basename
our %highlight_basename = (
@@ -3663,6 +3666,10 @@ sub git_footer_html {
print "<div class=\"page_footer\">\n";
print "<div class=\"cachetime\">Cache Last Updated: ". gmtime( time ) ." GMT</div>\n";
+ print "<!--\n".
+ " Full URL: |$full_url|\n".
+ " URL Hash: |$urlhash|\n".
+ "-->\n" if ($cache_enable);
if (defined $project) {
my $descr = git_get_project_description($project);
if (defined $descr) {
diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
index fafc028..63dbe9e 100644
--- a/gitweb/lib/cache.pl
+++ b/gitweb/lib/cache.pl
@@ -30,8 +30,8 @@ sub cache_fetch {
print "Cache directory created successfully\n";
}
- our $full_url = "$my_url?". $ENV{'QUERY_STRING'};
- our $urlhash = md5_hex($full_url);
+ $full_url = "$my_url?". $ENV{'QUERY_STRING'};
+ $urlhash = md5_hex($full_url);
our $fullhashdir = "$cachedir/". substr( $urlhash, 0, 2) ."/";
eval { mkpath( $fullhashdir, 0, 0777 ) };
--
1.7.2.3
^ permalink raw reply related
* [PATCH 15/18] gitweb: Add show_warning() to display an immediate warning, with refresh
From: John 'Warthog9' Hawley @ 2010-12-09 21:57 UTC (permalink / raw)
To: git
In-Reply-To: <1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>
die_error() is an immediate and abrupt action. show_warning() more or less
functions identically, except that the page generated doesn't use the
gitweb header or footer (in case they are broken) and has an auto-refresh
(10 seconds) built into it.
This makes use of print_transient_header() which is also used in the
'Generating...' page. Currently the only warning it throws is about
the cache needing to be created. If that fails it's a fatal error
and we call die_error()
Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
---
gitweb/lib/cache.pl | 36 +++++++++++++++++++++++++++++++++---
1 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
index 723ae9b..28e4240 100644
--- a/gitweb/lib/cache.pl
+++ b/gitweb/lib/cache.pl
@@ -25,9 +25,13 @@ sub cache_fetch {
my $cacheTime = 0;
if(! -d $cachedir){
- print "*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\n";
- mkdir ("cache", 0755) || die "Cannot create cache dir - you will need to manually create";
- print "Cache directory created successfully\n";
+ mkdir ("cache", 0755) || die_error(500, "Internal Server Error", "Cannot create cache dir () - you will need to manually create");
+ show_warning(
+ "<p>".
+ "<strong>*** Warning ***:</strong> Caching enabled but cache directory did not exsist. ($cachedir)<br/>/\n".
+ "Cache directory created successfully\n".
+ "<p>"
+ );
}
$full_url = "$my_url?". $ENV{'QUERY_STRING'};
@@ -119,6 +123,32 @@ sub print_transient_header {
return;
}
+sub show_warning {
+ $| = 1;
+
+ my $warning = esc_html(shift) || "Unknown Warning";
+
+ print_transient_header();
+
+ print <<EOF;
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www/w3.porg/TR/html4/strict.dtd">
+<!-- git web w/caching interface version $version, (C) 2006-2010, John 'Warthog9' Hawley <warthog9\@kernel.org> -->
+<!-- git core binaries version $git_version -->
+<head>
+<meta http-equiv="content-type" content="$content_type; charset=utf-8"/>
+<meta name="generator" content="gitweb/$version git/$git_version"/>
+<meta name="robots" content="index, nofollow"/>
+<meta http-equiv="refresh" content="10"/>
+<title>$title</title>
+</head>
+<body>
+$warning
+</body>
+</html>
+EOF
+ exit(0);
+}
+
sub isBinaryAction {
my ($action) = @_;
--
1.7.2.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox