[PATCH] Fix jobs -p to be POSIX compliant

[PATCH] Fix jobs -p to be POSIX compliant

[Wesley Schwengle]

In Debian the following bug was reported:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=346162

POSIX says that with "jobs -p", only the PID is output, whereas zsh
outputs full information.

There is discussion in the bug regarding POSIX_BUILTINS and refers to
workers/21366. The latter is interesting because it refers to setopt long_list_jobs.
I would want to argue that jobs -p should only show the PID regardless
of that setting and POSIX_BUILTINS. jobs has the -l option, where the
full line is displayed.

These patch makes the jobs -p POSIX compliant

Signed-off-by: Wesley Schwengle <wesleys@opperschaap.net>
---
 Src/jobs.c        | 18 ++++++++----------
 Test/W02jobs.ztst |  2 +-
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/Src/jobs.c b/Src/jobs.c
index 118c5e61b..50f4fa3cf 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1209,6 +1209,13 @@ printjob(Job jn, int lng, int synch)
 	    putc('\n', fout);
 	}
 	for (pn = jn->procs; pn;) {
+	    if (lng & 2) {
+		pid_t x = jn->gleader;
+		fprintf(fout, "%ld", (long) x);
+		putc('\n', fout);
+		doneprint = 1;
+		break;
+	    }
 	    len2 = (thisfmt ? 5 : 10) + len;	/* 2 spaces */
 	    if (lng & 3)
 		qn = pn->next;
@@ -1235,16 +1242,7 @@ printjob(Job jn, int lng, int synch)
 		    fprintf(fout, "zsh: ");
 		if (lng & 1)
 		    fprintf(fout, "%ld ", (long) pn->pid);
-		else if (lng & 2) {
-		    pid_t x = jn->gleader;
-
-		    fprintf(fout, "%ld ", (long) x);
-		    do
-			skip++;
-		    while ((x /= 10));
-		    skip++;
-		    lng &= ~3;
-		} else
+		else
 		    fprintf(fout, "%*s", skip, "");
 		if (pn->status == SP_RUNNING) {
 		    if (!conted)
diff --git a/Test/W02jobs.ztst b/Test/W02jobs.ztst
index d52888dd9..11bbdbf43 100644
--- a/Test/W02jobs.ztst
+++ b/Test/W02jobs.ztst
@@ -128,7 +128,7 @@
 0:`jobs -l` and `jobs -p` with running job
 *>\[1] [0-9]##
 *>\[1]  + [0-9]## running*sleep*
-*>\[1]  + [0-9]## running*sleep*
+*>[0-9]##
 *>zsh:*SIGHUPed*
 
   zpty_start
-- 
2.42.0.1140.gf01f4dc781

[PATCH v2] Fix jobs -p to be POSIX compliant

[Wesley Schwengle]

In Debian the following bug was reported a couple of year ago and
someone re-raised it:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=346162

POSIX says that with "jobs -p", only the PID is output, whereas zsh
outputs full information.

There is discussion in the bug regarding POSIX_BUILTINS and refers to
workers/21366. The latter is interesting because it refers to setopt
long_list_jobs. I would want to argue that jobs -p should only show the
PID regardless of that setting and POSIX_BUILTINS. jobs has the -l
option, where the full line is displayed. From that point it doesn't
make a whole lot of sense to have jobs -p do the same thing. From a UI
standpoint I think it is more logical to let jobs -l do a long listing
and jobs -p only show the pid.

Than there is the issue where its argued that zsh shows the group pid
and not the actual pid of the process that is running. I looked at the
output of bash and with this patch zsh and bash act similar. They both
show the pid of the command group:

On bash:

$ ( sleep 20 && echo foo) & sleep 5 &
$ jobs -p && jobs -l
255028
255029
[1]- 255028 Running                 ( sleep 20 && echo foo ) &
[2]+ 255029 Done                    sleep 5

On zsh with this patch:

$ ( sleep 20 && echo foo) & sleep 5 &
$ jobs -l && jobs -p
[1] 255065
[2] 255066
[1]  - 255065 running    ( sleep 20 && echo foo; )
[2]  + 255066 running    sleep 5
255065
255066

Signed-off-by: Wesley Schwengle <wesleys@opperschaap.net>
---
 Src/jobs.c        | 16 ++++++----------
 Test/W02jobs.ztst |  2 +-
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/Src/jobs.c b/Src/jobs.c
index 118c5e61b..14f1be831 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1209,6 +1209,11 @@ printjob(Job jn, int lng, int synch)
 	    putc('\n', fout);
 	}
 	for (pn = jn->procs; pn;) {
+	    if (lng & 2) {
+		fprintf(fout, "%ld\n", (long) jn->gleader);
+		doneprint = 1;
+		break;
+	    }
 	    len2 = (thisfmt ? 5 : 10) + len;	/* 2 spaces */
 	    if (lng & 3)
 		qn = pn->next;
@@ -1235,16 +1240,7 @@ printjob(Job jn, int lng, int synch)
 		    fprintf(fout, "zsh: ");
 		if (lng & 1)
 		    fprintf(fout, "%ld ", (long) pn->pid);
-		else if (lng & 2) {
-		    pid_t x = jn->gleader;
-
-		    fprintf(fout, "%ld ", (long) x);
-		    do
-			skip++;
-		    while ((x /= 10));
-		    skip++;
-		    lng &= ~3;
-		} else
+		else
 		    fprintf(fout, "%*s", skip, "");
 		if (pn->status == SP_RUNNING) {
 		    if (!conted)
diff --git a/Test/W02jobs.ztst b/Test/W02jobs.ztst
index d52888dd9..11bbdbf43 100644
--- a/Test/W02jobs.ztst
+++ b/Test/W02jobs.ztst
@@ -128,7 +128,7 @@
 0:`jobs -l` and `jobs -p` with running job
 *>\[1] [0-9]##
 *>\[1]  + [0-9]## running*sleep*
-*>\[1]  + [0-9]## running*sleep*
+*>[0-9]##
 *>zsh:*SIGHUPed*
 
   zpty_start
-- 
2.42.0.1140.gf01f4dc781

Re: [PATCH v2] Fix jobs -p to be POSIX compliant

[Bart Schaefer]

On Sat, Jan 20, 2024 at 4:31 AM Wesley Schwengle
<wesleys@opperschaap.net> wrote:
>
> POSIX says that with "jobs -p", only the PID is output, whereas zsh
> outputs full information.
>
> There is discussion in the bug regarding POSIX_BUILTINS and refers to
> workers/21366. The latter is interesting because it refers to setopt
> long_list_jobs. I would want to argue that jobs -p should only show the
> PID regardless of that setting and POSIX_BUILTINS. jobs has the -l
> option, where the full line is displayed. From that point it doesn't
> make a whole lot of sense to have jobs -p do the same thing.

This is not correct.  As you noted later, "jobs -p" lists information
about process group leaders only, whereas "jobs -l" lists all jobs.
This differs mainly when there's a pipeline:

% sleep 30 | sleep 20 | sleep 10 &
[1] 67157 67158 67159
% jobs -p
[1]  + 67157 running    sleep 30 |
             running    sleep 20 | sleep 10
% jobs -l
[1]  + 67157 running    sleep 30 |
       67158 running    sleep 20 |
       67159 running    sleep 10
%

> Than there is the issue where its argued that zsh shows the group pid
> and not the actual pid of the process that is running. I looked at the
> output of bash and with this patch zsh and bash act similar.

More relevant is how they compare without the patch.

If we really wanted to make this POSIX compliant, we should arrange
that "jobs -lp" shows "more information" only about process group
leaders (currently -l simply overrides -p) and add "jobs -n" to
produce the notifications that are suppressed by "setopt no_notify"
(setopt long_list_jobs would apply here).

However, I'm generally not in favor of changing longstanding zsh
behavior for POSIX compliance without some kind of additional
conditions, such as [[ -o POSIX_JOBS ]] in this case.