[Toybox] macOS realpath test failures

[enh]

On Thu, Jan 12, 2023 at 3:36 PM Rob Landley <rob at landley.net> wrote:

>
>
> On 1/12/23 16:19, enh wrote:
> >
> >
> > On Thu, Jan 12, 2023 at 1:40 PM Rob Landley <rob at landley.net
> > <mailto:rob at landley.net>> wrote:
> >
> >     On 1/11/23 16:53, enh wrote:
> >     > ah, which is probably because you won't typically have $PWD set on
> Android...
> >     >
> >     >   // If this isn't an absolute path, start with cwd or $PWD.
> >     >   if (*path != '/') {
> >     >     if ((flags & ABS_KEEP) && (str = getenv("PWD")))
> >     >       splitpath(path, splitpath(str, &todo));
> >     >     else {
> >     >       splitpath(path, splitpath(str = xgetcwd(), &todo));
> >     >       free(str);
> >     >     }
> >
> >     Which would explain the behavior difference, yes: getcwd() asks the
> OS for the
> >     current path to "." which is always going to be fully resolved,
> which means
> >     peeling off one member gives us the observed output.
> >
> >     >   } else splitpath(path, &todo);
> >     >
> >     > mksh fakes it so `echo $PWD` works, but there's no _exported_ $PWD:
> >
> >     It's not faking it, it's setting the shell variable. But bash
> exports PWD by
> >     default (and thus so does toybox), but mksh doesn't.
> >
> >     Toysh implemented the bash behavior and mksh... is mksh:
> >
> >     $ env -i bash --noprofile --norc -c 'declare -p PWD'
> >     declare -x PWD="/home/landley/toybox/toybox"
> >     $ env -i ./sh --noprofile --norc -c 'declare -p PWD'
> >     declare -x PWD="/home/landley/toybox/toybox"
> >     $ env -i mksh --noprofile --norc -c 'declare -p PWD'
> >     mksh: mksh: --: unknown option
> >
> >     I added an "export PWD" to the test, does that fix it?
> >
> > yeah, that passes for me locally. thanks!
> >
> > worth adding explicit PWD-unset tests for the other codepath, since it
> exists?
>
> Ooh that's a pointy question. I'm not sure you knew how pointy when you
> asked
> it. The answer is probably "no because TEST_HOST should not become a
> conceptual
> forkbomb"?
>

not at all :-)

all i was thinking was "hmm... since Android's default shell doesn't export
PWD, the currently-untested case is our default case".


> The problem is the behavior varies: bash and mksh are different,
> gnu/toybox/busybox are different, filesystems give different behavior... I
> don't
> want the tests to be more brittle than necessary in the absence of a spec.
> I'm
> testing for "correct" behavior and testing error paths reject "incorrect"
> behavior, but this is imprecisely defined fallback behavior that could
> change if
> a better option shows up.
>
> We're already in a "toyonly" test because gnu/realpath is doing getcwd()
> rather
> than $PWD for expanding relative paths, and my plumbing is going "if it's
> told
> not to expand symlinks then use $PWD instead of getcwd() to expand relative
> paths when available, falling back to the other when it isn't. (Which
> seems to
> be to be the "more correct" behavior, in the absence of an actual spec that
> covers this stuff.) If there was an additional way to get $PWD when the
> variable
> wasn't set, it probably should, because -s asks it to.
>
> By the way, the NEXT hair-splitting corner case here is that toysh will
> export
> PWD from its internal list variable list into environ where getenv() can
> find it
> even when running builtins, but NOT when running NOFORK commands. Those
> have to
> use getvar() instead of getenv() to see current variable values.
>
> I was initially doing export on assignment instead of export on fork and it
> turns out bash's nested local variable semantics require export on fork.
> You can
> export a local variable, have the next nested function call unset its own
> local
> of the same variable to create a "whiteout", and then the function call
> after
> that declare a local variable that isn't exported:
>
>   $ x() { local X=potato; declare -p X; }; y() { local X; unset X; declare
> -p X;
> x;}; z() { local X=fruit; export X; y; declare -p X; }; z
>   declare -- X
>   declare -- X="potato"
>   declare -x X="fruit"
>
> So anyway, the xabspath() fallback path is doing the best it can in the
> absence
> of complete information, and I'm not comfortable defining "fail in this
> way" as
> the SPECIFIC best it can do? If more sources of information become
> available,
> should it NOT use them? If xrealpath() did wind up getting called from
> inside
> the shell, then possibly it would need to learn about getvar() and the
> local
> version of PWD. (Or just have a wrapper function layer with the real one
> growing
> another argument to pass PWD through...)
>
> There's a reason all this stuff was in pending for so long...
>

given how hairy this is (and most people are probably expecting PWD to be
exported), i wonder whether i should change the default mksh rc file to
include `export PWD`. that's a break with the past, but a potentially
useful dose of consistency generally...


> Rob
>
> P.S. It would be GREAT if an actual standard was on this level, but it
> isn't.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20230112/8b9e5fad/attachment.htm>