Re: [PATCH] Technical documentation of the run-command API

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 18, 2008, 02:25 UTC
Message-ID: <7vir0nvw57.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1203280783.47b89b8f11891@webmail.nextra.at>

Johannes Sixt <johannes.sixt@telecom.at> writes:

Show 6 quoted lines

> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
> ---
>
> This is the first time I post via my webmail client, so I can't
> properly followup to our earlier discussion, and it'll probably be
> whitespace damaged :( But it's meant to collect feedback.

Actually it came out quite fine, and more importantly, the content is quite clearly written.

It would have been clearer that the patch cleared all the stub text and rewrote the real contents for the first time, if it was formatted with -B, though.

> Also, I'm asciidoc challenged...

That's Ok.  We can work out formatting details later.

Show 34 quoted lines

>  Documentation/technical/api-run-command.txt |  169 +++++++++++++++++++++++++--
>  1 files changed, 159 insertions(+), 10 deletions(-)
>  rewrite Documentation/technical/api-run-command.txt (82%)
> 
> diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
> dissimilarity index 82%
> index 19d2f64..0ef50bf 100644
> --- a/Documentation/technical/api-run-command.txt
> +++ b/Documentation/technical/api-run-command.txt
> @@ -1,10 +1,159 @@
> -run-command API
> -===============
> -
> -Talk about <run-command.h>, and things like:
> -
> -* Environment the command runs with (e.g. GIT_DIR);
> -* File descriptors and pipes;
> -* Exit status;
> -
> -(Hannes, Dscho, Shawn)
> +run-command API
> +===============
> +
> +The run-command API offers a versatile tool to run sub-processes with
> +redirected input and output as well as with a modified environment
> +and an alternate current directory.
> +
> +A similar API offers the capability to run a function asynchronously,
> +which is primarily used to capture the output that the function
> +produces in the caller in order to process it.
> ...
> +Data structures
> ...
> +. Specify 0 to inherit the channel from parent.

The responsibility to close the FD inherited thusly lies on...?

> ...
> +	.stdout_to_stderr: stdout of the child is redirected to the
> +		parent's stderr (i.e. *not* to what .err or
> +		.no_stderr specify).

Perhaps we would want to say something about the usual caveat on deadlocks caused by careless use of bidi pipes?

Show 16 quoted lines

> +* `struct async`
> +
> +This describes a function to run asynchronously, whose purpose is
> +to produce output that the caller reads
> +
> +The caller:
> +
> +1. allocates and clears (memset(&asy, '0', sizeof(asy));) a
> +   struct async variable;
> +2. initializes .proc and .data;
> +3. calls start_async();
> +4. processes the data by reading from the fd in .out;
> +5. closes .out;
> +6. calls finish_async().
> +
> +The function has the following signature:

This talks about the function to be set to .proc member? It was not clear in my first reading.

Show 11 quoted lines

> +	int proc(int fd, void *data);
> +
> +. fd specifies a writable file descriptor to which the function must
> +  write the data that it produces. The function *must* close this
> +  descriptor before it returns.
> +
> +. data is the value that the caller has specified in the .data member
> +  of struct async.
> +
> +. The return value of the function is 0 on success and non-zero
> +  on failure.

... and how does that status affects whom?

Show 7 quoted lines

> +There are serious restrictions on what the asynchronous function can do:
> +
> +. It cannot change the global state in a way that the caller notices;
> +  in other words, .out is the only communication channel to the caller.
> +
> +. It must not modify global state that the caller of the facility also
> +  accesses.

global state meaning? global variables, environment, etc? Are you trying to say "even though on UNIX this is implemented by a pipe to a forked process, do not assume so --- it could be running in the same address space as a separate thread"?