[Toybox] macOS realpath test failures

[Rob Landley]

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"?

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...

Rob

P.S. It would be GREAT if an actual standard was on this level, but it isn't.