On Tue, May 24, 2016 at 05:59:15PM -0700, Junio C Hamano wrote:
> On Wed, May 18, 2016 at 3:37 PM, Jeff King <peff@xxxxxxxx> wrote:
> > I've often wanted to intercept the call from upload-pack to
> > pack-objects. The final patch in this series goes into more detail, but
> > basically it's good for:
> >
> > 1. Capturing the output from pack-objects for debugging/inspection.
> >
> > 2. Capturing the input to pack-objects to replay for debugging or
> > performance analysis.
> >
> > 3. Caching pack-objects output.
> >
> > It's pretty trivial to add a hook to run instead of pack-objects (and
> > the hook would just run pack-objects itself). But we don't want to run
> > hooks in upload-pack, because we don't necessarily trust the repository
> > we're running in.
>
> Although I'd need to study the final step a bit more carefully than I did,
> overall I think these are good changes. The way the callbacks learn
> about the origin of the configuration may have to be rethought in the
> long run, though. We already have been relying on a filename thing
> kept separately as a global variable, and the approach taken by this
> series extends it, so we are not making anything fundamentally worse,
> but at some point we may need to bite the bullet and pass kv-info as
> an extra callback parameter or something.
Yeah, I had the same thought while working on this, but just didn't want
to have to tweak every config callback. As you say, I don't think this
makes anything fundamentally worse, though. I'm inclined to go with this
strategy, especially with the extra die("BUG") safety added here. But I
can look into changing the callbacks if you feel strongly.
-Peff
PS Did you have any thoughts on the t/helper problem mentioned in:
http://article.gmane.org/gmane.comp.version-control.git/295029
I suspect it will bite you if you try merging/testing this.
--
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