[Toybox] [PATCH] Seperate 'userdel' from testing syntax

[enh]

On Wed, Feb 18, 2015 at 11:10 AM, Rob Landley <rob at landley.net> wrote:
> On 01/30/2015 05:55 PM, enh wrote:
>> On Fri, Jan 30, 2015 at 2:53 PM, Rob Landley <rob at landley.net> wrote:
>>> On 01/30/2015 01:56 AM, Yeongdeok Suh wrote:
>>>> Hi, all,
>>>>
>>>> When I test toybox with toybox/tests/*.test scripts,
>>>> I got many false FAILs from it. So, I tried to fix useradd.test file.
>>>> What I fixed are as below.
>>>
>>> This week has been entirely eaten by $DAYJOB having me do kernel stuff.
>>> I hope to catch up a bit this weekend...
>>>
>>>> 1. I added the checking routine at the head of file whether it is run by
>>>> root.
>>>> useradd.test should be PASSed only by root user.
>>>
>>> Existing tests (ifconfig, losetup, chgrp, chown) output "SKIPPED"
>>> instead of FAIL. (Not running the test as root isn't a failure per say,
>>> it just means we couldn't perform this test in this context. That way
>>> when we run "make test" to test all the commands enabled in the current
>>> config, we don't get spurious failures.)
>>>
>>>> 2. I Separated 'userdel' from 'testing' syntax.
>>>> in existing code,
>>>>
>>>> /testing "adduser user_name (text)" "useradd toyTestUser $arg ||/
>>>> /   grep '^toyTestUser:' /etc/passwd $arg && test -d /home/toyTestUser &&/
>>>> /   *userdel toyTestUser $arg && rm -rf /home/toyTestUser &&* echo 'yes'" \/
>>>> /  "yes\n" "" "$pass" /
>>>>
>>>> */userdel toyTestUser $arg && rm -rf /home/toyTestUser/* is not related
>>>> with useradd directly. But if userdel or rm occur error, this test case
>>>> is FAILed. So I moved this part out of testing syntax.
>>>
>>> It's a cleanup step, yes.
>>>
>>>> Could you share your opinion about this patch?
>>>
>>> useradd and userdel are still in pending, in part because Android uses a
>>> different userlist mechanism than /etc/passwd files and I need to do
>>> some research on the correct way to handle that.
>>>
>>> Oddly, enh (the Android guy) sent us this patch:
>>>
>>> https://www.mail-archive.com/toybox@lists.landley.net/msg01664.html
>>>
>>> Which reaches out and touches /etc/passwd for testing chown. It's on my
>>> list of things to think about in a lot more depth than I've had time to yet.
>>
>> oops! sorry to have caused confusion...
>>
>> right now i don't have any good way to run your tests on Android. i
>> need to make a "simon says"-style mess with "adb shell" (at least at
>> run time; hopefully not actually in the test scripts themselves), and
>> haven't got round to doing that yet.
>
> I need to upgrade android toysh to the point where both the toybox build
> and the toybox test suite can run under the toybox shell.
>
> (Always a goal, just... kind of a big job.)
>
> I poked at the mirbsd guys online, but their license doesn't allow their
> code to be merged into toybox as is (yes, I'm a stickler for this sort
> of thing), and they have no interest in offering different license terms
> to the code. (Alas, would have saved some time to rebase and start
> fixing things up from there.)
>
> I've recently been poking at Aboriginal Linux (since part of my toybox
> release testing is building linux from scratch under that) and I
> recently found out that the bash 2.05b I've been using in my Aboriginal
> Linux project won't run the new parallel toybox build scripts (where
> "new" is like 6 months ago now). So the "rebuild itself under itself"
> thing is broken until I either fix up bash or get toysh to the point I
> can swap it in.
>
>> so right now if i make a code
>> change, i actually run the tests on the host (with either bionic or
>> glibc, depending on what exactly i'm trying to test).
>
> I need to set up a bionic host test environment. I want to add that to
> my standard regression testing. Alas, its build is an android.mk and I
> dunno how to run that outside of the giant AOSP build blob.
>
>> on the device i
>> usually just rely on manual testing. (in this specific case, i rewrote
>> the file using "root" and "shell" for Android.)
>
> You could also add a hook to scripts/runtest.sh replacing the "eval" on
> line 86 with a function similar to the do_loudly function starting on
> scripts/make.sh line 19.
>
> What does an adb remote invocation look like? Anything like testing via
> ssh? I've vaguely pondered adding a "passwordly ssh to remote machine
> and run the test there" thing so I could set up an aboriginal linux
> powerpc instance, do a static cross compile build and run the tests
> remotely inside the emulated instance with the host's test script
> driving things.

so that's the first problem... you give adb more credit than it
deserves. of its many misfeatures, pertinent ones here are: it won't
work in a pipe, it won't return the exit status of the command it ran,
and it will add a \r to every line of output. i need to fix all of
those, but none of them have become the most urgent fire yet, and no
one's sent completely convincing patches. facebook wrote their own adb
client (but it's an extended subset and doesn't support Windows [or
the Mac?] so it's not really a replacement).

anyway... something like this lets you run stuff like the pwd tests on
a connected Android device. ($ANDROID_SERIAL is how you tell adb which
Android device to talk to if there are several connected, without
having to specify it on every command line. since you'd want to set
that anyway before running the tests, that seemed reasonable.)

diff --git a/scripts/runtest.sh b/scripts/runtest.sh
index 8da1089..c4244a8 100644
--- a/scripts/runtest.sh
+++ b/scripts/runtest.sh
@@ -83,8 +83,16 @@ testing()

   echo -ne "$3" > expected
   echo -ne "$4" > input
-  echo -ne "$5" | eval "$2" > actual
-  RETVAL=$?
+  if [ -n "$ANDROID_SERIAL" ]
+  then
+    adb shell "echo -ne '$5' | eval '$2' > /data/local/tmp/actual ;
echo $? > /data/local/tmp/retval"
+    adb pull /data/local/tmp/actual .
+    adb pull /data/local/tmp/retval .
+    RETVAL=`cat retval`
+  else
+    echo -ne "$5" | eval "$2" > actual
+    RETVAL=$?
+  fi

   cmp expected actual > /dev/null 2>&1
   if [ $? -ne 0 ]


> (Alas, a generic solution would also require a shared filesystem since
> the test script is doing mkdir and such to set up temporary files to
> test on. But I could probably botch something up...)

...but, yeah, this is a problem. any test script that does some file
system setup is a problem.

one possibility i've wondered about is factoring that out into a
script of its own. this would have two advantages: 1. only one place
to fix all this stuff for Android/ssh/whatever and 2. less boilerplate
in each test script. if there was some known file system tree for
commands to work on.

>> as i've said, Android doesn't have /etc/passwd (or /etc/group), and it
>> doesn't have setpwent/getpwent/endpwent (or equivalents). you *can* do
>> uid/gid or name lookups though, because they do make some degree of
>> sense. (so id(1) works.) i've considered implementing getpwent so that
>> it would cycle through the well known users, but we don't actually
>> have an example of anything that would use it, and it's really not
>> obvious that we'd be doing anyone any favors --- code calling getpwent
>> that wants to run on Android needs to think long and hard about
>> exactly what it means by that, and whether it makes any sense at all.
>
> This I'm still trying to wrap my head around. Most of the android
> educational resources I find assume you're programming an app in java.
>
> Even if all your uid/gid pairs are dynamically allocated, can't you have
> an app that requests and tracks uid/gid pairs, and assigns names for
> them? Ok, they wouldn't be sequential (although the containers
> infrastructure can remap them so they could _appear_ sequential if it
> really mattered because somebody wanted to run a cifs fileserver or
> something)...

no, there's no API for any of that. i doubt non-system apps even have
the ability to read the relevant databases.

>> so going back to your "in pending ... because Android", i think you
>> should just ignore us as far as adduser/userdel are concerned. i don't
>> think there's anything useful you can do.
>
> I've heard that before. :)
>
> It still sounds fixable. Containers are awesome. Pity the crazy systemd
> guys keep trying to hog the infrastructure over in legacy linux land,
> but one big advantage android has is that the systemd being gpl keeps it
> _off_ of android, so you don't have to get any of "devfsd-tng" on you
> and can await the train wreck with popcorn...

i'm not saying the implementation won't ever change, but afaik no one
is working on it. your best hope is for someone to decide that there's
a big enough security advantage to make it worth the disruption.

>> the fact that Android tends to be pretty locked down with SELinux
>> means it's going to be tricky to test a lot of toybox things, but
>> getting to a point where i can run the test suite is definitely a
>> goal. just not a priority (because i can run the tests on the desktop
>> with bionic).
>
> I'm all for having a billion unadministered full posix systems with
> broadband access being tightly locked down, but I'm far more a fan of
> the containers approach (make the entire system nest so you can contain
> the damage) than SELinux's whack-a-mole complexity. It would be nice if
> there could be some kind of migration path eventually, but again I would
> know where to start...
>
>>> There are some minor hiccups with your patch (&> redirects both stdout
>>> and stderr, and then you redirect stderr again?) but it's better than
>>> what was there. I'll probably apply it this weekend. (Bured by $DAYJOB
>>> right now...)
>
> Did I remember to do that?
>
> Sigh. No, I did not. Ok, now it's in.
>
> Thanks,
>
> Rob

 1424454397.0