landley / toybox

toybox
http://landley.net/toybox
BSD Zero Clause License
2.44k stars 338 forks source link

tar tests fail on WSL2 after 4fca350fb34c31e681fd6750692d6089b9810c3d #328

Open LongPingWEI opened 2 years ago

LongPingWEI commented 2 years ago

PASS: tail noseek -c+ out of bonds PASS: tail -c 12345 -n 3 bigfile PASS: tail -n 3 -c 12345 bigfile PASS: tail -f one PASS: tail -f one two three PASS: tail -F PASS: tar create file PASS: tar pass file PASS: tar pass user tar: bad group 'nobody' tar: Not tar FAIL: tar pass group echo -ne '' | tar c --owner root --group nobody --mtime @0 file | LST --- expected 2022-03-23 11:04:31.921672300 +0000 +++ actual 2022-03-23 11:04:31.931672300 +0000 @@ -1 +0,0 @@ --rw-rw-r-- root/nobody 0 1970-01-01 00:00 file make: *** [Makefile:77: tests] Error 1

LongPingWEI commented 2 years ago

@landley My environment Ubuntu 20.04 WSL.

LongPingWEI commented 2 years ago

I found that this commit modified the perm of tar.test from 0644 to 0755. If I set perm 0644 to tar.test, make tests can pass.

LongPingWEI commented 2 years ago
id nobody
uid=65534(nobody) gid=65534(nogroup) groups=65534(nogroup)

but tar.test try to use nobody as a group,

skipnot id nobody >/dev/null
testing "pass group" "tar c --owner root --group nobody --mtime @0 file | LST" \
LongPingWEI commented 2 years ago

The other failure in tar.test:

yes | (dd bs=$((1<<16)) count=1 status=none; dd bs=8192 seek=14 count=1 status=none; dd bs=4096 seek=64 count=5 status=none) > fweep
testing "sparse without overflow" "$TAR --sparse fweep | SUM 3" \
  "e1560110293247934493626d564c8f03c357cec5\n" "" ""

rm fweep

The length of fweep differs from what generated by the same command on Ubuntu.

weilongping@Desktop-of-LongPing:~/toybox$ ls -lh ./generated/testdir/testdir/fweep
-rw-rw-r-- 1 weilongping weilongping 381K Mar 23 20:38 ./generated/testdir/testdir/fweep
weilongping@Desktop-of-LongPing:~/toybox$ ls -lh ./fweep
-rw-r--r-- 1 weilongping weilongping 428K Mar 23 20:33 ./fweep

Some issue in yes?

~/toybox/generated/testdir$ ./yes | (dd bs=$((1<<16)) count=1 ; dd bs=8192 seek=14 count=1 ; dd bs=4096 seek=64 count=5  ) > testdir/fweep2
0+1 records in
0+1 records out
556 bytes copied, 0.0002737 s, 2.0 MB/s
1+0 records in
1+0 records out
8192 bytes (8.2 kB, 8.0 KiB) copied, 4.3e-05 s, 191 MB/s
dd: warning: partial read (3892 bytes); suggest iflag=fullblock
2+3 records in
2+3 records out
12676 bytes (13 kB, 12 KiB) copied, 8.32e-05 s, 152 MB/s
enh-google commented 2 years ago
id nobody
uid=65534(nobody) gid=65534(nogroup) groups=65534(nogroup)

but tar.test try to use nobody as a group,

skipnot id nobody >/dev/null
testing "pass group" "tar c --owner root --group nobody --mtime @0 file | LST" \

one problem here is that Android has the opposite problem: it doesn't have as clear a user/group distinction as desktop unix, so Android's "nobody" is nobody:nobody, and there is no "nogroup". (our nobody is 9999 too, so you can't cheat and use numbers without a special case either.)

landley commented 2 years ago

The execute bit on the test file tells "make tests" which ones to run and which ones to skip. You can always run them individually, but "make tests" skips a bunch for commands whose tests aren't expected to work portably yet. I switched on the tar tests because I'd just fixed a failure I would have seen earlier if it was in "make tests", and they all passed for me. (And with TEST_HOST=1.)

The nobody user is a cross-distro convention that Ubuntu should support? It's UID 65534, one of only 2 UIDs hardwired into the kernel (for NFS), see https://wiki.ubuntu.com/nobody

It's hard to test tar recording user/group names and IDs without at least one non-root user and group that's expected to be there. :(

landley commented 2 years ago

A large part of the reason I haven't promoted "dd" out of pending yet is I'm trying to work out what the correct behavior is mapping short reads and short writes to dd's block sizes. This failure implies it's not even consistent across distros and/or versions. How nice.

enh-google commented 2 years ago

It's hard to test tar recording user/group names and IDs without at least one non-root user and group that's expected to be there. :(

i thought we'd already had this problem and worked around it, but couldn't find it when i looked this morning... but what i thought we'd done was grep /etc/passwd or /etc/groups for the one we were looking for, and fall back to an Android default if not found? so here we'd search for "nogroup" and fall back to "nobody" (since it seems debian has nobody:nogroup where Android has nobody:nobody)?

landley commented 2 years ago

In the chown and cpio tests, it uses the last user in /etc/passwd as the test user. I'm not sure how to apply that to making known tar test images though?

landley commented 2 years ago

Ok, it looks like debian's tar can --group=name:gid and --owner=name:uid and for testing we need to use that, meaning I need to implement it...

enh-google commented 2 years ago

oh, you can just hard-code some specific number? yeah, that sgtm.

landley commented 2 years ago

In theory it's doing a "skipnot id nobody" before each of these tests, but aparently that's not working for some reason? (Downside of testing failure paths: if it's working for me, I don't see it. In this case, it's testing failure paths IN THE TEST SUITE...)

LongPingWEI commented 2 years ago

In theory it's doing a "skipnot id nobody" before each of these tests, but aparently that's not working for some reason? (Downside of testing failure paths: if it's working for me, I don't see it. In this case, it's testing failure paths IN THE TEST SUITE...)

id checks a uid but not a gid. We need another command to check a gid.

LongPingWEI commented 2 years ago

In theory it's doing a "skipnot id nobody" before each of these tests, but aparently that's not working for some reason? (Downside of testing failure paths: if it's working for me, I don't see it. In this case, it's testing failure paths IN THE TEST SUITE...)

id checks a uid but not a gid. We need another command to check a gid.

@landley https://github.com/landley/toybox/pull/329/commits/366e5c00f628e9346c49218a37299acfc0eec86e getent can test a group name.

landley commented 2 years ago

Sorry for the delay, I've implemented the --owner name:uid --group name:gid functionality locally but I'm doing unrelated board bringup this week so haven't had time/energy to test it and convert the test suite. (I THINK not having any tests that fetch the data from /etc/password is fine here because that would really just be testing lib/ functionality that gets exercised elsewhere. And it is still loading "root" from /etc.)

(Fixing it so the tests run more portably is better than improving the skip logic.)

enh-google commented 2 years ago

hmm... in our recent discussions on the mailing list about groups shared between Linux and macOS, i don't think either of us remembered WSL!

@LongPingWEI --- can you share the low-numbered entries from WSL2's /etc/group file? (just the single-digit groups.)

landley commented 2 years ago

I thought they'd copied ubuntu, but don't have a test system for that available...

LongPingWEI commented 2 years ago

cat /etc/group | grep no nogroup:x:65534: