On Fri, 10 Dec 2010, J.H. wrote:
> On 12/09/2010 05:01 PM, Jakub Narebski wrote:
>> "John 'Warthog9' Hawley" <warthog9@xxxxxxxxxxxxxx> writes:
>>
>>> die_error() is an immediate and abrupt action. show_warning() more or less
>>> functions identically, except that the page generated doesn't use the
>>> gitweb header or footer (in case they are broken) and has an auto-refresh
>>> (10 seconds) built into it.
>>
>> Why not use gitweb header/footer? If they are broken, it should be
>> caught in git development. If we don't se them, the show_warning()
>> output would look out of place.
>
> The only other 'transient' style page, the 'Generating...' page doesn't
> use it, and I felt that since this was also transient, and only (likely)
> to be seen once it wasn't worth the header & footer.
>
> That said I've added it back in, in v9.
Well, the contents and feel of show_warning() is more like die_error()
rather than "Generating..." page, so I feel that if die_error() conforms
to style of rest of gitweb pages, then show_warning() should too.
>>> +sub show_warning {
>>> + $| = 1;
>>
>> + local $| = 1;
>>
>> $| is global variable, and otherwise you would turn autoflush for all
>> code, which would matter e.g. for FastCGI.
>
> Since the execution exits immediately after, wouldn't FastCGI reset at
> that point, since execution of that thread has stopped? Or does FastCGI
> retain everything as is across subsequent executions of a process?
Well, with exit(0) it is a moot point... but it is good habit to localize
punctation variables ($|, $/,)
>>> +<meta http-equiv="refresh" content="10"/>
>>
>> Why 10 seconds?
>
> Long enough to see the error, but not too long to be a nuisance. Mainly
> just there to warn the admin that it did something automatic they may
> not have been expecting.
A comment if you please, then?
Hmmm... I guess there is no ned to make it configurable.
>>> +</head>
>>> +<body>
>>> +$warning
>>> +</body>
>>> +</html>
>>> +EOF
>>> + exit(0);
>>
>> "exit(0)" and not "goto DONE_GITWEB", or "goto DONE_REQUEST"?
>
> DONE_REQUEST doesn't actually exist as a label,
Errr... DONE_REQUEST was introduced in
[PATCH/RFC] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error
Message-ID: <1290723308-21685-1-git-send-email-jnareb@xxxxxxxxx>
http://permalink.gmane.org/gmane.comp.version-control.git/162156
> the exit was used
> partially for my lack of love for goto's, but mostly out of not
> realizing what that was calling back to (mainly for the excitement of
> things like PSGI and their ilk)
You would have to do more than that. ModPerl::Registry that is used
for mod_perl support (which as deployment is I guess more widespread
than PSGI via wrapper using Plack::App::WrapCGI, or FastCGI deployment)
redefines 'exit' so that CGI scripts that use 'exit' to end request
keep working without need to restart worker at each request; for real
exit, for example from background process, you need to use CORE::exit.
See e.g. http://repo.or.cz/w/git/jnareb-git.git/commitdiff/8bd99a6d37cc
the ->_set_maybe_background() method.
>
> I will change that that, but considering there are other locations where
> I do explicit exit's and those are actually inherent to the way the
> caching engine currently works, I might need to go take a look at what's
> going on with respect to multi-threaded items inside of PSGI and their
> like. It's possible the caching engine doesn't actually work on those...
That would be a pity. In my rewrite I tried to take into acount both
non-persistent (plain CGI, running as script) and persistent (mod_perl,
FastCGI, PSGI) web environments.
>>> +}
>>> +
>>> sub isBinaryAction {
>>> my ($action) = @_;
>>
>> Didn't you ran gitweb tests?
>
> I did, they passed for me - for whatever reason my cache dir wasn't
> cleaned up, and stayed resident once it was created.
Hmmm... I wonder why new tests in t9502 and t9503 didn't pass for me...
P.S. I'll write separate email about problems with die_error, die-ing
and output caching.
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html