* Re: [PATCH] t3419-*.sh: Fix arithmetic expansion syntax error
From: Clemens Buchacher @ 2010-12-22 18:42 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <4D10F707.1000206@ramsay1.demon.co.uk>
On Tue, Dec 21, 2010 at 06:50:47PM +0000, Ramsay Jones wrote:
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Thanks.
> Note that this test is unique in having an '#!/bin/bash' line (rather
> than '#!/bin/sh'), which was (indirectly) responsible for me not
> noticing this failure for a while. I don't see anything that would
> require bash, so I suspect this is not intensional.
Indeed. I actually removed all bashisms from the script, but then
apparently forgot to get rid of #!/bin/bash as well.
Clemens
^ permalink raw reply
* git-svn author file and svn2cl compatibility
From: strk @ 2010-12-22 18:41 UTC (permalink / raw)
To: git
The svn2cl tool [1] supports an author file to use for
generating the ChangeLog.
[1] http://arthurdejong.org/svn2cl/
Format of the file is:
nick1:Name <email>
nick2:Name 2 <email2>
It would be nice if git-svn could support the same format
in addition to the one using ``='' instead of ``:''.
That way you might have the authors file already available
in projects that made use of svn2cl.
Does it make sense ?
Please include me in reply as I'm not subscribed.
--strk;
() Free GIS & Flash consultant/developer
/\ http://strk.keybit.net/services.html
^ permalink raw reply
* Re: 'show' pretty %B without a diff
From: peter @ 2010-12-22 18:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Jakub Narebski, Junio C Hamano, git
In-Reply-To: <20101221180459.GA25812@burratino>
On Tue, 21 Dec 2010 12:04:59 -0600, Jonathan Nieder
<jrnieder@gmail.com> wrote:
> Or even "git diff-tree -s --pretty=%B <commit>" if that is the intent.
Exactly the what I asked for. Thanks!
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #06; Tue, 21)
From: Junio C Hamano @ 2010-12-22 20:48 UTC (permalink / raw)
To: Thiago Farina; +Cc: git
In-Reply-To: <AANLkTin_u9FiZf-hbnhY0Dp+LifctxH8wKDL=yRrSpm+@mail.gmail.com>
Thiago Farina <tfransosi@gmail.com> writes:
> On Tue, Dec 21, 2010 at 11:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> * tf/commit-list-prefix (2010-11-26) 1 commit
>> (merged to 'next' on 2010-12-21 at 16e1351)
>> + commit: Add commit_list prefix in two function names.
>>
>> This churn
> Since you said that, can could you drop this patch? I don't mind if
> you discard this patch since you consider it a CHURN[1].
>
>> already introduced an unnecessary conflict.
> Which conflict? If you say, I could try to fix it.
>
>> It is not by itself a biggie, but these things tend to add up.
>
> How *these things* add a conflict? This is a new thing to me really.
Look at output from "git show 16e1351 sha1_name.c".
The damage (i.e. my time wasted to deal with the conflict resulting from
the rename of the function) has already been done. There is nothing to
fix.
One thing you _could_ fix is to keep an eye on what are cooking (e.g. the
diff between maint and pu), and refrain from throwing "trivial clean-up"
patches that may overlap with them at the list until the dust settles.
That would greatly reduce the annoyance factor.
The same comment applies to your patches to reduce use of alloc_nr(). If
absolutely nothing else is going on in the project, these are genuinely
good clean-up patches, but when other patches that give us real-life
benefit are in flight, just having to check if they overlap with whatever
is cooking now is already annoying.
Thanks.
^ permalink raw reply
* [PATCH] unpack_trees(): skip trees that are the same in all input
From: Junio C Hamano @ 2010-12-22 22:13 UTC (permalink / raw)
To: git; +Cc: Linus Torvalds
unpack_trees() merges two trees (the current HEAD and the destination
commit) when switching to another branch, checking and updating the index
entry where the destination differs from the current HEAD. It merges three
trees (the common ancestor, the current HEAD and the other commit) when
performing a three-way merge, checking and updating the index entry when
the merge result differs from the current HEAD. It does so by walking the
input trees in parallel all the way down to the leaves.
One common special case is a directory is identical across the trees
involved in the merge. In such a case, we do not have to descend into the
directory at all---we know that the end result is to keep the entries in
the current index.
This optimization cannot be applied in a few special cases in
unpack_trees(), though. We need to descend into the directory and update
the index entries from the target tree in the following cases:
- When resetting (e.g. "git reset --hard"); and
- When checking out a tree for the first time into an empty working tree
(e.g. "git read-tree -m -u HEAD HEAD" with missing .git/index).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
With and without this patch I tried to recreate a handful of recent
merges in the kernel repository. The result is somewhat mixed but
overall in favor.
<resulting merge commit>
*** CURRENT ***
/usr/bin/time output from "git merge" to reproduce it, without the patch
*** NEW ***
/usr/bin/time output from "git merge" to reproduce it, with the patch
6313e3c21743cc88bb5bd8aa72948ee1e83937b6
*** CURRENT ***
3.88user 0.43system 0:04.31elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+47688outputs (0major+205724minor)pagefaults 0swaps
*** NEW ***
3.78user 0.46system 0:04.26elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+47688outputs (0major+185004minor)pagefaults 0swaps
6dde39be39e636c1d41e73590668d5903b11535b
*** CURRENT ***
0.32user 0.12system 0:00.44elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+15272outputs (0major+23180minor)pagefaults 0swaps
*** NEW ***
0.24user 0.11system 0:00.34elapsed 102%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+15264outputs (0major+18288minor)pagefaults 0swaps
f8f5d4f11dc7d321fb372b09fc8767069a18bf30
*** CURRENT ***
1.37user 0.14system 0:01.52elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+15336outputs (0major+43258minor)pagefaults 0swaps
*** NEW ***
1.40user 0.14system 0:01.54elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+15336outputs (0major+41767minor)pagefaults 0swaps
2cedcc4f122934c3ad38dfb2a400b98a62703e6d
*** CURRENT ***
0.43user 0.10system 0:00.54elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+13696outputs (0major+24652minor)pagefaults 0swaps
*** NEW ***
0.30user 0.12system 0:00.43elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+13696outputs (0major+22156minor)pagefaults 0swaps
81e8d2162566379adcf4b3700f03845c62577145
*** CURRENT ***
0.42user 0.12system 0:00.54elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+13544outputs (0major+24128minor)pagefaults 0swaps
*** NEW ***
0.35user 0.11system 0:00.50elapsed 91%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+13536outputs (0major+21667minor)pagefaults 0swaps
unpack-trees.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index aa585be..7c0c963 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -436,6 +436,10 @@ static int switch_cache_bottom(struct traverse_info *info)
return ret;
}
+static int fast_forward_merge(int n, unsigned long dirmask,
+ struct name_entry *names,
+ struct traverse_info *info);
+
static int traverse_trees_recursive(int n, unsigned long dirmask,
unsigned long df_conflicts,
struct name_entry *names,
@@ -447,6 +451,11 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
struct traverse_info newinfo;
struct name_entry *p;
+ if (!df_conflicts) {
+ int status = fast_forward_merge(n, dirmask, names, info);
+ if (status)
+ return status;
+ }
p = names;
while (!p->mode)
p++;
@@ -694,6 +703,45 @@ static struct cache_entry *find_cache_entry(struct traverse_info *info,
return NULL;
}
+static int fast_forward_merge(int n, unsigned long dirmask,
+ struct name_entry *names,
+ struct traverse_info *info)
+{
+ int i;
+ struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };
+ struct unpack_trees_options *o = info->data;
+
+ /* merging two or more trees with an identical subdirectory? */
+ if ((n < 2) || ((1UL << n) - 1) != dirmask ||
+ !o->merge || o->reset || o->initial_checkout)
+ return 0;
+ for (i = 1; i < n; i++)
+ if (hashcmp(names[i-1].sha1, names[i].sha1))
+ return 0;
+
+ /*
+ * Instead of descending into the directory, keep the contents
+ * of the current index.
+ */
+ while (1) {
+ struct cache_entry *ce;
+ ce = next_cache_entry(o);
+ if (!ce)
+ break;
+ /* Is the entry still in that directory? */
+ if (do_compare_entry(ce, info, names))
+ break;
+ for (i = 0; i < n + 1; i++)
+ src[i] = ce;
+ mark_ce_used(ce, o);
+ if (call_unpack_fn(src, o) < 0)
+ return unpack_failed(o, NULL);
+ if (ce_stage(ce))
+ mark_ce_used_same_name(ce, o);
+ }
+ return dirmask;
+}
+
static void debug_path(struct traverse_info *info)
{
if (info->prev) {
--
1.7.3.4.804.g8883a
^ permalink raw reply related
* Using overlay filesystem for "other" files idea
From: Evgeniy Ivanov @ 2010-12-22 23:02 UTC (permalink / raw)
To: git
Hello,
I got following idea.
If different branches seriously differ and you switch branches, then
it's more likely you will loose most part of build files (e.g. when
run build).
Also in projects that may have some bugs in build system (e.g. buggy
makefiles) there is a chance you will not get clean build environment.
E.g. somebody forgot to add include file as dependency in Makefile ---
oops, you're in trouble.
Even without bugs in build system, some temp files are overwritten
each time you run build for a custom branch.
I think using overlays for other files can help much to solve this
issue. You can have git repository in regular directory (like now) and
overlay fs mounted over this repository (for each branch its own
overlay).
Another benefit is clearing source tree after build.
What do you think, guys? If it sounds good, I could work on this,
maybe even on my work.
--
Evgeniy Ivanov
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #06; Tue, 21)
From: Thiago Farina @ 2010-12-22 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvd2l4jpx.fsf@alter.siamese.dyndns.org>
On Wed, Dec 22, 2010 at 6:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thiago Farina <tfransosi@gmail.com> writes:
>
>> On Tue, Dec 21, 2010 at 11:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> * tf/commit-list-prefix (2010-11-26) 1 commit
>>> (merged to 'next' on 2010-12-21 at 16e1351)
>>> + commit: Add commit_list prefix in two function names.
>>>
>>> This churn
>> Since you said that, can could you drop this patch? I don't mind if
>> you discard this patch since you consider it a CHURN[1].
>>
>>> already introduced an unnecessary conflict.
>> Which conflict? If you say, I could try to fix it.
>>
>>> It is not by itself a biggie, but these things tend to add up.
>>
>> How *these things* add a conflict? This is a new thing to me really.
>
> Look at output from "git show 16e1351 sha1_name.c".
>
> The damage (i.e. my time wasted to deal with the conflict resulting from
> the rename of the function) has already been done. There is nothing to
> fix.
>
> One thing you _could_ fix is to keep an eye on what are cooking (e.g. the
> diff between maint and pu), and refrain from throwing "trivial clean-up"
That should be the pain of being the maintainer. You have to deal with
this, like I have to deal with all the critics too.
> patches that may overlap with them at the list until the dust settles.
The dust never settles in this mailing list.
> That would greatly reduce the annoyance factor.
>
> The same comment applies to your patches to reduce use of alloc_nr(). If
> absolutely nothing else is going on in the project,
Which is impossible, due to the high traffic that this project attracts.
> these are genuinely good clean-up patches, but when other patches that give us real-life benefit are in flight,
Well, I'd say to just ignore it as you have done many times before.
> just having to check if they overlap with whatever
> is cooking now is already annoying.
>
What I can do if you are the ONLY one that has commit access to this project.
^ permalink raw reply
* Re: [PATCH v5] convert filter: supply path to external driver
From: Junio C Hamano @ 2010-12-22 23:22 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: Jonathan Nieder, git, Jeff King, Johannes Sixt
In-Reply-To: <7v4oa565lc.fsf@alter.siamese.dyndns.org>
Here is another, just FYI.
-- >8 --
Subject: [PATCH] t0021: avoid getting filter killed with SIGPIPE
The fake filter did not read from the standard input at all,
which caused the calling side to die with SIGPIPE, depending
on the timing.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0021-conversion.sh | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index aacfd00..9078b84 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -100,6 +100,7 @@ test_expect_success expanded_in_repo '
test_expect_success 'filter shell-escaped filenames' '
cat >argc.sh <<-EOF &&
#!$SHELL_PATH
+ cat >/dev/null
echo argc: \$# "\$@"
EOF
normal=name-no-magic &&
--
1.7.3.4.811.g38bda
^ permalink raw reply related
* [RFC PATCH v7 0/9] gitweb: Output caching, with eval/die based error handling
From: Jakub Narebski @ 2010-12-22 23:54 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
This is preliminary (proof of concept) version of shortened series
intended as replacement (rewrite) of "Gitweb caching v8" series from
John 'Warthog9' Hawley (J.H.).
This series shows how one can manage exception handling using
die_error like die, even in the presence of output caching. The
output caching engine has an option that allows to turn off (default)
or on caching of error pages.
This series is unfinished; it does not include adaptive cache
lifetime, nor support for other caching engines than the one provided
(like Cache::Cache or CHI), nor does it support background cache
generation or progress info indicator.
This is just intended as proof of concept.
---
Jakub Narebski (9):
gitweb: Add optional output caching
gitweb/lib - Cache captured output (using compute_fh)
gitweb/lib - Very simple file based cache
gitweb/lib - Simple output capture by redirecting STDOUT to file
t/test-lib.sh: Export also GIT_BUILD_DIR in test_external
gitweb: Prepare for splitting gitweb
gitweb: Introduce %actions_info, gathering information about actions
gitweb: use eval + die for error (exception) handling
gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error
gitweb/Makefile | 22 +
gitweb/README | 46 ++
gitweb/gitweb.perl | 280 +++++++++++++--
gitweb/lib/GitwebCache/CacheOutput.pm | 84 ++++
gitweb/lib/GitwebCache/Capture/ToFile.pm | 109 ++++++
gitweb/lib/GitwebCache/FileCacheWithLocking.pm | 452 ++++++++++++++++++++++++
t/gitweb-lib.sh | 11 +
t/t9500-gitweb-standalone-no-errors.sh | 20 +
t/t9501-gitweb-standalone-http-status.sh | 13 +
t/t9502-gitweb-standalone-parse-output.sh | 33 ++
t/t9510-gitweb-capture-interface.sh | 34 ++
t/t9510/test_capture_interface.pl | 132 +++++++
t/t9511-gitweb-caching-interface.sh | 34 ++
t/t9511/test_cache_interface.pl | 381 ++++++++++++++++++++
t/t9512-gitweb-cache-output-interface.sh | 34 ++
t/t9512/test_cache_output.pl | 162 +++++++++
t/test-lib.sh | 4
17 files changed, 1806 insertions(+), 45 deletions(-)
create mode 100644 gitweb/lib/GitwebCache/CacheOutput.pm
create mode 100644 gitweb/lib/GitwebCache/Capture/ToFile.pm
create mode 100644 gitweb/lib/GitwebCache/FileCacheWithLocking.pm
create mode 100755 t/t9510-gitweb-capture-interface.sh
create mode 100755 t/t9510/test_capture_interface.pl
create mode 100755 t/t9511-gitweb-caching-interface.sh
create mode 100755 t/t9511/test_cache_interface.pl
create mode 100755 t/t9512-gitweb-cache-output-interface.sh
create mode 100755 t/t9512/test_cache_output.pl
--
Jakub Narebski
ShadeHawk on #git
Poland
^ permalink raw reply
* [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error
From: Jakub Narebski @ 2010-12-22 23:55 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
End the request after die_error finishes, rather than exiting gitweb
instance (perhaps wrapped like in ModPerl::Registry or gitweb.psgi
case).
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4779618..724287b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1169,6 +1169,7 @@ sub run {
run_request();
+ DONE_REQUEST:
$post_dispatch_hook->()
if $post_dispatch_hook;
$first_request = 0;
@@ -3767,7 +3768,7 @@ EOF
print "</div>\n";
git_footer_html();
- goto DONE_GITWEB
+ goto DONE_REQUEST
unless ($opts{'-error_handler'});
}
^ permalink raw reply related
* [RFC PATCH v7 2/9] gitweb: use eval + die for error (exception) handling
From: Jakub Narebski @ 2010-12-22 23:55 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
In gitweb code it is assumed that calling die_error() subroutine would
end request, just like running "die" would. Up till now it was done by
having die_error() jump to DONE_REQUEST (earlier DONE_GITWEB), or in
earlier version just 'exit' (for mod_perl via ModPerl::Registry it ends
request instead of exiting worker).
Instead of using 'goto DONE_REQUEST' for longjmp-like nonlocal jump, or
using 'exit', gitweb uses now native for Perl exception handlingin the
form of eval / die pair ("eval BLOCK" to trap exceptions, "die LIST" to
raise/throw them).
Up till now the "goto DONE_REQUEST" solution was enough, but with the
coming output caching support and it adding modular structure to gitweb,
it would be difficult to continue to keep using this solution
(e.g. interaction with capturing output).
Because gitweb now traps all exceptions occuring run_request(), the
handle_errors_html handler (set via set_message from CGI::Carp) is not
needed; gitweb can call die_error in -error_handler mode itself. This
has the advantage that we can now set correct HTTP header (handler from
CGI::Carp::set_message was run after HTTP headers were already sent).
Gitweb assumes here that exceptions thrown by Perl would be simple
strings; die_error() throws hash reference (if not for minimal
extrenal dependencies, it would be probable object of Class::Exception
or Throwable class thrown).
Note: in newer versions of CGI::Carp there is set_die_handler(), where
handler have to set HTTP headers to the browser itself, but we cannot
rely on new enough CGI::Carp version to have been installed. Also
set_die_handler interferes with fatalsToBrowser.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 26 ++++++++------------------
1 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 724287b..c7a1892 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -12,7 +12,7 @@ use strict;
use warnings;
use CGI qw(:standard :escapeHTML -nosticky);
use CGI::Util qw(unescape);
-use CGI::Carp qw(fatalsToBrowser set_message);
+use CGI::Carp qw(fatalsToBrowser);
use Encode;
use Fcntl ':mode';
use File::Find qw();
@@ -1045,21 +1045,6 @@ sub configure_gitweb_features {
}
}
-# custom error handler: 'die <message>' is Internal Server Error
-sub handle_errors_html {
- my $msg = shift; # it is already HTML escaped
-
- # to avoid infinite loop where error occurs in die_error,
- # change handler to default handler, disabling handle_errors_html
- set_message("Error occured when inside die_error:\n$msg");
-
- # you cannot jump out of die_error when called as error handler;
- # the subroutine set via CGI::Carp::set_message is called _after_
- # HTTP headers are already written, so it cannot write them itself
- die_error(undef, undef, $msg, -error_handler => 1, -no_http_header => 1);
-}
-set_message(\&handle_errors_html);
-
# dispatch
sub dispatch {
if (!defined $action) {
@@ -1167,7 +1152,11 @@ sub run {
$pre_dispatch_hook->()
if $pre_dispatch_hook;
- run_request();
+ eval { run_request() };
+ if (defined $@ && !ref($@)) {
+ # some Perl error, but not one thrown by die_error
+ die_error(undef, undef, $@, -error_handler => 1);
+ }
DONE_REQUEST:
$post_dispatch_hook->()
@@ -3768,7 +3757,8 @@ EOF
print "</div>\n";
git_footer_html();
- goto DONE_REQUEST
+
+ die {'status' => $status, 'error' => $error}
unless ($opts{'-error_handler'});
}
^ permalink raw reply related
* [RFC PATCH v7 3/9] gitweb: Introduce %actions_info, gathering information about actions
From: Jakub Narebski @ 2010-12-22 23:55 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
Currently it only contains information about output format, and is not
used anywhere. It will be used to check whether current action
produces HTML output, and therefore is displaying HTML-based progress
info about (re)generating cache makes sense.
It can contain information about allowed extra options, whether to
display link to feed (Atom or RSS), etc. in easier and faster way than
listing all matching or all non-matching actions at appropriate place.
Currently not used; will be used in next commit, to check if action
produces HTML output and therefore we can use HTML-specific progress
indicator.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 57 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c7a1892..e50654b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -749,6 +749,54 @@ our %allowed_options = (
"--no-merges" => [ qw(rss atom log shortlog history) ],
);
+# action => {
+# # what is the output format (content-type) of action
+# 'output_format' => ('html' | 'text' | 'feed' | 'binary' | undef),
+# # does action require $project parameter to work
+# 'needs_project' => (boolean | undef),
+# # log-like action, can start with arbitrary ref or revision
+# 'log_like' => (boolean | undef),
+# # has no specific feed, or should lik to OPML / generic project feed
+# 'no_feed' => (boolean | undef),
+# # allowed options to be passed ussing 'opt' parameter
+# 'allowed_options' => { 'option_1' => 1 [, ... ] },
+# }
+our %actions_info = ();
+sub evaluate_actions_info {
+ our %actions_info;
+ our (%actions);
+
+ # unless explicitely stated otherwise, default output format is html
+ # most actions needs $project parameter
+ foreach my $action (keys %actions) {
+ $actions_info{$action}{'output_format'} = 'html';
+ $actions_info{$action}{'needs_project'} = 1;
+ }
+ # list all exceptions; undef means variable format (no definite format)
+ $actions_info{$_}{'output_format'} = 'text'
+ foreach qw(commitdiff_plain patch patches project_index blame_data);
+ $actions_info{$_}{'output_format'} = 'feed'
+ foreach qw(rss atom opml); # there are different types (document formats) of XML
+ $actions_info{$_}{'output_format'} = undef
+ foreach qw(blob_plain object);
+ $actions_info{'snapshot'}{'output_format'} = 'binary';
+
+ $actions_info{$_}{'needs_project'} = 0
+ foreach qw(opml project_list project_index);
+
+ $actions_info{$_}{'log_like'} = 1
+ foreach qw(log shortlog history);
+
+ $actions_info{$_}{'no_feed'} = 1
+ foreach qw(tags heads forks tag search);
+
+ foreach my $opt (keys %allowed_options) {
+ foreach my $act (@{$allowed_options{$opt}}) {
+ $actions_info{$act}{'allowed_options'}{$opt} = 1;
+ }
+ }
+}
+
# fill %input_params with the CGI parameters. All values except for 'opt'
# should be single values, but opt can be an array. We should probably
# build an array of parameters that can be multi-valued, but since for the time
@@ -980,7 +1028,7 @@ sub evaluate_and_validate_params {
if (not exists $allowed_options{$opt}) {
die_error(400, "Invalid option parameter");
}
- if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
+ if (!$actions_info{$action}{'allowed_options'}{$opt}) {
die_error(400, "Invalid option parameter for this action");
}
}
@@ -1061,7 +1109,7 @@ sub dispatch {
if (!defined($actions{$action})) {
die_error(400, "Unknown action");
}
- if ($action !~ m/^(?:opml|project_list|project_index)$/ &&
+ if ($actions_info{$action}{'needs_project'} &&
!$project) {
die_error(400, "Project needed");
}
@@ -1142,6 +1190,7 @@ sub evaluate_argv {
sub run {
evaluate_argv();
+ evaluate_actions_info();
$first_request = 1;
$pre_listen_hook->()
@@ -1803,7 +1852,7 @@ sub format_ref_marker {
if ($indirect) {
$dest_action = "tag" unless $action eq "tag";
- } elsif ($action =~ /^(history|(short)?log)$/) {
+ } elsif ($actions_info{$action}{'log_like'}) {
$dest_action = $action;
}
@@ -2277,7 +2326,7 @@ sub get_feed_info {
return unless (defined $project);
# some views should link to OPML, or to generic project feed,
# or don't have specific feed yet (so they should use generic)
- return if ($action =~ /^(?:tags|heads|forks|tag|search)$/x);
+ return if ($actions_info{$action}{'no_feed'});
my $branch;
# branches refs uses 'refs/heads/' prefix (fullname) to differentiate
^ permalink raw reply related
* [RFC PATCH v7 4/9] gitweb: Prepare for splitting gitweb
From: Jakub Narebski @ 2010-12-22 23:56 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
Prepare gitweb for having been split into modules that are to be
installed alongside gitweb in 'lib/' subdirectory, by adding
use lib __DIR__.'/lib';
to gitweb.perl (to main gitweb script), and preparing for putting
modules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.
This preparatory work allows to add new module to gitweb by simply
adding
GITWEB_MODULES += <module>
to gitweb/Makefile (assuming that the module is in 'gitweb/lib/'
directory).
While at it pass GITWEBLIBDIR in addition to GITWEB_TEST_INSTALLED to
allow testing installed version of gitweb and installed version of
modules (for future tests which would check individual (sub)modules).
Using __DIR__ from Dir::Self module (not in core, that's why currently
gitweb includes excerpt of code from Dir::Self defining __DIR__) was
chosen over using FindBin-based solution (in core since perl 5.00307,
while gitweb itself requires at least perl 5.8.0) because FindBin uses
BEGIN block, which is a problem under mod_perl and other persistent
Perl environments (thought there are workarounds).
At Pavan Kumar Sankara suggestion gitweb/Makefile uses
install [OPTION]... SOURCE... DIRECTORY
format (2nd format) with single SOURCE rather than
install [OPTION]... SOURCE DEST
format (1st format) because of security reasons (race conditions).
Modern GNU install has `-T' / `--no-target-directory' option, but we
cannot rely that the $(INSTALL) we are using supports this option.
The install-modules target in gitweb/Makefile uses shell 'for' loop,
instead of make's $(foreach) function, to avoid possible problem with
generating a command line that exceeded the maximum argument list
length.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/Makefile | 17 +++++++++++++++--
gitweb/gitweb.perl | 8 ++++++++
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 0a6ac00..e6029e1 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -57,6 +57,7 @@ PERL_PATH ?= /usr/bin/perl
bindir_SQ = $(subst ','\'',$(bindir))#'
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
+gitweblibdir_SQ = $(subst ','\'',$(gitwebdir)/lib)#'
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))#'
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))#'
@@ -153,20 +154,32 @@ test:
test-installed:
GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
+ GITWEBLIBDIR='$(DESTDIR_SQ)$(gitweblibdir_SQ)' \
$(MAKE) -C ../t gitweb-test
### Installation rules
-install: all
+install: all install-modules
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+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"; \
+ done
+ gitweb_modules="$(GITWEB_MODULES)" && \
+ for mod in $$gitweb_modules; do \
+ $(INSTALL) -m 644 "lib/$$mod" '$(DESTDIR_SQ)$(gitweblibdir_SQ)'/"$$(dirname $$mod)"; \
+ done
+
### Cleaning rules
clean:
$(RM) gitweb.cgi static/gitweb.min.js static/gitweb.min.css GITWEB-BUILD-OPTIONS
-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
+.PHONY: all clean install install-modules test test-installed .FORCE-GIT-VERSION-FILE FORCE
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e50654b..880fdf2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -10,6 +10,14 @@
use 5.008;
use strict;
use warnings;
+
+use File::Spec;
+# __DIR__ is taken from Dir::Self __DIR__ fragment
+sub __DIR__ () {
+ File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
+}
+use lib __DIR__ . '/lib';
+
use CGI qw(:standard :escapeHTML -nosticky);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser);
^ permalink raw reply related
* [RFC PATCH v7 5/9] t/test-lib.sh: Export also GIT_BUILD_DIR in test_external
From: Jakub Narebski @ 2010-12-22 23:56 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
This way we can use it in test scripts written in other languages
(e.g. in Perl), and not rely on "$TEST_DIRECTORY/.."
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
t/test-lib.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 48fa516..c077fa4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -552,9 +552,9 @@ test_external () {
# Announce the script to reduce confusion about the
# test output that follows.
say_color "" "# run $test_count: $descr ($*)"
- # Export TEST_DIRECTORY, TRASH_DIRECTORY and GIT_TEST_LONG
+ # Export TEST_DIRECTORY, GIT_BUILD_DIR, TRASH_DIRECTORY and GIT_TEST_LONG
# to be able to use them in script
- export TEST_DIRECTORY TRASH_DIRECTORY GIT_TEST_LONG
+ export TEST_DIRECTORY GIT_BUILD_DIR TRASH_DIRECTORY GIT_TEST_LONG
# Run command; redirect its stderr to &4 as in
# test_run_, but keep its stdout on our stdout even in
# non-verbose mode.
^ permalink raw reply related
* [RFC PATCH v7 6/9] gitweb/lib - Simple output capture by redirecting STDOUT to file
From: Jakub Narebski @ 2010-12-22 23:57 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
Add GitwebCache::Capture::ToFile package, which captures output by
redirecting STDOUT to given file (specified by filename, or given opened
filehandle), earlier saving original STDOUT to restore it when finished
capturing.
GitwebCache::Capture::ToFile preserves PerlIO layers, both those set
before started capturing output, and those set during capture.
No care was taken to handle the following special cases (prior to
starting capture): closed STDOUT, STDOUT reopened to scalar reference,
tied STDOUT. You shouldn't modify STDOUT during capture.
Includes separate tests for capturing output in
t9510/test_capture_interface.pl which is run as external test from
t9510-gitweb-capture-interface.sh. It tests capturing of utf8 data
printed in :utf8 mode, and of binary data (containing invalid utf8) in
:raw mode.
This patch was based on "gitweb: add output buffering and associated
functions" patch by John 'Warthog9' Hawley (J.H.) in "Gitweb caching v7"
series, and on code of Capture::Tiny by David Golden (Apache License 2.0).
Based-on-work-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/lib/GitwebCache/Capture/ToFile.pm | 109 +++++++++++++++++++++++++
t/t9510-gitweb-capture-interface.sh | 34 ++++++++
t/t9510/test_capture_interface.pl | 132 ++++++++++++++++++++++++++++++
3 files changed, 275 insertions(+), 0 deletions(-)
create mode 100644 gitweb/lib/GitwebCache/Capture/ToFile.pm
create mode 100755 t/t9510-gitweb-capture-interface.sh
create mode 100755 t/t9510/test_capture_interface.pl
diff --git a/gitweb/lib/GitwebCache/Capture/ToFile.pm b/gitweb/lib/GitwebCache/Capture/ToFile.pm
new file mode 100644
index 0000000..d2dbf0f
--- /dev/null
+++ b/gitweb/lib/GitwebCache/Capture/ToFile.pm
@@ -0,0 +1,109 @@
+# gitweb - simple web interface to track changes in git repositories
+#
+# (C) 2010, Jakub Narebski <jnareb@gmail.com>
+#
+# This program is licensed under the GPLv2
+
+#
+# Simple output capturing via redirecting STDOUT to given file.
+#
+
+# This is the same mechanism that Capture::Tiny uses, only simpler;
+# we don't capture STDERR at all, we don't tee, we capture to
+# explicitely provided file (or filehandle).
+
+package GitwebCache::Capture::ToFile;
+
+use strict;
+use warnings;
+
+use PerlIO;
+use Symbol qw(qualify_to_ref);
+
+# Constructor
+sub new {
+ my $class = shift;
+
+ my $self = {};
+ $self = bless($self, $class);
+
+ return $self;
+}
+
+sub capture {
+ my $self = shift;
+ my $code = shift;
+
+ $self->capture_start(@_); # pass rest of params
+ eval { $code->(); 1; };
+ my $exit_code = $?; # save this for later
+ my $error = $@; # save this for later
+
+ my $got_out = $self->capture_stop();
+ $? = $exit_code;
+ die $error if $error;
+
+ return $got_out;
+}
+
+# ----------------------------------------------------------------------
+
+# Start capturing data (STDOUT)
+sub capture_start {
+ my $self = shift;
+ my $to = shift;
+
+ # save copy of real STDOUT via duplicating it
+ my @layers = PerlIO::get_layers(\*STDOUT);
+ open $self->{'orig_stdout'}, ">&", \*STDOUT
+ or die "Couldn't dup STDOUT for capture: $!";
+
+ # close STDOUT, so that it isn't used anymode (to have it fd0)
+ close STDOUT;
+
+ $self->{'to'} = $to;
+ my $fileno = fileno(qualify_to_ref($to));
+ if (defined $fileno) {
+ # if $to is filehandle, redirect
+ open STDOUT, '>&', $fileno;
+ } elsif (! ref($to)) {
+ # if $to is name of file, open it
+ open STDOUT, '>', $to;
+ }
+ _relayer(\*STDOUT, \@layers);
+
+ # started capturing
+ $self->{'capturing'} = 1;
+}
+
+# Stop capturing data (required for die_error)
+sub capture_stop {
+ my $self = shift;
+
+ # return if we didn't start capturing
+ return unless delete $self->{'capturing'};
+
+ # close capture file, and restore original STDOUT
+ my @layers = PerlIO::get_layers(\*STDOUT);
+ close STDOUT;
+ open STDOUT, '>&', fileno($self->{'orig_stdout'});
+ _relayer(\*STDOUT, \@layers);
+
+ return exists $self->{'to'} ? $self->{'to'} : $self->{'data'};
+}
+
+# taken from Capture::Tiny by David Golden, Apache License 2.0
+# with debugging stripped out
+sub _relayer {
+ my ($fh, $layers) = @_;
+
+ my %seen = ( unix => 1, perlio => 1); # filter these out
+ my @unique = grep { !$seen{$_}++ } @$layers;
+
+ binmode($fh, join(":", ":raw", @unique));
+}
+
+
+1;
+__END__
+# end of package GitwebCache::Capture::ToFile
diff --git a/t/t9510-gitweb-capture-interface.sh b/t/t9510-gitweb-capture-interface.sh
new file mode 100755
index 0000000..9151454
--- /dev/null
+++ b/t/t9510-gitweb-capture-interface.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Jakub Narebski
+#
+
+test_description='gitweb capturing interface
+
+This test checks capturing interface used for capturing gitweb output
+in gitweb caching (GitwebCache::Capture* modules).'
+
+# for now we are running only cache interface tests
+. ./test-lib.sh
+
+# this test is present in gitweb-lib.sh
+if ! test_have_prereq PERL; then
+ skip_all='perl not available, skipping test'
+ test_done
+fi
+
+"$PERL_PATH" -MTest::More -e 0 >/dev/null 2>&1 || {
+ skip_all='perl module Test::More unavailable, skipping test'
+ test_done
+}
+
+# ----------------------------------------------------------------------
+
+# The external test will outputs its own plan
+test_external_has_tap=1
+
+test_external \
+ 'GitwebCache::Capture* Perl API (in gitweb/lib/)' \
+ "$PERL_PATH" "$TEST_DIRECTORY"/t9510/test_capture_interface.pl
+
+test_done
diff --git a/t/t9510/test_capture_interface.pl b/t/t9510/test_capture_interface.pl
new file mode 100755
index 0000000..6d90497
--- /dev/null
+++ b/t/t9510/test_capture_interface.pl
@@ -0,0 +1,132 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+use utf8;
+
+use Test::More;
+
+# test source version
+use lib $ENV{GITWEBLIBDIR} || "$ENV{GIT_BUILD_DIR}/gitweb/lib";
+
+# ....................................................................
+
+use_ok('GitwebCache::Capture::ToFile');
+note("Using lib '$INC[0]'");
+note("Testing '$INC{'GitwebCache/Capture/ToFile.pm'}'");
+
+# Test setting up capture
+#
+my $capture = new_ok('GitwebCache::Capture::ToFile' => [], 'The $capture');
+
+
+# Test capturing to file (given by filename) and to filehandle
+#
+sub capture_block (&;$) {
+ $capture->capture(shift, shift || 'actual');
+
+ open my $fh, '<', 'actual' or return;
+ local $/ = undef;
+ my $result = <$fh>;
+ close $fh;
+ return $result;
+}
+
+diag('Should not print anything except test results and diagnostic');
+my $test_data = 'Capture this';
+my $captured = capture_block {
+ print $test_data;
+};
+is($captured, $test_data, 'capture simple data: filename');
+
+open my $fh, '>', 'actual';
+$captured = capture_block(sub {
+ print $test_data;
+}, $fh);
+close $fh;
+is($captured, $test_data, 'capture simple data: filehandle');
+
+
+# Test capturing :utf8 and :raw data
+#
+binmode STDOUT, ':utf8';
+$test_data = <<'EOF';
+Zażółć gęsią jaźń
+EOF
+utf8::decode($test_data);
+$captured = capture_block {
+ binmode STDOUT, ':utf8';
+
+ print $test_data;
+};
+utf8::decode($captured);
+is($captured, $test_data, 'capture utf8 data');
+
+$test_data = '|\x{fe}\x{ff}|\x{9F}|\000|'; # invalid utf-8
+$captured = capture_block {
+ binmode STDOUT, ':raw';
+
+ print $test_data;
+};
+is($captured, $test_data, 'capture raw data');
+
+
+# Test nested capturing, useful for future GitwebCache::CacheOutput tests
+#
+sub read_file {
+ my $filename = shift;
+
+ open my $fh, '<', $filename or return;
+ local $/ = undef;
+ my $result = <$fh>;
+ close $fh;
+
+ return $result;
+}
+
+my $outer_capture = GitwebCache::Capture::ToFile->new();
+$captured = $outer_capture->capture(sub {
+ print "pre|";
+ my $captured = $capture->capture(sub {
+ print "INNER";
+ }, 'inner_actual');
+ print "|post";
+}, 'outer_actual');
+
+my $inner = read_file('inner_actual');
+my $outer = read_file('outer_actual');
+
+is($inner, "INNER", 'nested capture: inner');
+is($outer, "pre||post", 'nested capture: outer');
+
+
+# Testing capture when code dies
+#
+$captured = $outer_capture->capture(sub {
+ print "pre|";
+ eval {
+ my $captured = $capture->capture(sub {
+ print "INNER:pre|";
+ die "die from inner\n";
+ print "INNER:post|"
+ }, 'inner_actual');
+ };
+ print "@=$@" if $@;
+ print "|post";
+}, 'outer_actual');
+
+my $inner = read_file('inner_actual');
+my $outer = read_file('outer_actual');
+
+is($inner, "INNER:pre|",
+ 'nested capture with die: inner output captured up to die');
+is($outer, "pre|@=die from inner\n|post",
+ 'nested capture with die: outer caught rethrown exception from inner');
+
+
+done_testing();
+
+# Local Variables:
+# coding: utf-8
+# End:
^ permalink raw reply related
* [RFC PATCH v7 7/9] gitweb/lib - Very simple file based cache
From: Jakub Narebski @ 2010-12-22 23:57 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
This is first step towards implementing file based output (response)
caching layer that is used on such large sites as kernel.org.
This patch introduces GitwebCaching::SimpleFileCache package, which
follows Cache::Cache / CHI interface, although do not implement it
fully. The intent of following established convention for cache
interface is to be able to replace our simple file based cache,
e.g. by the one using memcached.
The data is stored in the cache as-is, without adding metadata (like
expiration date), and without serialization (which means that one can
store only scalar data). At this point there is no support for
expiring cache entries.
The code of GitwebCaching::SimpleFileCache package in gitweb/lib
was heavily based on file-based cache in Cache::Cache package, i.e.
on Cache::FileCache, Cache::FileBackend and Cache::BaseCache, and on
file-based cache in CHI, i.e. on CHI::Driver::File and CHI::Driver
(including implementing atomic write, something that original patch
lacks). It tries to follow more modern CHI architecture, but without
requiring Moose. It is much simplified compared to both interfaces
and their file-based drivers.
This patch does not yet enable output caching in gitweb (it doesn't
have all required features yet); on the other hand it includes tests
of cache Perl API in t/t9503-gitweb-caching-interface.sh.
Inspired-by-code-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/lib/GitwebCache/FileCacheWithLocking.pm | 452 ++++++++++++++++++++++++
t/t9511-gitweb-caching-interface.sh | 34 ++
t/t9511/test_cache_interface.pl | 381 ++++++++++++++++++++
3 files changed, 867 insertions(+), 0 deletions(-)
create mode 100644 gitweb/lib/GitwebCache/FileCacheWithLocking.pm
create mode 100755 t/t9511-gitweb-caching-interface.sh
create mode 100755 t/t9511/test_cache_interface.pl
diff --git a/gitweb/lib/GitwebCache/FileCacheWithLocking.pm b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
new file mode 100644
index 0000000..ecd0e18
--- /dev/null
+++ b/gitweb/lib/GitwebCache/FileCacheWithLocking.pm
@@ -0,0 +1,452 @@
+# gitweb - simple web interface to track changes in git repositories
+#
+# (C) 2006, John 'Warthog9' Hawley <warthog19@eaglescrag.net>
+# (C) 2010, Jakub Narebski <jnareb@gmail.com>
+#
+# This program is licensed under the GPLv2
+
+#
+# Gitweb caching engine, file-based cache with flock-based entry locking
+#
+
+# Minimalistic cache that stores data in the filesystem, without serialization.
+# It uses file locks (flock) to have only one process generating data and
+# writing to cache, when using CHI-like interface ->compute_fh() method.
+
+package GitwebCache::FileCacheWithLocking;
+
+use strict;
+use warnings;
+
+use Carp;
+use File::Path qw(mkpath);
+use Digest::MD5 qw(md5_hex);
+use Fcntl qw(:flock);
+use POSIX qw(setsid);
+
+# by default, the cache nests all entries on the filesystem single
+# directory deep, i.e. '60/b725f10c9c85c70d97880dfe8191b3' for
+# key name (key digest) 60b725f10c9c85c70d97880dfe8191b3.
+#
+our $DEFAULT_CACHE_DEPTH = 1;
+
+# by default, the root of the cache is located in 'cache'.
+#
+our $DEFAULT_CACHE_ROOT = "cache";
+
+# by default we don't use cache namespace (empty namespace);
+# empty namespace does not allow for simple implementation of clear() method.
+#
+our $DEFAULT_NAMESPACE = '';
+
+# anything less than 0 means to not expire
+#
+our $NEVER_EXPIRE = -1;
+
+# cache expiration of 0 means that entry is expired
+#
+our $EXPIRE_NOW = 0;
+
+# ......................................................................
+# constructor
+
+# The options are set by passing in hash or a reference to a hash containing
+# any of the following keys:
+# * 'namespace'
+# The namespace associated with this cache. This allows easy separation of
+# multiple, distinct caches without worrying about key collision. Defaults
+# to $DEFAULT_NAMESPACE. Might be empty string.
+# * 'cache_root' (Cache::FileCache compatibile),
+# 'root_dir' (CHI::Driver::File compatibile),
+# The location in the filesystem that will hold the root of the cache.
+# Defaults to $DEFAULT_CACHE_ROOT.
+# * 'cache_depth' (Cache::FileCache compatibile),
+# 'depth' (CHI::Driver::File compatibile),
+# The number of subdirectories deep to cache object item. This should be
+# large enough that no cache directory has more than a few hundred objects.
+# Defaults to $DEFAULT_CACHE_DEPTH unless explicitly set.
+# * 'default_expires_in' (Cache::Cache compatibile),
+# 'expires_in' (CHI compatibile) [seconds]
+# The expiration time for objects place in the cache.
+# Defaults to -1 (never expire) if not explicitly set.
+# * 'max_lifetime' [seconds]
+# If it is greater than 0, and cache entry is expired but not older
+# than it, serve stale data when waiting for cache entry to be
+# regenerated (refreshed). Non-adaptive.
+# * 'on_error' (similar to CHI 'on_get_error'/'on_set_error')
+# How to handle runtime errors occurring during cache gets and cache
+# sets, which may or may not be considered fatal in your application.
+# Options are:
+# * "die" (the default) - call die() with an appropriate message
+# * "warn" - call warn() with an appropriate message
+# * "ignore" - do nothing
+# * <coderef> - call this code reference with an appropriate message
+sub new {
+ my $class = shift;
+ my %opts = ref $_[0] ? %{ $_[0] } : @_;
+
+ my $self = {};
+ $self = bless($self, $class);
+
+ $self->{'root'} =
+ exists $opts{'cache_root'} ? $opts{'cache_root'} :
+ exists $opts{'root_dir'} ? $opts{'root_dir'} :
+ $DEFAULT_CACHE_ROOT;
+ $self->{'depth'} =
+ exists $opts{'cache_depth'} ? $opts{'cache_depth'} :
+ exists $opts{'depth'} ? $opts{'depth'} :
+ $DEFAULT_CACHE_DEPTH;
+ $self->{'namespace'} =
+ exists $opts{'namespace'} ? $opts{'namespace'} :
+ $DEFAULT_NAMESPACE;
+ $self->{'expires_in'} =
+ exists $opts{'default_expires_in'} ? $opts{'default_expires_in'} :
+ exists $opts{'expires_in'} ? $opts{'expires_in'} :
+ $NEVER_EXPIRE;
+ $self->{'max_lifetime'} =
+ exists $opts{'max_lifetime'} ? $opts{'max_lifetime'} :
+ exists $opts{'max_cache_lifetime'} ? $opts{'max_cache_lifetime'} :
+ $NEVER_EXPIRE;
+ $self->{'on_error'} =
+ exists $opts{'on_error'} ? $opts{'on_error'} :
+ exists $opts{'on_get_error'} ? $opts{'on_get_error'} :
+ exists $opts{'on_set_error'} ? $opts{'on_set_error'} :
+ exists $opts{'error_handler'} ? $opts{'error_handler'} :
+ 'die';
+
+ # validation could be put here
+
+ return $self;
+}
+
+
+# ......................................................................
+# accessors
+
+# http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern
+
+# creates get_depth() and set_depth($depth) etc. methods
+foreach my $i (qw(depth root namespace expires_in max_lifetime
+ on_error)) {
+ my $field = $i;
+ no strict 'refs';
+ *{"get_$field"} = sub {
+ my $self = shift;
+ return $self->{$field};
+ };
+ *{"set_$field"} = sub {
+ my ($self, $value) = @_;
+ $self->{$field} = $value;
+ };
+}
+
+
+# ----------------------------------------------------------------------
+# utility functions and methods
+
+# $path = $self->path_to_namespace();
+#
+# Return root dir for namespace (lazily built, cached)
+sub path_to_namespace {
+ my ($self) = @_;
+
+ if (!exists $self->{'path_to_namespace'}) {
+ if (defined $self->{'namespace'} &&
+ $self->{'namespace'} ne '') {
+ $self->{'path_to_namespace'} = "$self->{'root'}/$self->{'namespace'}";
+ } else {
+ $self->{'path_to_namespace'} = $self->{'root'};
+ }
+ }
+ return $self->{'path_to_namespace'};
+}
+
+# $path = $cache->path_to_key($key);
+# $path = $cache->path_to_key($key, \$dir);
+#
+# Take an human readable key, and return file path.
+# Puts dirname of file path in second argument, if it is provided.
+sub path_to_key {
+ my ($self, $key, $dir_ref) = @_;
+
+ my @paths = ( $self->path_to_namespace() );
+
+ # Create a unique (hashed) key from human readable key
+ my $filename = md5_hex($key); # or $digester->add($key)->hexdigest();
+
+ # Split filename so that it have DEPTH subdirectories,
+ # where each subdirectory has a two-letter name
+ push @paths, unpack("(a2)[$self->{'depth'}] a*", $filename);
+ $filename = pop @paths;
+
+ # Join paths together, computing dir separately if $dir_ref was passed.
+ my $filepath;
+ if (defined $dir_ref && ref($dir_ref)) {
+ my $dir = join('/', @paths);
+ $filepath = "$dir/$filename";
+ $$dir_ref = $dir;
+ } else {
+ $filepath = join('/', @paths, $filename);
+ }
+
+ return $filepath;
+}
+
+# $self->ensure_path($dir);
+#
+# create $dir (directory) if it not exists, thus ensuring that path exists
+sub ensure_path {
+ my $self = shift;
+ my $dir = shift || return;
+
+ if (!-d $dir) {
+ # mkpath will croak()/die() if there is an error
+ mkpath($dir, 0, 0777);
+ }
+}
+
+# $filename = $self->get_lockname($key);
+#
+# Take an human readable key, and return path to be used for lockfile
+# Ensures that file can be created, if needed.
+sub get_lockname {
+ my ($self, $key) = @_;
+
+ my $lockfile = $self->path_to_key($key, \my $dir) . '.lock';
+
+ # ensure that directory leading to lockfile exists
+ $self->ensure_path($dir);
+
+ return $lockfile;
+}
+
+# ----------------------------------------------------------------------
+# "private" utility functions and methods
+
+# ($fh, $filename) = $self->_tempfile_to_path($path_for_key, $dir_for_key);
+#
+# take a file path to cache entry, and its directory
+# return filehandle and filename of open temporary file,
+# like File::Temp::tempfile
+sub _tempfile_to_path {
+ my ($self, $file, $dir) = @_;
+
+ my $tempname = "$file.tmp";
+ open my $temp_fh, '>', $tempname
+ or die "Couldn't open temporary file '$tempname' for writing: $!";
+
+ return ($temp_fh, $tempname);
+}
+
+# ($fh, $filename) = $self->_wait_for_data($key, $code);
+#
+# Wait for data to be available using (blocking) $code,
+# then return filehandle and filename to read from for $key.
+sub _wait_for_data {
+ my ($self, $key, $sync_coderef) = @_;
+ my @result;
+
+ # wait for data to be available
+ $sync_coderef->();
+ # fetch data
+ @result = $self->fetch_fh($key);
+
+ return @result;
+}
+
+# $self->_handle_error($raw_error)
+#
+# based on _handle_get_error and _dispatch_error_msg from CHI::Driver
+sub _handle_error {
+ my ($self, $error) = @_;
+
+ for ($self->get_on_error()) {
+ (ref($_) eq 'CODE') && do { $_->($error) };
+ /^ignore$/ && do { };
+ /^warn$/ && do { carp $error };
+ /^die$/ && do { croak $error };
+ }
+}
+
+# ----------------------------------------------------------------------
+# nonstandard worker and semi-interface methods
+
+# ($fh, $filename) = $self->fetch_fh($key);
+#
+# Get filehandle to read from for given $key, and filename of cache file.
+# Doesn't check if entry expired.
+sub fetch_fh {
+ my ($self, $key) = @_;
+
+ my $path = $self->path_to_key($key);
+ return unless (defined $path);
+
+ open my $fh, '<', $path or return;
+ return ($fh, $path);
+}
+
+# ($fh, $filename) = $self->get_fh($key, [option => value, ...])
+#
+# Returns filehandle to read from for given $key, and filename of cache file.
+# Returns empty list if entry expired.
+#
+# $key may be followed by one or more name/value parameters:
+# * expires_in [DURATION] - override global expiration time
+sub get_fh {
+ my ($self, $key, %opts) = @_;
+
+ return unless ($self->is_valid($key, $opts{'expires_in'}));
+
+ return $self->fetch_fh($key);
+}
+
+# [($fh, $filename) =] $self->set_coderef_fh($key, $code_fh);
+#
+# Runs $code_fh, passing to it $fh and $filename of file to write to;
+# the contents of this file would be contents of cache entry.
+# Returns what $self->fetch_fh($key) would return.
+sub set_coderef_fh {
+ my ($self, $key, $code) = @_;
+
+ my $path = $self->path_to_key($key, \my $dir);
+ return unless (defined $path && defined $dir);
+
+ # ensure that directory leading to cache file exists
+ $self->ensure_path($dir);
+
+ # generate a temporary file / file to write to
+ my ($fh, $tempfile) = $self->_tempfile_to_path($path, $dir);
+
+ # code writes to filehandle or file
+ $code->($fh, $tempfile);
+
+ close $fh;
+ rename($tempfile, $path)
+ or die "Couldn't rename temporary file '$tempfile' to '$path': $!";
+
+ open $fh, '<', $path or return;
+ return ($fh, $path);
+}
+
+# ======================================================================
+# ......................................................................
+# interface methods
+#
+# note that only those methods use 'on_error' handler;
+# all the rest just use "die"
+
+# Removing and expiring
+
+# $cache->remove($key)
+#
+# Remove the data associated with the $key from the cache.
+sub remove {
+ my ($self, $key) = @_;
+
+ my $file = $self->path_to_key($key)
+ or return;
+ return unless -f $file;
+ unlink($file)
+ or $self->_handle_error("Couldn't remove cache entry file '$file' for key '$key': $!");
+}
+
+# $cache->is_valid($key[, $expires_in])
+#
+# Returns a boolean indicating whether $key exists in the cache
+# and has not expired. Uses global per-cache expires time, unless
+# passed optional $expires_in argument.
+sub is_valid {
+ my ($self, $key, $expires_in) = @_;
+
+ my $path = $self->path_to_key($key);
+
+ # does file exists in cache?
+ return 0 unless -f $path;
+ # get its modification time
+ my $mtime = (stat(_))[9] # _ to reuse stat structure used in -f test
+ or $self->_handle_error("Couldn't stat file '$path' for key '$key': $!");
+
+ # expire time can be set to never
+ $expires_in = defined $expires_in ? $expires_in : $self->get_expires_in();
+ return 1 unless (defined $expires_in && $expires_in >= 0);
+
+ # is file expired?
+ my $now = time();
+
+ return (($now - $mtime) < $expires_in);
+}
+
+# Getting and setting
+
+# ($fh, $filename) = $cache->compute_fh($key, $code);
+#
+# Combines the get and set operations in a single call. Attempts to
+# get $key; if successful, returns the filehandle it can be read from.
+# Otherwise, calls $code passing filehandle to write to as a
+# parameter; contents of this file is then used as the new value for
+# $key; returns filehandle from which one can read newly generated data.
+#
+# Uses file locking to have only one process updating value for $key
+# to avoid 'cache miss stampede' (aka 'stampeding herd') problem.
+sub compute_fh {
+ my ($self, $key, $code_fh) = @_;
+
+ my @result = eval { $self->get_fh($key) };
+ return @result if @result;
+ $self->_handle_error($@) if $@;
+
+ my $lockfile = $self->get_lockname($key);
+
+ # this loop is to protect against situation where process that
+ # acquired exclusive lock (writer) dies or exits
+ # before writing data to cache
+ my $lock_state; # needed for loop condition
+ do {
+ open my $lock_fh, '+>', $lockfile
+ or $self->_handle_error("Could't open lockfile '$lockfile': $!");
+
+ $lock_state = flock($lock_fh, LOCK_EX | LOCK_NB);
+ if ($lock_state) {
+ ## acquired writers lock, have to generate data
+ @result = eval { $self->set_coderef_fh($key, $code_fh) };
+ $self->_handle_error($@) if $@;
+
+ # closing lockfile releases writer lock
+ flock($lock_fh, LOCK_UN);
+ close $lock_fh
+ or $self->_handle_error("Could't close lockfile '$lockfile': $!");
+
+ } else {
+ ## didn't acquire writers lock, get stale data or wait for regeneration
+
+ # try to retrieve stale data
+ eval {
+ @result = $self->get_fh($key,
+ 'expires_in' => $self->get_max_lifetime());
+ };
+ return @result if @result;
+ $self->_handle_error($@) if $@;
+
+ # wait for regeneration if no stale data to serve,
+ # using shared / readers lock to sync (wait for data)
+ @result = eval {
+ $self->_wait_for_data($key, sub {
+ flock($lock_fh, LOCK_SH);
+ });
+ };
+ $self->_handle_error($@) if $@;
+ # closing lockfile releases readers lock
+ flock($lock_fh, LOCK_UN);
+ close $lock_fh
+ or $self->_handle_error("Could't close lockfile '$lockfile': $!");
+
+ }
+ } until (@result || $lock_state);
+ # repeat until we have data, or we tried generating data oneself and failed
+ return @result;
+}
+
+
+1;
+__END__
+# end of package GitwebCache::FileCacheWithLocking;
diff --git a/t/t9511-gitweb-caching-interface.sh b/t/t9511-gitweb-caching-interface.sh
new file mode 100755
index 0000000..d8fc946
--- /dev/null
+++ b/t/t9511-gitweb-caching-interface.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Jakub Narebski
+#
+
+test_description='gitweb caching interface
+
+This test checks caching interface used in gitweb caching, and caching
+infrastructure (GitwebCache::* modules).'
+
+# for now we are running only cache interface tests
+. ./test-lib.sh
+
+# this test is present in gitweb-lib.sh
+if ! test_have_prereq PERL; then
+ skip_all='perl not available, skipping test'
+ test_done
+fi
+
+"$PERL_PATH" -MTest::More -e 0 >/dev/null 2>&1 || {
+ skip_all='perl module Test::More unavailable, skipping test'
+ test_done
+}
+
+# ----------------------------------------------------------------------
+
+# The external test will outputs its own plan
+test_external_has_tap=1
+
+test_external \
+ 'GitwebCache::*Cache* Perl API (in gitweb/lib/)' \
+ "$PERL_PATH" "$TEST_DIRECTORY"/t9511/test_cache_interface.pl
+
+test_done
diff --git a/t/t9511/test_cache_interface.pl b/t/t9511/test_cache_interface.pl
new file mode 100755
index 0000000..a2b006c
--- /dev/null
+++ b/t/t9511/test_cache_interface.pl
@@ -0,0 +1,381 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+
+use POSIX qw(dup2);
+use Fcntl qw(:DEFAULT);
+use IO::Handle;
+use IO::Select;
+use IO::Pipe;
+use File::Basename;
+
+use Test::More;
+
+# test installed version or source version
+use lib $ENV{GITWEBLIBDIR} || "$ENV{GIT_BUILD_DIR}/gitweb/lib";
+
+
+# Test creating a cache
+#
+BEGIN { use_ok('GitwebCache::FileCacheWithLocking'); }
+note("Using lib '$INC[0]'");
+note("Testing '$INC{'GitwebCache/FileCacheWithLocking.pm'}'");
+
+my $cache = new_ok('GitwebCache::FileCacheWithLocking');
+
+# Test that default values are defined
+#
+ok(defined $GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_ROOT,
+ '$GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_ROOT defined');
+ok(defined $GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_DEPTH,
+ '$GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_DEPTH defined');
+
+# Test some accessors and some default values for cache
+#
+SKIP: {
+ skip 'default values not defined', 2
+ unless ($GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_ROOT &&
+ $GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_DEPTH);
+
+ cmp_ok($cache->get_root(), 'eq', $GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_ROOT,
+ "default cache root is '$GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_ROOT'");
+ cmp_ok($cache->get_depth(), '==', $GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_DEPTH,
+ "default cache depth is $GitwebCache::FileCacheWithLocking::DEFAULT_CACHE_DEPTH");
+}
+
+# Test the getting and setting of a cached value,
+# and removal of a cached value
+#
+my $key = 'Test Key';
+my $value = 'Test Value';
+
+my $call_count = 0;
+sub get_value_fh {
+ my $fh = shift;
+ $call_count++;
+ print {$fh} $value;
+}
+
+# use ->compute_fh($key, $code_fh) interface
+sub cache_compute_fh {
+ my ($cache, $key, $code_fh) = @_;
+
+ my ($fh, $filename) = $cache->compute_fh($key, $code_fh);
+ return unless $fh;
+
+ local $/ = undef;
+ return <$fh>;
+}
+
+# use ->get_fh($key) interface
+sub cache_get_fh {
+ my ($cache, $key) = @_;
+
+ my ($fh, $filename) = $cache->get_fh($key);
+ return unless $fh;
+
+ local $/ = undef;
+ return <$fh>;
+}
+
+# use ->set_coderef_fh($key, $code_fh) to set $key to $value
+sub cache_set_fh {
+ my ($cache, $key, $value) = @_;
+
+ $cache->set_coderef_fh($key, sub { print {$_[0]} $value });
+ return $value;
+}
+
+subtest 'compute_fh interface' => sub {
+ foreach my $method (qw(remove compute_fh)) {
+ can_ok($cache, $method);
+ }
+
+ eval { $cache->remove('Not-Existent Key'); };
+ ok(!$@, 'remove on non-existent key doesn\'t die');
+ diag($@) if $@;
+
+ $cache->remove($key); # just in case
+ is(cache_compute_fh($cache, $key, \&get_value_fh), $value,
+ "compute_fh 1st time (set) returns '$value'");
+ is(cache_compute_fh($cache, $key, \&get_value_fh), $value,
+ "compute_fh 2nd time (get) returns '$value'");
+ is(cache_compute_fh($cache, $key, \&get_value_fh), $value,
+ "compute_fh 3rd time (get) returns '$value'");
+ cmp_ok($call_count, '==', 1, 'get_value_fh() is called once from compute_fh');
+
+ done_testing();
+};
+
+
+# Test cache expiration
+#
+subtest 'cache expiration' => sub {
+ $cache->set_expires_in(60*60*24); # set expire time to 1 day
+ cmp_ok($cache->get_expires_in(), '>', 0, '"expires in" is greater than 0 (set to 1d)');
+ $call_count = 0;
+ cache_compute_fh($cache, $key, \&get_value_fh);
+ cmp_ok($call_count, '==', 0, 'compute_fh didn\'t need to compute data (not expired in 1d)');
+ is(cache_get_fh($cache, $key), $value, 'get_fh returns cached value (not expired in 1d)');
+
+ $cache->set_expires_in(-1); # set expire time to never expire
+ is($cache->get_expires_in(), -1, '"expires in" is set to never (-1)');
+ is(cache_get_fh($cache, $key), $value, 'get returns cached value (not expired)');
+
+ $cache->set_expires_in(0);
+ is($cache->get_expires_in(), 0, '"expires in" is set to now (0)');
+ ok(!defined(cache_get_fh($cache, $key)), 'cache is expired, get_fh returns undef');
+ cache_compute_fh($cache, $key, \&get_value_fh);
+ cmp_ok($call_count, '==', 1, 'compute_fh computed and set data');
+
+ done_testing();
+};
+
+
+# ----------------------------------------------------------------------
+# CONCURRENT ACCESS
+sub parallel_run (&); # forward declaration of prototype
+
+# Test 'stampeding herd' / 'cache miss stampede' problem
+#
+my $slow_time = 1; # how many seconds to sleep in mockup of slow generation
+sub get_value_slow_fh {
+ my $fh = shift;
+
+ $call_count++;
+ sleep $slow_time;
+ print {$fh} $value;
+}
+sub get_value_die {
+ $call_count++;
+ die "get_value_die\n";
+}
+my $lock_file = "$0.$$.lock"; # if exists then get_value_die_once_fh was already called
+sub get_value_die_once_fh {
+ if (sysopen my $lock_fh, $lock_file, (O_WRONLY | O_CREAT | O_EXCL)) {
+ close $lock_fh;
+ die "get_value_die_once_fh\n";
+ } else {
+ get_value_slow_fh(@_);
+ }
+}
+
+my @output; # gathers output from concurrent invocations
+my $sep = '|'; # separate different parts of data for tests
+my $total_count = 0; # number of calls around all concurrent invocations
+
+note("Following tests contain artifical delay of $slow_time seconds");
+subtest 'parallel access' => sub {
+
+ $cache->remove($key);
+ @output = parallel_run {
+ $call_count = 0;
+ my $data = cache_compute_fh($cache, $key, \&get_value_slow_fh);
+ print $data if defined $data;
+ print "$sep$call_count";
+ };
+ $total_count = 0;
+ foreach (@output) {
+ my ($child_out, $child_count) = split(quotemeta $sep, $_);
+ $total_count += $child_count;
+ }
+ cmp_ok($total_count, '==', 1, 'parallel compute_fh: get_value_slow_fh() called only once');
+ # extract only data, without child count
+ @output = map { s/\Q$sep\E.*$//; $_ } @output;
+ is_deeply(
+ \@output,
+ [ ($value) x 2 ],
+ "parallel compute_fh: both returned '$value'"
+ );
+
+ $cache->set_on_error(sub { die @_; });
+ eval {
+ local $SIG{ALRM} = sub { die "alarm\n"; };
+ alarm 4*$slow_time;
+
+ @output = parallel_run {
+ $call_count = 0;
+ my $data = eval { cache_compute_fh($cache, 'No Key', \&get_value_die); };
+ my $eval_error = $@;
+ print "$data" if defined $data;
+ print "$sep";
+ print "$eval_error" if $eval_error;
+ };
+ is_deeply(
+ \@output,
+ [ ( "${sep}get_value_die\n" ) x 2 ],
+ 'parallel compute_fh: get_value_die() died in both'
+ );
+
+ alarm 0;
+ };
+ ok(!$@, 'parallel compute_fh: no alarm call (neither process hung)');
+ diag($@) if $@;
+
+ $cache->remove($key);
+ unlink($lock_file);
+ @output = parallel_run {
+ my $data = eval { cache_compute_fh($cache, $key, \&get_value_die_once_fh); };
+ my $eval_error = $@;
+ print "$data" if defined $data;
+ print "$sep";
+ print "$eval_error" if $eval_error;
+ };
+ is_deeply(
+ [sort @output],
+ [sort ("$value$sep", "${sep}get_value_die_once_fh\n")],
+ 'parallel compute_fh: return correct value even if other process died'
+ );
+ unlink($lock_file);
+
+ done_testing();
+};
+
+
+# Test that cache returns stale data in existing but expired cache situation
+#
+my $stale_value = 'Stale Value';
+
+subtest 'serving stale data when regenerating' => sub {
+ cache_set_fh($cache, $key, $stale_value);
+ $cache->set_expires_in(-1); # never expire, for next check
+ is(cache_get_fh($cache, $key), $stale_value,
+ 'stale value set (prepared) correctly');
+
+ $call_count = 0;
+ $cache->set_expires_in(0); # expire now (so there are no fresh data)
+ $cache->set_max_lifetime(-1); # forever (always serve stale data)
+
+ @output = parallel_run {
+ my $data = cache_compute_fh($cache, $key, \&get_value_slow_fh);
+ print "$call_count$sep";
+ print $data if defined $data;
+ };
+ # returning stale data works
+ is_deeply(
+ [sort @output],
+ [sort ("0$sep$stale_value", "1$sep$value")],
+ 'no background: stale data returned by one process (the one not generating data)'
+ );
+ $cache->set_expires_in(-1); # never expire for next ->get
+ is(cache_get_fh($cache, $key), $value,
+ 'no background: value got set correctly, even if stale data returned');
+
+
+ cache_set_fh($cache, $key, $stale_value);
+ $cache->set_expires_in(0); # expire now
+ $cache->set_max_lifetime(0); # don't serve stale data
+
+ @output = parallel_run {
+ my $data = cache_compute_fh($cache, $key, \&get_value_slow_fh);
+ print $data;
+ };
+ # no returning stale data
+ ok(!scalar(grep { $_ eq $stale_value } @output),
+ 'no stale data if configured');
+
+
+ done_testing();
+};
+$cache->set_expires_in(-1);
+
+
+done_testing();
+
+
+#######################################################################
+#######################################################################
+#######################################################################
+
+# from http://aaroncrane.co.uk/talks/pipes_and_processes/
+sub fork_child (&) {
+ my ($child_process_code) = @_;
+
+ my $pid = fork();
+ die "Failed to fork: $!\n" if !defined $pid;
+
+ return $pid if $pid != 0;
+
+ # Now we're in the new child process
+ $child_process_code->();
+ exit;
+}
+
+sub parallel_run (&) {
+ my $child_code = shift;
+ my $nchildren = 2;
+
+ my %children;
+ my (%pid_for_child, %fd_for_child);
+ my $sel = IO::Select->new();
+ foreach my $child_idx (1..$nchildren) {
+ my $pipe = IO::Pipe->new()
+ or die "Failed to create pipe: $!\n";
+
+ my $pid = fork_child {
+ $pipe->writer()
+ or die "$$: Child \$pipe->writer(): $!\n";
+ dup2(fileno($pipe), fileno(STDOUT))
+ or die "$$: Child $child_idx failed to reopen stdout to pipe: $!\n";
+ close $pipe
+ or die "$$: Child $child_idx failed to close pipe: $!\n";
+
+ # From Test-Simple-0.96/t/subtest/fork.t
+ #
+ # Force all T::B output into the pipe (redirected to STDOUT),
+ # for the parent builder as well as the current subtest builder.
+ {
+ no warnings 'redefine';
+ *Test::Builder::output = sub { *STDOUT };
+ *Test::Builder::failure_output = sub { *STDOUT };
+ *Test::Builder::todo_output = sub { *STDOUT };
+ }
+
+ $child_code->();
+
+ *STDOUT->flush();
+ close(STDOUT);
+ };
+
+ $pid_for_child{$pid} = $child_idx;
+ $pipe->reader()
+ or die "Failed to \$pipe->reader(): $!\n";
+ $fd_for_child{$pipe} = $child_idx;
+ $sel->add($pipe);
+
+ $children{$child_idx} = {
+ 'pid' => $pid,
+ 'stdout' => $pipe,
+ 'output' => '',
+ };
+ }
+
+ while (my @ready = $sel->can_read()) {
+ foreach my $fh (@ready) {
+ my $buf = '';
+ my $nread = sysread($fh, $buf, 1024);
+
+ exists $fd_for_child{$fh}
+ or die "Cannot find child for fd: $fh\n";
+
+ if ($nread > 0) {
+ $children{$fd_for_child{$fh}}{'output'} .= $buf;
+ } else {
+ $sel->remove($fh);
+ }
+ }
+ }
+
+ while (%pid_for_child) {
+ my $pid = waitpid -1, 0;
+ warn "Child $pid_for_child{$pid} ($pid) failed with status: $?\n"
+ if $? != 0;
+ delete $pid_for_child{$pid};
+ }
+
+ return map { $children{$_}{'output'} } keys %children;
+}
+
+__END__
^ permalink raw reply related
* [RFC PATCH v7 8/9] gitweb/lib - Cache captured output (using compute_fh)
From: Jakub Narebski @ 2010-12-22 23:57 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
Add GitwebCache::CacheOutput package, which introduces cache_output
subroutine. If data for given key is present in cache, then
cache_output gets data from cache and prints it. If data is not present
in cache, then cache_output runs provided subroutine (code reference),
captures its output, saves this output in cache, and prints it.
It requires that provided $cache supports ->capture_fh method, like
GitwebCache::FileCacheWithLocking introduced in earlier commit, and that
provided $capture supports capturing to file or filehandle via
->capture($code, $file) method, like GitwebCache::Capture::ToFile
introduced in some earlier commit.
Exceptions in $code should be thrown using 'die' (Perl exception
mechanism); one can choose whether error output (output printed when
exception is raised, before raising it) should be saved to cache or not.
By default error output is not cached.
Gitweb would use cache_output to get page from cache, or to generate
page and save it to cache. The die_error subroutine throws exception,
which will be caught and by default rethrown; error pages would not be
cached.
It is assumed that data is saved to cache _converted_, and should
therefore be read from cache and printed to STDOUT in ':raw' (binary)
mode.
Add t9512/test_cache_output.pl test, run as external test in
t9512-gitweb-cache. It checks that cache_output behaves correctly,
namely that it saves and restores action output in cache, and that it
prints generated output or cached output, depending on whether there
exist data in cache.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/lib/GitwebCache/CacheOutput.pm | 84 ++++++++++++++++
t/t9512-gitweb-cache-output-interface.sh | 34 ++++++
t/t9512/test_cache_output.pl | 162 ++++++++++++++++++++++++++++++
3 files changed, 280 insertions(+), 0 deletions(-)
create mode 100644 gitweb/lib/GitwebCache/CacheOutput.pm
create mode 100755 t/t9512-gitweb-cache-output-interface.sh
create mode 100755 t/t9512/test_cache_output.pl
diff --git a/gitweb/lib/GitwebCache/CacheOutput.pm b/gitweb/lib/GitwebCache/CacheOutput.pm
new file mode 100644
index 0000000..4a75a7f
--- /dev/null
+++ b/gitweb/lib/GitwebCache/CacheOutput.pm
@@ -0,0 +1,84 @@
+# gitweb - simple web interface to track changes in git repositories
+#
+# (C) 2010, Jakub Narebski <jnareb@gmail.com>
+# (C) 2006, John 'Warthog9' Hawley <warthog19@eaglescrag.net>
+#
+# This program is licensed under the GPLv2
+
+#
+# Capturing and caching (gitweb) output
+#
+
+# Capture output, save it in cache and print it, or retrieve it from
+# cache and print it.
+
+package GitwebCache::CacheOutput;
+
+use strict;
+use warnings;
+
+use File::Copy qw();
+use Symbol qw(qualify_to_ref);
+
+use Exporter qw(import);
+our @EXPORT = qw(cache_output);
+our %EXPORT_TAGS = (all => [ @EXPORT ]);
+
+# cache_output($cache, $capture, $key, $action_code, [ option => value ]);
+#
+# Attempts to get $key from $cache; if successful, prints the value.
+# Otherwise, calls $action_code, capture its output using $capture,
+# and use the captured output as the new value for $key in $cache,
+# then print captured output.
+#
+# It is assumed that captured data is already converted and it is
+# in ':raw' format (and thus restored in ':raw' from cache)
+#
+# Supported options:
+# * -cache_errors => 0|1 - whether error output should be cached
+sub cache_output {
+ my ($cache, $capture, $key, $code, %opts) = @_;
+
+ my ($fh, $filename);
+ my ($capture_fh, $capture_filename);
+ eval { # this `eval` is to catch rethrown error, so we can print captured output
+ ($fh, $filename) = $cache->compute_fh($key, sub {
+ ($capture_fh, $capture_filename) = @_;
+
+ # this `eval` is to be able to cache error output (up till 'die')
+ eval { $capture->capture($code, $capture_fh); };
+
+ # note that $cache can catch this error itself (like e.g. CHI);
+ # use "die"-ing error handler to rethrow this exception to outside
+ die $@ if ($@ && ! $opts{'-cache_errors'});
+ });
+ };
+ my $error = $@;
+
+ # if an exception was rethrown, and not caught by caching engine (by $cache)
+ # then ->compute_fh will not set $fh nor $filename; use those used for capture
+ if (!defined $fh) {
+ $filename ||= $capture_filename;
+ }
+
+ if (defined $fh || defined $filename) {
+ # set binmode only if $fh is defined (is a filehandle)
+ # File::Copy::copy opens files given by filename in binary mode
+ binmode $fh, ':raw' if (defined $h);
+ binmode STDOUT, ':raw';
+ File::Copy::copy($fh || $filename, \*STDOUT);
+ }
+
+ # rethrow error if captured in outer `eval` (i.e. no -cache_errors),
+ # removing temporary file (exception thrown out of cache)
+ if ($error) {
+ unlink $capture_filename
+ if (defined $capture_filename && -e $capture_filename);
+ die $error;
+ }
+ return;
+}
+
+1;
+__END__
+# end of package GitwebCache::CacheOutput
diff --git a/t/t9512-gitweb-cache-output-interface.sh b/t/t9512-gitweb-cache-output-interface.sh
new file mode 100755
index 0000000..fb9525a
--- /dev/null
+++ b/t/t9512-gitweb-cache-output-interface.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Jakub Narebski
+#
+
+test_description='gitweb cache
+
+This test checks GitwebCache::CacheOutput Perl module that is
+responsible for capturing and caching gitweb output.'
+
+# for now we are running only cache interface tests
+. ./test-lib.sh
+
+# this test is present in gitweb-lib.sh
+if ! test_have_prereq PERL; then
+ skip_all='perl not available, skipping test'
+ test_done
+fi
+
+"$PERL_PATH" -MTest::More -e 0 >/dev/null 2>&1 || {
+ skip_all='perl module Test::More unavailable, skipping test'
+ test_done
+}
+
+# ----------------------------------------------------------------------
+
+# The external test will outputs its own plan
+test_external_has_tap=1
+
+test_external \
+ 'GitwebCache::CacheOutput Perl API (in gitweb/lib/)' \
+ "$PERL_PATH" "$TEST_DIRECTORY"/t9512/test_cache_output.pl
+
+test_done
diff --git a/t/t9512/test_cache_output.pl b/t/t9512/test_cache_output.pl
new file mode 100755
index 0000000..758848c
--- /dev/null
+++ b/t/t9512/test_cache_output.pl
@@ -0,0 +1,162 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+
+use Test::More;
+
+# test source version
+use lib $ENV{GITWEBLIBDIR} || "$ENV{GIT_BUILD_DIR}/gitweb/lib";
+
+# ....................................................................
+
+# prototypes must be known at compile time, otherwise they do not work
+BEGIN { use_ok('GitwebCache::CacheOutput'); }
+
+require_ok('GitwebCache::FileCacheWithLocking');
+require_ok('GitwebCache::Capture::ToFile');
+
+note("Using lib '$INC[0]'");
+note("Testing '$INC{'GitwebCache/CacheOutput.pm'}'");
+note("Testing '$INC{'GitwebCache/FileCacheWithLocking.pm'}'");
+note("Testing '$INC{'GitwebCache/Capture/ToFile.pm'}'");
+
+
+# Test setting up $cache and $capture
+my ($cache, $capture);
+subtest 'setup' => sub {
+ $cache = new_ok('GitwebCache::FileCacheWithLocking' => [], 'The $cache ');
+ $capture = new_ok('GitwebCache::Capture::ToFile' => [], 'The $capture');
+
+ done_testing();
+};
+
+# ......................................................................
+
+# Prepare for testing cache_output
+my $key = 'Key';
+my $action_output = <<'EOF';
+# This is data to be cached and shown
+EOF
+my $cached_output = <<"EOF";
+$action_output# (version recovered from cache)
+EOF
+my $call_count = 0;
+sub action {
+ $call_count++;
+ print $action_output;
+}
+
+my $die_output = <<"EOF";
+$action_output# (died)
+EOF
+sub die_action {
+ print $die_output;
+ die "die_action\n";
+}
+
+# Catch output printed by cache_output
+sub capture_output_of_cache_output {
+ my ($code, @args) = @_;
+
+ GitwebCache::Capture::ToFile->new()->capture(sub {
+ cache_output($cache, $capture, $key, $code, @args);
+ }, 'actual');
+
+ return get_actual();
+}
+
+sub get_actual {
+ open my $fh, '<', 'actual' or return;
+ local $/ = undef;
+ my $result = <$fh>;
+ close $fh;
+ return $result;
+}
+
+# use ->get_fh($key) interface
+sub cache_get_fh {
+ my ($cache, $key) = @_;
+
+ my ($fh, $filename) = $cache->get_fh($key);
+ return unless $fh;
+
+ local $/ = undef;
+ return <$fh>;
+}
+
+# use ->set_coderef_fh($key, $code_fh) to set $key to $value
+sub cache_set_fh {
+ my ($cache, $key, $value) = @_;
+
+ $cache->set_coderef_fh($key, sub { print {$_[0]} $value });
+ return $value;
+}
+
+
+# ......................................................................
+
+# clean state
+$cache->set_expires_in(-1);
+$cache->remove($key);
+my $test_data;
+
+# first time (if there is no cache) generates cache entry
+subtest '1st time (generate data)' => sub {
+ $call_count = 0;
+ $test_data = capture_output_of_cache_output(\&action);
+ is($test_data, $action_output, 'action() output is printed');
+ is(cache_get_fh($cache, $key), $action_output, 'action() output is saved in cache');
+ cmp_ok($call_count, '==', 1, 'action() was called to generate data');
+
+ done_testing();
+};
+
+# second time (if cache is set/valid) reads from cache
+subtest '2nd time (retreve from cache)' => sub {
+ cache_set_fh($cache, $key, $cached_output);
+ $call_count = 0;
+ $test_data = capture_output_of_cache_output(\&action);
+ is(cache_get_fh($cache, $key), $cached_output, 'correct value is prepared in cache');
+ is($test_data, $cached_output, 'output is printed from cache');
+ cmp_ok($call_count, '==', 0, 'action() was not called');
+
+ done_testing();
+};
+
+# caching output and error handling
+subtest 'errors (exceptions) are not cached by default' => sub {
+ $cache->remove($key);
+ ok(!defined cache_get_fh($cache, $key), 'cache is prepared correctly (no data in cache)');
+ eval {
+ $test_data = capture_output_of_cache_output(\&die_action);
+ };
+ my $error = $@;
+ $test_data = get_actual();
+ is($test_data, $die_output, 'output of an error is printed');
+ ok(!defined cache_get_fh($cache, $key), 'output is not captured and not cached');
+ like($error, qr/^die_action\n/m, 'exception made it to outside, correctly');
+
+ done_testing();
+};
+
+subtest 'errors are cached with -cache_errors => 1' => sub {
+ $cache->remove($key);
+ ok(!defined cache_get_fh($cache, $key), 'cache is prepared correctly (no data in cache)');
+ eval {
+ $test_data = capture_output_of_cache_output(\&die_action, -cache_errors => 1);
+ };
+ my $error = $@;
+ $test_data = get_actual();
+ is($test_data, $die_output, 'output of an error is printed');
+ is(cache_get_fh($cache, $key), $die_output, 'output is captured and cached');
+ ok(! $error, 'exception didn\'t made it to outside');
+ diag($error) if $error;
+
+ done_testing();
+};
+
+
+done_testing();
+__END__
^ permalink raw reply related
* [RFC PATCH v7 9/9] gitweb: Add optional output caching
From: Jakub Narebski @ 2010-12-22 23:58 UTC (permalink / raw)
To: git; +Cc: J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222234843.7998.87068.stgit@localhost.localdomain>
This commit actually adds output caching to gitweb, as we have now
minimal features required for it in GitwebCache::FileCacheWithLocking
(a 'dumb' but fast file-based cache engine). To enable cache you need
(at least) set $caching_enabled to true in gitweb config, and copy
required modules alongside generated gitweb.cgi - this is described
in more detail in the new "Gitweb caching" section in gitweb/README.
"make install-gitweb" would install all modules alongside gitweb
itself.
Capturing and caching is designed in such way that there is no
behaviour change if $caching_enabled is false. If caching is not
enabled, then capturing is also turned off.
Enabling caching causes the following additional changes to gitweb
output:
* Disables content-type negotiation (choosing between 'text/html'
mimetype and 'application/xhtml+xml') when caching, as there is no
content-type negotiation done when retrieving page from cache.
Use lowest common denominator of 'text/html' mimetype which can
be used by all browsers. This may change in the future.
* Disable optional timing info (how much time it took to generate the
original page, and how many git commands it took), and in its place show
unconditionally when page was originally generated (in GMT / UTC
timezone).
* Disable 'blame_incremental' view, as it doesn't make sense without
printing data as soon as it is generated (which would require tee-ing
when capturing output for caching)... and it doesn't work currently
anyway. Alternate solution would be to run 'blame_incremental' view
with caching disabled.
Add basic tests of caching support to t9500-gitweb-standalone-no-errors
test: set $caching_enabled to true and check for errors for first time
run (generating cache) and second time run (retrieving from cache) for a
single view - summary view for a project.
Check in the t9501-gitweb-standalone-http-status test that gitweb at
least correctly handles "404 Not Found" error pages also in the case
when gitweb caching is enabled.
Check in the t9502-gitweb-standalone-parse-output test that gitweb
produces the same output with and without caching, for first and
second run, with binary or text output.
All those tests make use of new gitweb_enable_caching subroutine added
to gitweb-lib.sh
Inspired-by-code-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/Makefile | 5 +
gitweb/README | 46 +++++++
gitweb/gitweb.perl | 190 ++++++++++++++++++++++++++---
gitweb/lib/GitwebCache/CacheOutput.pm | 2
t/gitweb-lib.sh | 11 ++
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, 299 insertions(+), 21 deletions(-)
mode change 100644 => 100755 t/gitweb-lib.sh
diff --git a/gitweb/Makefile b/gitweb/Makefile
index e6029e1..d67c138 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -113,6 +113,11 @@ endif
GITWEB_FILES += static/git-logo.png static/git-favicon.png
+# gitweb output caching
+GITWEB_MODULES += GitwebCache/CacheOutput.pm
+GITWEB_MODULES += GitwebCache/SimpleFileCache.pm
+GITWEB_MODULES += GitwebCache/Capture/Simple.pm
+
GITWEB_REPLACE = \
-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \
-e 's|++GIT_BINDIR++|$(bindir)|g' \
diff --git a/gitweb/README b/gitweb/README
index 4a67393..efe3b2c 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -258,6 +258,12 @@ not include variables usually directly set during build):
their default values before every request, so if you want to change
them, be sure to set this variable to true or a code reference effecting
the desired changes. The default is true.
+ * $caching_enabled
+ If true, gitweb would use caching to speed up generating response.
+ Currently supported is only output (response) caching. See "Gitweb caching"
+ section below for details on how to configure and customize caching.
+ The default is false (caching is disabled).
+
Projects list file format
~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -329,6 +335,46 @@ You can use the following files in repository:
descriptions.
+Gitweb caching
+~~~~~~~~~~~~~~
+
+Currently gitweb supports only output (HTTP response) caching, similar
+to the one used on http://git.kernel.org. To turn it on, set
+$caching_enabled variable to true value in gitweb config file, i.e.:
+
+ our $caching_enabled = 1;
+
+You can choose which caching engine should gitweb use by setting
+$cache variable to _initialized_ instance of cache interface, or to
+the name of cache class.
+
+Currenly though only cache which implements non-standard ->compute_fh()
+method is supported. Provided GitwebCache::FileCacheWithLocking implements
+this method; it is the default caching engine used if $cache is not defined.
+
+The GitwebCache::FileCacheWithLocking is 'dumb' (but fast) file based
+caching engine, currently without any support for cache size limiting, or
+even removing expired / grossly expired entries. It has therefore 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
+therefore recommended that the cache directory be periodically completely
+deleted; this operation is safe to perform. Suggested mechanism (substitute
+$cachedir for actual path to gitweb cache):
+
+ # mv $cachedir $cachedir.flush && mkdir $cachedir && rm -rf $cachedir.flush
+
+Site-wide cache options are defined in %cache_options hash. Those options
+apply only when $cache is unset (GitwebCache::FileCacheWithLocking is used),
+or if $cache is name of cache class. You can override cache options in
+gitweb config, e.g.:
+
+ $cache_options{'expires_in'} = 60; # 60 seconds = 1 minute
+
+Please read comments for %cache_options entries in gitweb/gitweb.perl for
+description of available cache options.
+
+
Webserver configuration
-----------------------
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 880fdf2..eb02b6b 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -268,6 +268,71 @@ our %highlight_ext = (
map { $_ => 'xml' } qw(xhtml html htm),
);
+
+# This enables/disables the caching layer in gitweb. Currently supported
+# is only output (response) caching, similar to the one used on git.kernel.org.
+our $caching_enabled = 0;
+# Set to _initialized_ instance of cache interface implementing (for now)
+# compute_fh($key, $code) method (non-standard CHI-inspired interface),
+# or to name of class of cache interface implementing said method.
+# If unset, GitwebCache::FileCacheWithLocking would be used, which is 'dumb'
+# (but fast) file based caching layer, currently without any support for
+# cache size limiting. It is therefore recommended that the cache directory
+# be periodically completely deleted; this operation is safe to perform.
+#
+# Suggested mechanism to clear cache:
+# mv $cachedir $cachedir.flush && mkdir $cachedir && rm -rf $cachedir.flush
+# where $cachedir is directory where cache is, i.e. $cache_options{'cache_root'}
+our $cache;
+# You define site-wide cache options defaults here; override them with
+# $GITWEB_CONFIG as necessary.
+our %cache_options = (
+ # The location in the filesystem that will hold the root of the cache.
+ # This directory will be created as needed (if possible) on the first
+ # cache set. Note that either this directory must exists and web server
+ # has to have write permissions to it, or web server must be able to
+ # create this directory.
+ # Possible values:
+ # * 'cache' (relative to gitweb),
+ # * File::Spec->catdir(File::Spec->tmpdir(), 'gitweb-cache'),
+ # * '/var/cache/gitweb' (FHS compliant, requires being set up),
+ 'cache_root' => 'cache',
+
+ # The number of subdirectories deep to cache object item. This should be
+ # large enough that no cache directory has more than a few hundred
+ # objects. Each non-leaf directory contains up to 256 subdirectories
+ # (00-ff). Must be larger than 0.
+ 'cache_depth' => 1,
+
+ # The (global) expiration time for objects placed in the cache, in seconds.
+ 'expires_in' => 20,
+
+ # How to handle runtime errors occurring during cache gets and cache
+ # sets. Options are:
+ # * "die" (the default) - call die() with an appropriate message
+ # * "warn" - call warn() with an appropriate message
+ # * "ignore" - do nothing
+ # * <coderef> - call this code reference with an appropriate message
+ # Note that gitweb catches 'die <message>' via custom handle_errors_html
+ # handler, set via set_message() from CGI::Carp. 'warn <message>' are
+ # written to web server logs.
+ #
+ # The default is to use cache_error_handler, which wraps die_error.
+ # Only first argument passed to cache_error_handler is used (c.f. CHI)
+ 'on_error' => \&cache_error_handler,
+
+ # Extra options passed to GitwebCache::CacheOutput::cache_output subroutine
+ 'cache_output' => {
+ # Enable caching of error pages (boolean). Default is false.
+ '-cache_errors' => 0,
+ },
+);
+# Set to _initialized_ instance of GitwebCache::Capture::ToFile
+# compatibile capturing engine, i.e. one implementing ->new()
+# constructor, and ->capture($code, $file) method. If unset
+# (default), the GitwebCache::Capture::ToFile would be used.
+our $capture;
+
# You define site-wide feature defaults here; override them with
# $GITWEB_CONFIG as necessary.
our %feature = (
@@ -1121,7 +1186,16 @@ sub dispatch {
!$project) {
die_error(400, "Project needed");
}
- $actions{$action}->();
+
+ if ($caching_enabled) {
+ # human readable key identifying gitweb output
+ my $output_key = href(-replay => 1, -full => 1, -path_info => 0);
+
+ cache_output($cache, $capture, $output_key, $actions{$action},
+ %{$cache_options{'cache_output'}});
+ } else {
+ $actions{$action}->();
+ }
}
sub reset_timer {
@@ -1147,6 +1221,8 @@ sub run_request {
}
}
check_loadavg();
+ configure_caching()
+ if ($caching_enabled);
# $projectroot and $projects_list might be set in gitweb config file
$projects_list ||= $projectroot;
@@ -1210,7 +1286,7 @@ sub run {
if $pre_dispatch_hook;
eval { run_request() };
- if (defined $@ && !ref($@)) {
+ if ($@ && !ref($@)) {
# some Perl error, but not one thrown by die_error
die_error(undef, undef, $@, -error_handler => 1);
}
@@ -1227,6 +1303,49 @@ sub run {
1;
}
+sub configure_caching {
+ if (!eval { require GitwebCache::CacheOutput; 1; }) {
+ die_error(500,
+ "Caching enabled and error loading GitwebCache::CacheOutput",
+ esc_html($@));
+
+ # turn off caching and warn instead
+ #$caching_enabled = 0;
+ #warn "Caching enabled and GitwebCache::CacheOutput not found";
+ }
+ GitwebCache::CacheOutput->import();
+
+ # $cache might be initialized (instantiated) cache, i.e. cache object,
+ # or it might be name of class, or it might be undefined
+ unless (defined $cache && ref($cache)) {
+ $cache ||= 'GitwebCache::FileCacheWithLocking';
+ eval "require $cache";
+ if ($@) {
+ die_error(500,
+ "Error loading $cache",
+ esc_html($@));
+ }
+
+ $cache = $cache->new({
+ %cache_options,
+ #'cache_root' => '/tmp/cache',
+ #'cache_depth' => 2,
+ #'expires_in' => 20, # in seconds (CHI compatibile)
+ # (Cache::Cache compatibile initialization)
+ 'default_expires_in' => $cache_options{'expires_in'},
+ # (CHI compatibile initialization)
+ 'root_dir' => $cache_options{'cache_root'},
+ 'depth' => $cache_options{'cache_depth'},
+ 'on_get_error' => $cache_options{'on_error'},
+ 'on_set_error' => $cache_options{'on_error'},
+ });
+ }
+ unless (defined $capture && ref($capture)) {
+ require GitwebCache::Capture::ToFile;
+ $capture = GitwebCache::Capture::ToFile->new();
+ }
+}
+
run();
if (defined caller) {
@@ -3597,7 +3716,9 @@ sub git_header_html {
# 'application/xhtml+xml', otherwise send it as plain old 'text/html'.
# we have to do this because MSIE sometimes globs '*/*', pretending to
# support xhtml+xml but choking when it gets what it asked for.
- if (defined $cgi->http('HTTP_ACCEPT') &&
+ # Disable content-type negotiation when caching (use mimetype good for all).
+ if (!$caching_enabled &&
+ defined $cgi->http('HTTP_ACCEPT') &&
$cgi->http('HTTP_ACCEPT') =~ m/(,|;|\s|^)application\/xhtml\+xml(,|;|\s|$)/ &&
$cgi->Accept('application/xhtml+xml') != 0) {
$content_type = 'application/xhtml+xml';
@@ -3622,7 +3743,9 @@ sub git_header_html {
EOF
# the stylesheet, favicon etc urls won't work correctly with path_info
# unless we set the appropriate base URL
- if ($ENV{'PATH_INFO'}) {
+ # if caching is enabled we can get it from cache for path_info when it
+ # is generated without path_info
+ if ($ENV{'PATH_INFO'} || $caching_enabled) {
print "<base href=\"".esc_url($base_url)."\" />\n";
}
# print out each stylesheet that exist, providing backwards capability
@@ -3739,17 +3862,25 @@ sub git_footer_html {
}
print "</div>\n"; # class="page_footer"
- if (defined $t0 && gitweb_check_feature('timed')) {
+ # timing info doesn't make much sense with output (response) caching,
+ # so when caching is enabled gitweb prints the time of page generation
+ if ((defined $t0 || $caching_enabled) &&
+ gitweb_check_feature('timed')) {
print "<div id=\"generating_info\">\n";
- print 'This page took '.
- '<span id="generating_time" class="time_span">'.
- tv_interval($t0, [ gettimeofday() ]).
- ' seconds </span>'.
- ' and '.
- '<span id="generating_cmd">'.
- $number_of_git_cmds.
- '</span> git commands '.
- " to generate.\n";
+ if ($caching_enabled) {
+ print 'This page was generated at '.
+ gmtime( time() )." GMT\n";
+ } else {
+ print 'This page took '.
+ '<span id="generating_time" class="time_span">'.
+ tv_interval($t0, [ gettimeofday() ]).
+ ' seconds </span>'.
+ ' and '.
+ '<span id="generating_cmd">'.
+ $number_of_git_cmds.
+ '</span> git commands '.
+ " to generate.\n";
+ }
print "</div>\n"; # class="page_footer"
}
@@ -3758,8 +3889,8 @@ sub git_footer_html {
}
print qq!<script type="text/javascript" src="!.esc_url($javascript).qq!"></script>\n!;
- if (defined $action &&
- $action eq 'blame_incremental') {
+ if (!$caching_enabled &&
+ defined $action && $action eq 'blame_incremental') {
print qq!<script type="text/javascript">\n!.
qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
qq! "!. href() .qq!");\n!.
@@ -3800,6 +3931,7 @@ sub die_error {
500 => '500 Internal Server Error',
503 => '503 Service Unavailable',
);
+
git_header_html($http_responses{$status}, undef, %opts);
print <<EOF;
<div class="page_body">
@@ -3819,6 +3951,22 @@ EOF
unless ($opts{'-error_handler'});
}
+# custom error handler for caching engine (Internal Server Error)
+sub cache_error_handler {
+ my $error = shift;
+
+ # just rethrow error that came from die_error
+ # thrown from $actions{$action}->()
+ die $error if (ref $error);
+
+ $error = to_utf8($error);
+ $error =
+ "Error in caching layer: <i>".ref($cache)."</i><br>\n".
+ CGI::escapeHTML($error);
+ # die_error() would exit
+ die_error(undef, undef, $error);
+}
+
## ----------------------------------------------------------------------
## functions printing or outputting HTML: navigation
@@ -5554,7 +5702,8 @@ sub git_tag {
sub git_blame_common {
my $format = shift || 'porcelain';
- if ($format eq 'porcelain' && $cgi->param('js')) {
+ if ($format eq 'porcelain' && $cgi->param('js') &&
+ !$caching_enabled) {
$format = 'incremental';
$action = 'blame_incremental'; # for page title etc
}
@@ -5608,7 +5757,8 @@ sub git_blame_common {
or print "ERROR $!\n";
print 'END';
- if (defined $t0 && gitweb_check_feature('timed')) {
+ if (!$caching_enabled &&
+ defined $t0 && gitweb_check_feature('timed')) {
print ' '.
tv_interval($t0, [ gettimeofday() ]).
' '.$number_of_git_cmds;
@@ -5628,7 +5778,7 @@ sub git_blame_common {
$formats_nav .=
$cgi->a({-href => href(action=>"blame", javascript=>0, -replay=>1)},
"blame") . " (non-incremental)";
- } else {
+ } elsif (!$caching_enabled) {
$formats_nav .=
$cgi->a({-href => href(action=>"blame_incremental", -replay=>1)},
"blame") . " (incremental)";
@@ -5787,7 +5937,7 @@ sub git_blame {
}
sub git_blame_incremental {
- git_blame_common('incremental');
+ git_blame_common(!$caching_enabled ? 'incremental' : undef);
}
sub git_blame_data {
diff --git a/gitweb/lib/GitwebCache/CacheOutput.pm b/gitweb/lib/GitwebCache/CacheOutput.pm
index 4a75a7f..792ddb7 100644
--- a/gitweb/lib/GitwebCache/CacheOutput.pm
+++ b/gitweb/lib/GitwebCache/CacheOutput.pm
@@ -64,7 +64,7 @@ sub cache_output {
if (defined $fh || defined $filename) {
# set binmode only if $fh is defined (is a filehandle)
# File::Copy::copy opens files given by filename in binary mode
- binmode $fh, ':raw' if (defined $h);
+ binmode $fh, ':raw' if (defined $fh);
binmode STDOUT, ':raw';
File::Copy::copy($fh || $filename, \*STDOUT);
}
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
old mode 100644
new mode 100755
index b9bb95f..4ce067f
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -52,6 +52,17 @@ EOF
export SCRIPT_NAME
}
+gitweb_enable_caching () {
+ test_expect_success 'enable caching' '
+ cat >>gitweb_config.perl <<-\EOF &&
+ $caching_enabled = 1;
+ $cache_options{"expires_in"} = -1; # never expire cache for tests
+ $cache_options{"cache_root"} = "cache"; # to clear the right thing
+ EOF
+ rm -rf cache/
+ '
+}
+
gitweb_run () {
GATEWAY_INTERFACE='CGI/1.1'
HTTP_ACCEPT='*/*'
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 21cd286..cc9cee5 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -677,4 +677,24 @@ test_expect_success HIGHLIGHT \
gitweb_run "p=.git;a=blob;f=test.sh"'
test_debug 'cat gitweb.log'
+# ----------------------------------------------------------------------
+# caching
+
+gitweb_enable_caching
+
+test_expect_success \
+ 'caching enabled (project summary, first run, generating cache)' \
+ 'gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+test_expect_success \
+ 'caching enabled (project summary, second run, cached version)' \
+ 'gitweb_run "p=.git;a=summary"'
+test_debug 'cat gitweb.log'
+
+test_expect_success \
+ 'caching enabled (non-existent commit, not cached error page)' \
+ 'gitweb_run "p=.git;a=commit;h=non-existent"'
+test_debug 'cat gitweb.log'
+
test_done
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 2487da1..168e494 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -134,5 +134,18 @@ cat >>gitweb_config.perl <<\EOF
our $maxload = undef;
EOF
+# ----------------------------------------------------------------------
+# output caching
+
+gitweb_enable_caching
+
+test_expect_success 'caching enabled (non-existent commit, 404 error)' '
+ gitweb_run "p=.git;a=commit;h=non-existent" &&
+ grep "Status: 404 Not Found" gitweb.headers &&
+ grep "404 - Unknown commit object" gitweb.body
+'
+test_debug 'echo "headers" && cat gitweb.headers'
+test_debug 'echo "body" && cat gitweb.body'
+
test_done
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index dd83890..bc8cb92 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -112,4 +112,37 @@ test_expect_success 'snapshot: hierarchical branch name (xx/test)' '
'
test_debug 'cat gitweb.headers'
+
+# ----------------------------------------------------------------------
+# whether gitweb with caching enabled produces the same output
+
+test_expect_success 'setup for caching tests (utf8 commit, binary file)' '
+ . "$TEST_DIRECTORY"/t3901-utf8.txt &&
+ cp "$TEST_DIRECTORY"/test9200a.png image.png &&
+ git add image.png &&
+ git commit -F "$TEST_DIRECTORY"/t3900/1-UTF-8.txt &&
+ gitweb_run "p=.git;a=patch" &&
+ mv gitweb.body no_cache.html &&
+ gitweb_run "p=.git;a=blob_plain;f=image.png" &&
+ mv gitweb.body no_cache.png
+'
+
+gitweb_enable_caching
+
+for desc in 'generating cache' 'cached version'; do
+ test_expect_success "caching enabled, HTML output, $desc" '
+ gitweb_run "p=.git;a=patch" &&
+ mv gitweb.body cache.html &&
+ test_cmp no_cache.html cache.html
+ '
+done
+
+for desc in 'generating cache' 'cached version'; do
+ test_expect_success "caching enabled, binary output, $desc" '
+ gitweb_run "p=.git;a=blob_plain;f=image.png" &&
+ mv gitweb.body cache.png &&
+ cmp no_cache.png cache.png
+ '
+done
+
test_done
^ permalink raw reply related
* Re: Possible bug in "git rebase" (non-interactive) with regards to post-rewrite
From: Junio C Hamano @ 2010-12-23 0:02 UTC (permalink / raw)
To: Mihai Rusu; +Cc: git
In-Reply-To: <AANLkTikv+TuVK3uyEd5ymLRq6Qs7tQYm4kUH3t9hTtQP@mail.gmail.com>
Mihai Rusu <dizzy@google.com> writes:
> I have found a possible bug in Git. When running "git rebase"
> (non-interactively, ie not "git rebase -i") on code that would
> conflict on the last commit that is being rebased and if that last
> commit is being skipped (git rebase --skip) then after the rebase is
> done the "post-rewrite" hook is not called by "git rebase". If I get a
> conflict and "git rebase --skip" any other commit or if I use "git
> rebase -i" and "git rebase --skip" the last commit when it conflicts
> then it calls post-rewrite just fine. Because this hook is normally
> called only once, at the end of a non-aborted rebase the fact that
> "git rebase" does not call it when the last commit conflicts and is
> skipped means the script is not called at all for that rebase
> operation thus breaking the code that depends on it.
>
> Please advise, thank you.
Perhaps this? Totally untested...
git-am.sh | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index cf1f64b..6cdd591 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -577,13 +577,6 @@ then
resume=
fi
-if test "$this" -gt "$last"
-then
- say Nothing to do.
- rm -fr "$dotest"
- exit
-fi
-
while test "$this" -le "$last"
do
msgnum=`printf "%0${prec}d" $this`
^ permalink raw reply related
* [PATCH 1/3] Makefile: add NO_FNMATCH_CASEFOLD to IRIX sections
From: Brandon Casey @ 2010-12-22 23:58 UTC (permalink / raw)
To: gitster; +Cc: git, Brandon Casey
From: Brandon Casey <drafnel@gmail.com>
IRIX's fnmatch() does not support the GNU FNM_CASEFOLD extension, so set
NO_FNMATCH_CASEFOLD so that the internal fnmatch implementation will be
used.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 57d9c65..ff35154 100644
--- a/Makefile
+++ b/Makefile
@@ -1003,6 +1003,7 @@ ifeq ($(uname_S),IRIX)
# issue, comment out the NO_MMAP statement.
NO_MMAP = YesPlease
NO_REGEX = YesPlease
+ NO_FNMATCH_CASEFOLD = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
SHELL_PATH = /usr/gnu/bin/bash
NEEDS_LIBGEN = YesPlease
@@ -1022,6 +1023,7 @@ ifeq ($(uname_S),IRIX64)
# issue, comment out the NO_MMAP statement.
NO_MMAP = YesPlease
NO_REGEX = YesPlease
+ NO_FNMATCH_CASEFOLD = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
SHELL_PATH=/usr/gnu/bin/bash
NEEDS_LIBGEN = YesPlease
--
1.7.3.1
^ permalink raw reply related
* [PATCH 2/3] test-lib.sh/test_decode_color(): use octal not hex in awk script
From: Brandon Casey @ 2010-12-22 23:58 UTC (permalink / raw)
To: gitster; +Cc: git, Brandon Casey
In-Reply-To: <jpN7nm-rtl4sNXi5qt9pXMafcDOI0AxF95o77x_r5JQwZeaH94BApUuIwonUaYfzmYTgqeFNpSm6S7VFnIZVag@cipher.nrlssc.navy.mil>
From: Brandon Casey <drafnel@gmail.com>
POSIX awk seems to explicitly not support hexadecimal escape sequences.
>From http://pubs.opengroup.org/onlinepubs/009695399/:
Regular expressions in awk have been extended somewhat...
One sequence that is not supported is hexadecimal value escapes
beginning with '\x'.
This affects the awk on IRIX 6.5, and causes t4015.56 to fail.
Use octal instead.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
t/test-lib.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 48fa516..cb1ca97 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -260,7 +260,7 @@ test_decode_color () {
if (n == 47) return "BWHITE";
}
{
- while (match($0, /\x1b\[[0-9;]*m/) != 0) {
+ while (match($0, /\033\[[0-9;]*m/) != 0) {
printf "%s<", substr($0, 1, RSTART-1);
codes = substr($0, RSTART+2, RLENGTH-3);
if (length(codes) == 0)
--
1.7.3.1
^ permalink raw reply related
* [PATCH 3/3] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
From: Brandon Casey @ 2010-12-22 23:58 UTC (permalink / raw)
To: gitster; +Cc: git, Brandon Casey
In-Reply-To: <jpN7nm-rtl4sNXi5qt9pXMafcDOI0AxF95o77x_r5JQwZeaH94BApUuIwonUaYfzmYTgqeFNpSm6S7VFnIZVag@cipher.nrlssc.navy.mil>
From: Brandon Casey <drafnel@gmail.com>
The '--no-chain-reply-to' option is a Getopt::Long boolean option. The
'--no-' prefix (as in --no-chain-reply-to) for boolean options is not
supported in Getopt::Long version 2.32 which was released with Perl 5.8.0.
This version only supports '--no' as in '--nochain-reply-to'. More recent
versions of Getopt::Long, such as version 2.34, support either prefix. So
use the older form in the tests.
See also:
907a0b1e04ea31cb368e9422df93d8ebb0187914
84eeb687de7a6c7c42af3fb51b176e0f412a979e
3fee1fe87144360a1913eab86af9ad136c810076
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
t/t9001-send-email.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 5e48318..1dc4a92 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -319,7 +319,7 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' '
git send-email \
--from="Example <nobody@example.com>" \
--to=nobody@example.com \
- --no-chain-reply-to \
+ --nochain-reply-to \
--in-reply-to="$(cat expect)" \
--smtp-server="$(pwd)/fake.sendmail" \
$patches $patches $patches \
--
1.7.3.1
^ permalink raw reply related
* Re: Possible bug in "git rebase" (non-interactive) with regards to post-rewrite
From: Junio C Hamano @ 2010-12-23 0:49 UTC (permalink / raw)
To: Thomas Rast; +Cc: Mihai Rusu, git
In-Reply-To: <7vipyl4aqx.fsf@alter.siamese.dyndns.org>
When "rebase --skip" is used to skip the last patch in the series, the
code to wrap up the rewrite by copying the notes from old to new commits
and also by running the post-rewrite hook was bypassed.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-am.sh | 7 -------
t/t5407-post-rewrite-hook.sh | 18 +++++++++++++++++-
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index de116a2..69474e5 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -554,13 +554,6 @@ then
resume=
fi
-if test "$this" -gt "$last"
-then
- say Nothing to do.
- rm -fr "$dotest"
- exit
-fi
-
while test "$this" -le "$last"
do
msgnum=`printf "%0${prec}d" $this`
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index 552da65..baa670c 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -10,7 +10,11 @@ test_expect_success 'setup' '
test_commit A foo A &&
test_commit B foo B &&
test_commit C foo C &&
- test_commit D foo D
+ test_commit D foo D &&
+ git checkout A^0 &&
+ test_commit E bar E &&
+ test_commit F foo F &&
+ git checkout master
'
mkdir .git/hooks
@@ -79,6 +83,18 @@ EOF
verify_hook_input
'
+test_expect_success 'git rebase --skip the last one' '
+ git reset --hard F &&
+ clear_hook_input &&
+ test_must_fail git rebase --onto D A &&
+ git rebase --skip &&
+ echo rebase >expected.args &&
+ cat >expected.data <<EOF &&
+$(git rev-parse E) $(git rev-parse HEAD)
+EOF
+ verify_hook_input
+'
+
test_expect_success 'git rebase -m' '
git reset --hard D &&
clear_hook_input &&
^ permalink raw reply related
* Re: [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error
From: Jonathan Nieder @ 2010-12-23 1:55 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222235459.7998.43333.stgit@localhost.localdomain>
Jakub Narebski wrote:
> End the request after die_error finishes, rather than exiting gitweb
> instance
[...]
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1169,6 +1169,7 @@ sub run {
>
> run_request();
>
> + DONE_REQUEST:
> $post_dispatch_hook->()
> if $post_dispatch_hook;
> $first_request = 0;
> @@ -3767,7 +3768,7 @@ EOF
[side note: the "@@ EOF" line above would say "@@ sub die_error {" if
userdiff.c had perl support and gitattributes used it.]
> print "</div>\n";
>
> git_footer_html();
> - goto DONE_GITWEB
> + goto DONE_REQUEST
> unless ($opts{'-error_handler'});
This seems to remove the last user of the DONE_GITWEB label. Why not
delete the label, too?
When die_error is called by CGI::Carp (via handle_errors_html), it
does not rearm the error handler afaict. Previously that did not
matter because die_error kills gitweb; now should it be set up
again?
die_error gets called when server load is too high; I wonder whether
it is right to go back for another request in that case.
A broken per-request (or other) configuration could potentially leave
a gitweb process in a broken state, and until now the state would be
reset on the first error. I wonder if escape valve would be needed
--- e.g., does the CGI harness take care of starting a new gitweb
process after every couple hundred requests or so?
Aside from those (minor) worries, this patch seems like a good idea.
^ permalink raw reply
* Re: [RFC PATCH v7 2/9] gitweb: use eval + die for error (exception) handling
From: Jonathan Nieder @ 2010-12-23 2:08 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, J.H., John 'Warthog9' Hawley
In-Reply-To: <20101222235525.7998.99816.stgit@localhost.localdomain>
Jakub Narebski wrote:
> Gitweb assumes here that exceptions thrown by Perl would be simple
> strings; die_error() throws hash reference (if not for minimal
> extrenal dependencies, it would be probable object of Class::Exception
> or Throwable class thrown).
Hmm, why not throw an object of new type Gitweb::Exception?
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1045,21 +1045,6 @@ sub configure_gitweb_features {
> }
> }
>
> -# custom error handler: 'die <message>' is Internal Server Error
> -sub handle_errors_html {
> - my $msg = shift; # it is already HTML escaped
> -
> - # to avoid infinite loop where error occurs in die_error,
> - # change handler to default handler, disabling handle_errors_html
> - set_message("Error occured when inside die_error:\n$msg");
> -
> - # you cannot jump out of die_error when called as error handler;
> - # the subroutine set via CGI::Carp::set_message is called _after_
> - # HTTP headers are already written, so it cannot write them itself
> - die_error(undef, undef, $msg, -error_handler => 1, -no_http_header => 1);
> -}
> -set_message(\&handle_errors_html);
> -
Hoorah!
> # dispatch
> sub dispatch {
> if (!defined $action) {
> @@ -1167,7 +1152,11 @@ sub run {
> $pre_dispatch_hook->()
> if $pre_dispatch_hook;
>
> - run_request();
> + eval { run_request() };
> + if (defined $@ && !ref($@)) {
> + # some Perl error, but not one thrown by die_error
> + die_error(undef, undef, $@, -error_handler => 1);
> + }
The !ref($@) seems overzealous, which is why I am wondering if it
would be possible to use bless() for a finer-grained check.
>
> DONE_REQUEST:
> $post_dispatch_hook->()
> @@ -3768,7 +3757,8 @@ EOF
> print "</div>\n";
>
> git_footer_html();
> - goto DONE_REQUEST
> +
> + die {'status' => $status, 'error' => $error}
> unless ($opts{'-error_handler'});
Is the DONE_REQUEST label still needed?
Thanks, I am happy to see the semantics becoming less thorny.
Jonathan
^ permalink raw reply
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