PATCH: Run builtins, functions, etc. under zpty

PATCH: Run builtins, functions, etc. under zpty

[Bart Schaefer]

This patch copies code from the clone module into newptycmd(), so that the
forked process set up on the pty is initially a clone of the current shell.
Then instead of calling execve(), the command can be parsed and executed
with execode().  This is thus roughly equivalent to a shell function:

	clone /dev/ttyxx
	[[ $TTY == /dev/ttyxx ]] && {
	    eval $*
	    exit
	}

where zpty takes care of allocating the pty for /dev/ttyxx.

I initially wanted to avoid the "eval", e.g. do the equivalent of executing
$* directly; but there isn't any simple function for converting a char** to
the wordcode for a simple command without re-parsing it, and anyway without
the eval you can't do conditionals, loops or pipelines.

This does mean that zpty can't test the goodness of the command until after
it is executed in the forked process (no more findcmd() call).  Does anyone
think this is a serious drawback?

Included is correction of a couple of inconsequential typos in utils.c.

Index: Doc/Zsh/mod_zpty.yo
===================================================================
@@ -5,12 +5,13 @@
 
 startitem()
 findex(zpty)
-item(tt(zpty) [ tt(-e) ] [ tt(-b) ] var(name) var(command) [ var(args ...) ])(
-In the first form, the var(command) is started with the var(args) as
-arguments.  The command runs under a newly assigned pseudo-terminal; this
-is useful for running commands non-interactively which expect an
-interactive environment.  The var(name) is used to refer to this command
-in later calls to tt(zpty).
+item(tt(zpty) [ tt(-e) ] [ tt(-b) ] var(name) [ var(arg ...) ])(
+The arguments following var(name) are concatenated with spaces between,
+then executed as a command, as if passed to the tt(eval) builtin.  The
+command runs under a newly assigned pseudo-terminal; this is useful for
+running commands non-interactively which expect an interactive
+environment.  The var(name) is not part of the command, but is used to
+refer to this command in later calls to tt(zpty).
 
 With the tt(-e) option, the pseudo-terminal is set up so that input
 characters are echoed.
Index: Src/utils.c
===================================================================
@@ -2548,7 +2548,7 @@
  *   META_NOALLOC:  buf points to a memory area which is long enough to hold *
  *                  the quoted form, just quote it and return buf.           *
  *   META_STATIC:   store the quoted string in a static area.  The original  *
- *                  sting should be at most PATH_MAX long.                   *
+ *                  string should be at most PATH_MAX long.                   *
  *   META_ALLOC:    allocate memory for the new string with zalloc().        *
  *   META_DUP:      leave buf unchanged and allocate space for the return    *
  *                  value even if buf does not contains special characters   *
@@ -2600,7 +2600,7 @@
 	case META_NOALLOC:
 	    break;
 	default:
-	    fprintf(stderr, "BUG: metafy called with invaild heap value\n");
+	    fprintf(stderr, "BUG: metafy called with invalid heap value\n");
 	    fflush(stderr);
 	    break;
 #endif
Index: Src/Modules/zpty.c
===================================================================
@@ -171,7 +171,7 @@
     int ret;
 
     if (master) {
-	if ((mfd = open("/dev/ptmx", O_RDWR)) < 0)
+	if ((mfd = open("/dev/ptmx", O_RDWR|O_NOCTTY)) < 0)
 	    return 1;
 
 	if (grantpt(mfd) || unlockpt(mfd) || !(name = ptsname(mfd))) {
@@ -182,7 +182,7 @@
 
 	return 0;
     }
-    if ((sfd = open(name, O_RDWR)) < 0) {
+    if ((sfd = open(name, O_RDWR|O_NOCTTY)) < 0) {
 	close(mfd);
 	return 1;
     }
@@ -242,7 +242,7 @@
 	    name[8] = *p1;
 	    for (p2 = char2; *p2; p2++) {
 		name[9] = *p2;
-		if ((mfd = open(name, O_RDWR)) >= 0) {
+		if ((mfd = open(name, O_RDWR|O_NOCTTY)) >= 0) {
 		    *retfd = mfd;
 
 		    return 0;
@@ -251,7 +251,7 @@
 	}
     }
     name[5] = 't';
-    if ((sfd = open(name, O_RDWR)) >= 0) {
+    if ((sfd = open(name, O_RDWR|O_NOCTTY)) >= 0) {
 	*retfd = sfd;
 
 	return 0;
@@ -268,12 +268,14 @@
 {
     Ptycmd p;
     int master, slave, pid;
-    char *cmd;
+    Eprog prog;
 
-    if (!(cmd = findcmd(*args, 1))) {
-	zwarnnam(nam, "unknown command: %s", *args, 0);
+    prog = parse_string(zjoin(args, ' ', 1), 0);
+    if (!prog) {
+	errflag = 0;
 	return 1;
     }
+
     if (get_pty(1, &master)) {
 	zwarnnam(nam, "can't open pseudo terminal: %e", NULL, errno);
 	return 1;
@@ -283,41 +285,40 @@
 	close(master);
 	return 1;
     } else if (!pid) {
-
-	pid = getpid();
+	/* This code copied from the clone module, except for getting *
+	 * the descriptor from get_pty() and duplicating it to 0/1/2. */
 
+	clearjobtab();
+	ppid = getppid();
+	mypid = getpid();
 #ifdef HAVE_SETSID
-	setsid();
-#else
+	if (setsid() != mypid) {
+	    zwarnnam(nam, "failed to create new session: %e", NULL, errno);
+#endif
 #ifdef TIOCNOTTY
-	{
-	    int t = open("/dev/tty", O_RDWR);
-
-	    ioctl(t, TIOCNOTTY, 0);
-	    close(t);
-	}
+	    if (ioctl(SHTTY, TIOCNOTTY, 0))
+		zwarnnam(nam, "%e", NULL, errno);
+	    setpgrp(0L, mypid);
 #endif
+#ifdef HAVE_SETSID
+	}
 #endif
 
 	if (get_pty(0, &slave))
 	    exit(1);
-
-#ifdef TIOCSCTTY
-	ioctl(slave, TIOCSCTTY, 0);
-#endif
-
-	/* This is taken from attachtty(). Should we exit in case of
-	 * failure? */
+#ifdef TIOCGWINSZ
+	/* Set the window size before associating with the terminal *
+	 * so that we don't get hit with a SIGWINCH.  I'm paranoid. */
+	if (interact) {
+	    struct ttyinfo info;
 
-#ifdef HAVE_TCSETPGRP
-	tcsetpgrp(slave, pid);
-#else
-# if ardent
-	setpgrp();
-# else
-	ioctl(slave, TIOCSPGRP, &pid);
-# endif
-#endif
+	    if (ioctl(slave, TIOCGWINSZ, (char *) &info.winsize) == 0) {
+		info.winsize.ws_row = lines;
+		info.winsize.ws_col = columns;
+		ioctl(slave, TIOCSWINSZ, (char *) &info.winsize);
+	    }
+	}
+#endif /* TIOCGWINSZ */
 
 	if (!echo) {
 	    struct ttyinfo info;
@@ -336,21 +337,9 @@
 	    }
 	}
 
-#ifdef TIOCGWINSZ
-	if (interact) {
-	    struct ttyinfo info;
-
-	    if (ioctl(slave, TIOCGWINSZ, (char *) &info.winsize) == 0) {
-		info.winsize.ws_row = lines;
-		info.winsize.ws_col = columns;
-		ioctl(slave, TIOCSWINSZ, (char *) &info.winsize);
-	    }
-	}
-#endif /* TIOCGWINSZ */
-
-	signal_default(SIGTERM);
-	signal_default(SIGINT);
-	signal_default(SIGQUIT);
+#ifdef TIOCSCTTY
+	ioctl(slave, TIOCSCTTY, 0);
+#endif
 
 	close(0);
 	close(1);
@@ -360,16 +349,16 @@
 	dup2(slave, 1);
 	dup2(slave, 2);
 
+	closem(0);
 	close(slave);
 	close(master);
-
-	if (SHTTY != -1)
-	    close(SHTTY);
-
-	closedumps();
+	close(coprocin);
+	close(coprocout);
+	init_io();
+	setsparam("TTY", ztrdup(ttystrname));
 
-	execve(cmd, args, environ);
-	exit(0);
+	execode(prog, 1, 0);
+	zexit(lastval, 0);
     }
     master = movefd(master);
 

-- 
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: Run builtins, functions, etc. under zpty

[Bart Schaefer]

On Nov 5, 10:37pm, Bart Schaefer wrote:
}
} This patch copies code from the clone module into newptycmd(), so that the
} forked process set up on the pty is initially a clone of the current shell.
} Then instead of calling execve(), the command can be parsed and executed
} with execode().

I forgot that this requires a change to nslookup.

Also, I committed a tweak to newptycmd() after sending the patch in 13123,
to unset `interactive' in the cloned shell before calling execode().  This
seems like the right thing to do; the clone itself isn't interactive any
more, even if the command it's eval'ing probably is, and it prevents the
saving of history or sourcing of .zlogout should an `exec' or `exit' be
performed in the clone.  (Patch for that not included here, it's in CVS.)

Index: Functions/Misc/nslookup
===================================================================
@@ -24,7 +24,7 @@
     [[ -z "$pager" ]] && pager="${opager:-more}"
 (( $#pmpt )) || pmpt=(-p '> ')
 
-zpty nslookup nslookup "$@"
+zpty nslookup command nslookup "${(q)@}"
 
 zpty -r nslookup line '*
 > '

-- 
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: Run builtins, functions, etc. under zpty

[Andrej Borsenkow]

Attempt to run any (external) command with zpty under Cygwin now results in:

zsh: can't set tty pgrp: not owner

(from child process) and hanging? zsh. That is what I have in process list:

$ ps -l
      PID    PPID    PGID     WINPID TTY  UID    STIME COMMAND
     1696       1    1696       1696   0 1006 15:22:33 /usr/bin/bash
      580       1     580        580   1 1006 15:22:34 /usr/bin/zsh
     2328     580    2328       2328  -1 1006 15:22:52 /usr/bin/zsh
     2188    1696    2188       1904   0 1006 15:28:35 /usr/bin/ps

for "zpty sh /bin/sh". Zsh does not appear to execute command at all, but it
does not exit either.

This happens with the version of cygwin that did work before, so it is very
much related to the changes in zsh. The messages above comes from attachtty,
but I am not sure when it is executed and what happens after this, i.e. why
zsh apparently "hangs".

-andrej

Re: PATCH: Run builtins, functions, etc. under zpty

[Bart Schaefer]

On Nov 8,  3:38pm, Andrej Borsenkow wrote:
} Subject: RE: PATCH: Run builtins, functions, etc. under zpty
} 
} Attempt to run any (external) command with zpty under Cygwin now results in:
} 
} zsh: can't set tty pgrp: not owner

Hrm.  Does the clone module fail under cygwin as well?  (I'm not sure
how you would test it, frankly, but if you can think of a way ...)

} (from child process) and hanging? zsh.

