-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Declaration utilities perform assignments immediately before expanding remaining command arguments #626
Comments
The issue is more fundamental than that. Consider preceding assignments:
(which is also the behaviour of most other shells) In ksh, declaration utilities are implemented by interpreting the assignment-arguments as real assignments. When you do
what actually gets executed is
Same with This is fundamental. Changing it would require a comprehensive rewrite/redesign. |
This is something POSIX leaves unspecified. It is not a bug for this to result in '2 2'.
(I cannot comment on the second part of your reply, as I am not sufficiently familiar with ksh internals.) |
Perhaps this will clarify. On the dev branch, I now use deparse.c for
So, that shows you how ksh has hacked declaration built-ins. |
Thanks, that makes sense. A trivial proof of concept modification to not do that works for this case: --- a/src/cmd/ksh93/sh/parse.c
+++ b/src/cmd/ksh93/sh/parse.c
@@ -1484,33 +1484,13 @@ static Shnode_t *simple(Lex_t *lexp,int flag, struct ionod *io)
if(associative && (!(argp->argflag&ARG_ASSIGN) || argp->argval[0]!='['))
sh_syntax(lexp);
/* check for assignment argument */
- if((argp->argflag&ARG_ASSIGN) && assignment!=2)
+ if((argp->argflag&ARG_ASSIGN) && assignment==0)
{
*settail = argp;
settail = &(argp->argnxt.ap);
lexp->assignok = (flag&SH_ASSIGN)?SH_ASSIGN:1;
- if(assignment)
- {
- struct argnod *ap=argp;
- char *last, *cp;
- if(assignment==1)
- {
- last = strchr(argp->argval,'=');
- if(last && (last[-1]==']'|| (last[-1]=='+' && last[-2]==']')) && (cp=strchr(argp->argval,'[')) && (cp < last) && cp[-1]!='.')
- last = cp;
- stkseek(stkp,ARGVAL);
- sfwrite(stkp,argp->argval,last-argp->argval);
- ap=(struct argnod*)stkfreeze(stkp,1);
- ap->argflag = ARG_RAW;
- ap->argchn.ap = 0;
- }
- *argtail = ap;
- argtail = &(ap->argnxt.ap);
- if(argno>=0)
- argno++;
- }
- else /* alias substitutions allowed */
- lexp->aliasok = 1;
+ /* alias substitutions allowed */
+ lexp->aliasok = 1;
}
else
{ This works because declaration utilities already have the logic for parsing assignments that were passed in as command line arguments, as they are needed to handle e.g. However, this cannot be easily extended to not also break e.g. |
IMO the other shells have the bug. The way ksh does this is consistent with the way assignments always work - the assignment words should be evaluated in order. I don't know why the others don't do that. |
My comment was based on what POSIX specifies. It requires the behaviour of other shells, for the reason I gave. I have no issue if you wish to argue that POSIX is defective, but IMO this is the wrong place to raise that, if you feel that way I would encourage you to report a bug at https://austingroupbugs.net/main_page.php. :) |
What requirement are you referring to? 2.9.1.1 section 2 says:
This section, confusingly, redundantly overviews the order of expansions for an assignment, but it definitely says nothing about modification of the overall evaluation of assignments for declaration builtins. I would interpret this as meaning assignments should be processed as usual one assignment word at a time. Also, the builtin itself almost never performs the assignments. This is usually reflected in xtrace output. If the assignments were being performed by the builtin then arguments would first need to be passed to the builtin, then any assignments exported to the builtin would have to evaluate, and last the builtin would evaluate the arguments with exported assignments also affecting these assignments. No shell does this for regular assignments, but a notable exception with bash serves to illustrate. Bash may conditionally modify evaluation such that assignment are performed by the builtin for non-assignment words that expand to valid assignments after the initial assignment word pass. This applies only to compound array assignments in bash.
Note the literal assignment word is evaluated first, before the builtin is executed, next exported assignments are evaluated, and last the remaining word is passed to the builtin which re-evaluates the word as an assignment, except now with a new |
That's because despite being parsed as assignment words, they have no such effect. Nothing in 2.9.1 says that 'export a=b' assigns to the variable 'a' the value 'b', that is entirely covered by the description of the 'export' command itself.
You're describing ksh implementation behaviour here, not POSIX. Per POSIX's description of the 'export' command:
Yes, this is exactly what POSIX specifies.
False, this is exactly how it works in dash and dash-derived shells. I have not dug into the behaviour of other shells but the fact that ksh is alone in the behaviour I originally reported makes me doubt your claim here for other shells too. |
The POSIX description is quite poor and doesn't really capture how things actually work. I would not infer just from the
That's because the way ksh and its derivatives process these was the entire precedent for adding declaration builtins as a separate class to the spec. The term "declaration command" literally originates from the ksh manual:
In dash, declaration builtins were a total afterthought. Dash is not a shining example. I remember the patch which made that change and it was a clumbsy half-hearted attempt at converting the argument processing into something that looks roughly like assignment processing. |
@ormaaj: You're talking to the developer of a dash-based shell who is intimately familiar with its internals. My view of what you just wrote is that that is wrong and insulting, and that continuing this conversation is a waste of time of everyone involved. I will withdraw from this conversation with you. @McDutchie: If you believe that the current ksh behaviour is permitted by POSIX, I will be happy to have that conversation if we can do it in a productive way. I believe it is clear that it is not. |
I'm not wrong. And nothing about that is intended to offend. Historically most ash derivatives did not treat their arguments as special assignments. That was a recent development for dash in reaction to POSIX. Ash gained the ability to assign with The sentences you're referring to are unchanged from issue 7, which doesn't even define special processing for assignment arguments. Unsurprisingly, words treated as ordinary arguments in that context are expanded as usual and also unsurprisingly the builtin would have to capture and evaluate them under the assumptions of a typical builtin. POSIX now mashes up these ideas and doesn't explain the design ideas underlying them. |
Not only that, it will expose those pseudo-assignment arguments to field splitting and pathname expansion, which is contra new POSIX. I might get the Austin Group involved in this when I have enough time/energy to be bothered. Ksh invented declaration builtins, it doesn't seem right that we should have to redesign them. I'm not a fan of them though, I think that (no matter how they are implemented) they're a kludge that papers over the real and fundamental problem, which is global field splitting and pathname expansion – and they do this at the expense of creating even more inconsistency and confusion (as in, we now have to explain to scripters why So, for now, I'm having a hard time getting excited about this issue. |
Indeed. A simple test case for this:
This is supposed to print
under new POSIX rules, and prints
with my proof of concept patch.
If you can propose POSIX wording to make this unspecified, while leaving enough specified for the concept of declaration utilities to remain sufficiently useful to standardise, I am certainly not going to object to that. I didn't open this issue because it's breaking existing scripts, I opened it because I happened to come across it comparing my own shell to others in corner cases.
At the moment, for your shell and mine, |
Please consider the following
This is supposed to print 'a=2 b=1'. Under ksh, it prints 'a=2 b=2'.
The reason it is supposed to print 'a=2 b=1' is that the word expansions are supposed to be performed first for each command argument, and only then does the command run, which performs the assignments. Therefore, when 'b=$a' is expanded, 'a' is supposed to still have its previous value of '1'. In ksh, when the command is recognised as a declaration utility, the assignment is performed for each argument immediately after it has been expanded.
This applies to 'declare', 'export', 'local', 'readonly', 'typeset'. Not all are covered by POSIX, but I expect it would be more work to treat these differently than it would be to change all of them in one go.
Workaround: It is possible to avoid the issue by writing the command in such a way that it does not get detected as a declaration utility, e.g.
The text was updated successfully, but these errors were encountered: