On Fri, May 13, 2022 at 10:35 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> Adding the official KUT maintainers, they undoubtedly know more about the getopt
> stuff than me.
Thanks.
> On Fri, May 13, 2022, Dan Cross wrote:
> > This change modifies the `configure` script to run under illumos
>
> Nit, use imperative mood. KUT follows the kernel's rules/guidelines for the most
> part. From Linux's Documentation/process/submitting-patches.rst:
> [snip]
Done locally, but see below.
> > diff --git a/configure b/configure
> > index 86c3095..7193811 100755
> > --- a/configure
> > +++ b/configure
> > @@ -15,6 +15,7 @@ objdump=objdump
> > ar=ar
> > addr2line=addr2line
> > arch=$(uname -m | sed -e 's/i.86/i386/;s/arm64/aarch64/;s/arm.*/arm/;s/ppc64.*/ppc64/')
> > +os=$(uname -s)
> > host=$arch
> > cross_prefix=
> > endian=""
> > @@ -317,9 +318,9 @@ EOF
> > rm -f lib-test.{o,S}
> > fi
> >
> > -# require enhanced getopt
> > +# require enhanced getopt everywhere except illumos
> > getopt -T > /dev/null
> > -if [ $? -ne 4 ]; then
> > +if [ $? -ne 4 ] && [ "$os" != "SunOS" ]; then
>
> What does illumos return for `getopt -T`?
Sadly, it returns "0". I was wrong in my earlier explorations
because I did not realize that `configure` does not use `getopt`
aside from that one check, which is repeated in `run_tests.sh`.
I would argue that the most straight-forward way to deal with
this is to just remove the check for "getopt" from "configure",
which doesn't otherwise use "getopt". The only place it is
used is in `run_tests.sh`, which is unlikely to be used directly
for illumos, and repeats the check anyway.
> Unless it's a direct collision with
> the "old" getopt, why not check for illumos' return?
It collides with "legacy" getopt. :-(
> The SunOS check could be
> kept (or not). E.g. IMO this is much more self-documenting (though does $? get
> clobbered by the check? I'm terrible at shell scripts...).
I think it is more or less moot now, but "$?" does get clobbered
by the check.
> if [ $? -ne 4 ] && [ "$os" != "SunOS" || $? -ne 666 ]; then
>
> Test if your getopt(1) is this enhanced version or an old version. This
> generates no output, and sets the error status to 4. Other implementations of
> getopt(1), and this version if the environment variable GETOPT_COMPATIBLE is
> set, will return '--' and error status 0.
Sigh. That's precisely what the illumos version does. :-(
But it's perhaps worth pointing out that `getopt` isn't used
by `configure` aside from just that check. Does it, perhaps,
make more sense just to remove it from `configure` entirely?
- Dan C.
