On Thu, Feb 29, 2024 at 5:03 PM Rob Landley <r...@landley.net> wrote: > > On 2/28/24 18:23, Oliver Webb via Toybox wrote: > > I've been looking at some of the other pending commands, And found a getopt > > implementation from 2017 by AOSP. > > Looking at the source code, it doesn't seem unclean, nor overly large > > (about 100 lines). > > It does use getopt_long_only, a GNU extension of glibc, but musl has that > > so it will work > > on both glibc and musl. It is GNU compatible too, all test cases pass (even > > for TEST_HOST). > > It's mostly that: > > A) I'm not a shell getopt user and haven't put in the focus to come up to > speed > on it yet, > > B) the shell has a "getopts" (plural) builtin that I need to write, but which > this doesn't share code with. And LOTS of stuff confuses the two. (There's a > comment in this source about "BSD getopts", when this is "getopt".) Of COURSE > the semantics of the two are subtly different. > > C) I'd really wanted to get it to use lib/args.c instead of having two > getopt() > implementations in toybox (one from lib/ and one pulled in from libc). > > Sigh. That said, I've been sitting on this for a long time without doing > anything about it, and the perfect is the enemy of the good. > > The libc version isn't _bad_, it just bloats the static binary pulling in two > implementations of approximately the same code. I don't know if switching it > over to lib/args.c is even possible (the semantics are close, but I'd need one > to be a strict subset of the other), but doing so _after_ merging it wouldn't > be > a much heavier lift than doing it before. (I guess that's what the regression > test suite is for?) > > I should bite the bullet and promote this... > > > There were a few things I changed (printf to xputsn/putchar/xprintf), and > > added a link > > to https://man7.org/linux/man-pages/man1/getopt.1.html, both in the > > attached patch. > > Applied, but I've got some fixups to do on top of this. The help text for one > thing, I dunno how this command works and the help is _not_ explaining it. > (What > does "parse" mean here? Details!) > > The existence of the -T option is a damning indictment of the entire gnu > project. As if we needed more of those. The -s option isn't doing them any > favors either: PICK ONE. But then again, this is gnu we're talking about: > > $ printf %s one two three > onetwothree$ echo -- one two three > -- one two three > > Not known for providing good options, are they? > > > There doesn't seem like a good reason for this command to be in pending, > > are the calls to > > getopt_long_only a problem? > > My lack of domain expertise in it has been the big blocker, plus only very > intermittent pokes about its absence.
yeah, i _almost_ poked you again recently because someone internally noticed the lack of symlink, but when i (just out of interest) asked them what they wanted it for, they never responded. > The man page does not have a usage example. I'm guessing there's a: > > $ for i in $(getopt "$@"); do blah; done > > or maybe > > $ getopt "$@" | while read i; do blah; done > > Hmmm... > > $ for i in $(echo '"one two" three'); do echo $i; done > "one > two" > three > > In what context is this "quote reparsing" supposed to occur? How does "using > eval" as the man page suggests happen _safely_ here? See above for two > attempts > at making "for i in $()" output collate them back together sanely. Sigh, what > does this sucker actually DO... > > $ getopt abc one two three > -- one two three > $ getopt abc one -a two three > -a -- one two three > $ getopt abc one -a two -x three > getopt: invalid option -- 'x' > -a -- one two three > $ getopt a:bc one -a two -x three > getopt: invalid option -- 'x' > -a two -- one three > $ getopt a:bc one -a two three > -a two -- one three > $ getopt a:bc one -a two three; echo $? > -a two -- one three > 0 > $ getopt a:bc one -a two -x three; echo $? > getopt: invalid option -- 'x' > -a two -- one three > 1 > $ getopt a -a > -a -- > > <coulson>So that's what it does.</coulson> > > Still no idea WHY. (And why does it output a leading space?) Do colons work > for > longopts... > > $ getopt -l walrus: abc hello -b --walrus x > -b --walrus 'x' -- 'hello' > $ getopt -l walrus:: abc hello --walrus -b > --walrus '' -b -- 'hello' > > Ok, the help text should PROBABLY MENTION THAT then. No obvious way to tell it > to pass through unknown arguments... > > $ getopt -q a ping -x; echo $? > -- 'ping' > 1 > $ getopt -Q a -x > getopt: invalid option -- 'x' > > And why is 'hello' quoted above but not --walrus? What's the logic... > > $ getopt -l 'wal rus':: abc hello '--wal rus' -b > getopt: unrecognized option '--wal rus' > -b -- 'hello' > > Lovely. That's just... bravo. (Ok, it's quoting any string that nominally > originated after the -- argument.) > > Sigh. Ok, assume there's a userbase out there that wants... "this". I don't > have > to understand WHY. (But I do want to actually DOCUMENT it in the help text, > which... grrr.) > > Why is this getopt_main() have a comment saying no -o means don't quote > output? > > $ getopt -l walrus:: abc hello --walrus -b 'one two' > --walrus '' -b -- 'hello' 'one two' > > Sigh, libc api's with random globals make me sad. (This came up with the time > zone parsing the other day too. You can pass a struct as an argument. But no, > opterr, optind, and optopt.) > > What is with the magic ? here? > > $ getopt -l long:,arg:: 'ab?c' command --long -b there --arg '-?' > --long '-b' --arg '' -- 'command' 'there' > $ echo $? > 1 > > Lovely. Nonzero exit with nothing written to stderr. > > It can ALMOST use error_msg() except the name isn't in which->this. > Frustratingly close. > > WHY is there a comment about BSD getopt? Was this tested on bsd targets before > being tested on Linux targets? Is a BSD workaround a big deal? (Does it > affect mac?) given that i wrote this, it was almost certainly the case that i wrote it on my mac laptop ... but also remember that bionic has a lot of BSD-based code, especially for stuff that "doesn't matter" [that is: that no app developer cares about] like getopt(3). so, yes, "macOS _and_ Android will need that workaround". > Hang on. why doesn't this match: > > } else if (!ch) { > xprintf(" --%s", lopts[i].name); > if (lopts[i].has_arg) out(optarg ? : ""); > } else { > xprintf(" -%c", ch); > if (optarg) out(optarg); > } > > It's checking has_arg for the longopts, but NOT checking it for the short > options? > > $ getopt ab::c -b > -b -- > $ getopt -l x ab::c --x > --x -- > $ getopt ab::c -bx > -b x -- > $ getopt ab::c -b'x d' > -b x d -- > > Ok, optional short arguments need collating (which I implemented in lib/args.c > already), but... why was the output quoted ABOVE but not quoted HERE? > > $ getopt -l x:: ab::c --x > --x '' -- > $ getopt -l abc ab::c boo -b'x d' > -b 'x d' -- 'boo' > > Why does feeding -l an argument longer than one character make a difference to > whether the output is quoted? > > $ getopt -l one:: abc thingy --one potato > --one '' -- 'thingy' 'potato' > $ getopt -l one:: abc thingy --one=potato > --one 'potato' -- 'thingy' > > Ok, optional longopts need to be attached too. Still doesn't explain the > difference in quote output... > > Rob > _______________________________________________ > Toybox mailing list > Toybox@lists.landley.net > http://lists.landley.net/listinfo.cgi/toybox-landley.net _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net