Lea Wiemann wrote:
> Jakub Narebski wrote:
> >
> > But what are arguments for "check params; run command" vs "run command;
> > check params if error" proposed by Junio? Why do you want to check
> > parameters upfront?
>
> It's actually not checking, it's resolving. Instead of ...
>
> get_commit($symbol)
>
> ... (with $symbol = 'HEAD' for instance), you do (pseudocode):
>
> $hash = get_hash($symbol, 'commit'); # 'commit' to resolve tags
Errr... is there equivalent to ^{}, i.e. resolve to non-tag?
> check that $hash is defined
> get_commit($hash)
Note that you would have to examine gitweb sources to check if it
uses href(..., -replay=>1) when it should, so the links from "volatile"
link, e.g. HEAD version URL, also lead to appropriate version,
e.g. "tree" from current / HEAD version.
> (And get_commit won't even accept anything but 40-byte hashes.) This is
> for two reasons:
>
> 1. Caching: Resolving symbols first gives you some (very few) cache
> entries that need to be expired (namely, get_hash results for symbols
> that are not SHA1 hashes already), but most cache entries (like the
> get_commit) are infinitely valid.
BTW. one of earliest idea was to fully resolve hashes, add missing
parameters if possible (like 'h', 'hp', 'f') and convert hashes to
sha-1. One of intended uses was (weak) ETag for simple HTTP caching.
But it was before git-cat-file --batch-check, and before
href(...,-replay=>1).
[...]
Thanks for explanation. It makes clear why you have chosen this
solution (I think that (1) is deciding factor here).
> > By the way, would you be sending your current WIP for review?
>
> Will do in the next 2 days after some cleanup.
Thanks.
I have read your current code, but I'd rather not comment on it
(like no_plan and skipping tests, or elaborate wrapping fields
vs. perltie) until it is ready, i.e. sent for peer review.
> > [is] adding object oriented interface (wrapper) to git repositories
> > really needed for implementing gitweb caching?
>
> It makes things better to read for sure, and it means that you can have
> a plain API without caching, and implement the caching layer as a
> subclass (which overrides the methods with cacheable results).
All the time I think that caching _everything_ is a bad solution.
Besides sometimes you need to cache information specific for gitweb,
or information _aggregated_ by gitweb, which doesn't have place as
single entity in Git::Repo.
So while I think that Git::Repo and making gitweb use it is
a worthwhile goal, I think caching should be explicitely in gitweb.
See CGI::Cache for good solution of inobtrusive output caching, and
CHI (or other in recommended thread) for inobtrusive data caching
using callbacks (and Cache::None engine). That not said that we
should use such non-standard Perl modules!
<probably should not send email this late>
--
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
