sh compatibility issue

sh compatibility issue

[Vincent Stemen]

Hi.

I have encountered a minor compatibility issue when using zsh in place
of /bin/sh on FreeBSD.

When attempting to compile the base system, it gets the error

    /usr/src/gnu/usr.bin/binutils/ld/genscripts.sh:43: parse error near `)'
    *** Error code 1

The error comes from this piece of code in genscripts.sh

    # XXX: arm hack : until those file are merged back into the FSF repo,
    # just
    # use the version in this directory.
    if !(test -f ${CUSTOMIZER_SCRIPT}"";) then
    CUSTOMIZER_SCRIPT="${srcdir}/emulparams/${EMULATION_NAME}.sh"
    fi

What is happening is that zsh does not like the '!' being next to the
opening '(' without a space in the if condition.  In my opinion, this is
kind of an unorthodox syntax.  I'm not even sure if it is traditionally
legal sh or ksh syntax.  Nevertheless, it works with BSD sh and bash.
A simple test is

    if !(echo hello); then echo "XXXX"; fi
    if ! (echo hello); then echo "XXXX"; fi

Zsh works fine in the second case, with a space after the '!'.

I also tested without the space under ksh on NetBSD and got a strange
result.  It ran the mail utility.  Weird.

My question is, is it possible and reasonable to modify zsh to work
without the space so that screwy GNU software such as this will compile?

Regards,
Vince

Re: sh compatibility issue

[Peter Stephenson]

On Thu, 17 Feb 2011 21:55:07 -0600
Vincent Stemen <vince.lists@hightek.org> wrote:
> I have encountered a minor compatibility issue when using zsh in place
> of /bin/sh on FreeBSD.
> The error comes from this piece of code in genscripts.sh
> 
>     # XXX: arm hack : until those file are merged back into the FSF
> repo, # just
>     # use the version in this directory.
>     if !(test -f ${CUSTOMIZER_SCRIPT}"";) then
>     CUSTOMIZER_SCRIPT="${srcdir}/emulparams/${EMULATION_NAME}.sh"
>     fi
> 
> What is happening is that zsh does not like the '!' being next to the
> opening '(' without a space in the if condition.  In my opinion, this
> is kind of an unorthodox syntax.  I'm not even sure if it is
> traditionally legal sh or ksh syntax.  Nevertheless, it works with
> BSD sh and bash. A simple test is
> 
>     if !(echo hello); then echo "XXXX"; fi
>     if ! (echo hello); then echo "XXXX"; fi
> 
> Zsh works fine in the second case, with a space after the '!'.

I don't think it is standard for this to work.  POSIX defines "!" as a
"reserved word", and if it's not followed by whitespace it's not a
word.

However, it looks like it's possible to get this to work specially in
this case without disrupting anything else; because the parentheses are
always special in one way another to zsh, the only other meaning (with
the exception below) would be a string "!" followed by something like a
globbing expression.  I can't think of a case where that other meaning
is useful (and it's sure as heck thoroughly confusing if you use it,
given all there are at least three meanings for "!" when not used
as a plain string).

The slightly odd handling is down to the fact ! isn't treated as
a token at this point since it's usually handled as a reserved word.

> I also tested without the space under ksh on NetBSD and got a strange
> result.  It ran the mail utility.  Weird.

That's because !(...) is globbing syntax in ksh.  It means "find all
files not matching the pattern in the parentheses".  So that can do
pretty much anything in command position.  So we need to avoid doing
this if the KSH_GLOB option is set (although the result isn't
particularly useful in practice).  Luckily KSH_GLOB isn't set for
emulating sh.

Index: Src/lex.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/lex.c,v
retrieving revision 1.63
diff -p -u -r1.63 lex.c
--- Src/lex.c	19 Dec 2010 17:42:10 -0000	1.63
+++ Src/lex.c	18 Feb 2011 10:18:13 -0000
@@ -487,6 +487,7 @@ ctxtlex(void)
 #define LX1_COMMENT 1
 #define LX1_NEWLIN 2
 #define LX1_SEMI 3
+#define LX1_BANG 4
 #define LX1_AMPER 5
 #define LX1_BAR 6
 #define LX1_INPAR 7
@@ -835,6 +836,30 @@ gettok(void)
 	hungetc(d);
 	lexstop = 0;
 	return SEMI;
+    case LX1_BANG:
+	/*
+	 * In command position, treat "!(" or "!{"
+	 * as "! (" or "! {".  "!" is a reserved word,
+	 * so not handled as a token at this level.
+	 * This is for compatibility; a "real"
+	 * reserved word wouldn't behave like this.
+	 *
+	 * With ksh globbing, !(...) is a special syntax.
+	 * Although it doesn't do anything very useful
+	 * in command position, we shouldn't disrupt it.
+	 */
+	if (incmdpos && !isset(KSHGLOB) && reswdtab->getnode(reswdtab, "!")) {
+	    d = hgetc();
+	    hungetc(d);
+	    lexstop = 0;
+	    if (d == '(' || d == '{') {
+		bptr = tokstr = (char *)hcalloc(2);
+		*bptr++ = '!';
+		*bptr++ = '\0';
+		return STRING;
+	    }
+	}
+	break;
     case LX1_AMPER:
 	d = hgetc();
 	if (d == '&')
Index: Test/A01grammar.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v
retrieving revision 1.27
diff -p -u -r1.27 A01grammar.ztst
--- Test/A01grammar.ztst	18 Mar 2010 16:30:58 -0000	1.27
+++ Test/A01grammar.ztst	18 Feb 2011 10:18:13 -0000
@@ -577,3 +577,11 @@
 0:$0 is traditionally if bizarrely set to the first argument with -c
 >myargzero
 >myargone
+
+  if ! (echo success1); then echo failure1; fi
+  if !(echo success2); then echo failure2; fi
+  if !{echo success3}; then echo failure3; fi
+0:Special handling of ! in command position.
+>success1
+>success2
+>success3

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: sh compatibility issue

[Vincent Stemen]

Thank you very much for the quick response Peter.

That patch seems to have worked fine.  I was able to successfully
compile the entire FreeBSD base system this time, using zsh as /bin/sh.
I patched against and tested with the master branch from the git
repository.

Regards,
Vince

Re: sh compatibility issue

[Jilles Tjoelker]

> !(...) without space

I think the POSIX spec requires this to work, but that may not be
intentional. '(' is an operator and therefore it does not need a space
between it and other characters (except to disambiguate between two
adjacent operators and one two-character operator). It may not be
intentional because it conflicts with ksh extended pattern matching.

More generally, this rule also means that things like
  while(true)do(pwd)done
do not need any spaces. Although this example is contrived, I can
imagine there are real scripts that zsh fails to parse because of this;
the other shells I tried execute it correctly.

The situation for '!{' is different. '{' is a reserved word and
therefore it is not recognized. Instead, this describes a utility (or
alias) that probably does not exist. I do not recommend making special
allowances for it.

On a related note, here is another quite insidious sh compatibility
issue:
  sh -c 'exec </nonexistent/a; echo wrong'
This should not print "wrong" because exec is a special builtin and
redirection errors on special builtins are fatal. Most shells get this
right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
  set -o posixbuiltins
does not help.

-- 
Jilles Tjoelker

Re: sh compatibility issue

[Vincent Stemen]

On Sun, Feb 20, 2011 at 12:05:40AM +0100, Jilles Tjoelker wrote:
> > !(...) without space
> 
> I think the POSIX spec requires this to work, but that may not be
> intentional. '(' is an operator and therefore it does not need a space
> between it and other characters (except to disambiguate between two
> adjacent operators and one two-character operator). It may not be
> intentional because it conflicts with ksh extended pattern matching.
> 
> More generally, this rule also means that things like
>   while(true)do(pwd)done
> do not need any spaces. Although this example is contrived, I can
> imagine there are real scripts that zsh fails to parse because of this;
> the other shells I tried execute it correctly.
> 
> The situation for '!{' is different. '{' is a reserved word and
> therefore it is not recognized. Instead, this describes a utility (or
> alias) that probably does not exist. I do not recommend making special
> allowances for it.
> 
> On a related note, here is another quite insidious sh compatibility
> issue:
>   sh -c 'exec </nonexistent/a; echo wrong'
> This should not print "wrong" because exec is a special builtin and
> redirection errors on special builtins are fatal. Most shells get this
> right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
>   set -o posixbuiltins
> does not help.
> 
> -- 
> Jilles Tjoelker

Interesting.

I ran these two tests tests with sh, ksh, bash, and zsh.

while(true)do(pwd)done
    This worked in sh, ksh, and bash.  It failed only in zsh.

sh -c 'exec </nonexistent/a; echo wrong'
    This did not print "wrong" in sh and ksh.

    Zsh prints wrong in both sh and zsh mode and bash only prints "wrong"
    without the --posix switch.


Vince

Re: sh compatibility issue

[Peter Stephenson]

On Sun, 20 Feb 2011 00:05:40 +0100
Jilles Tjoelker <jilles@stack.nl> wrote:
> > !(...) without space
> 
> I think the POSIX spec requires this to work, but that may not be
> intentional. '(' is an operator and therefore it does not need a space
> between it and other characters (except to disambiguate between two
> adjacent operators and one two-character operator). It may not be
> intentional because it conflicts with ksh extended pattern matching.
> 
> More generally, this rule also means that things like
>   while(true)do(pwd)done
> do not need any spaces. Although this example is contrived, I can
> imagine there are real scripts that zsh fails to parse because of this;
> the other shells I tried execute it correctly.

I think we can manage that when SH_GLOB is on (it'll screw up zsh
patterns royally otherwise), but it's quite a big change and we don't
have many tests for sh emulation, so someone ought to see what this
patch does before it goes much further.  All I've tested is what's in
the test patch.

This replaces the previous patch, but note that means you still need the
spaces in native zsh mode --- can't see a real problem there, in fact
it's probably safer as well as more consistent.

I had to exclude it with KSH_GLOB on because of the way we handle
KSH_GLOB expressions.  That's not the correct thing to do --- we should
handle KSH_GLOB expressions in the lexical analysis --- but KSH_GLOB
isn't on for normal sh emulation so I don't think it's a major problem.

> The situation for '!{' is different. '{' is a reserved word and
> therefore it is not recognized. Instead, this describes a utility (or
> alias) that probably does not exist. I do not recommend making special
> allowances for it.

You're right that '{' is different from '('.  I haven't done anything
special with it here.

> On a related note, here is another quite insidious sh compatibility
> issue:
>   sh -c 'exec </nonexistent/a; echo wrong'
> This should not print "wrong" because exec is a special builtin and
> redirection errors on special builtins are fatal. Most shells get this
> right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
>   set -o posixbuiltins
> does not help.

I think it does need to be fatal, but not because it's a special builtin
--- it looks like that's only a "may" rather than a "shall" --- but
because there is a rule for exec:

  If a redirection error occurs (see Consequences of Shell Errors ), the
  shell shall exit with a value in the range 1-125

which is certainly unambiguous on the subject.  This will need looking at.

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.99
diff -p -u -r1.99 options.yo
--- Doc/Zsh/options.yo	16 Dec 2010 13:55:35 -0000	1.99
+++ Doc/Zsh/options.yo	20 Feb 2011 19:04:30 -0000
@@ -683,8 +683,12 @@ item(tt(SH_GLOB) <K> <S>)(
 Disables the special meaning of `tt(LPAR())', `tt(|)', `tt(RPAR())'
 and 'tt(<)' for globbing the result of parameter and command substitutions,
 and in some other places where
-the shell accepts patterns.  This option is set by default if zsh is
-invoked as tt(sh) or tt(ksh).
+the shell accepts patterns.  If tt(SH_GLOB) is set but tt(KSH_GLOB) is
+not, the shell allows the interpretation of
+subshell expressions enclosed in parentheses in some cases where there
+is no space before the opening parenthesis, e.g. tt(!LPAR()true+RPAR())
+is interpreted as if there were a space after the tt(!).  This option is
+set by default if zsh is invoked as tt(sh) or tt(ksh).
 )
 pindex(UNSET)
 pindex(NO_UNSET)
Index: Src/lex.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/lex.c,v
retrieving revision 1.63
diff -p -u -r1.63 lex.c
--- Src/lex.c	19 Dec 2010 17:42:10 -0000	1.63
+++ Src/lex.c	20 Feb 2011 19:04:30 -0000
@@ -877,7 +877,7 @@ gettok(void)
 		dbparens = 1;
 		return DINPAR;
 	    }
-	    if (incmdpos) {
+	    if (incmdpos || (isset(SHGLOB) && !isset(KSHGLOB))) {
 		len = 0;
 		bptr = tokstr = (char *) hcalloc(bsiz = 32);
 		switch (cmd_or_math(CS_MATH)) {
@@ -1141,6 +1141,8 @@ gettokstr(int c, int sub)
 		    break;
 		if (incasepat && !len)
 		    return INPAR;
+		if (!isset(KSHGLOB) && len)
+		    goto brk;
 	    }
 	    if (!in_brace_param) {
 		if (!sub) {
Index: Test/A01grammar.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v
retrieving revision 1.27
diff -p -u -r1.27 A01grammar.ztst
--- Test/A01grammar.ztst	18 Mar 2010 16:30:58 -0000	1.27
+++ Test/A01grammar.ztst	20 Feb 2011 19:04:30 -0000
@@ -577,3 +577,15 @@
 0:$0 is traditionally if bizarrely set to the first argument with -c
 >myargzero
 >myargone
+
+  (setopt shglob
+  eval '
+  if ! (echo success1); then echo failure1; fi
+  if !(echo success2); then echo failure2; fi
+  print -l one two | while(read foo)do(print read it)done
+  ')
+0:Parentheses in shglob
+>success1
+>success2
+>read it
+>read it


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: sh compatibility issue

[Peter Stephenson]

On Sun, 20 Feb 2011 19:11:59 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Sun, 20 Feb 2011 00:05:40 +0100
> Jilles Tjoelker <jilles@stack.nl> wrote:
> > On a related note, here is another quite insidious sh compatibility
> > issue:
> >   sh -c 'exec </nonexistent/a; echo wrong'
> > This should not print "wrong" because exec is a special builtin and
> > redirection errors on special builtins are fatal. Most shells get this
> > right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
> >   set -o posixbuiltins
> > does not help.
> 
> I think it does need to be fatal, but not because it's a special builtin
> --- it looks like that's only a "may" rather than a "shall" --- but
> because there is a rule for exec:
> 
>   If a redirection error occurs (see Consequences of Shell Errors ), the
>   shell shall exit with a value in the range 1-125
> 
> which is certainly unambiguous on the subject.  This will need looking at.

I think this is unproblematic.  The relevant hooks are already mostly
there.

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.99
diff -p -u -r1.99 options.yo
--- Doc/Zsh/options.yo	16 Dec 2010 13:55:35 -0000	1.99
+++ Doc/Zsh/options.yo	20 Feb 2011 20:11:45 -0000
@@ -1866,6 +1870,10 @@ tt(source),
 tt(times),
 tt(trap) and
 tt(unset).
+
+In addition, a failed redirection after tt(exec) causes a non-interactive
+shell to exit and an interactive shell to return to its top-level
+processing.
 )
 pindex(POSIX_IDENTIFIERS)
 pindex(NO_POSIX_IDENTIFIERS)
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.15
diff -p -u -r1.15 A04redirect.ztst
--- Test/A04redirect.ztst	14 Sep 2010 14:46:26 -0000	1.15
+++ Test/A04redirect.ztst	20 Feb 2011 20:11:45 -0000
@@ -366,3 +366,15 @@
 >more output:
 >This file contains data.
 >This file contains data.
+
+  $ZTST_testdir/../Src/zsh -fc 'exec >/nonexistent/nonexistent
+  echo output'
+0:failed exec redir, no POSIX_BUILTINS
+>output
+?zsh:1: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  exec >/nonexistent/nonexistent
+  echo output'
+1:failed exec redir, POSIX_BUILTINS
+?zsh:2: no such file or directory: /nonexistent/nonexistent
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.188
diff -p -u -r1.188 exec.c
--- Src/exec.c	17 Feb 2011 19:52:42 -0000	1.188
+++ Src/exec.c	20 Feb 2011 20:11:46 -0000
@@ -181,7 +181,15 @@ struct execstack *exstack;
 /**/
 mod_export Funcstack funcstack;
 
-#define execerr() if (!forked) { lastval = 1; goto done; } else _exit(1)
+#define execerr()				\
+    do {					\
+	if (!forked) {				\
+	    redir_err = lastval = 1;		\
+	    goto done;				\
+	} else {				\
+	    _exit(1);				\
+	}					\
+    } while (0)
 
 static int doneps4;
 static char *STTYval;
@@ -2312,7 +2320,7 @@ execcmd(Estate state, int input, int out
     struct multio *mfds[10];
     char *text;
     int save[10];
-    int fil, dfil, is_cursh, type, do_exec = 0, i, htok = 0;
+    int fil, dfil, is_cursh, type, do_exec = 0, redir_err = 0, i, htok = 0;
     int nullexec = 0, assign = 0, forked = 0;
     int is_shfunc = 0, is_builtin = 0, is_exec = 0, use_defpath = 0;
     /* Various flags to the command. */
@@ -3289,6 +3297,13 @@ execcmd(Estate state, int input, int out
     fixfds(save);
 
  done:
+    if (redir_err && isset(POSIXBUILTINS)) {
+	if (!isset(INTERACTIVE)) {
+	    /* We've already _exit'ed if forked */
+	    exit(1);
+	}
+	errflag = 1;
+    }
     if (newxtrerr) {
 	fil = fileno(newxtrerr);
 	fclose(newxtrerr);


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: sh compatibility issue

[Vincent Stemen]

OK.  I applied both patches.  So far as I can tell it all works on
freebsd.

  $ setopt shglob
  
  $ if !(echo hello); then echo "XXXX"; fi
  hello
  
  $ while(true)do(pwd)done
  /home/vince/src
  /home/vince/src
  /home/vince/src
  ...
  ^C
  
  $ /usr/bin/zsh -c 'exec </nonexistent/a; echo wrong'
  zsh:1: no such file or directory: /nonexistent/a
  wrong
  
  $ /usr/bin/zsh -c 'emulate sh; exec </nonexistent/a; echo wrong'
  zsh:1: no such file or directory: /nonexistent/a
  
Although, I am a little unclear about this last one.  Since it is
a redirection error of a built in function, shouldn't it exit, even if
it is not emulating another shell?

Vince

Re: sh compatibility issue

[Peter Stephenson]

On Sun, 20 Feb 2011 16:39:29 -0500
Chet Ramey <chet.ramey@case.edu> wrote:
> On 2/20/11 2:11 PM, Peter Stephenson wrote:
> >> On a related note, here is another quite insidious sh compatibility
> >> issue:
> >>   sh -c 'exec </nonexistent/a; echo wrong'
> >> This should not print "wrong" because exec is a special builtin and
> >> redirection errors on special builtins are fatal. Most shells get this
> >> right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
> >>   set -o posixbuiltins
> >> does not help.
> > 
> > I think it does need to be fatal, but not because it's a special builtin
> > --- it looks like that's only a "may" rather than a "shall" --- but
> > because there is a rule for exec:
> 
> No, that's actually a `shall'.  Section 2.8.1, special builtin, redirection
> error.

