Re: [PATCH/RFC 0/6] pack-objects hook for upload-pack

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 [off-list ref] wrote:

quoted

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.