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. 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?) 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 [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
