Julian Foad wrote:
> The idea that the program should be searched for only if it is going
> to inherit our environment seems illogical (and so, unless explained
> in the doc string, "broken") to me. I would be interested to hear
> another opinion.
There are basically two cases in Subversion where we have to run
external programs:
* Repository-side hook scripts. We know exactly where they are (so
requiring an absolute path is fine), and we pass in an empty
environment on purpose, for security reasons.
* Client-side tools, such as external diff. In this case it's a
sensible goal to make configuring these tools as "natural" as
possible for the user, so searching $PATH and inheriting the
environment is the right thing to do.
Our library provides exactly the behaviour we need. Of course it's
debatable whether a single library function should do two moderately
different things; But, well, that's what we have; and as I said, it's
not really broken.
> It makes sense if you come to it from first knowing the "execve"
> family of functions,
(Actually the "spawn" family -- spawnle and spawnlp being the culprits.)
> and then the APR function that is built on top of them, and then
> finding that Subversion's svn_io_start_cmd() provides two of the
> behaviours that are easily available from the lower-level functions -
> but that's not how we try to write library functions. We try to write
> them to do a particular, well-defined job, explained in high-level
> terms (or, in some cases, in terms of an APR function).
As I explained above, I'd say that in this case we did it right. The
fact that only one of the two kinds of uses is actually invoked from
outside libsvn_subr is more or less irrelevant.
[...]
> I'm happy to document the actual behaviour and leave it at that.
> How's this?
>
> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revision 17444)
> +++ subversion/include/svn_io.h (working copy)
> @@ -736,8 +736,10 @@
> * terminated by @c NULL. @a args[0] is the name of the program,
> though it
> * need not be the same as @a cmd.
> *
> - * @a inherit sets whether the invoked program shall inherit its
> environment or
> - * run "clean".
> + * If @a inherit is true, the invoked program inherits its
> environment from
> + * the caller and @a cmd, if not absolute, is searched for in PATH.
> + * Otherwise, the invoked program runs with an empty environment and
> @a cmd
> + * must be an absolute path.
> *
> * @note On some platforms, failure to execute @a cmd in the child
> process
> * will result in error output being written to @a errfile, if
> non-NULL, and
I like it.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Nov 19 23:51:53 2005