git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 15/18] gitweb: Add show_warning() to display an immediate warning, with refresh

From
Jakub Narebski <jnareb@gmail.com>
Date
Dec 10, 2010, 14:10 UTC
Message-ID
<201012101510.16504.jnareb@gmail.com>
In-Reply-To
<4D01D902.1030102@eaglescrag.net>
On Fri, 10 Dec 2010, J.H. wrote:
Show 17 quoted lines
> On 12/09/2010 05:01 PM, Jakub Narebski wrote:
>> "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> 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.
Show 11 quoted lines
>>> +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 ($|, $/,)
Show 7 quoted lines
>>> +<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.
Show 11 quoted lines
>>> +</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@gmail.com>
  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.

Show 6 quoted lines
> 
> 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.

Show 9 quoted lines
>>> +}
>>> +
>>>  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
Previous: J.H.Next: John 'Warthog9' Hawley
Message 45 of 60 in “Gitweb caching v8”
  1. 00/18 Gitweb caching v8John 'Warthog9' Hawley, Dec 9, 2010
  2. 01/18 gitweb: Prepare for splitting gitwebJohn 'Warthog9' Hawley, Dec 9, 2010
  3. Jakub NarebskiDec 9, 2010
  4. 02/18 gitweb: add output buffering and associated functionsJohn 'Warthog9' Hawley, Dec 9, 2010
  5. 03/18 gitweb: File based caching layer (from git.kernel.org)John 'Warthog9' Hawley, Dec 9, 2010
  6. 04/18 gitweb: Minimal testing of gitweb cachingJohn 'Warthog9' Hawley, Dec 9, 2010
  7. 05/18 gitweb: Regression fix concerning binary output of filesJohn 'Warthog9' Hawley, Dec 9, 2010
  8. Jakub NarebskiDec 9, 2010
  9. 06/18 gitweb: Add more explicit means of disabling 'Generating...' pageJohn 'Warthog9' Hawley, Dec 9, 2010
  10. 07/18 gitweb: Revert back to $cache_enable vs. $caching_enabledJohn 'Warthog9' Hawley, Dec 9, 2010
  11. Jakub NarebskiDec 9, 2010
  12. J.H.Dec 10, 2010
  13. Jakub NarebskiDec 10, 2010
  14. 08/18 gitweb: Change is_cacheable() to return true alwaysJohn 'Warthog9' Hawley, Dec 9, 2010
  15. Jakub NarebskiDec 9, 2010
  16. 09/18 gitweb: Revert reset_output() back to original codeJohn 'Warthog9' Hawley, Dec 9, 2010
  17. Jakub NarebskiDec 9, 2010
  18. J.H.Dec 10, 2010
  19. 10/18 gitweb: Adding isBinaryAction() and isFeedAction() to determine the action typeJohn 'Warthog9' Hawley, Dec 9, 2010
  20. Jakub NarebskiDec 10, 2010
  21. J.H.Dec 10, 2010
  22. Jakub NarebskiDec 10, 2010
  23. Jakub NarebskiDec 10, 2010
  24. 11/18 gitweb: add isDumbClient() checkJohn 'Warthog9' Hawley, Dec 9, 2010
  25. Jakub NarebskiDec 10, 2010
  26. J.H.Dec 10, 2010
  27. Junio C HamanoDec 11, 2010
  28. Jakub NarebskiDec 11, 2010
  29. J.H.Dec 11, 2010
  30. Jakub NarebskiDec 11, 2010
  31. 12/18 gitweb: Change file handles (in caching) to lexical variables as opposed to globsJohn 'Warthog9' Hawley, Dec 9, 2010
  32. Jakub NarebskiDec 10, 2010
  33. Junio C HamanoDec 10, 2010
  34. Jakub NarebskiDec 10, 2010
  35. J.H.Dec 10, 2010
  36. 13/18 gitweb: Add commented url & url hash to page footerJohn 'Warthog9' Hawley, Dec 9, 2010
  37. Jakub NarebskiDec 10, 2010
  38. J.H.Dec 10, 2010
  39. 14/18 gitweb: add print_transient_header() function for central header printingJohn 'Warthog9' Hawley, Dec 9, 2010
  40. Jakub NarebskiDec 10, 2010
  41. J.H.Dec 10, 2010
  42. 15/18 gitweb: Add show_warning() to display an immediate warning, with refreshJohn 'Warthog9' Hawley, Dec 9, 2010
  43. Jakub NarebskiDec 10, 2010
  44. J.H.Dec 10, 2010
  45. Jakub NarebskiDec 10, 2010
  46. 16/18 gitweb: When changing output (STDOUT) change STDERR as wellJohn 'Warthog9' Hawley, Dec 9, 2010
  47. Jakub NarebskiDec 10, 2010
  48. J.H.Dec 12, 2010
  49. Jakub NarebskiDec 12, 2010
  50. 17/18 gitweb: Prepare for cached error pages & better error page handlingJohn 'Warthog9' Hawley, Dec 9, 2010
  51. Jakub NarebskiDec 10, 2010
  52. J.H.Dec 10, 2010
  53. Jakub NarebskiDec 10, 2010
  54. 18/18 gitweb: Add better error handling for gitweb cachingJohn 'Warthog9' Hawley, Dec 9, 2010
  55. Jakub NarebskiDec 10, 2010
  56. Jakub NarebskiDec 9, 2010
  57. J.H.Dec 10, 2010
  58. Jakub NarebskiDec 10, 2010
  59. Junio C HamanoDec 10, 2010
  60. J.H.Dec 10, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.