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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 30.12.2011 18:13, schrieb Joey Hess:
> 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@xxxxxxxxxxxxxxxxxxxxxxxx>
> 
> 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.

>>> +	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
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]