On 10/02/2025 22:49, Sertonix wrote:
On Mon Feb 10, 2025 at 11:14 PM CET, Harald van Dijk wrote:
On 10/02/2025 21:46, Sertonix wrote:
On Mon Feb 10, 2025 at 10:37 PM CET, Harald van Dijk wrote:
On 10/02/2025 21:15, Sertonix wrote:
On Mon Feb 10, 2025 at 10:08 PM CET, Harald van Dijk wrote:
Hi,
On 10/02/2025 19:14, Sertonix wrote:
The test-test example in the comment wasn't actually fixed before as
one can see by running
env -i "test-test=test" busybox ash -c 'export -p'
This previously incorrectly resulted in
export test
You're right that that output is incorrect. However...
But now it correctly results in
export 'test-test=test'
...this is not the correct result either. As the comment in the code
says, the output is meant to be usable for 'eval'. This output is not
usable for 'eval' and would result in a "bad variable name" error.
This is not the case. I changed the variable name checking to be more
relaxed for export so it works as expected. You can try this to verify:
env -i "test-test=test" busybox ash -c 'eval "$(export -p)"'
Oh, apologies, I missed that, I tested the output in an unpatched
busybox instead. I did not see in your message that you changed that as
well.
That change still looks wrong: unless there is some special permission
that I am not seeing, 'export test-test=test' is supposed to either
result in an error, or in the specified variable being assigned in the
way that POSIX specifies variable assignments work. The way POSIX
specifies variable assignments work mean the variable behaves like any
other variable. The syntax might not always allow that variable to be
referenced, but consider
env -i ./busybox ash -c 'export foo=bar baz-=quux; export -p; set'
*If* the assignment of baz- succeeds, then the requirements for 'export'
say that it becomes a shell variable, and the requirements for 'set' say
it gets printed there as well. That does not happen with your patch.
The POSIX doc for set (and only for set) states that "the output shall
be suitable for reinput to the shell, setting or resetting, as far as
possible". The way I read this is that skipping a variable name that
can't be parsed again would be ok.
There is no provision that permits variables to be skipped though. It
simultaneously requires that all variables be printed, and that they be
printed in a format that allows reinput into the shell. The
impossibility of printing baz-=quux in a way that allows reinput into
the shell does not negate the requirement that "set shall write the
names and values of all shell variables in the collation sequence of the
current locale."
The section about "Shell variables" mentions that "Shell variables
shall be initialized only from environment variables that have valid names".
So there is a destinction between shell variables and environment variables.
There is, yes. The 'export' command sets shell variables with a specific
attribute, it does not set environment variables directly. It is that
specific attribute that subsequently causes environment variables of the
same name to be set. If it worked any other way, then
export foo=bar
echo $foo
would not be required to print 'bar': $foo only expands shell variables.
This is specified in the description of the 'export' command:
The shell shall give the export attribute to the variables
corresponding to the specified names, which shall cause them to be in
the environment of subsequently executed commands.
Unfortunatly it seems like the POSIX spec tries to avoides this discussion
by making some things undefined behavior:
It is unspecified whether environment variables that were passed to the
invoking
shell when it was invoked itself, but were not used to initialize shell
variables
(see 2.5.3 Shell Variables) because they had invalid names, are
included in the
invoked utility's environment.
Unspecified and undefined are two different things.
This says that if test-test=test is not used to initialize a shell
variable, the shell may either remove test-test=test from the
environment of executed commands, or it may include it. Those are the
only two options. Undefined would permit anything.
busybox ash has previously made the choice to ensure that test-test=test
is not used to initialize a shell variable, and to do so by not
including itin the environment of executed commands. This is one choice
permitted by POSIX.
busybox ash then attempted to switch to the other permitted choice, to
continue to ensure that test-test=test is not used to initialize a shell
variable, but to still include it in the environment of executed
commands, in response to a bug report (bug 10231). Even though the prior
behaviour was permitted, it was not considered the best choice for its
users.
So handling these names at all might be an extension to the POSIX spec
but one that (in my opinion) is needed for export -p to be used reliably.
A change is needed, but the only change that I think is needed is:
diff --git a/shell/ash.c b/shell/ash.c
index 9173b8608..a6638c57f 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -11597,6 +11597,8 @@ showvars(const char *sep_prefix, int on, int off)
q = nullstr;
if (*p == '=')
q = single_quote(++p);
+ else if (*p != '\0')
+ continue;
out1fmt("%s%s%.*s%s\n", sep_prefix, sep, (int)(p -
*ep), *ep, q);
}
return 0;
This is already enough to permit export -p to be used reliably, this
makes busybox ash do what it currently attempts to do, and matches what
bash does.
There are two ways around that conflict: either the shell can act in a
way in which baz- is never accepted as anything that makes it a shell
variable name, or the shell can add some extended syntax to permit
variables with such a name to be set so that the set command can print
them. This would be similar to how you extended the 'export' command to
print such variable names.
That said, even if you are not convinced on 'set', there is another way
to show that this patch is problematic: it also affects the printing and
argument processing of the 'readonly' command. Consider:
$ env test-test=test ./busybox ash -c 'readonly test-test; readonly -p'
readonly 'test-test=test'
$ ./busybox ash -c "readonly 'test-test=test'"
ash: readonly: line 0: test-test: bad variable name
You are right that case is still broken. yash (which I took some
inspiration from) seems to allow any name for readonly as well. When
making the change I suggest it would be more consistent to do that for
readonly as well.
yash makes it so that things behave consistently there (regardless of
what POSIX does and does not permit): it is possible to set *any* shell
variable with any name, not just shell variables with specific
attributes. This is because yash has the 'typeset' command and also
permits 'typeset test-test=test'. busybox ash lacks the 'typeset'
command so cannot do exactly the same thing.
bash, which busybox ash much more often tries to copy, does have the
'typeset' command as well, but enforces variable names there in exactly
the same way it does in the 'export' and 'readonly' commands:
$ bash -c 'typeset test-test=test'
bash: line 1: typeset: `test-test=test': not a valid identifier
$ bash -c 'export test-test=test'
bash: line 1: export: `test-test=test': not a valid identifier
$ bash -c 'readonly test-test=test'
bash: line 1: readonly: `test-test=test': not a valid identifier
Except for the lack of the 'typeset' command, this is also the behaviour
of dash, which busybox ash is based on.
In my opinion, regardless of whether POSIX permits it, these shells are
so much more widely used than yash that it would be a mistake to deviate
from them.
I believe my below comment still applies, the variable needs to be
skipped when printing instead.
The variable needs to be skipped when printing instead.
Cheers,
Harald van Dijk
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
