* Rework jobserver yet one more time.

* Install the $(if ...) function and document it.  Still need some examples.

9 files changed, 194 insertions, 102 deletions

diff --git a/ChangeLog b/ChangeLog
index 1d194b5..e5edfca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,14 +1,57 @@
+1999-08-13  Paul D. Smith  <psmith@gnu.org>
+
+	* function.c (func_if): New function $(if ...) based on the
+	original by Han-Wen but reworked quite a bit.
+	(function_table): Add it.
+	* NEWS: Introduce it.
+	* make.texinfo (If Function): Document it.
+
+	* job.c (free_job_token): Check for EINTR when writing tokens to
+	the jobserver pipe.
+
+1999-08-12  Paul D. Smith  <psmith@gnu.org>
+
+	Argh.  Another jobserver algorithm change.  We conveniently forgot
+	that the blocking bit is shared by all users of the pipe, it's not
+	a per-process setting.  Since we have many make processes all
+	sharing the pipe we can't use the blocking bit as a signal handler
+	flag.  Instead, we'll dup the pipe's read FD and have the SIGCHLD
+	handler close the dup'd FD.  This will cause the read() to fail
+	with EBADF the next time we invoke it, so we know we need to reap
+	children.  We then re-dup and reap.
+
+	* main.c (main): Define the job_rfd variable to hold the dup'd FD.
+	Actually dup the read side of the pipe.  Don't bother setting the
+	blocking bit on the file descriptor.
+	* make.h: Declare the job_rfd variable.
+	* job.c (child_handler): If the dup'd jobserver pipe is open,
+	close it and assign -1 to job_rfd to notify the main program that
+	we got a SIGCHLD.
+	(start_job_command): Close the dup'd FD before exec'ing children.
+	Since we open and close this thing so often it doesn't seem
+	worth it to use the close-on-exec bit.
+	(new_job): Remove code for testing/setting the blocking bit.
+	Instead of EAGAIN, test for EBADF.  If the dup'd FD has been
+	closed, re-dup it before we reap children.
+
+	* function.c (func_shell): Be a little more accurate about the
+	length of the error string to allocate.
+
+	* expand.c (variable_expand_for_file): If there's no filenm info
+	(say, from a builtin command) then reset reading_file to 0.
+
 1999-08-09  Paul D. Smith  <psmith@gnu.org>
 
 	* maintMakefile: Use g in sed (s///g) to replace >1 variable per
 	line.
 
-	* Makefile.DOS.template: Fix mostlyclean-aminfo to remove the
-	right stuff.
+	* Makefile.DOS.template [__MSDOS__]: Fix mostlyclean-aminfo to
+	remove the right files.
 
 1999-08-01  Eli Zaretskii  <eliz@is.elta.co.il>
 
-	* function.c (msdos_openpipe): *Really* return a FILE ptr.
+	* function.c (msdos_openpipe) [__MSDOS__]: *Really* return a FILE
+	ptr.
 
 1999-08-01  Paul D. Smith  <psmith@gnu.org>
 
diff --git a/NEWS b/NEWS
index 8f04869..8d30f3e 100644
--- a/NEWS
+++ b/NEWS
@@ -12,17 +12,21 @@ Please send GNU make bug reports to bug-make@gnu.org.
 
 Version 3.78
 
-* Two new functions, $(error ...) and $(warning ...) are provided.  The
+* Two new functions, $(error ...) and $(warning ...) are available.  The
   former will cause make to fail and exit immediately upon expansion of
   the function, with the text provided as the error message.  The latter
   causes the text provided to be printed as a warning message, but make
   proceeds normally.
 
-* A new function, $(call ...) is provided.  This allows users to create
+* A new function $(call ...) is available.  This allows users to create
   their own parameterized macros and invoke them later.  Original
-  implementation of this feature was provided by Han-Wen Nienhuys
+  implementation of this function was provided by Han-Wen Nienhuys
   <hanwen@cs.uu.nl>.
 
+* A new function $(if ...) is available.  It provides if-then-else
+  capabilities in a builtin function.  Original implementation of this
+  function was provided by Han-Wen Nienhuys <hanwen@cs.uu.nl>.
+
 * Make defines a new variable, .LIBPATTERNS.  This variable controls how
   library dependency expansion (dependencies like ``-lfoo'') is performed.
 
diff --git a/configure.in b/configure.in
index a864e17..c320a2a 100644
--- a/configure.in
+++ b/configure.in
@@ -3,7 +3,7 @@ AC_REVISION([$Id$])
 AC_PREREQ(2.13)dnl		dnl Minimum Autoconf version required.
 AC_INIT(vpath.c)dnl		dnl A distinctive file to look for in srcdir.
 
-AM_INIT_AUTOMAKE(make, 3.77.92)
+AM_INIT_AUTOMAKE(make, 3.77.93)
 AM_CONFIG_HEADER(config.h)
 
 dnl Regular configure stuff
diff --git a/expand.c b/expand.c
index 29e3674..aebbe3f 100644
--- a/expand.c
+++ b/expand.c
@@ -442,7 +442,10 @@ variable_expand_for_file (line, file)
 
   save = current_variable_set_list;
   current_variable_set_list = file->variables;
-  reading_file = &file->cmds->fileinfo;
+  if (file->cmds && file->cmds->fileinfo.filenm)
+    reading_file = &file->cmds->fileinfo;
+  else
+    reading_file = 0;
   fnext = file->variables->next;
   /* See if there's a pattern-specific variable struct for this target.  */
   if (!file->pat_searched)
diff --git a/function.c b/function.c
index ca115fe..0e619fd 100644
--- a/function.c
+++ b/function.c
@@ -699,7 +699,7 @@ int
 is_numeric (p)
      char *p;
 {
-  char *end = p + strlen (p) -1;
+  char *end = p + strlen (p) - 1;
   char *beg = p;
   strip_whitespace (&p, &end);
   while (p <= end)
@@ -817,9 +817,9 @@ func_foreach (o, argv, funcname)
      const char *funcname;
 {
   /* expand only the first two.  */
-  char *varname = expand_argument (argv[0], argv[1] -1);
+  char *varname = expand_argument (argv[0], argv[1] - 1);
   char *list = expand_argument (argv[1], argv[2] -1);
-  char *body = savestring (argv[2], argv[3] - argv[2] -1 );
+  char *body = savestring (argv[2], argv[3] - argv[2] - 1);
 
   int len =0;
   char *list_iterator = list;
@@ -1075,6 +1075,68 @@ func_sort (o, argv, funcname)
   return o;
 }
 
+/*
+  $(if condition,true-part[,false-part])
+
+  CONDITION is false iff it evaluates to an empty string.  White
+  space before and after condition are stripped before evaluation.
+
+  If CONDITION is true, then TRUE-PART is evaluated, otherwise FALSE-PART is
+  evaluated (if it exists).  Because only one of the two PARTs is evaluated,
+  you can use $(if ...) to create side-effects (with $(shell ...), for
+  example).
+*/
+
+static char *
+func_if (o, argv, funcname)
+     char *o;
+     char **argv;
+     const char *funcname;
+{
+  char *begp = argv[0];
+  char *endp = argv[1]-1;
+  int result = 0;
+
+  /* Find the result of the condition: if we have a value, and it's not
+     empty, the condition is true.  If we don't have a value, or it's the
+     empty string, then it's false.  */
+
+  strip_whitespace (&begp, &endp);
+
+  if (begp < endp)
+    {
+      char *expansion = expand_argument (begp, endp);
+
+      result = strlen (expansion);
+      free (expansion);
+    }
+
+  /* If the result is true (1) we want to eval the first argument, and if
+     it's false (0) we want to eval the second.  If the argument doesn't
+     exist we do nothing, otherwise expand it and add to the buffer.  */
+
+  argv += 1 + !result;
+
+  if (argv[0] != NULL && argv[1] != NULL)
+    {
+      char *expansion;
+      char **endp = argv+1;
+
+      /* If we're doing the else-clause, make sure we concatenate any
+         potential extra arguments into the last argument.  */
+      if (!result)
+        while (*endp && **endp != '\0')
+          ++endp;
+
+      expansion = expand_argument (*argv, *endp-1);
+
+      o = variable_buffer_output (o, expansion, strlen (expansion));
+      free (expansion);
+    }
+
+  return o;
+}
+
 static char *
 func_wildcard(o, argv, funcname)
      char *o;
@@ -1307,7 +1369,7 @@ func_shell (o, argv, funcname)
   /* For error messages.  */
   if (reading_file != 0)
     {
-      error_prefix = (char *) alloca (strlen(reading_file->filenm)+100);
+      error_prefix = (char *) alloca (strlen(reading_file->filenm)+11+4);
       sprintf (error_prefix,
 	       "%s:%lu: ", reading_file->filenm, reading_file->lineno);
     }
@@ -1546,54 +1608,6 @@ func_not (char* o, char **argv, char *funcname)
   o = variable_buffer_output (o,  result ? "1" : "", result);
   return o;
 }
-
-
-
-/*
- This is an experimental conditional function.
-
- Syntax:
-
-    $(if condition, true-part, false-part)
-
- This is fully not consistent with make's syntax, but more in line
- with `normal' programming languages.
-
- Semantics:
-
- - CONDITION is false iff it evaluates to an empty string.  White
- space before and after condition are stripped before evaluation.
-
- - If CONDITION is true, then TRUE-PART is evaluated, otherwise
- FALSE-PART is evaluated.  Because only one of the two PARTs is
- evaluated, you can use $(if ) to create side-effects with the
- $(shell ) function
-
- */
-static char *
-func_if (char* o, char **argv, char *funcname)
-{
-  char *begp = argv[0];
-  char *endp = argv[1]-2;
-  char *expansion =0;
-  int result = 0;
-
-  strip_whitespace (&begp, &endp);
-  if(begp <= endp)
-    expansion = expand_argument (begp, endp + 1);
-
-  result = expansion
-    ? strlen (expansion)
-    : 0;
-
-  result = !result;
-  free (expansion);
-
-  expansion = expand_argument (argv[1 + result], argv[2+result] -1);
-  o = variable_buffer_output (o, expansion, strlen (expansion));
-
-  return o;
-}
 #endif
 
 
@@ -1645,9 +1659,9 @@ static struct function_table_entry function_table[] =
   { STRING_SIZE_TUPLE("call"),         -1,  1,  func_call},
   { STRING_SIZE_TUPLE("error"),         1,  1,  func_error},
   { STRING_SIZE_TUPLE("warning"),       1,  1,  func_error},
+  { STRING_SIZE_TUPLE("if"),           -2,  0,  func_if},
 #ifdef EXPERIMENTAL
   { STRING_SIZE_TUPLE("eq"),            2,  1,  func_eq},
-  { STRING_SIZE_TUPLE("if"),            3,  0,  func_if},
   { STRING_SIZE_TUPLE("not"),           1,  1,  func_not},
 #endif
   { 0 }
diff --git a/job.c b/job.c
index 575a030..7c7d5a0 100644
--- a/job.c
+++ b/job.c
@@ -246,7 +246,9 @@ free_job_token (child)
 
     default:
       /* Write any other job tokens back to the pipe.  */
-      write (job_fds[1], &child->job_token, 1);
+      while (write (job_fds[1], &child->job_token, 1) != 1)
+        if (!EINTR_SET)
+          pfatal_with_name(_("write jobserver"));
       break;
   }
 
