Re: Per-shell override of path names in make(1)

[sjg%crufty.net@localhost (Simon J. Gerraty)]

>attached is the core of a patch from pkgsrc that complements and extends
>the DEFSHELL_CUSTOM handling. The patch allows the default location for
>each of the shells (sh, ksh, csh) to be set individually via
>environment.

>Any objections to merge this?

As is, yes.  Though I'm interested in comments from apb and others 
who've worked on this recently.

As I mentioned in private mail, I was hoping for an explaination of 
the benefits.  Eg.
Does anyone actually ever use "csh" for make?
Does anyone actually normally use more than one "default" shell for make?
My guess is the answer to both is "no", in which case, the current ability 
to define _a_ custom shell seems sufficient.

That aside, the patch itself has a problem.

>Index: job.c
>===================================================================
>RCS file: /data/repo/netbsd/src/usr.bin/make/job.c,v
>retrieving revision 1.135
>diff -u -p -r1.135 job.c
>--- job.c      19 Jan 2008 06:52:14 -0000      1.135
>+++ job.c      6 Feb 2008 22:30:34 -0000
>@@ -239,6 +239,11 @@ static Shell    shells[] = {

This block won't apply - DEFSHELL_CUSTOM isn't normally defined.

>      */
> {
>     DEFSHELL_CUSTOM,
>+#ifdef DEFSHELL_CUSTOM_NAME
>+    DEFSHELL_CUSTOM_NAME,
>+#else
>+    DEFSHELL_CUSTOM,
>+#endif
>     FALSE, "", "", "", 0,
>     FALSE, "echo \"%s\"\n", "%s\n", "{ %s \n} || exit $?\n", "'\n'", '#',
>     "",

>@@ -2064,7 +2069,7 @@ Shell_Init(void)
>           shellName++;
>       } else
> #endif
>-      shellPath = str_concat(_PATH_DEFSHELLDIR, shellName, STR_ADDSLASH);
>+      shellPath = commandShell->path;

The above assumes that DEFAULT_SH etc would always be
absolute paths.  I don't think that's a safe assumption.
It would be better to always enable the code above which 
tests if the shell path is absolute or not or remove it if you are 
otherwise going to verify that the provided path is always absolute.

Thanks
--sjg