disowning stopped jobs

disowning stopped jobs

[Sven Wischnowsky]

If one tries bash-2.04:

  % sleep 80
  ^Z
  [1]+  Stopped                    sleep 80
  % disown %1
  bash: warning: deleting stopped job 1 with process group 988

I.e., it prints what Bart suggested we should print.  I answered that by
saying that this is slightly more complicated for us because there may
also be a sub-job to be continued in such cases.

Maybe we can `solve' (or, rather, circumvent) this problem by being more
verbose, saying `warning: job is stopped, use `kill -CONT ...' to resume'.
That way, users can even use cut&paste to make the job running.

Below is a patch for this, which I won't commit until I get positive
replies.

Any better wording?  Should we print the `kill ...' indented on its own
line? (The list of pids can get a bit longer, because the patch makes it
use the same loops used by killjb() for super-jobs where we send all
processes the CONT signal.)


Bye
  Sven

diff -ur -r ../oz/Src/jobs.c ./Src/jobs.c
--- ../oz/Src/jobs.c	Sat Jun 23 22:12:56 2001
+++ ./Src/jobs.c	Sat Jun 23 22:42:33 2001
@@ -1392,14 +1392,36 @@
 	    printjob(job + jobtab, lng, 2);
 	    break;
 	case BIN_DISOWN:
-	    if (jobtab[job].stat & STAT_STOPPED)
+	    if (jobtab[job].stat & STAT_STOPPED) {
+		char buf[20], *pids = "";
+
+		if (jobtab[job].stat & STAT_SUPERJOB) {
+		    Process pn;
+
+		    for (pn = jobtab[jobtab[job].other].procs; pn; pn = pn->next) {
+			sprintf(buf, " -%d", pn->pid);
+			pids = dyncat(pids, buf);
+		    }
+		    for (pn = jobtab[job].procs; pn->next; pn = pn->next) {
+			sprintf(buf, " %d", pn->pid);
+			pids = dyncat(pids, buf);
+		    }
+		    if (!jobtab[jobtab[job].other].procs && pn) {
+			sprintf(buf, " %d", pn->pid);
+			pids = dyncat(pids, buf);
+		    }
+		} else {
+		    sprintf(buf, " -%d", jobtab[job].gleader);
+		    pids = buf;
+		}
                 zwarnnam(name,
 #ifdef USE_SUSPENDED
-                         "warning: job is suspended",
+                         "warning: job is suspended, use `kill -CONT%s' to resume",
 #else
-                         "warning: job is stopped",
+                         "warning: job is stopped, use `kill -CONT%s' to resume",
 #endif
-                         NULL, 0);
+                         pids, 0);
+	    }
 	    deletejob(jobtab + job);
 	    break;
 	}

-- 
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: disowning stopped jobs

[Thomas Köhler]

