[Toybox] Did I mention the release?

Rob Landley rob at landley.net
Sat Mar 21 12:16:00 PDT 2015


On 03/20/2015 12:42 AM, enh wrote:
> On Thu, Mar 19, 2015 at 6:52 PM, Rob Landley <rob at landley.net> wrote:
>> On 03/15/2015 08:47 PM, enh wrote:
>>> as a particularly observant follower of my AOSP gerrit activity might
>>> discern, from the fact i committed a small improvement to the
>>> _tool_box ps last week, i'm not expecting to switch to the toybox ps
>>> in the near future.
>>
>> I wouldn't switch to what we've got right now either. That's why I'm
>> fixing it. :)
>>
>> That said, I should look at toolbox ps and see what _it_ does, because
>> it's at least as much an example as ubuntu.
> 
> i think the most important thing is to implement -o well.

Yeah, -o is what I'm wrestling with. For most fields, the column name
and the -o value are case-smashed versions of each other, and "nice/NI"
and "comm/COMMAND" are abbreviations of each other (in opposite
directions), but then you get "etime/ELAPSED" and... Grrr.

There is a cleanup to be done here. Making it not ugly is... tricky.

I'm leaning towards just matching "longest common initial segment" with
an early stop searching at complete matches, but I _can't_ have a
minimum length requirement because etime/elapsed have _one_ letter in
common. (Luckily they're the only "e" at the moment, but still.) And
that means typos can give unexpected results instead of errors which is
nasty. (I can at least continue searching if it didn't end and complain
if it was ambiguous, ala "we only matched the initial p and there's 3 of
those...")

On the other hand, I still need to special case args/COMMAND and if
etime is the other special case then requiring one of the strings to
_end_ to give me a match...

This is why simple things take so long, when there's a better way on the
tip of my tongue but I can't figure out what it _is_...

> other than
> -Z which Ubuntu has too, i think the only oddity that we wouldn't be
> better off without is --abi. right now developers often care about
> whether a process is 32 or 64 bit.

Is making abi an -o column acceptable?

> the main behavioral oddity is that
> it defaults to showing all processes (because people are usually doing
> "adb shell ps" rather than "ps")

Is having "alias" commands in /etc/profile or similar a sufficient fix
for this sort of thing?

> and the last unrecognized argument
> that don't start '-' or with a digit is taken as a "name filter" ---

What if you get more than one unrecognized argument that doesn't start
with a digit?

I'm ok with implementing "pgrep" and "ps" in the same toys/*/filename.c
and having the pgrep functionality available to ps. (Would having the
filter be interpreted as a regex cause problems, do you think?)

> only processes whose COMM or NAME exactly matches are shown. i suspect
> that this is an exact match means it's not used all that much, but
> it's hard to know without taking it away and seeing if anyone
> complains. i might be reduced to that :-(

I like figuring out how to fit special cases into a plausibly deniable
"I meant to do that" general case. (Sometimes the elegant solution is to
back up and solve a bigger problem.)

> it might be worth my while introducing toybox's pgrep as soon as
> possible and encourage people to switch over.

Which is also in pending.

Lemme merge pgrep into the ps cleanup, and have unrecognized ps
arguments... oh, that's nasty. Is "ps ax" unreognized? Hmmm. It's a
thing people are going to try to do...

(I think I can make this work, I just have to figure out what behavior
we _want_. I guess start the option string with "?&" and presumably
lib/args.c will handle it? The fiddly bit is that nondash arguments and
dash arguments have _different_meanings_ and we don't currently record
whether or not we got a dash because things like tar don't care. Hmmm...
I don't think one flagset maps cleanly onto the other either, so
rejecting _invalid_ flags becomes tricky...)

>> I really want to genericize lib/lib.c:human_readable() so it can
>> reproduce the variants we need, and factor out the column spacing code.
>>
>> What would really help here is test cases. (Extract this tarball, cd in
>> and ls -h, and the output should look like this textfile...)
> 
> a tarball or function for all tests to use to test with is a great
> idea. it would make it easier to have interesting cases available for
> all the commands to be tested against. (dangling symlinks, empty and
> non-empty directories, non-regular files, ...)

Indeed. Right now I've hijacked blkid's filesystem images for a couple
other tests that just need a known file, but a tarball with different
kinds of stuff in it would be nice.

Unfortunately, this opens up the "test as root" can of worms because
"device nodes" are an obvious thing to put in there, so possibly _two_
tarballs, one "root" and one "not root".

(Hmmm, would moving the blkid files into said tarball make sense?
Unfortunately, uncompressed they're huge, the f2fs one alone is 128
megs. Maybe if I get tar extracting sparse files by default?)

In case it isn't obvious, the pending directory isn't the only one I
need to do a cleanup/extend/rewrite pass over. The test suite could use
several months of fulltime poking.

>>> toolbox/top.c
>>>   output, options, and behavior all differ enough that any switchover
>>> is going to be interesting. punt for now, worry about these later.
>>
>> I would very much like to unify top, htop, and iotop (but haven't looked
>> into how the third is implemented). Also there's the possibility of
>> common code with vmstat and ps.
>>
>> The other thing is that "top" does terminal control similar to "less".
>> And "watch" should do the same, and ps does a _little_ of the same.
>> (They query the terminal size and truncate output at screen width and
>> screen height.) So my plan was to genericize _that_ code, in a way that
>> works with shell command line history editing and vi.
>>
>> None of which is directly relevant to you, you just want it to work. But
>> this is why promoting it is harder than it looks...
> 
> for me "as good as the desktop" is a long-term goal that can take
> several releases.

Unfortunately "as good as the desktop" is under-specified. Take Red Hat
6-ish circa 1999: that was a desktop, was that good enough? The current
commands are much much bigger and more elaborate, but are they enough
better to justify the bloat? (Answer: it depends.)

I intend to keep improving things after 1.0, but "crazy things gnu did"
is not an interesting thing to chase. (I also consider interface churn
to be a cost, just like complexity. Which means some things aren't worth
changing even if the result would be an improvement because we break
people's scripts, and the scripts in their heads. For example, I type
"ps ax" on busybox systems all the time, it doesn't parse that but it
doesn't complain either.)

> "better than toolbox" is enough for right now. it
> pains me to waste time on things like WCHAN for toolbox ps when i
> could have been doing that for toybox ps instead, but until we're
> switched over...

Working on it. :)

>>> toolbox/renice.c
>>>   ours has nonstandard -r, -t (equivalent to -n?), -g (“get”); do we need these?
>>
>> Android is as least a much a standard as gnu. If you've got existing
>> users who will miss the options, patches welcome.
> 
> things like renice are tricky because they're not heavily used. i need
> to work out first whether the extra stuff is useful or just random.
> (one particular problem is that people have added things that weren't
> quite what they really wanted because it was easier to do that and
> good enough for what they needed to do, than it was to implement the
> full standard option they'd have used if it had been available.)

If you do work it out, please document the hell out of it. All toybox
commands have complete --help as condition of promotion, I've spent more
time on help text than on command implementation more than once.

>> Possibly I should have a "how to make testcases" section in my toybox
>> talk on wednesday. A good test suite really helps development. (Alas, as
>> I've learned, what counts as good is another thing apparently requiring
>> some explanation...)
> 
> (my [lame] excuse is that even with my patch the output still isn't
> _exactly_ the desktop output, and i'm not sure how anal we should be
> about that.)

I'm way behind on testsuite work too.

>>> toolbox/uptime.c
>>>   Android's output format is completely different (all about time
>>> spent idle/sleeping, not concurrent users or processes); not clear
>>> that merging makes sense.

(I note our uptime is producing what ubuntu produces. Producing what
toolbox produces isn't hard.)

>> I should take a look. (Is there documentation on any of the android
>> commands? Each toybox command at least produces --help output, and the
>> test suite is gives a gazillion usage examples, or at least _should_...)
> 
> no. most of the toolbox commands don't/didn't even have _usage_
> output. the built-in help is one of my favorite things about toybox.
> (it was when i was trying to show it off this morning that i found the
> "top --help" bug i sent the patch for. so that was embarrassing.)

I'm tellin ya, toys/pending is there for a reason. :)

> Android's uptime looks like this:
> 
> $ adb shell uptime
> up time: 04:05:17, idle time: 08:00:53, sleep time: 00:00:00
>
> how would you offer both formats?

Add an uptime -a, and use alias in .profile the same way ls --color is
generally aliased?

/proc/uptime gives uptime and idle time. Easy enough to grab:

  if (readfile("/proc/uptime", toybuf, 4096) < 0 ||
      2 != sscanf(toybuf, "%f %f", uptime, idle))
        perror_exit("/proc/uptime");

Not sure how you get sleep time, hmmm...

Your uptime.c is ignoring the uptime from /proc and fetching "elapsed"
as either clock_gettime(CLOCK_BOOTTIME) or an
ioctl(ANDROID_ALARM_ELAPSED_REALTIME) on /dev/alarm. Why is it doing
that? (In fs/proc/uptime.c uptime_proc_show() is doing
get_monotonic_boottime(), which is what sysinfo is also getting but only
with second granularity...)

It looks like sleep is uptime from /proc (defined as "wall clock time"
in Documentation/filesystems/proc.txt) minus gettime(CLOCK_MONOTONIC)
which is presumably "jiffies spent" or some such.

Hmmm... it's test cases that are hard. I _think_ the vanilla kernel
should provide an api for all the right clock variants now, so in theory
I can do this with the if/else stack in the android implementation, but
since the old one is using android specific apis to fetch stuff I'd have
to get an android device with an android kernel and put it to sleep for
a bit (I should be able to wget a static toybox into that champion
gnuroot thing on my nexus 5, but what counts as "sleep" here? When the
screen is off?)

> or were you just thinking that we'd have a big if (ANDROID)?

While I _could_ have a CONFIG entry for UPTIME_DEFAULT_A (so -a is the
default and supplying it switches it off), and even have a compile time
probe to set its default value based on whether or not we're building
against bionic... it wouldn't be my first choice.

If you sit down at toybox, it should behave like toybox, otherwise users
are confused. Each deviation from that needs to be justified.

> about the only code the two formats would
> share would be the duration formatting. and Android doesn't have
> anything like utmp, so it's not obvious we could show the regular form
> output even if we wanted to.

Eh, half the command's data collection. The output is:

$ ./uptime
01:09:18 up 53 days, 43 min,  170 users,  load average: 5.94, 4.04, 2.55

The bit at the start is:

// Time
xprintf(" %02d:%02d:%02d up ", now->tm_hour, now->tm_min, now->tm_sec);

And doing three variants of that is just a for loop based off 3
different pointers, with the other output format as an else.


>> Toybox was always intended to work alongside other packagesin the same
>> $PATH, but toybox is also explicitly designed to allow forks to add new
>> commands unobtrusively. Not only is each command self-contained (a trick
>> I had to explain in person Denys Vlasenko at the 2010 ELC before he
>> started doing it in busybox), but the directories are self-contained too.
>>
>> So if you want a locally-maintained directory of android commands, all
>> you have to do is:
>>
>> 1) mkdir toys/android
>> 2) echo "Android" > toys/android/README
>> 3) cp mycommand.c toys/android
>> 4) make distclean
>> 5) make defconfig
>> 6) make
>>
>> (Once upon a time there was a directory list in scripts/genconfig.sh to
>> manually control the ordering, but I ripped it back out and just did a
>> "sort -r" to put posix first and examples last and called it good enough.)
> 
> oh, great. i hadn't actually looked; i'd just assumed there was a
> hard-coded list.

