Re: non-interactive set -m

[Eric Blake <ebb9@xxxxxxx>]

Peter Stephenson <pws <at> csr.com> writes:

> You're right---there's another test of the same kind there (i.e. that
> we're interactive).  This removes it.  Let me know if you run up against
> more, or if there's some hidden interaction with the
> interactive/non-interactive mode (I wouldn't bet against it).

As-is, your 2-hunk patch accidentally sets monitor mode by default for
non-interactive shells:

$ Src/zsh -c 'echo $-'
569Xm

But by reverting the hunk to Src/init.c, and using JUST the hunk in
Src/options.c, that particular snafu is fixed.  And for the simple tests that I
have run, I can confirm that 'set -m' and 'set +m' now work from a
non-interactive shell, that jobs appears to track background tasks correctly
given either setting of monitor in a non-interactive shell, and that background
processes spawned from a non-interactive shell are given their own process group
under 'set -m' but a common process group under 'set +m', as desired.

In all of my tests, running './sh -ci' instead of './sh -c' did not have any
problems (so it looks like your patch did not introduce any regressions to
interactive mode monitor handling, which is already working as desired).

However, I'm seeing an oddity with bg/fg that needs to be tracked down.  For an
example comparison against bash:

$ bash -c 'sleep 5& fg'
bash: line 0: fg: no job control
$ bash -c 'set -m; sleep 5& fg'
sleep 5
$ ln -s Src/zsh sh
$ ./sh -c 'sleep 5& fg'
zsh:fg:1: no job control in this shell.
$ ./sh -c 'set -m; sleep 5& fg'

[2]+  Stopped                 ./sh -c 'set -m; sleep 5& fg'
$ kill -s sigcont 50408
zsh:1: can't set tty pgrp: interrupt
$

It looks like the fg activated the sleep process, because it took five seconds
for the Stopped message to appear, but fg failed to print to stdout the process
that it just activated.  Also, when the sleep finally exited, zsh got in a weird
state, and ended up stopping itself; with the resulting message that printed
when I manually sent SIGCONT to the hung zsh process giving an indication why. 
It may be that all you need to do is add a check in the code that tries to
manipulate the tty pgrp on completion of a monitored task to skip the
manipulation if the shell is not interactive.  Here's my shot at that patch,
which passed the above test case for me (module the bug that fg still didn't
print anything to stdout), but I have no idea if it is doing the correct thing.

Meanwhile, Autoconf does not use fg or bg in implementing parallel tests, so I
will try to find time to see whether my patch below is sufficient to get through
a parallel testsuite run.



From: Eric Blake <ebb9@xxxxxxx>
Date: Thu, 9 Jul 2009 12:04:58 -0600
Subject: [PATCH] 27100: Touch up non-interactive MONITOR handling.

---
 ChangeLog   |    6 ++++++
 Src/init.c  |    2 +-
 Src/utils.c |    2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3a4ca1f..2797835 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2009-07-09  Eric Blake  <ebb9@xxxxxxx>
+
+	* 27100: Touch up non-interactive MONITOR handling.
+	Don't enable MONITOR when initializing non-interactive shells.
+	Don't mess with tty when MONITOR but not interactive.
+
 2009-07-08  Peter Stephenson  <pws@xxxxxxx>

 	* 27100: Allow MONITOR option in non-interactive shells.
diff --git a/Src/init.c b/Src/init.c
index 1894519..41e5ecf 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -486,7 +486,7 @@ init_io(void)
      * process group leader.
      */
     mypid = (zlong)getpid();
-    if (opts[MONITOR] && (SHTTY != -1)) {
+    if (opts[MONITOR] && interact && (SHTTY != -1)) {
 	origpgrp = GETPGRP();
         acquire_pgrp(); /* might also clear opts[MONITOR] */
     } else
diff --git a/Src/utils.c b/Src/utils.c
index 510a02b..ad4ffca 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3590,7 +3590,7 @@ attachtty(pid_t pgrp)
 {
     static int ep = 0;

-    if (jobbing) {
+    if (jobbing && interact) {
 #ifdef HAVE_TCSETPGRP
 	if (SHTTY != -1 && tcsetpgrp(SHTTY, pgrp) == -1 && !ep)
 #else
-- 
1.6.3.2