[-- Attachment #1: Type: text/plain, Size: 3094 bytes --]

On Mon, Jun 25, 2001 at 11:00:56AM +0200,
Sven Wischnowsky <wischnow@informatik.hu-berlin.de> wrote:
> If one tries bash-2.04:
> 
>   % sleep 80
>   ^Z
>   [1]+  Stopped                    sleep 80
>   % disown %1
>   bash: warning: deleting stopped job 1 with process group 988
> 
> I.e., it prints what Bart suggested we should print.  I answered that by
> saying that this is slightly more complicated for us because there may
> also be a sub-job to be continued in such cases.
> 
> Maybe we can `solve' (or, rather, circumvent) this problem by being more
> verbose, saying `warning: job is stopped, use `kill -CONT ...' to resume'.
> That way, users can even use cut&paste to make the job running.

Good idea, IMHO.

> Below is a patch for this, which I won't commit until I get positive
> replies.

Well, I just have a minor quetion, see at end.

[...]
> diff -ur -r ../oz/Src/jobs.c ./Src/jobs.c
> --- ../oz/Src/jobs.c	Sat Jun 23 22:12:56 2001
> +++ ./Src/jobs.c	Sat Jun 23 22:42:33 2001
> @@ -1392,14 +1392,36 @@
>  	    printjob(job + jobtab, lng, 2);
>  	    break;
>  	case BIN_DISOWN:
> -	    if (jobtab[job].stat & STAT_STOPPED)
> +	    if (jobtab[job].stat & STAT_STOPPED) {
> +		char buf[20], *pids = "";
> +
> +		if (jobtab[job].stat & STAT_SUPERJOB) {
> +		    Process pn;
> +
> +		    for (pn = jobtab[jobtab[job].other].procs; pn; pn = pn->next) {
> +			sprintf(buf, " -%d", pn->pid);
> +			pids = dyncat(pids, buf);
> +		    }
> +		    for (pn = jobtab[job].procs; pn->next; pn = pn->next) {
> +			sprintf(buf, " %d", pn->pid);
> +			pids = dyncat(pids, buf);
> +		    }
> +		    if (!jobtab[jobtab[job].other].procs && pn) {
> +			sprintf(buf, " %d", pn->pid);
> +			pids = dyncat(pids, buf);
> +		    }
> +		} else {
> +		    sprintf(buf, " -%d", jobtab[job].gleader);
> +		    pids = buf;
> +		}
>                  zwarnnam(name,
>  #ifdef USE_SUSPENDED
> -                         "warning: job is suspended",
> +                         "warning: job is suspended, use `kill -CONT%s' to resume",
                                                                      ^^
>  #else
> -                         "warning: job is stopped",
> +                         "warning: job is stopped, use `kill -CONT%s' to resume",
                                                                    ^^
>  #endif
> -                         NULL, 0);
> +                         pids, 0);
> +	    }
>  	    deletejob(jobtab + job);
>  	    break;
>  	}

Shouldn't there be a space between 'T' and '%' in the marked places?
Or does "pids" contain a space at the beginning?
(Just thrown in because it looks strange, I don't really know anything
about that "pids" variable, what does dyncat() do to it?)

Ciao,
Thomas

-- 
 Thomas Köhler Email:   jean-luc@picard.franken.de     | LCARS - Linux
     <><        WWW:     http://jeanluc-picard.de      | for Computers
                IRC:             jeanluc               | on All Real
               PGP public key available from Homepage! | Starships

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: disowning stopped jobs

[Sven Wischnowsky]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

Thomas Köhler wrote:

> ...
> 
> Shouldn't there be a space between 'T' and '%' in the marked places?
> Or does "pids" contain a space at the beginning?

It does, look at the sprintf()s, they all have a space at the beginning
so that concatenating them with dyncat() makes it look right and we
don't have a space at the end of the string.

> (Just thrown in because it looks strange, I don't really know anything
> about that "pids" variable, what does dyncat() do to it?)

It concatenates its arguments in `dynamic' memory, i.e., in
heap-allocated memory (a zsh heap, not the system-heap).


Bye
  Sven


-- 
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: disowning stopped jobs

[Thomas Köhler]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

On Mon, Jun 25, 2001 at 11:17:58AM +0200,
Sven Wischnowsky <wischnow@informatik.hu-berlin.de> wrote:
> Thomas Köhler wrote:
> 
> > ...
> > 
> > Shouldn't there be a space between 'T' and '%' in the marked places?
> > Or does "pids" contain a space at the beginning?
> 
> It does, look at the sprintf()s, they all have a space at the beginning
> so that concatenating them with dyncat() makes it look right and we
> don't have a space at the end of the string.

Ah, alright. So looks like all is fine then. Thanks for explaining :-)

> > (Just thrown in because it looks strange, I don't really know anything
> > about that "pids" variable, what does dyncat() do to it?)
> 
> It concatenates its arguments in `dynamic' memory, i.e., in
> heap-allocated memory (a zsh heap, not the system-heap).

Ah. Perhaps worth a look some time...

> Bye
>   Sven

Ciao,
Thomas

-- 
 Thomas Köhler Email:   jean-luc@picard.franken.de     | LCARS - Linux
     <><        WWW:     http://jeanluc-picard.de      | for Computers
                IRC:             jeanluc               | on All Real
               PGP public key available from Homepage! | Starships

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: disowning stopped jobs

[Bart Schaefer]

On Jun 25, 11:00am, Sven Wischnowsky wrote:
} Subject: disowning stopped jobs
}
} Maybe we can `solve' (or, rather, circumvent) this problem by being more
} verbose, saying `warning: job is stopped, use `kill -CONT ...' to resume'.
} That way, users can even use cut&paste to make the job running.
} 
} Any better wording?  Should we print the `kill ...' indented on its own
} line?