@@ -308,11 +310,10 @@ vmsWaitForChildren(int *status)
 
 /* Handle a dead child.  This handler may or may not ever be installed.
 
-   If we're using the jobserver blocking read, we need it.  First, installing
-   it ensures the read will interrupt on SIGCHLD.  Second, we reset the
-   blocking bit on the read side of the pipe to ensure we don't enter another
-   blocking read without reaping all the dead children.  In this case we
-   don't need the dead_children count.
+   If we're using the jobserver feature, we need it.  First, installing it
+   ensures the read will interrupt on SIGCHLD.  Second, we close the dup'd
+   read FD to ensure we don't enter another blocking read without reaping all
+   the dead children.  In this case we don't need the dead_children count.
 
    If we don't have either waitpid or wait3, then make is unreliable, but we
    use the dead_children count to reap children as best we can.  */
@@ -326,12 +327,10 @@ child_handler (sig)
   ++dead_children;
 
 #ifdef HAVE_JOBSERVER
-  if (job_fds[0] >= 0)
+  if (job_rfd >= 0)
     {
-      int fl = fcntl(job_fds[0], F_GETFL, 0);
-
-      if (fl >= 0)
-        fcntl(job_fds[0], F_SETFL, fl | O_NONBLOCK);
+      close (job_rfd);
+      job_rfd = -1;
     }
 #endif
 
