Fix for zpty under Cygwin

Fix for zpty under Cygwin

[áÎÄÒÅÊ âÏÒÚÅÎËÏ×]

Our office connection was down today and I had some spare time to get a look 
at this problem. It turned out to be quite simple (as usual). The problem is 
this line in Src/Modules/zpty.c:

#if defined(__SVR4) || defined(sinix) || defined(__CYGWIN__)                   
.... 
static int
get_pty(int master, int *retfd)
{
.....
    if ((sfd = open(name, O_RDWR|O_NOCTTY)) < 0) {                             
 
It was O_NOCTTY. Looks like O_NOCTTY is "sticky" under Cygwin and prevents 
this tty from becoming ctty when dup()'ing it later. I do not remeber when it 
was put in, so there are two possibilities:

- we added this and that broke things
- at some point O_NOCTTY became sticky and that broke zpty
(unfortunately, my current connection is so slow I really cannot connect to 
Sourceforge CVS to check it).

So the fix under Cygwin is quite simple - just remove O_NOCTTY. Actually, I 
wonder what it does here at all - it is exactly the descriptor that *will* be 
ctty later. In case it may break on some system we can do it under ifdef 
__CYGWIN__ and then check again after 4.0.1 is out. I'd really like 4.0.1 to 
be as most feature complete on Cygwin as possible.

Related problem is testing. Completion tests consistently failed; the reason 
was this line in comptest:

zpty -b zsh "$comptest_zsh -f +Z" 

i.e. non-blocking mode. As a result the next read returned empty string and 
failed. I could not reproduce it in interactive shell, so I suspect some 
obscure scheduling issues here. Anyway, non-blocking mode looks wrong here to 
me. We cannot expect that the whole output will always be available when we 
try to read. In this case much more better would be read-with-timeout ... O.K. 
after 4.0.1 may be. With -b removed all completion tests passed under Cygwin.

Peter, could you test if above two changes work for you? In this case could 
you commit it (unfortunately, my CVS is in office and it looks like connection 
is gone for several days at least).                                           

I'll drop a line to cygwin list when I'm back again. I still consider 
stickyness of O_NOCTTY a bug ... any comments? 

-andrej

P.S. Could you please Cc me replies to this thread?

Re: Fix for zpty under Cygwin

[Bart Schaefer]

On May 7, 10:15pm, áÎÄÒÅÊ âÏÒÚÅÎËÏ× wrote:
> Subject: Fix for zpty under Cygwin
>  
> It was O_NOCTTY. Looks like O_NOCTTY is "sticky" under Cygwin and
> prevents this tty from becoming ctty when dup()'ing it later. I do not
> remeber when it was put in, so there are two possibilities:
> 
> - we added this and that broke things

That's probably it; I copied that part from clone.c along with several
other changes (rev 1.15).

> So the fix under Cygwin is quite simple - just remove O_NOCTTY.
> Actually, I wonder what it does here at all - it is exactly the
> descriptor that *will* be ctty later.

It will be ctty in the forked-off child that runs on the pty, but we have
to open the descriptor originally in the parent before forking.  O_NOCTTY
is to prevent the pty from becoming the parent's controlling terminal.

> Related problem is testing. Completion tests consistently failed; the
> reason was this line in comptest:
>
> zpty -b zsh "$comptest_zsh -f +Z"

The -b was added in 13035 when non-blocking mode was the default and -b
selected blocking mode, then never removed when the default switched again
in 13120.  This was my error; the -b should not be there.

Re[2]: Fix for zpty under Cygwin

[áÎÄÒÅÊ âÏÒÚÅÎËÏ×]

-----Original Message-----
From: "Bart Schaefer" <schaefer@candle.brasslantern.com>
To: "Андрей Борзенков" <arvidjaar@mail.ru>
Date: Mon, 7 May 2001 12:47:39 -0700
Subject: Re: Fix for zpty under Cygwin

> 
> > So the fix under Cygwin is quite simple - just
remove O NOCTTY.
> > Actually, I wonder what it does here at all - it is
exactly the
> > descriptor that *will* be ctty later.
> 
> It will be ctty in the forked-off child that runs on
the pty, but we have
> to open the descriptor originally in the parent
before forking.  O NOCTTY
> is to prevent the pty from becoming the parent's
controlling terminal.
> 

That's wrong. get_pty() has two threads - one is
executed in parent (master) and one in child after fork
(slave). So, sfd descriptor is opened just once in
child and fix looks applicable. I've tried it on my
Unix as well without any obvious ill effects.

Anyway, I suggested to put it under ifdef __CYGWIN__
for the time being to avoid last-time breaking on other
systems. It does fix things under Cygwin (it is nice to
convince Cygwin folks to change O_NOCTTY semantics of
course).

-andrej

Re: Fix for zpty under Cygwin

[Peter Stephenson]

Someone claiming to be Andrej wrote:
> So the fix under Cygwin is quite simple - just remove O_NOCTTY. Actually, I 
> wonder what it does here at all - it is exactly the descriptor that *will* be
>
> Related problem is testing. Completion tests consistently failed; the reason 
> was this line in comptest:
> 
> zpty -b zsh "$comptest_zsh -f +Z" 
> 
> i.e. non-blocking mode. As a result the next read returned empty string and 
> failed. I could not reproduce it in interactive shell, so I suspect some 
> obscure scheduling issues here. Anyway, non-blocking mode looks wrong here to
>  
> me. We cannot expect that the whole output will always be available when we 
> try to read. In this case much more better would be read-with-timeout ... O.K
> . 
> after 4.0.1 may be. With -b removed all completion tests passed under Cygwin.

I had problems with the tilde test (below, if it means anything --- part of
the problem is my /etc/zshenv which isn't as clean as usual on my PC), but
it seems clear that zpty is functioning OK, so I'm committing the two
changes (with the #ifdef __CYGWIN__ for now).

By the way, we probably need to be clearer about tests that might fail on a
network mount.

*** /tmp/zsh.ztst.out.334       Tue May  8 11:17:03 2001
--- /tmp/zsh.ztst.tout.334      Tue May  8 11:17:05 2001
***************
*** 1,7 ****
! line: {: ~user}{}
! line: {: ~user}{}
  NO:{user1}
  NO:{user2}
  line: {: ~user1}{}
  line: {: ~user2}{}
! line: {: ~user1}{}
--- 1,10 ----
! line: {: ~}{}
  NO:{user1}
  NO:{user2}
+ DESCRIPTION:{named directory}
+ NO:{c}
+ NO:{dev}
  line: {: ~user1}{}
  line: {: ~user2}{}
! line: {: ~c/}{}
! line: {: ~dev/}{}
Test ../../Test/Y01completion.ztst failed: output differs from expected as shown
 above for:
  comptesteval '_users () { compadd user1 user2 }'
  comptest $': ~\t\t\t\t\t'
Was testing: tilde

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

RE: Fix for zpty under Cygwin

[Andrej Borsenkow]

> Someone claiming to be Andrej wrote:

No, it was really me :-)