Patchwork gdb/configure.ac: add --enable-source-highlight

login
register
mail settings
Submitter Sergei Trofimovich
Date March 13, 2019, 9:50 p.m.
Message ID <20190313215020.11959-1-slyfox@gentoo.org>
Download mbox | patch
Permalink /patch/31848/
State New
Headers show

Comments

Sergei Trofimovich - March 13, 2019, 9:50 p.m.
Allow disabling source-highlight dependency autodetection even
it exists in the system. More details on problem of automatic
dependencies:
https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies

Noticed by Jeroen Roovers in https://bugs.gentoo.org/680238

gd/ChangeLog Sergei Trofimovich <slyfox@gentoo.org>
	* configure.ac: add --enable-source-highlight switch.
	* configure: Regenerate.
---
 gdb/ChangeLog    |  5 +++++
 gdb/configure    | 38 +++++++++++++++++++++++++++++++-------
 gdb/configure.ac | 33 ++++++++++++++++++++++++++-------
 3 files changed, 62 insertions(+), 14 deletions(-)
Tom Tromey - March 15, 2019, 2:18 p.m.
>>>>> "Sergei" == Sergei Trofimovich <slyfox@gentoo.org> writes:

Sergei> Allow disabling source-highlight dependency autodetection even
Sergei> it exists in the system. More details on problem of automatic
Sergei> dependencies:
Sergei> https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies

Sergei> Noticed by Jeroen Roovers in https://bugs.gentoo.org/680238

Hi.  Thank you for the patch.

This looks essentially fine to me.  Do you have a copyright assignment
in place?  (Though I wonder whether one is really needed, given that the
actual change here is pretty small.)

This probably could use some minor follow-on patches, e.g. for "show
configuration", but I'm happy to do those.

Tom
Sergei Trofimovich - March 17, 2019, 10:47 p.m.
On Fri, 15 Mar 2019 08:18:42 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Sergei" == Sergei Trofimovich <slyfox@gentoo.org> writes:  
> 
> Sergei> Allow disabling source-highlight dependency autodetection even
> Sergei> it exists in the system. More details on problem of automatic
> Sergei> dependencies:
> Sergei> https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies  
> 
> Sergei> Noticed by Jeroen Roovers in https://bugs.gentoo.org/680238  
> 
> Hi.  Thank you for the patch.
> 
> This looks essentially fine to me.  Do you have a copyright assignment
> in place?  (Though I wonder whether one is really needed, given that the
> actual change here is pretty small.)

Yes, my company has the assignment. Will resend with according email
address in v2.

> This probably could use some minor follow-on patches, e.g. for "show
> configuration", but I'm happy to do those.

I can have a stab at it as well. Anything else worth adding besides
"show configuration"? I'll send v2 meanwhile.
Sergio Durigan Junior - March 18, 2019, 3:52 a.m.
On Sunday, March 17 2019, Sergei Trofimovich wrote:

> On Fri, 15 Mar 2019 08:18:42 -0600
> Tom Tromey <tom@tromey.com> wrote:
>> This probably could use some minor follow-on patches, e.g. for "show
>> configuration", but I'm happy to do those.
>
> I can have a stab at it as well. Anything else worth adding besides
> "show configuration"? I'll send v2 meanwhile.

Thanks, Sergei.

Something Keith and I were talking about the other day is adding a
command line switch to enable/disable source highlight, so the user can
do:

  gdb --disable-source-highlight ...

  (or some such)

WDYT?

Thanks,
Tom Tromey - March 18, 2019, 4:01 p.m.
>>>>> "Sergei" == Sergei Trofimovich <slyfox@gentoo.org> writes:

>> This probably could use some minor follow-on patches, e.g. for "show
>> configuration", but I'm happy to do those.

Sergei> I can have a stab at it as well. Anything else worth adding besides
Sergei> "show configuration"? I'll send v2 meanwhile.

