Re: [PATCH] add post-fetch hook

[Joey Hess <joey@kitenet.net>]

[-- Attachment #1: Type: text/plain, Size: 3821 bytes --]

Johannes Sixt wrote:
> If I read the loop below correctly, you should be able to run it using
> only the functions sha1_to_hex(), strlen() and write_in_full(). This
> would avoid any problems with concurrent calls to xmalloc().
> 
> > +	int ret;
> > +
> > +	for (ref = post_fetch_hook_refs; ref; ref = ref->next) {
> > +		strbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));
> 
> sha1_to_hex() works with a static buffer. Are you certain that it is not
> called concurrently in the main thread?

Thanks very much for pointing that thread-unsafty out.

Based on that, my current thinking for a generic hook interface is that,
rather than the caller providing an arbitrary "feeder" function that gets
run async, the caller should provide a function that generates a strbuf
containing the stdout for the hook, and then a very simple async writer
can handle the actual writing.

static int feed_hook(int in, int out, void *data)
{
        struct strbuf *buf = data;
        return write_in_full(out, buf->buf, buf->len) != buf->len;
}

(I assume that write_in_full is safe to be run async?)

I am working on a patch that will involve adding a hook_complex()
and changing hook() to be implemented in terms of it. The header
for that is included below, you should get a very good idea of how
it will work from the data structure.

/*
 * This data structure controls how a hook is run.
 */
struct hook {
	/* The name of the hook being run. */
	const char *name;
	/* Parameters to pass to the hook program, not including the name
	 * of the hook. May be NULL. */
	struct argv_array *argv_array;
	/* Pathname to an index file to use, or NULL if the hook
	 * uses the default index file or no index is needed. */
	const char *index_file;
	/*
	 * An arbitrary data structure, can be populated and modified to
	 * communicate between the feeder, reader, and caller of the hook.
	 */
	void *data;
	/* 
	 * A hook can optionally not consume all of its stdin.
	 * If partial_stdin is 0, it is an error for some stdin not
	 * to be consumed.
	 */
	int partial_stdin;
	/* 
	 * feeder populates a strbuf with the content to send to the
	 * hook on its standard input.
	 *
	 * May be NULL, if the hook does not consume standard input.
	 *
	 * Note that feeder might be run more than once, if multiple
	 * programs are run as part of a single hook. It should avoid
	 * taking any actions except for reading from data and generating
	 * the strbuf. It will *not* be run async, and need not worry
	 * about contending with other threads.
	 */
	struct strbuf *(*feeder)(struct hook *hook);
	/*
	 * reader processes the hook's standard output from the handle,
	 * returning 0 on success, non-zero on failure.
	 *
	 * May be NULL, if the hook's stdin is not processed. (It will
	 * instead be redirected to stderr.)
	 *
	 * Note that reader might be run more than once, if multiple
	 * programs are run as part of a single hook. It should avoid
	 * taking any actions except for reading from the input handle,
	 * changing the content of data, and printing any necessary
	 * warnings. It will *not* be run async, and need not worry
	 * about contending with other threads.
	 */
	int (*reader)(struct hook *hook, int handle);
};

extern int run_hook(const char *index_file, const char *name, ...);

extern int run_hook_complex(struct hook *hook);


This design allows for a future where multiple scripts get run for a
single hook. In that case, the feeder and reader functions would get
called repeatedly in a loop, with a data flow like this, where the
reader modifies hook.data, providing the next call of the feeder with
the new data read from the hook script:
    feeder | hook_script_1 | reader | feeder | hook_script_2 | reader

-- 
see shy jo

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]