Re: [PATCH] autoconf: fix handling absolute $PYTHON path

[Ian Jackson <iwj@xxxxxxxxxxxxxx>]

Marek Marczykowski-Górecki writes ("[PATCH] autoconf: fix handling absolute 
$PYTHON path"):
> Don't strip full path from $PYTHON variable. This is especially
> relevant, if it points outside of $PATH. This is the case
> for RPM build on CentOS 8 (%{python3} macro points at
> /usr/libexec/platform-python).
> 
> For this reason, adjust also python-config handling - AC_PATH_PROG
> doesn't work on already absolute path, so make it conditional.

Sorry for the delay replying and thanks for trying to improve this
area.

> -AC_PATH_PROG([pyconfig], [$PYTHON-config], [no])
> +AS_IF([echo "$PYTHON" | grep -q "^/"], [
> +    pyconfig="$PYTHON-config"
> +], [
> +    AC_PATH_PROG([pyconfig], [$PYTHON-config], [no])
> +])

I'm not sure this logic is right.  I haven't looked at this area in
detail but it seems confusing to me.  I don't quite understand why the
preexisting code calls AC_CHECK_PROG followed by AC_PATH_PROG.

I also don't understand why we ever need an absolute path for
$PYTHON-config.  Why don't we just rely on PATH lookups when in invoke
it ?

> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -368,7 +368,6 @@ AS_IF([test -z "$PYTHON"], [AC_CHECK_PROGS([PYTHON], 
> [python3 python python2], e
>  AS_IF([test "$PYTHON" = "err"], [AC_MSG_ERROR([No python interpreter 
> found])])
>  AS_IF([echo "$PYTHON" | grep -q "^/"], [], [AC_PATH_PROG([PYTHON], 
> [$PYTHON])])
>  PYTHONPATH=$PYTHON
> -PYTHON=`basename $PYTHONPATH`

I'm not sure this is right.  I think we sometimes try to look at
PTYHON to see if we should be doing python-3-like things or
python-2-like things, and maybe that logic depends on PYTHON just
being the basename.

Contrary to what I said about leaving $PYTHON-config unresolved and
expecting it to be looked up at the time of use, maybe the right fix
is simply to change python_devel.m4 to use $PYTHONPATH-config instead.

Also using echo | grep -q ^/ seems poor style when case is available.
I think we can rely on case.  But I see that's in the old code
already.

As you can tell, trying to write down what I think doesn't seem to
have unconfused me.  Maybe you can explain ?  Or maybe I need more
coffee.

Thanks,
Ian.