It would be good to update the documentation.  gdb.texinfo has a section
on configuration, and there is also some text in the README, though
without looking I don't know if it needs to be updated or not.

Tom
Tom Tromey - March 18, 2019, 4:01 p.m.
>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> Something Keith and I were talking about the other day is adding a
Sergio> command line switch to enable/disable source highlight, so the user can
Sergio> do:

Sergio>   gdb --disable-source-highlight ...

Sergio>   (or some such)

Sergio> WDYT?

FWIW you can "set style source off" now.

Tom
Eli Zaretskii - March 18, 2019, 4:45 p.m.
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sun, 17 Mar 2019 23:52:51 -0400
> 
> Something Keith and I were talking about the other day is adding a
> command line switch to enable/disable source highlight, so the user can
> do:
> 
>   gdb --disable-source-highlight ...
> 
>   (or some such)

We already have a variable, which you can set from your ~/.gdbinit,
isn't that enough?
Sergio Durigan Junior - March 18, 2019, 5:10 p.m.
On Monday, March 18 2019, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
>> Date: Sun, 17 Mar 2019 23:52:51 -0400
>> 
>> Something Keith and I were talking about the other day is adding a
>> command line switch to enable/disable source highlight, so the user can
>> do:
>> 
>>   gdb --disable-source-highlight ...
>> 
>>   (or some such)
>
> We already have a variable, which you can set from your ~/.gdbinit,
> isn't that enough?

In my experience as a GDB user, it is.  But I'm thinking about people
who, for some reason, wouldn't like to keep (or don't know about) a
~/.gdbinit, and would like to disable the feature from the start.  It's
more convenient for the user to do:

  gdb --disable-source-highlight ...

  (The parameter could be named in a different, more concise way, of
  course)

than to do:

  echo 'set style source off' >> ~/.gdbinit
  gdb ...

or:

  gdb -ex 'set style source off' ...

  (IME, it's common for a user to not know about -ex)

But anyway, I don't want to bikeshed over this.  If it's a hard thing to
do, I'm totally fine with keeping things as is.

Thanks,
Eli Zaretskii - March 18, 2019, 5:56 p.m.
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: slyfox@gentoo.org,  tom@tromey.com,  gdb-patches@sourceware.org
> Date: Mon, 18 Mar 2019 13:10:01 -0400
> 
> But anyway, I don't want to bikeshed over this.  If it's a hard thing to
> do, I'm totally fine with keeping things as is.

No bikeshedding wanted here as well.

I don't think this is hard, it just strikes me as strange that we
introduce a feature, and then immediately devise no less than 3 ways
to disable it.

But I don't have strong feelings either.

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 24de651bf9..35b66f02a6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2019-03-13  Sergei Trofimovich <slyfox@gentoo.org>
+
+	* configure.ac: add --enable-source-highlight switch.
+	* configure: Regenerate.
+
 2019-03-13  Tom Tromey  <tromey@adacore.com>
 
 	* i386-gnu-nat.c (i386_gnu_nat_target::fetch_registers)
diff --git a/gdb/configure b/gdb/configure
index f2d271e23a..15a96afcca 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -879,6 +879,7 @@  with_mpfr
 with_libmpfr_prefix
 with_python
 with_guile
+enable_source_highlight
 enable_libmcheck
 with_intel_pt
 with_libipt_prefix
@@ -1554,6 +1555,8 @@  Optional Features:
   --enable-profiling      enable profiling of GDB
   --enable-codesign=CERT  sign gdb with 'codesign -s CERT'
   --disable-rpath         do not hardcode runtime library paths
+  --enable-source-highlight
+                          enable source-highlight for source listings
   --enable-libmcheck      Try linking with -lmcheck if available
   --enable-werror         treat compile warnings as errors
   --enable-build-warnings enable build-time compiler warnings if gcc is used
@@ -11393,13 +11396,30 @@  fi
 
 SRCHIGH_LIBS=
 SRCHIGH_CFLAGS=
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for the source-highlight library" >&5
+
+# Check whether --enable-source-highlight was given.
+if test "${enable_source_highlight+set}" = set; then :
+  enableval=$enable_source_highlight; case "${enableval}" in
+  yes)  enable_source_highlight=yes ;;
+  no)   enable_source_highlight=no  ;;
+  *)    as_fn_error $? "bad value ${enableval} for source-highlight option" "$LINENO" 5 ;;
+esac
+else
+  enable_source_highlight=auto
+fi
+
+
+if test "${enable_source_highlight}" != "no"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for the source-highlight library" >&5
 $as_echo_n "checking for the source-highlight library... " >&6; }
