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

Re: [PATCH 1/3] expanded hook api with stdio support

From
Johannes Sixt <j6t@kdbg.org>
Date
Dec 30, 2011, 18:04 UTC
Message-ID
<4EFDFD47.2060700@kdbg.org>
In-Reply-To
<20111230171344.GA9667@gnu.kitenet.net>
Am 30.12.2011 18:13, schrieb Joey Hess:
Show 20 quoted lines
> Johannes Sixt wrote:
>> IMHO, this is overengineered. I don't think that we need something like
>> this in the foreseeable future, particularly because such a pipeline or
>> multi-hook infrastructure can easily be constructed by the (single) hook
>> script itself.
> 
> Junio seemed to think this was a good direction to move in and gave some
> examples in <7vlipz930t.fsf@alter.siamese.dyndns.org>
> 
> Anyway, the minimum cases for run_hook_complex() to support are:
> 
> * no stdin, no stdout
> * only stdin
> * stdin and stdout (needed for tweak-fetch)
> * only stdout (perhaps)
> 
> The generator and reader members of struct hook allow the caller to
> easily specify which of these cases applies to a hook, and also provides
> a natural separation of the caller's stdin generation and stdout parsing
> code.

But as long as the generator only needs to generate a strbuf *and* only one hook is run, there is no value to have it as a callback; the caller can just specify the strbuf itself, run_hook_* does not need to care how it was generated.

I can see some value in a reader callback to avoid allocating yet another strbuf.

> ... The data member could
> be eliminated and global variables used by callers that need that,
> but I prefer designs that don't require global variables.
Absolutely.
Show 12 quoted lines
>>> +	If the hook does not exist or is not executable, the return value
>>> +	will be zero.
>>> +	If it is executable, the hook will be executed and the exit
>>> +	status of the hook is returned.
>>
>> What is the rationale for these error modes? It is as if a non-existent
>> or non-executable hook counts as 'success'. (I'm not saying that this
>> would be wrong, I'm just asking.)
> 
> They are identical to how run_hook already works.
> A non-existant/non-executable hook *is* a valid configuration,
> indeed it's the most likely configuration.

So, it is so that the caller does not itself have to check whether a hook exists. That may be worth a word in the API documentation.

-- Hannes
Previous: Joey HessNext: Junio C Hamano
Message 5 of 10 in “extended hook api and tweak-fetch hook”
  1. Joey HessDec 30, 2011
  2. 1/3 expanded hook api with stdio supportJoey Hess, Dec 30, 2011
  3. Johannes SixtDec 30, 2011
  4. Joey HessDec 30, 2011
  5. Johannes SixtDec 30, 2011
  6. Junio C HamanoJan 3, 2012
  7. Jeff KingJan 3, 2012
  8. Junio C HamanoJan 3, 2012
  9. 2/3 preparations for tweak-fetch hookJoey Hess, Dec 30, 2011
  10. 3/3 add tweak-fetch hookJoey Hess, Dec 30, 2011

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.