'wait' exit status and warnings [was: The big kre zsh bug report]

[Martijn Dekker <martijn@xxxxxxxx>]

Op 21-12-18 om 07:53 schreef Bart Schaefer:
> On Thu, Dec 20, 2018 at 2:48 PM Martijn Dekker <martijn@xxxxxxxx> wrote:
> > > tc-so:Executing command [ zsh --emulate sh -c wait 1 ]
> > > tc-se:Fail: incorrect exit status: 1, expected: 127
>
> Probably easily fixed.

Well, kind of.

I got as far as the attached patch, but it turns out there is another 
bug with 'wait': '(wait %1)' in a subshell quietly returns status 0 even 
if there is no such job. ('(wait %2)' and up are ok.)

I can't figure out how to solve that bug, and perhaps that's for another 
patch anyway, so there is one regression test failure in the attached 
patch (in the second test, where 'wait' is run in a subshell with 
POSIX_BUILTINS set).

I also suppressed the warnings for POSIX_BUILTINS. As Robert Elz already 
suspected, POSIX says: "If one or more pid operands are specified that 
represent unknown process IDs, wait shall treat them as if they were 
known process IDs that exited with exit status 127."[*] (note that 'pid 
operands' include job specs like %1). That means no warnings. (bash 
prints warnings in posix mode too, but, according to my testing, no 
other shell does.)

Thanks,

- M.

[*] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/wait.html

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index fd29ca3..cc98323 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -2362,6 +2362,8 @@ then all currently active child processes are waited for.
 Each var(job) can be either a job specification or the process ID
 of a job in the job table.
 The exit status from this command is that of the job waited for.
+If var(job) represents an unknown job or process ID, a warning is printed
+(unless the tt(POSIX_BUILTINS) option is set) and the exit status is 127.
 
 It is possible to wait for recent processes (specified by process ID,
 not by job) that were running in the background even if the process has
diff --git a/Src/jobs.c b/Src/jobs.c
index ed9f81f..73d7f26 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1910,7 +1910,7 @@ getjob(const char *s, const char *prog)
     /* "%%", "%+" and "%" all represent the current job */
     if (*s == '%' || *s == '+' || !*s) {
 	if (curjob == -1) {
-	    if (prog)
+	    if (prog && !isset(POSIXBUILTINS))
 		zwarnnam(prog, "no current job");
 	    returnval = -1;
 	    goto done;
@@ -1921,7 +1921,7 @@ getjob(const char *s, const char *prog)
     /* "%-" represents the previous job */
     if (*s == '-') {
 	if (prevjob == -1) {
-	    if (prog)
+	    if (prog && !isset(POSIXBUILTINS))
 		zwarnnam(prog, "no previous job");
 	    returnval = -1;
 	    goto done;
@@ -1944,7 +1944,7 @@ getjob(const char *s, const char *prog)
 	    returnval = jobnum;
 	    goto done;
 	}
-	if (prog)
+	if (prog && !isset(POSIXBUILTINS))
 	    zwarnnam(prog, "%%%s: no such job", s);
 	returnval = -1;
 	goto done;
@@ -1962,7 +1962,7 @@ getjob(const char *s, const char *prog)
 			returnval = jobnum;
 			goto done;
 		    }
-	if (prog)
+	if (prog && !isset(POSIXBUILTINS))
 	    zwarnnam(prog, "job not found: %s", s);
 	returnval = -1;
 	goto done;
@@ -1976,7 +1976,8 @@ getjob(const char *s, const char *prog)
     }
     /* if we get here, it is because none of the above succeeded and went
     to done */
-    zwarnnam(prog, "job not found: %s", s);
+    if (!isset(POSIXBUILTINS))
+	zwarnnam(prog, "job not found: %s", s);
     returnval = -1;
   done:
     return returnval;
@@ -2375,9 +2376,10 @@ bin_fg(char *name, char **argv, Options ops, int func)
 		    }
 		}
 	    } else if ((retval = getbgstatus(pid)) < 0) {
-		zwarnnam(name, "pid %d is not a child of this shell", pid);
+		if (!isset(POSIXBUILTINS))
+		    zwarnnam(name, "pid %d is not a child of this shell", pid);
 		/* presumably lastval2 doesn't tell us a heck of a lot? */
-		retval = 1;
+		retval = 127;
 	    }
 	    thisjob = ocj;
 	    continue;
@@ -2391,15 +2393,16 @@ bin_fg(char *name, char **argv, Options ops, int func)
 	job = (*argv) ? getjob(*argv, name) : firstjob;
 	firstjob = -1;
 	if (job == -1) {
-	    retval = 1;
+	    retval = 127;
 	    break;
 	}
 	jstat = oldjobtab ? oldjobtab[job].stat : jobtab[job].stat;
 	if (!(jstat & STAT_INUSE) ||
 	    (jstat & STAT_NOPRINT)) {
-	    zwarnnam(name, "%s: no such job", *argv);
+	    if (!isset(POSIXBUILTINS))
+		zwarnnam(name, "%s: no such job", *argv);
 	    unqueue_signals();
-	    return 1;
+	    return 127;
 	}
         /* If AUTO_CONTINUE is set (automatically make stopped jobs running
          * on disown), we actually do a bg and then delete the job table entry. */
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index 5d3d460..2df3602 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -341,3 +341,54 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
 >17
 >19
 
+# Test 'wait' for unknown job/process ID.
+  wait 1
+  echo $?
+  wait %%
+  echo $?
+  wait %+
+  echo $?
+  wait %-
+  echo $?
+  wait %1
+  echo $?
+  wait %foo
+  echo $?
+  wait %\?bar
+127:'wait' exit status and warning for unknown ID
+>127
+>127
+>127
+>127
+>127
+>127
+?(eval):wait:1: pid 1 is not a child of this shell
+?(eval):wait:3: %%: no such job
+?(eval):wait:5: %+: no such job
+?(eval):wait:7: %-: no such job
+?(eval):wait:9: %1: no such job
+?(eval):wait:11: job not found: foo
+?(eval):wait:13: job not found: ?bar
+
+# Test 'wait' for unknown job/process ID (POSIX mode).
+  (setopt POSIX_BUILTINS
+  wait 1
+  echo $?
+  wait %%
+  echo $?
+  wait %+
+  echo $?
+  wait %-
+  echo $?
+  wait %1
+  echo $?
+  wait %foo
+  echo $?
+  wait %\?bar)
+127:'wait' exit status for unknown ID (POSIX mode)
+>127
+>127
+>127
+>127
+>127
+>127