[review] Simplify Python checks in configure.ac
Commit Message
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241
......................................................................
Simplify Python checks in configure.ac
The version checking code is not necessary. It is only used to define
HAVE_LIBPYTHON2_6 or HAVE_LIBPYTHON2_7, which is not used anywhere.
If a version check is desired, the PY_{MAJOR,MINOR}_VERSION macro from
the Python headers can be (and is) used, which does not require updating
configure.ac whenever a new Python version is released.
gdb/ChangeLog:
2019-10-23 Christian Biesinger <cbiesinger@google.com>
* config.in: Regenerate.
* configure: Regenerate.
* configure.ac: Remove the code that uses sed to get the python
version and defines HAVE_LIBPYTHON2_6 / HAVE_LIBPYTHON2_7.
Change-Id: I07073870d9040c2bc8519882c8b3c1368edd4513
---
M gdb/config.in
M gdb/configure
M gdb/configure.ac
3 files changed, 21 insertions(+), 94 deletions(-)
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I07073870d9040c2bc8519882c8b3c1368edd4513
Gerrit-Change-Number: 241
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-MessageType: newchange
Comments
On 2019-10-23 6:04 p.m., Christian Biesinger (Code Review) wrote:
> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241
> ......................................................................
>
> Simplify Python checks in configure.ac
>
> The version checking code is not necessary. It is only used to define
> HAVE_LIBPYTHON2_6 or HAVE_LIBPYTHON2_7, which is not used anywhere.
>
> If a version check is desired, the PY_{MAJOR,MINOR}_VERSION macro from
> the Python headers can be (and is) used, which does not require updating
> configure.ac whenever a new Python version is released.
This is a test comment in the commit message.
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index d929b89..f11dccd 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -691,19 +691,17 @@
> # --------------------- #
>
> dnl Utility to simplify finding libpython.
> -dnl $1 = pythonX.Y
> -dnl $2 = the shell variable to assign the result to
> +dnl $1 = the shell variable to assign the result to
> dnl If libpython is found we store $version here.
> -dnl $3 = additional flags to add to CPPFLAGS
> -dnl $4 = additional flags to add to LIBS
> +dnl $2 = additional flags to add to CPPFLAGS
> +dnl $3 = additional flags to add to LIBS
>
> AC_DEFUN([AC_TRY_LIBPYTHON],
> [
> - version=$1
> - define([have_libpython_var],$2)
> - new_CPPFLAGS=$3
> - new_LIBS=$4
> - AC_MSG_CHECKING([for ${version}])
> + define([have_libpython_var],$1)
> + new_CPPFLAGS=$2
> + new_LIBS=$3
> + AC_MSG_CHECKING([for python])
This is a test comment in configure.ac.
Simon
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241
......................................................................
Patch Set 1:
(1 comment)
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241/1/gdb/configure.ac
File gdb/configure.ac:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241/1/gdb/configure.ac@694
PS1, Line 694: dnl $1 = the shell variable to assign the result to
This is a test comment.
On 2019-10-24 9:52 a.m., Simon Marchi (Code Review) wrote:
> Simon Marchi has posted comments on this change.
>
> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241
> ......................................................................
>
>
> Patch Set 1:
>
> (1 comment)
>
> https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241/1/gdb/configure.ac
> File gdb/configure.ac:
>
> https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241/1/gdb/configure.ac@694
> PS1, Line 694: dnl $1 = the shell variable to assign the result to
> This is a test comment.
This is a test reply to the test comment.
Simon
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241
......................................................................
Patch Set 1: Code-Review+2
Thank you. I think this is ok.
Christian Biesinger has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/241
......................................................................
Patch Set 2:
> Patch Set 1:
>
> Thanks, that LGTM.
>
> It seems like the `python_has_threads` stuff is unused too...
Oops, I did not see your comment. Yeah it looks like python_has_threads has been unused since 404f29021abaef86a341663444fb069eb1f0282a, I'll make a separate patch for that.
@@ -246,12 +246,6 @@
/* Define if you have the mpfr library. */
#undef HAVE_LIBMPFR
-/* Define if Python 2.6 is being used. */
-#undef HAVE_LIBPYTHON2_6
-
-/* Define if Python 2.7 is being used. */
-#undef HAVE_LIBPYTHON2_7
-
/* Define to 1 if you have the <libunwind-ia64.h> header file. */
#undef HAVE_LIBUNWIND_IA64_H
@@ -10439,32 +10439,12 @@
have_libpython=no
if test "${have_python_config}" = yes; then
- # Determine the Python version by extracting "-lpython<version>"
- # part of the python_libs. <version> is usually X.Y with X and Y
- # being decimal numbers, but can also be XY (seen on Windows).
- #
- # The extraction is performed using sed with a regular expression.
- # Initially, the regexp used was using the '?' quantifier to make
- # the dot in the version number optional. Unfortunately, this
- # does not work with non-GNU versions of sed because, because of
- # what looks like a limitation (the '?' quantifier does not work
- # with back-references). We work around this limitation by using
- # the '*' quantifier instead. It means that, in theory, we might
- # match unexpected version strings such as "-lpython2..7", but
- # this seems unlikely in practice. And even if that happens,
- # an error will be triggered later on, when checking that version
- # number.
- python_version=`echo " ${python_libs} " \
- | sed -e 's,^.* -l\(python[0-9]*[.]*[0-9]*\).*$,\1,'`
- case "${python_version}" in
- python*)
- version=${python_version}
new_CPPFLAGS=${python_includes}
new_LIBS=${python_libs}
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ${version}" >&5
-$as_echo_n "checking for ${version}... " >&6; }
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for python" >&5
+$as_echo_n "checking for python... " >&6; }
save_CPPFLAGS=$CPPFLAGS
save_LIBS=$LIBS
CPPFLAGS="$CPPFLAGS $new_CPPFLAGS"
@@ -10482,7 +10462,7 @@
}
_ACEOF
if ac_fn_c_try_link "$LINENO"; then :
- have_libpython=${version}
+ have_libpython=yes
found_usable_python=yes
PYTHON_CPPFLAGS=$new_CPPFLAGS
PYTHON_LIBS=$new_LIBS
@@ -10494,20 +10474,14 @@
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: ${found_usable_python}" >&5
$as_echo "${found_usable_python}" >&6; }
- ;;
- *)
- as_fn_error $? "unable to determine python version from ${python_libs}" "$LINENO" 5
- ;;
- esac
elif test "${have_python_config}" != failed; then
if test "${have_libpython}" = no; then
- version=python2.7
new_CPPFLAGS=${python_includes}
new_LIBS="-lpython2.7 ${python_libs}"
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ${version}" >&5
-$as_echo_n "checking for ${version}... " >&6; }
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for python" >&5
+$as_echo_n "checking for python... " >&6; }
save_CPPFLAGS=$CPPFLAGS
save_LIBS=$LIBS
CPPFLAGS="$CPPFLAGS $new_CPPFLAGS"
@@ -10525,7 +10499,7 @@
}
_ACEOF
if ac_fn_c_try_link "$LINENO"; then :
- have_libpython=${version}
+ have_libpython=yes
found_usable_python=yes
PYTHON_CPPFLAGS=$new_CPPFLAGS
PYTHON_LIBS=$new_LIBS
@@ -10540,12 +10514,11 @@
fi
if test "${have_libpython}" = no; then
- version=python2.6
new_CPPFLAGS=${python_includes}
new_LIBS="-lpython2.6 ${python_libs}"
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for ${version}" >&5
-$as_echo_n "checking for ${version}... " >&6; }
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for python" >&5
+$as_echo_n "checking for python... " >&6; }
save_CPPFLAGS=$CPPFLAGS
save_LIBS=$LIBS
CPPFLAGS="$CPPFLAGS $new_CPPFLAGS"
@@ -10563,7 +10536,7 @@
}
_ACEOF
if ac_fn_c_try_link "$LINENO"; then :
- have_libpython=${version}
+ have_libpython=yes
found_usable_python=yes
PYTHON_CPPFLAGS=$new_CPPFLAGS
PYTHON_LIBS=$new_LIBS
@@ -10577,15 +10550,6 @@
fi
fi
- if test "${have_libpython}" = python2.7 -o "${have_libpython}" = python27; then
-
-$as_echo "#define HAVE_LIBPYTHON2_7 1" >>confdefs.h
-
- elif test "${have_libpython}" = python2.6 -o "${have_libpython}" = python26; then
-
-$as_echo "#define HAVE_LIBPYTHON2_6 1" >>confdefs.h
-
- fi
if test "${have_libpython}" = no; then
case "${with_python}" in
@@ -691,19 +691,17 @@
# --------------------- #
dnl Utility to simplify finding libpython.
-dnl $1 = pythonX.Y
-dnl $2 = the shell variable to assign the result to
+dnl $1 = the shell variable to assign the result to
dnl If libpython is found we store $version here.
-dnl $3 = additional flags to add to CPPFLAGS
-dnl $4 = additional flags to add to LIBS
+dnl $2 = additional flags to add to CPPFLAGS
+dnl $3 = additional flags to add to LIBS
AC_DEFUN([AC_TRY_LIBPYTHON],
[
- version=$1
- define([have_libpython_var],$2)
- new_CPPFLAGS=$3
- new_LIBS=$4
- AC_MSG_CHECKING([for ${version}])
+ define([have_libpython_var],$1)
+ new_CPPFLAGS=$2
+ new_LIBS=$3
+ AC_MSG_CHECKING([for python])
save_CPPFLAGS=$CPPFLAGS
save_LIBS=$LIBS
CPPFLAGS="$CPPFLAGS $new_CPPFLAGS"
@@ -711,7 +709,7 @@
found_usable_python=no
AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include "Python.h"]],
[[Py_Initialize ();]])],
- [have_libpython_var=${version}
+ [have_libpython_var=yes
found_usable_python=yes
PYTHON_CPPFLAGS=$new_CPPFLAGS
PYTHON_LIBS=$new_LIBS])
@@ -868,47 +866,18 @@
have_libpython=no
if test "${have_python_config}" = yes; then
- # Determine the Python version by extracting "-lpython<version>"
- # part of the python_libs. <version> is usually X.Y with X and Y
- # being decimal numbers, but can also be XY (seen on Windows).
- #
- # The extraction is performed using sed with a regular expression.
- # Initially, the regexp used was using the '?' quantifier to make
- # the dot in the version number optional. Unfortunately, this
- # does not work with non-GNU versions of sed because, because of
- # what looks like a limitation (the '?' quantifier does not work
- # with back-references). We work around this limitation by using
- # the '*' quantifier instead. It means that, in theory, we might
- # match unexpected version strings such as "-lpython2..7", but
- # this seems unlikely in practice. And even if that happens,
- # an error will be triggered later on, when checking that version
- # number.
- python_version=`echo " ${python_libs} " \
- | sed -e 's,^.* -l\(python[[0-9]]*[[.]]*[[0-9]]*\).*$,\1,'`
- case "${python_version}" in
- python*)
- AC_TRY_LIBPYTHON(${python_version}, have_libpython,
- ${python_includes}, ${python_libs})
- ;;
- *)
- AC_MSG_ERROR([unable to determine python version from ${python_libs}])
- ;;
- esac
+ AC_TRY_LIBPYTHON(have_libpython,
+ ${python_includes}, ${python_libs})
elif test "${have_python_config}" != failed; then
if test "${have_libpython}" = no; then
- AC_TRY_LIBPYTHON(python2.7, have_libpython,
+ AC_TRY_LIBPYTHON(have_libpython,
${python_includes}, "-lpython2.7 ${python_libs}")
fi
if test "${have_libpython}" = no; then
- AC_TRY_LIBPYTHON(python2.6, have_libpython,
+ AC_TRY_LIBPYTHON(have_libpython,
${python_includes}, "-lpython2.6 ${python_libs}")
fi
fi
- if test "${have_libpython}" = python2.7 -o "${have_libpython}" = python27; then
- AC_DEFINE(HAVE_LIBPYTHON2_7, 1, [Define if Python 2.7 is being used.])
- elif test "${have_libpython}" = python2.6 -o "${have_libpython}" = python26; then
- AC_DEFINE(HAVE_LIBPYTHON2_6, 1, [Define if Python 2.6 is being used.])
- fi
if test "${have_libpython}" = no; then
case "${with_python}" in