Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [review] Simplify Python checks in configure.ac
@ 2019-10-23 22:04 Christian Biesinger (Code Review)
  2019-10-24 13:44 ` Simon Marchi
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-10-23 22:04 UTC (permalink / raw)
  To: gdb-patches; +Cc: Christian Biesinger

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(-)



diff --git a/gdb/config.in b/gdb/config.in
index da365a7..a76ac9f 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -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
 
diff --git a/gdb/configure b/gdb/configure
index 289c91b..dcd6141 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -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
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])
   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

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [review] Simplify Python checks in configure.ac
  2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
@ 2019-10-24 13:44 ` Simon Marchi
  2019-10-24 13:52 ` Simon Marchi (Code Review)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Simon Marchi @ 2019-10-24 13:44 UTC (permalink / raw)
  To: cbiesinger, gdb-patches, Christian Biesinger (Code Review)

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [review] Simplify Python checks in configure.ac
  2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
  2019-10-24 13:44 ` Simon Marchi
@ 2019-10-24 13:52 ` Simon Marchi (Code Review)
  2019-10-24 13:54   ` Simon Marchi
  2019-10-24 18:45 ` Tom Tromey (Code Review)
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi (Code Review) @ 2019-10-24 13:52 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches

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.



-- 
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-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Thu, 24 Oct 2019 13:52:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [review] Simplify Python checks in configure.ac
  2019-10-24 13:52 ` Simon Marchi (Code Review)
@ 2019-10-24 13:54   ` Simon Marchi
  0 siblings, 0 replies; 8+ messages in thread
From: Simon Marchi @ 2019-10-24 13:54 UTC (permalink / raw)
  To: gnutoolchain-gerrit, Christian Biesinger, gdb-patches

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [review] Simplify Python checks in configure.ac
  2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
  2019-10-24 13:44 ` Simon Marchi
  2019-10-24 13:52 ` Simon Marchi (Code Review)
@ 2019-10-24 18:45 ` Tom Tromey (Code Review)
  2019-10-24 18:48 ` [pushed] " Sourceware to Gerrit sync (Code Review)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey (Code Review) @ 2019-10-24 18:45 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches; +Cc: Simon Marchi

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.


-- 
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-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Thu, 24 Oct 2019 18:44:44 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pushed] Simplify Python checks in configure.ac
  2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
                   ` (3 preceding siblings ...)
  2019-10-24 18:48 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-10-24 18:48 ` Sourceware to Gerrit sync (Code Review)
  2019-10-24 18:50 ` [review v2] " Christian Biesinger (Code Review)
  5 siblings, 0 replies; 8+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-10-24 18:48 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches; +Cc: Tom Tromey, Simon Marchi

Sourceware to Gerrit sync has submitted this change.

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-24  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/ChangeLog
M gdb/config.in
M gdb/configure
M gdb/configure.ac
4 files changed, 28 insertions(+), 94 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 887c7fb..0545f4a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-10-24  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.
+
 2019-10-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* python/py-progspace.c (pspy_block_for_pc): Return None for all
diff --git a/gdb/config.in b/gdb/config.in
index da365a7..a76ac9f 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -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
 
diff --git a/gdb/configure b/gdb/configure
index 289c91b..dcd6141 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -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
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])
   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

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I07073870d9040c2bc8519882c8b3c1368edd4513
Gerrit-Change-Number: 241
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: merged


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pushed] Simplify Python checks in configure.ac
  2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
                   ` (2 preceding siblings ...)
  2019-10-24 18:45 ` Tom Tromey (Code Review)
@ 2019-10-24 18:48 ` Sourceware to Gerrit sync (Code Review)
  2019-10-24 18:48 ` Sourceware to Gerrit sync (Code Review)
  2019-10-24 18:50 ` [review v2] " Christian Biesinger (Code Review)
  5 siblings, 0 replies; 8+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-10-24 18:48 UTC (permalink / raw)
  To: Christian Biesinger, Tom Tromey, gdb-patches; +Cc: Simon Marchi

The original change was created by Christian Biesinger.

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-24  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/ChangeLog
M gdb/config.in
M gdb/configure
M gdb/configure.ac
4 files changed, 28 insertions(+), 94 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 887c7fb..0545f4a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-10-24  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.
+
 2019-10-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* python/py-progspace.c (pspy_block_for_pc): Return None for all
diff --git a/gdb/config.in b/gdb/config.in
index da365a7..a76ac9f 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -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
 
diff --git a/gdb/configure b/gdb/configure
index 289c91b..dcd6141 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -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
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])
   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

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I07073870d9040c2bc8519882c8b3c1368edd4513
Gerrit-Change-Number: 241
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [review v2] Simplify Python checks in configure.ac
  2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
                   ` (4 preceding siblings ...)
  2019-10-24 18:48 ` Sourceware to Gerrit sync (Code Review)
@ 2019-10-24 18:50 ` Christian Biesinger (Code Review)
  5 siblings, 0 replies; 8+ messages in thread
From: Christian Biesinger (Code Review) @ 2019-10-24 18:50 UTC (permalink / raw)
  To: Christian Biesinger, gdb-patches; +Cc: Tom Tromey, Simon Marchi

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.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I07073870d9040c2bc8519882c8b3c1368edd4513
Gerrit-Change-Number: 241
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Thu, 24 Oct 2019 18:50:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-10-24 18:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23 22:04 [review] Simplify Python checks in configure.ac Christian Biesinger (Code Review)
2019-10-24 13:44 ` Simon Marchi
2019-10-24 13:52 ` Simon Marchi (Code Review)
2019-10-24 13:54   ` Simon Marchi
2019-10-24 18:45 ` Tom Tromey (Code Review)
2019-10-24 18:48 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-10-24 18:48 ` Sourceware to Gerrit sync (Code Review)
2019-10-24 18:50 ` [review v2] " Christian Biesinger (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox