Re: [PATCH 2/2] kvm-unit-tests: configure changes for illumos.

[Sean Christopherson <seanjc@google.com>]

Adding the official KUT maintainers, they undoubtedly know more about the getopt
stuff than me.

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:

  Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
  instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
  to do frotz", as if you are giving orders to the codebase to change
  its behaviour.


E.g.

  Exempt illumos, which reports itself as SunOS, from the `getopt -T` check
  for enhanced getopt.   blah blah blah... 

> by not probing for, `getopt -T` (illumos `getopt` supports the
> required functionality, but exits with a different return status
> when invoked with `-T`).
> 
> Signed-off-by: Dan Cross <cross@oxidecomputer.com>
> ---
>  configure | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> 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`?  Unless it's a direct collision with
the "old" getopt, why not check for illumos' return?  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...).

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.

>      echo "Enhanced getopt is not available, add it to your PATH?"
>      exit 1
>  fi
> -- 
> 2.31.1
>