Thanks, didn't follow the link... it looks like the "may" here means
"may be required to".  The table shows essentially all errors for
special builtins are fatal.

I didn't actually test for a special builtin in the previous patch,
either.  Here's a revised patch with more careful tests.

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.101
diff -p -u -r1.101 options.yo
--- Doc/Zsh/options.yo	21 Feb 2011 11:32:47 -0000	1.101
+++ Doc/Zsh/options.yo	22 Feb 2011 20:00:52 -0000
@@ -1871,9 +1871,9 @@ tt(times),
 tt(trap) and
 tt(unset).
 
-In addition, a failed redirection after tt(exec) causes a non-interactive
-shell to exit and an interactive shell to return to its top-level
-processing.
+In addition, various error conditions associated with the above builtins
+or tt(exec) cause a non-interactive shell to exit and an interactive
+shell to return to its top-level processing.
 )
 pindex(POSIX_IDENTIFIERS)
 pindex(NO_POSIX_IDENTIFIERS)
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.246
diff -p -u -r1.246 builtin.c
--- Src/builtin.c	7 Jan 2011 10:05:35 -0000	1.246
+++ Src/builtin.c	22 Feb 2011 20:00:53 -0000
@@ -4821,8 +4821,14 @@ bin_dot(char *name, char **argv, UNUSED(
 	freearray(pparams);
 	pparams = old;
     }
-    if (ret == SOURCE_NOT_FOUND)
-	zwarnnam(name, "%e: %s", errno, enam);
+    if (ret == SOURCE_NOT_FOUND) {
+	if (isset(POSIXBUILTINS)) {
+	    /* hard error in POSIX (we'll exit later) */
+	    zerrnam(name, "%e: %s", errno, enam);
+	} else {
+	    zwarnnam(name, "%e: %s", errno, enam);
+	}
+    }
     zsfree(arg0);
     if (old0) {
 	zsfree(argzero);
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.189
diff -p -u -r1.189 exec.c
--- Src/exec.c	21 Feb 2011 11:28:49 -0000	1.189
+++ Src/exec.c	22 Feb 2011 20:00:53 -0000
@@ -2416,6 +2416,8 @@ execcmd(Estate state, int input, int out
 		checked = !(cflags & BINF_BUILTIN);
 		break;
 	    }
+	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
+	    cflags |= hn->flags;
 	    if (!(hn->flags & BINF_PREFIX)) {
 		is_builtin = 1;
 
@@ -2425,8 +2427,6 @@ execcmd(Estate state, int input, int out
 		assign = (hn->flags & BINF_MAGICEQUALS);
 		break;
 	    }
-	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
-	    cflags |= hn->flags;
 	    checked = 0;
 	    if ((cflags & BINF_COMMAND) && nextnode(firstnode(args))) {
 		/* check for options to command builtin */
@@ -3297,12 +3297,20 @@ execcmd(Estate state, int input, int out
     fixfds(save);
 
  done:
-    if (redir_err && isset(POSIXBUILTINS)) {
-	if (!isset(INTERACTIVE)) {
-	    /* We've already _exit'ed if forked */
-	    exit(1);
+    if (isset(POSIXBUILTINS) &&
+	(cflags & (BINF_PSPECIAL|BINF_EXEC))) {
+	/*
+	 * For POSIX-compatibile behaviour with special
+	 * builtins (including exec which we don't usually
+	 * classify as a builtin, we treat all errors as fatal.
+	 */
+	if (redir_err || errflag) {
+	    if (!isset(INTERACTIVE)) {
+		/* We've already _exit'ed if forked */
+		exit(1);
+	    }
+	    errflag = 1;
 	}
-	errflag = 1;
     }
     if (newxtrerr) {
 	fil = fileno(newxtrerr);
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.16
diff -p -u -r1.16 A04redirect.ztst
--- Test/A04redirect.ztst	21 Feb 2011 11:28:49 -0000	1.16
+++ Test/A04redirect.ztst	22 Feb 2011 20:00:53 -0000
@@ -378,3 +378,52 @@
   echo output'
 1:failed exec redir, POSIX_BUILTINS
 ?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  set >/nonexistent/nonexistent
+  echo output'
+1:failed special builtin redir, POSIX_BUILTINS
+?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  echo >/nonexistent/nonexistent
+  echo output'
+0:failed unspecial builtin redir, POSIX_BUILTINS
+>output
+?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  . /nonexistent/nonexistent
+  echo output'
+1:failed dot, POSIX_BUILTINS
+?zsh:.:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -c '
+  . /nonexistent/nonexistent
+  echo output'
+0:failed dot, NO_POSIX_BUILTINS
+>output
+?zsh:.:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f <<<'
+  readonly foo
+  foo=bar set output
+  echo output'
+0:failed assignment on posix special, NO_POSIX_BUILTINS
+>output
+?zsh: read-only variable: foo
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS <<<'
+  readonly foo
+  foo=bar set output
+  echo output'
+1:failed assignment on posix special, POSIX_BUILTINS
+?zsh: read-only variable: foo
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS <<<'
+  readonly foo
+  foo=bar echo output
+  echo output'
+0:failed assignment on non-posix-special, POSIX_BUILTINS
+>output
+?zsh: read-only variable: foo

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: sh compatibility issue

[Vincent Stemen]

On Tue, Feb 22, 2011 at 08:02:59PM +0000, Peter Stephenson wrote:
> On Sun, 20 Feb 2011 16:39:29 -0500
> Chet Ramey <chet.ramey@case.edu> wrote:
> > On 2/20/11 2:11 PM, Peter Stephenson wrote:
> > >> On a related note, here is another quite insidious sh compatibility
> > >> issue:
> > >>   sh -c 'exec </nonexistent/a; echo wrong'
> > >> This should not print "wrong" because exec is a special builtin and
> > >> redirection errors on special builtins are fatal. Most shells get this
> > >> right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
> > >>   set -o posixbuiltins
> > >> does not help.
> > > 
> > > I think it does need to be fatal, but not because it's a special builtin
> > > --- it looks like that's only a "may" rather than a "shall" --- but
> > > because there is a rule for exec:
> > 
> > No, that's actually a `shall'.  Section 2.8.1, special builtin, redirection
> > error.
> 
> Thanks, didn't follow the link... it looks like the "may" here means
> "may be required to".  The table shows essentially all errors for
> special builtins are fatal.
> 
> I didn't actually test for a special builtin in the previous patch,
> either.  Here's a revised patch with more careful tests.

I don't think it worked.  With this patch added, the bahaviour did not
change from the previous patch.  Fatal error in sh mode not not in zsh
mode.

    $ /usr/bin/zsh -c 'exec </nonexistent/a; echo wrong'
    zsh:1: no such file or directory: /nonexistent/a
    wrong
    
    $ /usr/bin/zsh -c 'emulate sh; exec </nonexistent/a; echo wrong'
    zsh:1: no such file or directory: /nonexistent/a

Re: sh compatibility issue

[Bart Schaefer]

On Tuesday, February 22, 2011, Vincent Stemen <vince.lists@hightek.org> wrote:
>
> I don't think it worked.  With this patch added, the bahaviour did not
> change from the previous patch.  Fatal error in sh mode not not in zsh
> mode.

That's intentional.  Zsh is not a POSIX shell unless you tell it to
be, so in zsh mode it behaves the way zsh always has.

As I understand it, the patch should change the behavior of other
special builtins (in addition to exec), but only when POSIXBUILTINS is
set.

Re: sh compatibility issue

[Vincent Stemen]

On Tue, Feb 22, 2011 at 05:51:18PM -0800, Bart Schaefer wrote:
> On Tuesday, February 22, 2011, Vincent Stemen <vince.lists@hightek.org> wrote:
> >
> > I don't think it worked. ?With this patch added, the bahaviour did not
> > change from the previous patch. ?Fatal error in sh mode not not in zsh
> > mode.
> 
> That's intentional.  Zsh is not a POSIX shell unless you tell it to
> be, so in zsh mode it behaves the way zsh always has.
> 
> As I understand it, the patch should change the behavior of other
> special builtins (in addition to exec), but only when POSIXBUILTINS is
> set.

Oh.  OK.  If that's the case, then it does appear to work as intended 
on BSD :-).

    $ /usr/bin/zsh -c 'setopt posixbuiltins; exec </nonexistent/a; echo wrong'
    zsh:1: no such file or directory: /nonexistent/a

Re: sh compatibility issue

[Peter Stephenson]

On Tue, 22 Feb 2011 17:51:18 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Tuesday, February 22, 2011, Vincent Stemen <vince.lists@hightek.org> wrote:
> >
> > I don't think it worked.  With this patch added, the bahaviour did not
> > change from the previous patch.  Fatal error in sh mode not not in zsh
> > mode.
> 
> That's intentional.  Zsh is not a POSIX shell unless you tell it to
> be, so in zsh mode it behaves the way zsh always has.
> 
> As I understand it, the patch should change the behavior of other
> special builtins (in addition to exec), but only when POSIXBUILTINS is
> set.

Bart's right, sorry, I wasn't clear enough.  Yes, this is the
long-standing policy.

If we'd been designing zsh from scratch now, there would be no point in
all these small areas where it's a bit different from POSIX by accident
rather than design.  As it is, given the shell's manifold differences in
native mode, there's no point squeezing extra POSIX-compliance out of
native mode at the expense of backward compatibility.

By the way, one small tweak which should be rarely visible: now I've
upped the ante, there may be cases where we're forked at the point we
need to exit.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.190
diff -p -u -r1.190 exec.c
--- Src/exec.c	22 Feb 2011 20:09:20 -0000	1.190
+++ Src/exec.c	23 Feb 2011 09:21:32 -0000
@@ -3306,8 +3306,10 @@ execcmd(Estate state, int input, int out
 	 */
 	if (redir_err || errflag) {
 	    if (!isset(INTERACTIVE)) {
-		/* We've already _exit'ed if forked */
-		exit(1);
+		if (forked)
+		    _exit(1);
+		else
+		    exit(1);
 	    }
 	    errflag = 1;
 	}
-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: sh compatibility issue

[Jilles Tjoelker]

I notice that the patches were committed. This indeed fixes some things
(FreeBSD sh testsuite errors/redirection-error.0 which checks that
redirection errors for special builtins are fatal) but breaks another
test which happened to pass before: errors/redirection-error3.0 which
checks that redirection errors are not fatal for special builtins
preceded by the command builtin.

Although 'command' is often implemented as some sort of precommand
modifier, the POSIX specification says it is a regular builtin,
therefore redirection errors are not fatal (and variable assignments do
not persist). The specification also says that 'command' causes operand
(syntax) errors to be non-fatal.

A useful example is in the Application Usage section of the 'command'
page:
  command exec > unwritable-file
This does not cause the shell to abort, so the exit status can be
checked by the script.

This works as specified in ksh93, bash --posix and FreeBSD 9 sh, for
example.

-- 
Jilles Tjoelker

Re: sh compatibility issue

[Peter Stephenson]

On Fri, 4 Mar 2011 14:36:34 +0100
Jilles Tjoelker <jilles@stack.nl> wrote:
> Although 'command' is often implemented as some sort of precommand
> modifier, the POSIX specification says it is a regular builtin,
> therefore redirection errors are not fatal (and variable assignments do
> not persist). The specification also says that 'command' causes operand
> (syntax) errors to be non-fatal.

This should fix at least some such problems.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.191
diff -p -u -r1.191 exec.c
--- Src/exec.c	23 Feb 2011 09:47:06 -0000	1.191
+++ Src/exec.c	6 Mar 2011 20:25:15 -0000
@@ -2324,7 +2324,7 @@ execcmd(Estate state, int input, int out
     int nullexec = 0, assign = 0, forked = 0;
     int is_shfunc = 0, is_builtin = 0, is_exec = 0, use_defpath = 0;
     /* Various flags to the command. */
-    int cflags = 0, checked = 0, oautocont = -1;
+    int cflags = 0, orig_cflags = 0, checked = 0, oautocont = -1;
     LinkList redir;
     wordcode code;
     Wordcode beg = state->pc, varspc;
@@ -2416,6 +2416,7 @@ execcmd(Estate state, int input, int out
 		checked = !(cflags & BINF_BUILTIN);
 		break;
 	    }
+	    orig_cflags |= cflags;
 	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
 	    cflags |= hn->flags;
 	    if (!(hn->flags & BINF_PREFIX)) {
@@ -3298,11 +3299,13 @@ execcmd(Estate state, int input, int out
 
  done:
     if (isset(POSIXBUILTINS) &&
-	(cflags & (BINF_PSPECIAL|BINF_EXEC))) {
+	(cflags & (BINF_PSPECIAL|BINF_EXEC)) &&
+	!(orig_cflags & BINF_COMMAND)) {
 	/*
 	 * For POSIX-compatible behaviour with special
 	 * builtins (including exec which we don't usually
 	 * classify as a builtin) we treat all errors as fatal.
+	 * The "command" builtin is not special so resets this behaviour.
 	 */
 	if (redir_err || errflag) {
 	    if (!isset(INTERACTIVE)) {
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.17
diff -p -u -r1.17 A04redirect.ztst
--- Test/A04redirect.ztst	22 Feb 2011 20:09:20 -0000	1.17
+++ Test/A04redirect.ztst	6 Mar 2011 20:25:15 -0000
@@ -386,6 +386,13 @@
 ?zsh:2: no such file or directory: /nonexistent/nonexistent
 
   $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  command set >/nonexistent/nonexistent
+  echo output'
+0:failed special builtin redir with command prefix, POSIX_BUILTINS
+>output
+?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
   echo >/nonexistent/nonexistent
   echo output'
 0:failed unspecial builtin redir, POSIX_BUILTINS

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/