[-- Attachment #1: Type: text/plain, Size: 934 bytes --] testfunc() { echo $*; echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`"; echo 'I do getopts :a: varname'; getopts ':a:' varname; echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`"; echo 'I do getopts :a: varname'; getopts ':a:' varname; echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`"; echo 'I do getopts :a: varname'; getopts ':a:' varname; echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`"; } (testfunc -a -w -e -r -a) Execution in bash: -a -w -e -r -a OPTIND is 1, next -a I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 4, next -r I do getopts :a: varname OPTIND is 5, next -a execution in zsh: -a -w -e -r -a OPTIND is 1, next -a I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 4, next -r Best regards
On 9 Jan 2018, at 10:22, Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote: >Execution in bash: ... OPTIND is 5, next -a >execution in zsh: ... OPTIND is 4, next -r I think this fixes it? At least, zsh gives the same output as bash and dash when i do this. Peter's left an ominous warning about changes to this function that deserves recognition though... dana diff --git a/Src/builtin.c b/Src/builtin.c index 0211f2721..2550b29f2 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5437,6 +5437,9 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) { p = "?"; err: + /* Keep OPTIND correct if the user doesn't return after the error */ + optcind = 0; + zoptind++; zsfree(zoptarg); setsparam(var, ztrdup(p)); if(quiet) { diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst index 7eba5a4b1..f6977accb 100644 --- a/Test/B10getopts.ztst +++ b/Test/B10getopts.ztst @@ -79,3 +79,20 @@ test_getopts +x 1:one illegal option, + variant >test_getopts:3: bad option: +x + + t() { local o; repeat $1 { getopts a: o "${@:2}" 2>&1 }; print -r $OPTIND } + t 4 -a -w -e -r -a + t 5 -a -w -e -a -w -e + t 5 -a -w -e -r -ax -a +0:OPTIND calculation after error (workers/42248) +*>*: bad option: -e +*>*: bad option: -r +*>*: argument expected after -a option +>6 +*>*: bad option: -e +*>*: bad option: -e +>7 +*>*: bad option: -e +*>*: bad option: -r +*>*: argument expected after -a option +>7
On Tue, Jan 9, 2018 at 2:48 PM, dana <dana@dana.is> wrote:
>
> I think this fixes it? At least, zsh gives the same output as bash and dash when
> i do this. Peter's left an ominous warning about changes to this function that
> deserves recognition though...
There's a whole thread from 2015 related to this, beginning with workers/35317
On 9 Jan 2018, at 16:57, Bart Schaefer <schaefer@brasslantern.com> wrote:
>There's a whole thread from 2015 related to this, beginning with workers/35317
Oops, sure enough. Sorry.
Well, i'm not sure about POSIX correctness, but for whatever it's worth, i can
confirm that all of the following are in agreement as to how it should behave:
ash (BusyBox)
bash
dash
ksh93
mksh
posh
The ksh that comes with OpenBSD (derived from pdksh i think) seems to stop
updating OPTIND after it encounters an invalid option until it finds either a
legal one or the end of the argument list; after that it's set back to the
correct value.
yash just seems broken somehow, it gives me nonsense.
dana
Such quick, much wow! On Jan 9, 2018 8:58 PM, "dana" <dana@dana.is> wrote: > > On 9 Jan 2018, at 16:57, Bart Schaefer <schaefer@brasslantern.com> wrote: > >There's a whole thread from 2015 related to this, beginning with workers/35317 > OK, I just read workers/35317 and also the previous thread http://www.zsh.org/mla/workers//2015/msg00196.html and also the previous from 2007 http://www.zsh.org/mla/users/2007/msg01191.html 2 key responses from Peter Stephenson: 2007: > This is the way you'd normally use it: you don't know how many options > there are, if any, until getopts returns 1. > > However, it's still a bit fishy. The internal option index handling logic > has always been a bit curious; I've lost count of the number of bugs we've > had in it. And also On Thu, 28 May 2015 18:17:40 +0100 Peter Stephenson <p.stephenson@xxxxxxxxxxx> wrote: > This is documented behaviour (well, sort of -- the documentation didn't > say quite what we actually do) so this needs another compatibility fix. > POSIX_BUILTINS seems appropriate. (and then submits a cool patch) I checked man zshbuiltins getopts optstring name [ arg ... ] (...) The first option to be examined may be changed by explicitly assigning to OPTIND. OPTIND has an initial value of 1, and is normally set to 1 upon entry to a shell function and restored upon exit (this is disabled by the POSIX_BUILTINS option). and man zshoptions POSIX_BUILTINS <K> <S> (...) Furthermore, the getopts builtin behaves in a POSIX-compatible fashion in that the associated variable OPTIND is not made local to functions. Now here things get funky. If I try this: ( testfunc() { echo POSIX_BUILTINS is $options[posixbuiltins] echo $*; for i in 1 2 3 4; do echo "OPTIND is $OPTIND, `(shift "$(($OPTIND - 1))"; echo next $1)`"; echo 'I do getopts :a: varname'; getopts ':a:' varname; done } ( setopt POSIX_BUILTINS testfunc -a -w -e -r ) ( unsetopt POSIX_BUILTINS testfunc -a -w -e -r ) ) I get: POSIX_BUILTINS is on -a -w -e -r OPTIND is 1, next -a I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 4, next -r I do getopts :a: varname POSIX_BUILTINS is off -a -w -e -r OPTIND is 1, next -a I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 3, next -e I do getopts :a: varname OPTIND is 4, next -r I do getopts :a: varname hmmmmmmm... I's it a bug? > Well, i'm not sure about POSIX correctness, but for whatever it's worth, i can > confirm that all of the following are in agreement as to how it should behave: > > ash (BusyBox) > bash > dash > ksh93 > mksh > posh > I think there are 2 things to discuss, one is this (possible) bug. The other is if POSIX_BUILTINS should be on by default or not. If off by default, New users will get bugs in their code if they migrate from bash If on by default, downstream projects will get bugs in their working code This options does other things as well: When this option is set the command builtin can be used to execute shell builtin commands. Parameter assignments specified before shell functions and special builtins are kept after the command completes unless the special builtin is prefixed with the command builtin. Special builtins are ., :, break, continue, declare, eval, exit, export, integer, local, readonly, return, set, shift, source, times, trap and unset. In addition, various error conditions associated with the above builtins or exec cause a non-interactive shell to exit and an interactive shell to return to its top-level processing. So this is a broather subject and may be better discussed in a new thread with a suitable title
On Tue, 9 Jan 2018 16:48:17 -0600
dana <dana@dana.is> wrote:
> On 9 Jan 2018, at 10:22, Francisco de Zuviría Allende <franciscodezuviria@gmail.com> wrote:
> >Execution in bash: ... OPTIND is 5, next -a
> >execution in zsh: ... OPTIND is 4, next -r
>
> I think this fixes it? At least, zsh gives the same output as bash and dash when
> i do this. Peter's left an ominous warning about changes to this function that
> deserves recognition though...
I've completely lost track of the cases that are real compatibility
problems and which are just broken, because, as just got quoted, the
code is pretty hair-raising anyway.
If we think we can get away with wrapping this change in
isset(POSIXBUILTINS) that will make it easier. It doesn't sound like it
can be wrong in that case, anyway.
pws
(Resurrecting this per workers/48509) bin_getopts() has changed a little since i posted my patch before, this looks a bit weirder. The test is also weird. But i confirmed that this makes it behave like dash, bash, and mksh (my ksh93 doesn't support local, and i still don't know what yash is doing): % pbpaste | zsh-dev -f <1><1><3><5><7><6> % pbpaste | zsh-dev -f --posix-builtins <2><2><3><6><7><7> % pbpaste | dash <2><2><3><6><7><7> % pbpaste | bash <2><2><3><6><7><7> % pbpaste | mksh <2><2><3><6><7><7> % pbpaste | yash <3><3><3><7><7><7> tbh i don't think i fully understand why this needs to work this way, or whether there are other cases that should be tested. Open to review obv dana diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index a7afe42cf..4b9778e40 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -982,7 +982,8 @@ vindex(OPTARG, use of) The first option to be examined may be changed by explicitly assigning to tt(OPTIND). tt(OPTIND) has an initial value of tt(1), and is normally set to tt(1) upon entry to a shell function and restored -upon exit (this is disabled by the tt(POSIX_BUILTINS) option). tt(OPTARG) +upon exit. (The tt(POSIX_BUILTINS) option disables this, and also changes +the way the value is calculated to match other shells). tt(OPTARG) is not reset and retains its value from the most recent call to tt(getopts). If either of tt(OPTIND) or tt(OPTARG) is explicitly unset, it remains unset, and the index or option argument is not diff --git a/Src/builtin.c b/Src/builtin.c index 26335a2e8..13dfdf8be 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5556,6 +5556,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun /* check for legality */ if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) { p = "?"; + /* Keep OPTIND correct if the user doesn't return after the error */ + if (isset(POSIXBUILTINS)) { + optcind = 0; + zoptind++; + } zsfree(zoptarg); setsparam(var, ztrdup(p)); if(quiet) { @@ -5572,6 +5577,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun if(p[1] == ':') { if(optcind == lenstr) { if(!args[zoptind]) { + /* Fix OPTIND as above */ + if (isset(POSIXBUILTINS)) { + optcind = 0; + zoptind++; + } zsfree(zoptarg); if(quiet) { setsparam(var, ztrdup(":")); diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst index 72c9e209e..69b3d63f4 100644 --- a/Test/B10getopts.ztst +++ b/Test/B10getopts.ztst @@ -96,3 +96,29 @@ done 0:missing option-argument (quiet mode) >:,x + + # This function is written so it can be easily referenced against other shells + t() { + local o i=0 n=$1 + shift + while [ $i -lt $n ]; do + i=$(( i + 1 )) + getopts a: o "$@" 2> /dev/null + done + printf '<%d>' "$OPTIND" + } + # Try all these the native way, then the POSIX_BUILTINS way + for 1 in no_posix_builtins posix_builtins; do ( + setopt $1 + print -rn - "$1: " + t 1 -a + t 1 -w + t 2 -a -w + t 4 -a -w -e -r -a + t 5 -a -w -e -a -w -e + t 5 -a -w -e -r -ax -a + print + ); done +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248) +>no_posix_builtins: <1><1><3><5><7><6> +>posix_builtins: <2><2><3><6><7><7>
dana wrote on Tue, Apr 13, 2021 at 18:28:50 -0500: > + # This function is written so it can be easily referenced against other shells > + t() { > + local o i=0 n=$1 > + shift > + while [ $i -lt $n ]; do > + i=$(( i + 1 )) > + getopts a: o "$@" 2> /dev/null > + done > + printf '<%d>' "$OPTIND" > + } > + # Try all these the native way, then the POSIX_BUILTINS way > + for 1 in no_posix_builtins posix_builtins; do ( > + setopt $1 > + print -rn - "$1: " > + <3> t 1 -a I'm guessing yash notices that -a (at index 1) takes an argument, so it goes to the top of the loop for the word at index 2, where it increments OPTIND before noticing that index 2 is actually past the end of the array. > + <3> t 1 -w I get "1:2" in yash 2.51 interactively. > + <3> t 2 -a -w Consistent with other shells. > + <7> t 4 -a -w -e -r -a - The first word is the -a option. - The second word is the argument to the -a option. - The third word is -w, so this call will return an error. Before returning, since the next word to parse is the fourth one, set OPTIND to 4. > + <7> t 5 -a -w -e -a -w -e Consistent with other shells. > + <7> t 5 -a -w -e -r -ax -a Consistent with other shells. > + print > + ); done > +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248) > +>no_posix_builtins: <1><1><3><5><7><6> > +>posix_builtins: <2><2><3><6><7><7> > >
dana wrote on Tue, Apr 13, 2021 at 18:28:50 -0500: > (Resurrecting this per workers/48509) > > bin_getopts() has changed a little since i posted my patch before, this looks > a bit weirder. The test is also weird. But i confirmed that this makes it > behave like dash, bash, and mksh (my ksh93 doesn't support local, and i still > don't know what yash is doing): > > % pbpaste | zsh-dev -f > <1><1><3><5><7><6> > % pbpaste | zsh-dev -f --posix-builtins > <2><2><3><6><7><7> > % pbpaste | dash > <2><2><3><6><7><7> > % pbpaste | bash > <2><2><3><6><7><7> > % pbpaste | mksh > <2><2><3><6><7><7> > % pbpaste | yash > <3><3><3><7><7><7> > > tbh i don't think i fully understand why this needs to work this way, or > whether there are other cases that should be tested. Open to review obv Should the descriptions of OPTIND and/or POSIX_BUILTINS in the manual be extended as well? (Once the behaviour is decided on, if there are still open questions about that.) Some more cases to test: . t 0 t 1 foo t 1 -- foo t 1 -b . where -b doesn't take an argument. > @@ -96,3 +96,29 @@ > + # Try all these the native way, then the POSIX_BUILTINS way > + for 1 in no_posix_builtins posix_builtins; do ( > + setopt $1 > + print -rn - "$1: " > + t 1 -a > + t 1 -w > + t 2 -a -w > + t 4 -a -w -e -r -a > + t 5 -a -w -e -a -w -e > + t 5 -a -w -e -r -ax -a > + print > + ); done > +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248) > +>no_posix_builtins: <1><1><3><5><7><6> > +>posix_builtins: <2><2><3><6><7><7> > >
On 14 Apr 2021, at 08:08, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Should the descriptions of OPTIND and/or POSIX_BUILTINS in the manual be > extended as well? The latter, yes; done that. The former, idk, it doesn't currently mention anything about how it's calculated or about POSIX_BUILTINS effects so i'm inclined to leave it alone On 14 Apr 2021, at 08:08, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Some more cases to test: > . > t 0 > t 1 foo > t 1 -- foo > t 1 -b > . > where -b doesn't take an argument. `t 0` doesn't test anything, the loop is just skipped. `t 1 -b` tests the same thing as `t 1 -w`, but i guess it's confusing that i picked -a -w -e -r as the options; i've changed them to -a -b -c -d. I've also added the two foo ones you suggested, as well as just `t 1`. (All three behaved the same as other shells already) dana diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index a7afe42cf..4b9778e40 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -982,7 +982,8 @@ vindex(OPTARG, use of) The first option to be examined may be changed by explicitly assigning to tt(OPTIND). tt(OPTIND) has an initial value of tt(1), and is normally set to tt(1) upon entry to a shell function and restored -upon exit (this is disabled by the tt(POSIX_BUILTINS) option). tt(OPTARG) +upon exit. (The tt(POSIX_BUILTINS) option disables this, and also changes +the way the value is calculated to match other shells). tt(OPTARG) is not reset and retains its value from the most recent call to tt(getopts). If either of tt(OPTIND) or tt(OPTARG) is explicitly unset, it remains unset, and the index or option argument is not diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo index 714e8a1a1..ffe2d1a0d 100644 --- a/Doc/Zsh/options.yo +++ b/Doc/Zsh/options.yo @@ -2239,7 +2239,8 @@ command found in the path. Furthermore, the tt(getopts) builtin behaves in a POSIX-compatible fashion in that the associated variable tt(OPTIND) is not made -local to functions. +local to functions, and its value is calculated differently to match +other shells. Moreover, the warning and special exit code from tt([[ -o )var(non_existent_option)tt( ]]) are suppressed. diff --git a/Src/builtin.c b/Src/builtin.c index 26335a2e8..13dfdf8be 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -5556,6 +5556,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun /* check for legality */ if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) { p = "?"; + /* Keep OPTIND correct if the user doesn't return after the error */ + if (isset(POSIXBUILTINS)) { + optcind = 0; + zoptind++; + } zsfree(zoptarg); setsparam(var, ztrdup(p)); if(quiet) { @@ -5572,6 +5577,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun if(p[1] == ':') { if(optcind == lenstr) { if(!args[zoptind]) { + /* Fix OPTIND as above */ + if (isset(POSIXBUILTINS)) { + optcind = 0; + zoptind++; + } zsfree(zoptarg); if(quiet) { setsparam(var, ztrdup(":")); diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst index 72c9e209e..e50d177c7 100644 --- a/Test/B10getopts.ztst +++ b/Test/B10getopts.ztst @@ -96,3 +96,32 @@ done 0:missing option-argument (quiet mode) >:,x + + # This function is written so it can be easily referenced against other shells + t() { + local o i=0 n=$1 + shift + while [ $i -lt $n ]; do + i=$(( i + 1 )) + getopts a: o "$@" 2> /dev/null + done + printf '<%d>' "$OPTIND" + } + # Try all these the native way, then the POSIX_BUILTINS way + for 1 in no_posix_builtins posix_builtins; do ( + setopt $1 + print -rn - "$1: " + t 1 + t 1 foo + t 1 -- foo + t 1 -a + t 1 -b + t 2 -a -b + t 4 -a -b -c -d -a + t 5 -a -b -c -a -b -c + t 5 -a -b -c -d -ax -a + print + ); done +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248) +>no_posix_builtins: <1><1><2><1><1><3><5><7><6> +>posix_builtins: <1><1><2><2><2><3><6><7><7>
dana wrote on Sun, Apr 18, 2021 at 00:16:52 -0500: > On 14 Apr 2021, at 08:08, Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Some more cases to test: > > . > > t 0 > > t 1 foo > > t 1 -- foo > > t 1 -b > > . > > where -b doesn't take an argument. > > `t 0` doesn't test anything, the loop is just skipped. Yeah, but OPTIND still gets printed, so it does test *something*. > `t 1 -b` tests the same thing as `t 1 -w`, but i guess it's confusing > that i picked -a -w -e -r as the options; i've changed them to -a -b > -c -d. I've also added the two foo ones you suggested, as well as just > `t 1`. (All three behaved the same as other shells already) *nod* > +upon exit. (The tt(POSIX_BUILTINS) option disables this, and also changes > +the way the value is calculated to match other shells). tt(OPTARG) s/)./.)/ Cheers, Daniel > +++ b/Test/B10getopts.ztst > @@ -96,3 +96,32 @@ > done > 0:missing option-argument (quiet mode) > >:,x > + > + # This function is written so it can be easily referenced against other shells > + t() { > + local o i=0 n=$1 > + shift > + while [ $i -lt $n ]; do > + i=$(( i + 1 )) > + getopts a: o "$@" 2> /dev/null > + done > + printf '<%d>' "$OPTIND" > + } > + # Try all these the native way, then the POSIX_BUILTINS way > + for 1 in no_posix_builtins posix_builtins; do ( > + setopt $1 > + print -rn - "$1: " > + t 1 > + t 1 foo > + t 1 -- foo > + t 1 -a > + t 1 -b > + t 2 -a -b > + t 4 -a -b -c -d -a > + t 5 -a -b -c -a -b -c > + t 5 -a -b -c -d -ax -a > + print > + ); done > +0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248) > +>no_posix_builtins: <1><1><2><1><1><3><5><7><6> > +>posix_builtins: <1><1><2><2><2><3><6><7><7> > >
On 20 Apr 2021, at 16:31, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Yeah, but OPTIND still gets printed, so it does test *something*.
That's true. I'm not sure what guarantees there are about the state of OPTIND
when this test runs, though, so i ended up leaving it out. We can add it later
if you want
Anyway, i've committed this now, with the rest of the feedback + a mention in
README. Sorry i lost track of it before
dana