I think it's OK as it is, at least until we get some empirical feedback.

What about my other suggestion, that `%job &!' should use `bg' and then
handle the job table in exec.c ?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

PATCH: Re: disowning stopped jobs

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Jun 25, 11:00am, Sven Wischnowsky wrote:
> } Subject: disowning stopped jobs
> }
> } Maybe we can `solve' (or, rather, circumvent) this problem by being more
> } verbose, saying `warning: job is stopped, use `kill -CONT ...' to resume'.
> } That way, users can even use cut&paste to make the job running.
> } 
> } Any better wording?  Should we print the `kill ...' indented on its own
> } line?
> 
> I think it's OK as it is, at least until we get some empirical feedback.

Fine, I'm going to commit it to the development branch, should it go
into 4.0.2, too?

> What about my other suggestion, that `%job &!' should use `bg' and then
> handle the job table in exec.c ?

I looked into this shortly, it would require us to export getjob() and
then should be relatively simple.  But.  Just had an idea.  Why not give
an option to disown, say `-c' for `continue', that makes it wake up
stopped jobs before disowning them?  With that, users could alias
`disown' to `disown -c' to never have to worry about that anymore and in
execcmd() we just would have to stick the `-c' into the list.


Bye
  Sven

Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.11
diff -u -r1.11 jobs.c
--- Src/jobs.c	2001/05/29 09:49:58	1.11
+++ Src/jobs.c	2001/06/26 08:12:37
@@ -1392,14 +1392,36 @@
 	    printjob(job + jobtab, lng, 2);
 	    break;
 	case BIN_DISOWN:
-	    if (jobtab[job].stat & STAT_STOPPED)
+	    if (jobtab[job].stat & STAT_STOPPED) {
+		char buf[20], *pids = "";
+
+		if (jobtab[job].stat & STAT_SUPERJOB) {
+		    Process pn;
+
+		    for (pn = jobtab[jobtab[job].other].procs; pn; pn = pn->next) {
+			sprintf(buf, " -%d", pn->pid);
+			pids = dyncat(pids, buf);
+		    }
+		    for (pn = jobtab[job].procs; pn->next; pn = pn->next) {
+			sprintf(buf, " %d", pn->pid);
+			pids = dyncat(pids, buf);
+		    }
+		    if (!jobtab[jobtab[job].other].procs && pn) {
+			sprintf(buf, " %d", pn->pid);
+			pids = dyncat(pids, buf);
+		    }
+		} else {
+		    sprintf(buf, " -%d", jobtab[job].gleader);
+		    pids = buf;
+		}
                 zwarnnam(name,
 #ifdef USE_SUSPENDED
-                         "warning: job is suspended",
+                         "warning: job is suspended, use `kill -CONT%s' to resume",
 #else
-                         "warning: job is stopped",
+                         "warning: job is stopped, use `kill -CONT%s' to resume",
 #endif
-                         NULL, 0);
+                         pids, 0);
+	    }
 	    deletejob(jobtab + job);
 	    break;
 	}

-- 
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: disowning stopped jobs

[Sven Wischnowsky]

I wrote:

> ...
> 
> I looked into this shortly, it would require us to export getjob() and
> then should be relatively simple.  But.  Just had an idea.  Why not give
> an option to disown, say `-c' for `continue', that makes it wake up
> stopped jobs before disowning them?  With that, users could alias
> `disown' to `disown -c' to never have to worry about that anymore and in
> execcmd() we just would have to stick the `-c' into the list.

Err... of course users could already write their own little functions
that call bg, then disown to get that.  But it still seems useful to
make this as convenient as possible -- maybe a setopt option (didn't
someone suggest this lately?).


Bye
  Sven


-- 
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: disowning stopped jobs

[Bart Schaefer]

On Jun 26, 10:15am, Sven Wischnowsky wrote:
} Subject: PATCH: Re: disowning stopped jobs
}
} Just had an idea. Why not give an option to disown, say `-c' for
} `continue', that makes it wake up stopped jobs before disowning them?

I think I'd rather have a setopt, as presently `-c' would be interpreted
by disown as a job spec.  However, I don't feel strongly about it.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: Re: disowning stopped jobs

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Jun 26, 10:15am, Sven Wischnowsky wrote:
> } Subject: PATCH: Re: disowning stopped jobs
> }
> } Just had an idea. Why not give an option to disown, say `-c' for
> } `continue', that makes it wake up stopped jobs before disowning them?
> 
> I think I'd rather have a setopt, as presently `-c' would be interpreted
> by disown as a job spec.  However, I don't feel strongly about it.

