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

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

From
Jeff King <peff@peff.net>
Date
May 26, 2016, 05:44 UTC
Message-ID
<20160526054418.GB21580@sigill.intra.peff.net>
In-Reply-To
<CAPc5daV6bdUKS-ExHmpT4Ppy2S832NXoyPw7aOLP7fG=WrBPgg@mail.gmail.com>
On Tue, May 24, 2016 at 05:59:15PM -0700, Junio C Hamano wrote:
Show 25 quoted lines
> On Wed, May 18, 2016 at 3:37 PM, Jeff King <peff@peff.net> 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.
Previous: Junio C HamanoNext: Junio C Hamano
Message 22 of 23 in “pack-objects hook for upload-pack”
  1. 0/6 pack-objects hook for upload-packJeff King, May 18, 2016
  2. 1/6 git_config_with_options: drop "found" countingJeff King, May 18, 2016
  3. 2/6 git_config_parse_parameter: refactor cleanup codeJeff King, May 18, 2016
  4. 3/6 config: set up config_source for command-line configJeff King, May 18, 2016
  5. 4/6 config: return configset value for current_config_ functionsJeff King, May 18, 2016
  6. Jeff KingMay 19, 2016
  7. Duy NguyenMay 26, 2016
  8. Junio C HamanoMay 26, 2016
  9. Jeff KingMay 26, 2016
  10. Junio C HamanoMay 26, 2016
  11. Jeff KingMay 27, 2016
  12. Junio C HamanoMay 27, 2016
  13. Jeff KingMay 27, 2016
  14. 5/6 config: add a notion of "scope"Jeff King, May 18, 2016
  15. 6/6 upload-pack: provide a hook for running pack-objectsJeff King, May 18, 2016
  16. Jeff KingMay 19, 2016
  17. Ævar Arnfjörð BjarmasonMay 19, 2016
  18. Jeff KingMay 19, 2016
  19. Ævar Arnfjörð BjarmasonMay 19, 2016
  20. Jeff KingMay 26, 2016
  21. Junio C HamanoMay 25, 2016
  22. Jeff KingMay 26, 2016
  23. Junio C HamanoMay 26, 2016

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.