Do you mean that the "can't set tty pgrp" message appears on the pty,
that is, that you read it back with "zpty -r"?  (If it shows up on the
parent zsh's tty, it can't be from the child, because stdin/out/err
are dup2'd before any such message could be generated.)

} This happens with the version of cygwin that did work before, so it is very
} much related to the changes in zsh. The messages above comes from attachtty,
} but I am not sure when it is executed

The only way attachtty() gets called (with 13123 applied) is from init_io(),
and the message above can only happen if tcsetpgrp() or ioctl(TIOCSPGRP)
both failed.  This means that the old code at abount line 310 of (the pre-
13123) zpty.c must also have been failing, but was never testing its return
value and so was silent.

The same set of terminal-initialization system calls that was happening
before 13123 should still be happening, just in slightly different order
because of using init_io() -- e.g. some calls that were previously made
directly on the `slave' descriptor are now made on the dup'd copies of
that descriptor.  But that *shouldn't* make any difference -- unless
ttyname(0) and ttyname(1) are both failing.

} and what happens after this, i.e. why zsh apparently "hangs".

Just to be sure I understand -- which zsh appears to be hung?  The parent
from which you issued the zpty command, or the child that's supposed to
be on the new pty?  I think you mean the child.

} That is what I have in process list:
} 
} $ ps -l
}       PID    PPID    PGID     WINPID TTY  UID    STIME COMMAND
}      1696       1    1696       1696   0 1006 15:22:33 /usr/bin/bash
}       580       1     580        580   1 1006 15:22:34 /usr/bin/zsh
}      2328     580    2328       2328  -1 1006 15:22:52 /usr/bin/zsh
}      2188    1696    2188       1904   0 1006 15:28:35 /usr/bin/ps
} 
} for "zpty sh /bin/sh". Zsh does not appear to execute command at all, but it
} does not exit either.

The only thing that happens after attachtty() is that the command is
executed with execode().  If attachtty() has failed, MONITOR will have
been turned off (as well as INTERACTIVE) before execode() runs.

To see whether the hang is execode()-related, try replacing that with a
call to
	execve(findcmd(args[0], 1), args, environ);

That should be at line 361 of zpty.c, as it stands in cvs.

-- 
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: Run builtins, functions, etc. under zpty

[Andrej Borsenkow]

>
> On Nov 8,  3:38pm, Andrej Borsenkow wrote:
> } Subject: RE: PATCH: Run builtins, functions, etc. under zpty
> }
> } Attempt to run any (external) command with zpty under Cygwin now
> results in:
> }
> } zsh: can't set tty pgrp: not owner
>
> Hrm.  Does the clone module fail under cygwin as well?  (I'm not sure
> how you would test it, frankly, but if you can think of a way ...)
>

I'm not sure how to do it (in clean way). Some explanation: the very first
cygwin program checks if it has valid console window. If not, it spawns a
thread that controls console IO and creates a pipe. The other end of this pipe
is then our terminal (it adds tty emulation, 'course). Every cygwin program
started in this conosle inherits this "tty". So, you do not have
"free-standing" ttys under cygwin and cloning to some busy (allocated) tty
probably fails as well.

> } (from child process) and hanging? zsh.
>
> Do you mean that the "can't set tty pgrp" message appears on the pty,
> that is, that you read it back with "zpty -r"?  (If it shows up on the
> parent zsh's tty, it can't be from the child, because stdin/out/err
> are dup2'd before any such message could be generated.)
>

Yes, I can read it witj zpty -r (this itself was not as easy :-)

> } This happens with the version of cygwin that did work before, so
> it is very
> } much related to the changes in zsh. The messages above comes from
> attachtty,
> } but I am not sure when it is executed
>
> The only way attachtty() gets called (with 13123 applied) is from init_io(),
> and the message above can only happen if tcsetpgrp() or ioctl(TIOCSPGRP)
> both failed.  This means that the old code at abount line 310 of (the pre-
> 13123) zpty.c must also have been failing, but was never testing its return
> value and so was silent.
>

Before I've seen external program attached to new tty. So, something must have
changed.

>
> } and what happens after this, i.e. why zsh apparently "hangs".
>
> Just to be sure I understand -- which zsh appears to be hung?  The parent
> from which you issued the zpty command, or the child that's supposed to
> be on the new pty?  I think you mean the child.
>

I mean the child. After this message  I do not get any output at all. It
simply does nothing.

> } That is what I have in process list:
> }
> } $ ps -l
> }       PID    PPID    PGID     WINPID TTY  UID    STIME COMMAND
> }      1696       1    1696       1696   0 1006 15:22:33 /usr/bin/bash
> }       580       1     580        580   1 1006 15:22:34 /usr/bin/zsh
> }      2328     580    2328       2328  -1 1006 15:22:52 /usr/bin/zsh
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
child
> }      2188    1696    2188       1904   0 1006 15:28:35 /usr/bin/ps
> }
> } for "zpty sh /bin/sh". Zsh does not appear to execute command at
> all, but it
> } does not exit either.
>
> The only thing that happens after attachtty() is that the command is
> executed with execode().  If attachtty() has failed, MONITOR will have
> been turned off (as well as INTERACTIVE) before execode() runs.
>
> To see whether the hang is execode()-related, try replacing that with a
> call to
> 	execve(findcmd(args[0], 1), args, environ);
>
> That should be at line 361 of zpty.c, as it stands in cvs.
>

I'll give it a try. I'm currently busy trying to solve cygwin select problem
:-) It did not work for master pty in the way zsh is using it. Zsh seems to be
good stress test for cygwin. If we had somebody with intimate knowledge of
both at the same time ... :-)

-andrej