-if test "${pkg_config_prog_path}" = "missing"; then
-   { $as_echo "$as_me:${as_lineno-$LINENO}: result: no - pkg-config not found" >&5
+  if test "${pkg_config_prog_path}" = "missing"; then
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: no - pkg-config not found" >&5
 $as_echo "no - pkg-config not found" >&6; }
-else
-   if ${pkg_config_prog_path} --exists source-highlight; then
+    if test "${enable_source_highlight}" = "yes"; then
+      as_fn_error $? "pkg-config was not found in your system" "$LINENO" 5
+    fi
+  else
+    if ${pkg_config_prog_path} --exists source-highlight; then
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
 
@@ -11407,10 +11427,14 @@  $as_echo "#define HAVE_SOURCE_HIGHLIGHT 1" >>confdefs.h
 
       { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
 $as_echo "yes" >&6; }
-   else
+    else
       { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
 $as_echo "no" >&6; }
-   fi
+      if test "${enable_source_highlight}" = "yes"; then
+        as_fn_error $? "source-highlight was not found in your system" "$LINENO" 5
+      fi
+    fi
+  fi
 fi
 
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 8ddd0fda61..1318c8d008 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1220,19 +1220,38 @@  AM_CONDITIONAL(HAVE_GUILE, test "${have_libguile}" != no)
 
 SRCHIGH_LIBS=
 SRCHIGH_CFLAGS=
-AC_MSG_CHECKING([for the source-highlight library])
-if test "${pkg_config_prog_path}" = "missing"; then
-   AC_MSG_RESULT([no - pkg-config not found])
-else
-   if ${pkg_config_prog_path} --exists source-highlight; then
+
+AC_ARG_ENABLE(source-highlight,
+  AS_HELP_STRING([--enable-source-highlight],
+    [enable source-highlight for source listings]),
+  [case "${enableval}" in
+  yes)  enable_source_highlight=yes ;;
+  no)   enable_source_highlight=no  ;;
+  *)    AC_MSG_ERROR(bad value ${enableval} for source-highlight option) ;;
+esac],
+[enable_source_highlight=auto])
+
+if test "${enable_source_highlight}" != "no"; then
+  AC_MSG_CHECKING([for the source-highlight library])
+  if test "${pkg_config_prog_path}" = "missing"; then
+    AC_MSG_RESULT([no - pkg-config not found])
+    if test "${enable_source_highlight}" = "yes"; then
+      AC_MSG_ERROR([pkg-config was not found in your system])
+    fi
+  else
+    if ${pkg_config_prog_path} --exists source-highlight; then
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
       AC_DEFINE([HAVE_SOURCE_HIGHLIGHT], 1,
                 [Define to 1 if the source-highlight library is available])
       AC_MSG_RESULT([yes])
-   else
+    else
       AC_MSG_RESULT([no])
-   fi
+      if test "${enable_source_highlight}" = "yes"; then
+        AC_MSG_ERROR([source-highlight was not found in your system])
+      fi
+    fi
+  fi
 fi
 AC_SUBST(SRCHIGH_LIBS)
 AC_SUBST(SRCHIGH_CFLAGS)