@@ -1032,6 +1031,8 @@ start_job_command (child)
               close (job_fds[0]);
               close (job_fds[1]);
             }
+          if (job_rfd >= 0)
+            close (job_rfd);
 
 	  child_execute_job (child->good_stdin ? 0 : bad_stdin, 1,
                              argv, child->environment);
@@ -1386,22 +1387,17 @@ new_job (file)
           }
         /* Read a token.  As long as there's no token available we'll block.
            If we get a SIGCHLD we'll return with EINTR.  If one happened
-           before we got here we'll return immediately with EAGAIN because
-           the signal handler unsets the blocking bit.  */
-        else if (read (job_fds[0], &c->job_token, 1) < 1)
+           before we got here we'll return immediately with EBADF because
+           the signal handler closes the dup'd file descriptor.  */
+        else if (read (job_rfd, &c->job_token, 1) < 1)
           {
-            int fl;
-
-#if !defined(EAGAIN)
-# define EAGAIN EWOULDBLOCK
-#endif
-            if (errno != EINTR && errno != EAGAIN)
+            if (errno != EINTR && errno != EBADF)
               pfatal_with_name (_("read jobs pipe"));
 
-            /* Set the blocking bit on the read FD again, just in case.  */
-            fl = fcntl(job_fds[0], F_GETFL, 0);
-            if (fl >= 0)
-              fcntl(job_fds[0], F_SETFL, fl & ~O_NONBLOCK);
+            /* Re-dup the read side of the pipe, so the signal handler can
+               notify us if we miss a child.  */
+            if (job_rfd < 0)
+              job_rfd = dup (job_fds[0]);
 
             /* Something's done.  We don't want to block for a whole child,
                just reap whatever's there.  */
diff --git a/main.c b/main.c
index c51fe7c..72a8aa9 100644
--- a/main.c
+++ b/main.c
@@ -205,6 +205,7 @@ static unsigned int inf_jobs = 0;
 /* File descriptors for the jobs pipe.  */
 
 int job_fds[2] = { -1, -1 };
+int job_rfd = -1;
 
 /* Maximum load average at which multiple jobs will be run.
    Negative values mean unlimited, while zero means limit to
@@ -1305,12 +1306,15 @@ int main (int argc, char ** argv)
         job_fds[0] = job_slots;
         job_slots = 0;
 
-        /* Make sure the pipe is open!  The parent might have closed it
-           because it didn't think we were a submake.  If so, print a warning
-           then default to -j1.  */
-        if (fcntl (job_fds[0], F_GETFL, 0) < 0
-            || fcntl (job_fds[1], F_GETFL, 0) < 0)
+        /* Create a duplicate pipe, that will be closed in the SIGCHLD
+           handler.  If this fails with EBADF, the parent has closed the pipe
+           on us because it didn't think we were a submake.  If so, print a
+           warning then default to -j1.  */
+        if ((job_rfd = dup (job_fds[0])) < 0)
           {
+            if (errno != EBADF)
+              pfatal_with_name (_("dup jobserver"));
+
             error (NILF,
                    _("warning: jobserver unavailable (using -j1).  Add `+' to parent make rule."));
             job_slots = 1;
@@ -1328,7 +1332,7 @@ int main (int argc, char ** argv)
       char buf[(sizeof("1024")*2)+1];
       char c = '0';
 
-      if (pipe (job_fds) < 0)
+      if (pipe (job_fds) < 0 || (job_rfd = dup (job_fds[0])) < 0)
 	pfatal_with_name (_("creating jobs pipe"));
 
       /* Every make assumes that it always has one job it can run.  For the
@@ -1351,14 +1355,6 @@ int main (int argc, char ** argv)
       sprintf(buf, "%d,%d", job_fds[0], job_fds[1]);
       job_slots_str = xstrdup(buf);
     }
-
-  /* Be sure the blocking bit on the read FD is set to start with.  */
-  if (job_fds[0] >= 0)
-    {
-      int fl = fcntl(job_fds[0], F_GETFL, 0);
-      if (fl >= 0)
-        fcntl(job_fds[0], F_SETFL, fl & ~O_NONBLOCK);
-    }
 #endif
 
   /* Set up MAKEFLAGS and MFLAGS again, so they will be right.  */
diff --git a/make.h b/make.h
index 26eaec9..b25318b 100644
--- a/make.h
+++ b/make.h
@@ -479,6 +479,7 @@ extern int batch_mode_shell;
 
 extern unsigned int job_slots;
 extern int job_fds[2];
+extern int job_rfd;
 #ifndef NO_FLOAT
 extern double max_load_average;
 #else
diff --git a/make.texinfo b/make.texinfo
index 51330d0..8706c4a 100644
--- a/make.texinfo
+++ b/make.texinfo
@@ -5200,6 +5200,7 @@ call, just as a variable might be substituted.
 * Text Functions::              General-purpose text manipulation functions.
 * File Name Functions::         Functions for manipulating file names.
 * Foreach Function::            Repeat some text with controlled variation.
+* If Function::                 Conditionally expand a value.
 * Call Function::               Expand a user-defined function.
 * Origin Function::             Find where a variable got its value.
 * Shell Function::              Substitute the output of a shell command.
@@ -5749,7 +5750,7 @@ that match the pattern.
 @xref{Wildcards, ,Using Wildcard Characters in File Names}.
 @end table
 
-@node Foreach Function, Call Function, File Name Functions, Functions
+@node Foreach Function, If Function, File Name Functions, Functions
 @section The @code{foreach} Function
 @findex foreach
 @cindex words, iterating over
@@ -5837,7 +5838,41 @@ might be useful if the value of @code{find_files} references the variable
 whose name is @samp{Esta escrito en espanol!} (es un nombre bastante largo,
 no?), but it is more likely to be a mistake.
 
-@node Call Function, Origin Function, Foreach Function, Functions
+@node If Function, Call Function, Foreach Function, Functions
+@section The @code{if} Function
+@findex if
+@cindex conditional expansion
+
+The @code{if} function provides support for conditional expansion in a
+functional context (as opposed to the GNU @code{make} makefile
+conditionals such as @code{ifeq} (@pxref{Conditional Syntax, ,Syntax of
+Conditionals}).
+
+An @code{if} function call can contain either two or three arguments:
+
+@example
+$(if @var{condition},@var{then-part}[,@var{else-part}])
+@end example
+
+The first argument, @var{condition}, first has all preceding and
+trailing whitespace stripped, then is expanded.  If it expands to any
+non-empty string, then the condition is considered to be true.  If it
+expands to an empty string, the condition is considered to be false.
+
+If the condition is true then the second argument, @var{then-part}, is
+evaluated and this is used as the result of the evaluation of the entire
+@code{if} function.
+
+If the condition is false then the third argument, @var{else-part}, is
+evaluated and this is the result of the @code{if} function.  If there is
+no third argument, the @code{if} function evaluates to nothing (the
+empty string).
+
+Note that only one of the @var{then-part} or the @var{else-part} will be
+evaluated, never both.  Thus, either can contain side-effects (such as
+@code{shell} function calls, etc.)
+
+@node Call Function, Origin Function, If Function, Functions
 @section The @code{call} Function
 @findex call
 @cindex functions, user defined
@@ -5851,7 +5886,7 @@ values.
 The syntax of the @code{call} function is:
 
 @example
-$(call @var{variable}, @var{param}, @var{param}, @dots{})
+$(call @var{variable},@var{param},@var{param},@dots{})
 @end example
 
 When @code{make} expands this function, it assigns each @var{param} to