[Toybox] [PATCH] tar.test: fix the tests when run as root.

[enh]

On Tue, Sep 6, 2022 at 5:11 PM enh <enh at google.com> wrote:

>
>
> On Tue, Sep 6, 2022 at 5:09 PM enh <enh at google.com> wrote:
>
>>
>>
>> On Sun, Sep 4, 2022 at 2:16 AM Rob Landley <rob at landley.net> wrote:
>>
>>> On 9/1/22 18:00, enh via Toybox wrote:
>>> > Afaik, I'm the only person regularly running the toybox tests as
>>> > root (in Android's CI), so I haven't actually fixed the character
>>> > special and block special tests for macOS here --- I've fixed them
>>> > so they work again on Android and Linux, and am just assuming we'll
>>> > "skipnot" them on macOS. The first person to want to run the tests
>>> > as root on macOS can fix this more thoroughly.
>>>
>>> I need to run the root tests more myself (part of what running the tests
>>> under
>>> mkroot is supposed to allow). I'm trying to fix it to work with "sys"
>>> and...
>>>
>>> -crw-rw---- 1 root sys 12,  34 1970-01-01 00:00 dir/char
>>> +crw-rw-r-- 1 root sys 12, 34 Jan  1  1970 dir/char
>>>
>>> Why would that change between root and non-root... because it didn't,
>>> devuan
>>> changed the default format out from under me in a version "upgrade".
>>> (Yes, this
>>> is more "make test_tar" without the rest of toybox, meaning it's not
>>> using
>>> toybox ls, meaning your previous argument circles around again...)
>>>
>>> I can force the time format with --full-time, although does the mac ls
>>> command
>>> have that...
>>>
>>>
>>> https://apple.stackexchange.com/questions/15170/how-do-i-change-the-time-format-used-by-ls-command-line-on-osx
>>>
>>> No it does  not. Nor does it provide an obvious alternative, they
>>> recommend
>>> "brew install coreutils". Right, I'm pretty happy saying mac only
>>> supports "make
>>> tests" all at once with macos_defconfig, and if you want more than that
>>> you can
>>> set up the $PATH to appropriate commands yourself. (Comparing toybox,
>>> busybox,
>>> and debian makes sense. Comparing with macosx is out of scope.)
>>>
>>
>> it's also useful for debugging specific mac issues... i currently have to
>> `rm tests/*.test` and then `git checkout -- tests/tar.test` to test tar on
>> macos in a reasonable amount of time.
>>
>>
>>> Except with --full-time it goes:
>>>
>>> crw-rw---- 1 root sys 12, 34 1970-01-01 00:00:00.000000000 +0000 dir/char
>>>
>>> Luckily we forced TZ=UTC but what if the underlying filesystem doesn't
>>> SUPPORT
>>> nanoseconds? (Will it still display them? Are any filesystems that don't
>>> support
>>> nanoseconds still a concern?)
>>>
>>> Possibly toybox ls should support --time-format=long-iso (ahem:
>>> time-style)...
>>> except, even then the spacing is off when I list /dev? Why is the
>>> spacing off?
>>> Because a couple entries in /dev have:
>>>
>>> crw-rw----+ 1 root    video    81,   0 2022-09-01 11:12 video0
>>> crw-rw----+ 1 root    video    81,   1 2022-09-01 11:12 video1
>>>
>>> What does the plus mean? I've never seen the plus before... Sigh:
>>>
>>>
>>> https://en.wikipedia.org/wiki/File-system_permissions#:~:text=access%20control%20list
>>>
>>> Debian does not have selinux enabled! Grrr. I'm guessing udevd has
>>> common rules
>>> or something, not chasing it down right now and in any case "system it's
>>> running
>>> on" can have arbitrary nuts spacing, so NOSPACE=1 the test. Does mksh
>>> support
>>> prefix assignments on functions...
>>>
>>>   $ mksh -c 'x() echo "$ABC"; ABC=def x'
>>>   def
>>>
>>> Yes it does. Right.
>>>
>>> Alright, put all this together and:
>>>
>>> -crw-rw---- 1 root sys 12, 34 1970-01-01 00:00:00.000000000 +0000
>>> dir/char
>>> +crw-rw-r-- 1 root sys 12, 34 1970-01-01 00:00:00.000000000 +0000
>>> dir/char
>>>
>>> The file creation permissions are still different. Your patch didn't
>>> change the
>>> permissions? The umask 0002 is zapping o+w but not o+r...?
>>>
>>> Ah, it's another "toybox" vs "debian du jour". Can coreutils please STOP
>>> RANDOMLY CHANGING STUFF? Grrr. Luckily, mknod has -m. (Checking to see
>>> if macos'
>>> mknod has -m, I find that macos does not appear to HAVE a command line
>>> mknod
>>> utility, just the system call? Anyway: gestalt toybox testing there,
>>> yay.)
>>>
>>> And a TODO item added to see if mknod's default permissions should
>>> change. (I.E.
>>> track down when/why gnu/mknod changed.)
>>>
>>> > Also use a consistent group as well as user for the "ownership"
>>> > test.
>>>
>>> Except it's "wheel" on the BSD-alikes, that change was made for a
>>> reason. Let's
>>> see... teach "skipnot" to exit with true or false and have an && thingy
>>> on the
>>> end to chgrp.
>>>
>>> > With this patch, the tests still pass on Linux and macOS but also
>>> > pass as root on Android.
>>>
>>> I can't seem to fix the last one:
>>>
>>> FAIL: tar ownership
>>> echo -ne '' | tar c --owner root --group sys --mtime @1234567890
>>> dir/file | SUM 3
>>> --- expected    2022-09-04 09:11:48.899556402 +0000
>>> +++ actual      2022-09-04 09:11:48.903556402 +0000
>>> @@ -1 +1 @@
>>> -2d7b96c7025987215f5a41f10eaa84311160afdb
>>> +d9e7fb3884430d29e7eed0dc04a2593dd260df14
>>>
>>> And alas I dunno what the difference IS, the downside of an opaque hash.
>>> (It's
>>> consistently doing it with debian's tar and my tar so the file's got
>>> something
>>> different. Deleting and recreating dir/file right before the test isn't
>>> changing
>>> it...)
>>>
>>> The problem is, if you're still seeing the OLD hash, I can't just change
>>> it to
>>> the one I'm seeing. If you are, could you send me the tarball? (You saw
>>> the
>>> "save all tarballs" hack a couple emails back, right? I should work the
>>> test
>>> name in there or something, although "sha1sum *" should let you know
>>> which one
>>> has the matching hash...)
>>>
>>
>> i saw it, but didn't you see my reply saying that it didn't work?
>>
>>
>>> I checked in the rest of it, lemme know what I broke?
>>>
>>
>> it doesn't seem to work at all?
>>
>> you got the test the wrong way round (it should have been `&& SKIP=1` not
>> `|| SKIP=1`), but then it only seems to last for one test anyway?
>>
>> oh, because you're supposed to count the number of tests to skip? i'm not
>> sure i have the words to express how terrible of an idea this seems to me?
>> that's so unreadable an idea even its own author got it wrong? :-P
>>
>> i still don't understand why there isn't just a "positive" version of
>> skipnot? personally i think the only thing wrong with skipnot is that it's
>> backwards, which makes the tests hard to read. i actually _like_ having a
>> `skip_if` on tests; each test is understandable in its own right, without
>> you needing to read any context. (that's actually part of why _i_ wasn't
>> offended by the repetition of all the uname checks in my version, but i'd
>> much rather factor out a DARWIN=1 or something if you want something brief
>> like `skip_if DARWIN` instead.)
>>
>> but expecting humans to _count_ the number of tests to skip (and to know,
>> when adding/removing/changing existing tests, that they might invisibly be
>> in such a "block") sounds like the kind of thing that even a FORTRAN IV
>> programmer might have said "dude, are you sure?" to...
>>
>> from this line:
>>
>> SKIP=0 # End of tests that don't work on MacOS X
>>
>> i don't think you even believed in it yourself :-) did you accidentally
>> check in something that was between two changes?
>>
>
> forgot to say that i've sent you the minimal patch to get this working
> anyway, even though i'm really unconvinced that this is a good route to go
> down!
>

and the good news is that the tests do now pass on Android as root. (and
only github runs mac CI, so i should be able to get this update in!)


> i think your `skipnot` was really close; it just needed to not be so
> negative :-)
>
>
>> Thanks,
>>>
>>> Rob
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20220906/d5d76029/attachment-0001.htm>