Hm, I was thinking the same after I'd sent that.

Here is the patch, adding the option `AUTO_CONTINUE' (any better name?).
It also makes the `%foo &!' and `%foo &|' forms make the job running
automatically, independent of the option's setting.


Bye
  Sven

Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.30
diff -u -r1.30 builtins.yo
--- Doc/Zsh/builtins.yo	2001/06/14 09:49:02	1.30
+++ Doc/Zsh/builtins.yo	2001/06/27 11:20:09
@@ -230,6 +230,12 @@
 no longer report their status, and will not complain if you
 try to exit an interactive shell with them running or stopped.
 If no var(job) is specified, disown the current job.
+
+If the var(job)s are currently stopped and the tt(AUTO_CONTINUE) option
+is not set, a warning is printed containing information about how to
+make them running after they have been disowned.  If one of the latter
+two forms is used, the var(job)s will automatically be made running,
+independent of the setting of the tt(AUTO_CONTINUE) option.
 )
 findex(echo)
 item(tt(echo) [ tt(-neE) ] [ var(arg) ... ])(
Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.20
diff -u -r1.20 options.yo
--- Doc/Zsh/options.yo	2001/06/12 10:34:57	1.20
+++ Doc/Zsh/options.yo	2001/06/27 11:20:10
@@ -91,6 +91,14 @@
 and the command is the name of a directory, perform the tt(cd)
 command to that directory.
 )
+pindex(AUTO_CONTINUE)
+cindex(jobs, continuing automatically)
+cindex(continuing jobs automatically)
+item(tt(AUTO_CONT))(
+With this option set, stopped jobs that are removed from the job table
+with the tt(disown) builtin command are automatically sent a tt(CONT)
+signal to make them running.
+)
 pindex(AUTO_LIST)
 cindex(completion, listing choices)
 item(tt(AUTO_LIST) (tt(-9)) <D>)(
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.32
diff -u -r1.32 exec.c
--- Src/exec.c	2001/06/25 00:15:05	1.32
+++ Src/exec.c	2001/06/27 11:20:11
@@ -1645,7 +1645,7 @@
     int nullexec = 0, assign = 0, forked = 0;
     int is_shfunc = 0, is_builtin = 0, is_exec = 0;
     /* Various flags to the command. */
-    int cflags = 0, checked = 0;
+    int cflags = 0, checked = 0, oautocont = opts[AUTOCONTINUE];
     LinkList redir;
     wordcode code;
     Wordcode beg = state->pc, varspc;
@@ -1680,6 +1680,8 @@
      * reference to a job in the job table.                */
     if (type == WC_SIMPLE && args && nonempty(args) &&
 	*(char *)peekfirst(args) == '%') {
+        if (how & Z_DISOWN)
+            opts[AUTOCONTINUE] = 1;
 	pushnode(args, dupstring((how & Z_DISOWN)
 				 ? "disown" : (how & Z_ASYNC) ? "bg" : "fg"));
 	how = Z_SYNC;
@@ -1833,6 +1835,7 @@
 		if (cflags & BINF_BUILTIN) {
 		    zwarn("no such builtin: %s", cmdarg, 0);
 		    lastval = 1;
+                    opts[AUTOCONTINUE] = oautocont;
 		    return;
 		}
 		break;
@@ -1856,6 +1859,7 @@
 
     if (errflag) {
 	lastval = 1;
+        opts[AUTOCONTINUE] = oautocont;
 	return;
     }
 
@@ -1899,6 +1903,7 @@
 
     if (errflag) {
 	lastval = 1;
+        opts[AUTOCONTINUE] = oautocont;
 	return;
     }
 
@@ -1981,6 +1986,7 @@
 	if ((pid = zfork()) == -1) {
 	    close(synch[0]);
 	    close(synch[1]);
+            opts[AUTOCONTINUE] = oautocont;
 	    return;
 	} if (pid) {
 	    close(synch[1]);
@@ -2006,6 +2012,7 @@
 		}
 	    }
 	    addproc(pid, text);
+            opts[AUTOCONTINUE] = oautocont;
 	    return;
 	}
 	/* pid == 0 */
