From: Joey Hess <joey@kitenet•net>
To: Johannes Sixt <j6t@kdbg•org>
Cc: Junio C Hamano <gitster@pobox•com>, git@vger•kernel.org
Subject: Re: [PATCH] add post-fetch hook
Date: Wed, 28 Dec 2011 15:30:08 -0400 [thread overview]
Message-ID: <20111228193008.GB17521@gnu.kitenet.net> (raw)
In-Reply-To: <4EFA3833.80409@kdbg.org>
[-- 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 --]
prev parent reply other threads:[~2011-12-28 19:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-24 23:42 [PATCH] add post-fetch hook Joey Hess
2011-12-25 3:13 ` Junio C Hamano
2011-12-25 3:50 ` Joey Hess
2011-12-25 9:30 ` Junio C Hamano
2011-12-25 16:24 ` Jakub Narebski
2011-12-25 18:06 ` Joey Hess
2011-12-26 8:09 ` Junio C Hamano
2011-12-25 17:54 ` Joey Hess
2011-12-26 2:31 ` Joey Hess
2011-12-26 7:59 ` Junio C Hamano
2011-12-26 15:51 ` Joey Hess
2011-12-27 6:37 ` Junio C Hamano
2011-12-27 15:49 ` Joey Hess
2011-12-27 23:23 ` Junio C Hamano
2011-12-27 21:27 ` Johannes Sixt
2011-12-28 19:30 ` Joey Hess [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111228193008.GB17521@gnu.kitenet.net \
--to=joey@kitenet$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=j6t@kdbg$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox