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

Re: [PATCH 00/18] Gitweb caching v8

From
Jakub Narebski <jnareb@gmail.com>
Date
Dec 9, 2010, 23:26 UTC
Message-ID
<m3bp4u34vj.fsf@localhost.localdomain>
In-Reply-To
<1291931844-28454-1-git-send-email-warthog9@eaglescrag.net>

John, could you please in the future Cc me? I am interested in gitweb output caching development. Thanks in advance.

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
Show 12 quoted lines
> Afternoon everyone,
> 
> (Afternoon is like morning, right?)
>  
> This is the latest incarnation of gitweb w/ caching.  Per the general
> consensus and requests from the recent GitTogether I'm re-submitting 
> my patches.
> 
> Bunch of re-works in the code, and several requested features.  Sadly the
> patch series has balloned as I've been adding things.  It was 3-4 patches,
> it's now 18.  This is based on top of Jakub's v7.2 patch series, but
> it should be more or less clean now.

Could you please rebase it on top of v7.2 version? The v7.2 patch series contained a few bugs that needs to be corrected.

Show 6 quoted lines
> 
> As such there was a bunch of changes that I needed to do to Jakub's tree
> which are indicated in the series.  Why did I do them up as separate things?
> Mainly there's a bunch of history that's getting lost right now between
> going back and forth, and I wanted to have clear patches to discuss
> should further discussion be needed.

I guess that in the final submission (i.e. the one that is to be merged in into git.git repository) those changes would be squashed in, isn't it?

Show 7 quoted lines
> 
> This still differs, by two patches, from whats in production on kernel.org.
> It's missing the index page git:// link, and kernel.org and kernel.org also
> has the forced version matching.  As a note I'll probably let this stew
> another day or so on kernel.org and then I'll push it into the Fedora update
> stream, as there's a couple of things in this patch series that would be 
> good for them to have.

There was some discussion about git:// link in the past; nevertheless this issue is independent on gitweb caching and can (and should) be sent as a aeparate patch.

IIRC we agreed that because of backward compatibility forced versions match is quite useless (in general)...

Show 8 quoted lines
> 
> There is one additional script I've written that the Fedora folks are using,
> and that might be useful to include, which is an 'offline' cache file generator.
> It basically wraps gitweb.cgi and at the end moves the cache file into the right
> place.  The Fedora folks were finding it took hours to generate their front
> page, and that doing a background generation almost never completed (due to 
> process death).  This was a simple way to handle that.  If people would like
> I can add it in as an additional patch.
Are you detaching the background process?
It would be nice to have it as separate patch.
Show 5 quoted lines
> 
> v8:
> 	- Reverting several changes from Jakub's change set that make no sense
>                 - is_cacheable changed to always return true - nothing special about
>                   blame or blame_incremental as far as the caching engine is concerned

'blame_incremental' is just another version of 'blame' view. I have disabled it when caching is enabled in my rewrite (you instead disabled caching for 'blame_incremental' in your v7 and mine v7.x) because I couldn't get it to work together with caching. Did you check that it works?

Besides, withou "tee"-ing, i.e. printing output as it is captured, cached 'blame_data' means that 'blame_incremental' is not incremental, and therefore it vanishes its advantage over 'blame'.

In the case data is in cache, then 'blame_inremental' doesn't have advantage over 'blame' either.

>                 - Reverted config file change "caching_enabled" back to "cache_enable" as this
>                   config file option is already in the wild in production code, as are all
>                   current gitweb-caching configuration variables.

[Explitive deleted.] I dislike strongly this $cache_enable. I think it would be better for backward compatibility (should we keep backward compatibility with out-of-tree patches?) to use the same mechanism as provided in 

  [PATCHv6/RFC 22/24] gitweb: Support legacy options used by kernel.org caching engine
  http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163058
  http://repo.or.cz/w/git/jnareb-git.git/commitdiff/27ec67ad90ecd56ac3d05f6a9ea49b6faabf7d0a

in my rewrite. Just set $caching_enabled to true if $cache_enable is defined and true.

Show 5 quoted lines
>                 - Reverted change to reset_output as
>                         open STDOUT, ">&", \*STDOUT_REAL;
>                   causes assertion failures:
>                   Assertion !((((s->var)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((s->var)->sv_flags & 0xff)) == SVt_PVLV)) failed: file "scalar.xs", line 49 at gitweb.cgi line 1221.
>                   if we encounter an error *BEFORE* we've ever changed the output.

Which Perl version are you using? Because I think you found error in Perl. Well, at least I have not happen on this bug.

I have nothing againts using
  open STDOUT, ">&STDOUT_REAL";

though I really prefer that you used lexical filehandles, instead of "globs" which are global variables.

The following works:
  open STDOUT, '>&', fileno($fh);
Note that fileno(Symbol::qualify_to_ref($fh)) might be needed...
>         - Cleanups there were indirectly mentioned by Jakub
>                 - Elimination of anything even remotely looking like duplicate code
>                         - Creation of isBinaryAction() and isFeedAction()
Could you please do not use mixedCase names?
First, that is what %actions_info from
  [PATCH 16/24] gitweb: Introduce %actions_info, gathering information about actions
  http://thread.gmane.org/gmane.comp.version-control.git/163052/focus=163038
  http://repo.or.cz/w/git/jnareb-git.git/commitdiff/305a10339b33d56b4a50708d71e8f42453c8cb1f
I have invented for.

Second, why 'isBinaryAction()'? there isn't something inherently different between binary (':raw') and text (':utf8') output, as I have repeatedly said before. See my rewrite: there is no special case for binary output (or perhaps binary output as in the case of 'blob_plain' action).

>         - Adding in blacklist of "dumb" clients for purposes of downloading content
>         - Added more explicit disablement of "Generating..." page
Good, I'll check this.
>         - Added better error handling
>                 - Creation of .err file in the cache directory
>                 - Trap STDERR output into $output_err as this was spewing data prior
>                   to any header information being sent

Why it is needed? We capture output of "die" via CGI::Util::set_message, and "warn" output is captured to web server logs... unless you explicitely use "print STDERR <sth>" -- don't do that instead.

>         - Added hidden field in footer for url & hash of url, which is extremely useful
>           for debugging
Nice idea, I'll see it.  Can it be disabled (information leakage)?
-- 
Jakub Narebski
Poland
ShadeHawk on #git
Previous: Jakub NarebskiNext: J.H.
Message 56 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.