There was. I decided it was a bad idea and removed it to make life
easier for exactly this use case. :)

(Sort of the opposite of the busybox approach. I want this code to be
_useful_.)

Note that the command namespace is actually flat, so if I add some
random thing like "fstrim" to toys/other and you've got one in
toys/android, your build will break. (You can locally delete the
duplicate out of toys/other, but making git not conflict is left as an
exercise to the user.) So there's still _some_ incentive to get stuff
upstream, but even if you don't, if you only resync each release the
cleanup's trivial.

(And as always lib/ may change a bit over the years and break out of
tree commands. I don't try to do it, but I can't _not_ do it if I can't
test the command. Fixup's probably pretty obvious after a bisect, though.)

>> As for additions to the lib directory, the build just grabs lib/*.c and
>> then lets "-ffunction-sections -fdata-sections -Wl,--gc-sections" sort
>> it out, so you can add a lib/android.c and as long as the names don't
>> conflict life is good. (I'm aware your build doesn't, but presumably
>> adjusting your build is easier for you than for me. :)
> 
> yes, given that i'd prefer not to have differences from upstream
> (which is why i'm not committing any of my own changes locally; i'm
> merging them back if/when they make it upstream) it's useful to me
> that your build will just pick up extra lib/*.c files too. (i usually
> test on the host first.)

I'm all for it. :)

Sorry I'm slow at merging, as you can see from above just _discussing_
this stuff spawns design tangents...

>> Adding stuff to lib/lib.h is bad (merge conflicts), but you could
>> #include "toys/android/lib.h" after the #include "toys.h" in your local
>> commands, and have the library header live there.
>>
>> (You could even build with just the android commands enabled, mv toybox
>> toolbox, and point symlinks at that during the install if all you wanted
>> was the toybox infrastructure but wanted to keep it separate. Or use
>> scripts/single.sh or the new "make change" target, although I dunno how
>> that translates to your build infrastructure.)
> 
> no, there's no particular benefit to keeping things separate. although
> if long term you plan on having a toys/android upstream i might want
> to go with toys/android-pending or something so i can keep the two
> sets straight.

If you would like to submit android-specific commands just so they could
use the toybox infrastructure, I'd be ok with a toys/android directory.
I would probably have the things in it default "n" in defconfig, though.

I need to set up a local bionic test build, but the half hour I spent
poking at it just confirmed that bionic hasn't got a makefile that lets
it build out of tree, and extracting it from the whole android build is
another one of those problems best explained by linking to xkcd:

https://xkcd.com/214/

>> I am behind on my reading for android.
>> Possibly these commands have different permissions... selinux extended
>> attributes? Or maybe setprop is only a debuging command or something?)
> 
> no. everything in toolbox is in one binary (with the exception of r),
> so they're all the same command as far as SELinux is concerned.
> setprop works by talking to the property service over a socket, and
> that uses SO_PEERCRED, and then there's some kind of SELinux lookup,
> and there are extra checks for stuff like the "ro.*" properties, and
> ...

You see my problem!

I need to reread this thread after my california trip and update the
roadmap. Quite POSSIBLY what I need is a slushpile.html where instead of
"why this command should go in" I have a list of commands and notes
about what I plan to do with each command. (Brief implementation details
like the above, "why this is hard", "what's blocking it", "remember to
handle these cases"...)

>>> i also plan on sticking with BSD grep for now (we can worry about the
>>> quality of the BSD regex implementation in bionic and fleshing out the
>>> missing grep features in toybox when everything else is settled):
>>
>> No rush. The "regex|regex" thing with multiple grep -e is something I
>> need to figure out how to deal with anyway. (The problem is | only means
>> something in extended regexes. I had a patch to musl to allow \| to work
>> in non-extended regexes, but then they rewrote their regex plumbing. I
>> may have to break down and do multiple passes over the data, or maybe
>> extend and factor out the ghostwheel() function in sed.c.)
> 
> (BSD just tries them all in order, separately.)

And if they overlap?

Somebody linked me to a post about the old grep being fast by ignoring
lines breaks and operating on large data blocks instead (ideally
mmap)... Ah:

https://lists.freebsd.org/pipermail/freebsd-current/2010-August/019310.html

And my reply after reading that was basically "I'm not doing that".
Locality of reference and avoiding multiple passes over the data still
seems like a good idea, but iterating over readline() is a common
pattern. (What I worry about there is "readline returns a gigabyte".
There's no length limit on the line, and no obvious way of _adding_ a
length limit that doesn't boil down to not properly handling long lines...)

*shrug* I did "fast" and "slow" modes for tail (mmap vs stream) because
the stream mode is _so_ pathologically slow when dealing with large
files, but you can't have only the mmap mode because not everything is
mmapable. I could presumably do mmap() and non-mmap() modes for grep
too. But given that the non-mmap() mode isn't pathological there, I lean
against bothering until somebody really complains...

(What might be better in this case was a getline() variant that returned
a chunk of mmapped file, start pointer and length. I designed toybox sed
to work with start and length so it could handle built-in nulls,
although A) I don't think that part's finished, B) it's probably also
assuming each segment is null terminated because there's no way to get
start/length semantics out of regex() and copying the data into a
temporary buffer would kill performance. And of course mmap() backing
store means you don't free() the result _and_ that on a 32 bit system
you run out of virtual address space when dealing with large files...)

Anyway, there's potential things to do here, but after 1.0...

Rob

 1426965360.0


More information about the Toybox mailing list