@@ -2373,6 +2380,7 @@
 
     zsfree(STTYval);
     STTYval = 0;
+    opts[AUTOCONTINUE] = oautocont;
 }
 
 /* Arrange to have variables restored. */
Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.12
diff -u -r1.12 jobs.c
--- Src/jobs.c	2001/06/26 08:18:23	1.12
+++ Src/jobs.c	2001/06/27 11:20:12
@@ -1215,7 +1215,7 @@
 int
 bin_fg(char *name, char **argv, char *ops, int func)
 {
-    int job, lng, firstjob = -1, retval = 0;
+    int job, lng, firstjob = -1, retval = 0, ofunc = func;
 
     if (ops['Z']) {
 	int len;
@@ -1299,6 +1299,8 @@
     for (; (firstjob != -1) || *argv; (void)(*argv && argv++)) {
 	int stopped, ocj = thisjob;
 
+        func = ofunc;
+
 	if (func == BIN_WAIT && isanum(*argv)) {
 	    /* wait can take a pid; the others can't. */
 	    pid_t pid = (long)atoi(*argv);
@@ -1326,6 +1328,13 @@
 	    unqueue_signals();
 	    return 1;
 	}
+        /* If AUTO_CONTINUE is set (automatically make stopped jobs running
+         * on disown), we actually do a bg and then delete the job table entry. */
+
+        if (isset(AUTOCONTINUE) && func == BIN_DISOWN &&
+            jobtab[job].stat & STAT_STOPPED)
+            func = BIN_BG;
+
 	/* We have a job number.  Now decide what to do with it. */
 	switch (func) {
 	case BIN_FG:
@@ -1386,7 +1395,8 @@
 	    if (func != BIN_BG) {
 		waitjobs();
 		retval = lastval2;
-	    }
+	    } else if (ofunc == BIN_DISOWN)
+	        deletejob(jobtab + job);
 	    break;
 	case BIN_JOBS:
 	    printjob(job + jobtab, lng, 2);
Index: Src/options.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/options.c,v
retrieving revision 1.9
diff -u -r1.9 options.c
--- Src/options.c	2001/06/12 10:34:57	1.9
+++ Src/options.c	2001/06/27 11:20:12
@@ -75,6 +75,7 @@
 {NULL, "alwaystoend",	      0,			 ALWAYSTOEND},
 {NULL, "appendhistory",	      OPT_ALL,			 APPENDHISTORY},
 {NULL, "autocd",	      OPT_EMULATE,		 AUTOCD},
+{NULL, "autocontinue",	      0,			 AUTOCONTINUE},
 {NULL, "autolist",	      OPT_ALL,			 AUTOLIST},
 {NULL, "automenu",	      OPT_ALL,			 AUTOMENU},
 {NULL, "autonamedirs",	      0,			 AUTONAMEDIRS},
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.31
diff -u -r1.31 zsh.h
--- Src/zsh.h	2001/06/12 10:34:57	1.31
+++ Src/zsh.h	2001/06/27 11:20:13
@@ -1314,6 +1314,7 @@
     ALWAYSTOEND,
     APPENDHISTORY,
     AUTOCD,
+    AUTOCONTINUE,
     AUTOLIST,
     AUTOMENU,
     AUTONAMEDIRS,

-- 
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: Re: disowning stopped jobs

[Clint Adams]

> Fine, I'm going to commit it to the development branch, should it go
> into 4.0.2, too?

[...]

> Here is the patch, adding the option `AUTO_CONTINUE' (any better name?).
> It also makes the `%foo &!' and `%foo &|' forms make the job running
> automatically, independent of the option's setting.

Someone just complained to me about the 4.0.2 behavior.  I referred
him to 4.1.0, but it seems to me that this is innocuous enough to be
merged into the trunk in preparation for 4.0.3.