Don't enable gdbtk in testsuite

Message ID 1416903049-30419-1-git-send-email-yao@codesourcery.com
State New, archived
Headers

Commit Message

Yao Qi Nov. 25, 2014, 8:10 a.m. UTC
  When I skim configure.ac and Makefile.in in gdb/testsuite, I happen to
see that directory gdb.gdbtk is added to subdirs, however it doesn't
exist.  gdb/testsuite/gdb.gdbtk was removed by the patch below,

  [rfa] git repo fixup: delete gdb/testsuite/gdb.gdbtk
  http://thread.gmane.org/gmane.comp.gdb.patches/61489

and we should cleanup configure.ac accordingly.

gdb/testsuite:

2014-11-25  Yao Qi  <yao@codesourcery.com>

	* configure.ac: Remove AC_ARG_ENABLE for gdbtk.  Don't invoke
	AC_CONFIG_SUBDIRS(gdb.gdbtk).
	* configure: Re-generated.
---
 gdb/testsuite/configure    | 177 +--------------------------------------------
 gdb/testsuite/configure.ac |  19 -----
 2 files changed, 1 insertion(+), 195 deletions(-)
  

Comments

Joel Brobecker Nov. 30, 2014, 3:23 p.m. UTC | #1
> When I skim configure.ac and Makefile.in in gdb/testsuite, I happen to
> see that directory gdb.gdbtk is added to subdirs, however it doesn't
> exist.  gdb/testsuite/gdb.gdbtk was removed by the patch below,
> 
>   [rfa] git repo fixup: delete gdb/testsuite/gdb.gdbtk
>   http://thread.gmane.org/gmane.comp.gdb.patches/61489
> 
> and we should cleanup configure.ac accordingly.
> 
> gdb/testsuite:
> 
> 2014-11-25  Yao Qi  <yao@codesourcery.com>
> 
> 	* configure.ac: Remove AC_ARG_ENABLE for gdbtk.  Don't invoke
> 	AC_CONFIG_SUBDIRS(gdb.gdbtk).
> 	* configure: Re-generated.

FWIW, the patch looks good to me, so you can go ahead and push.
  
Yao Qi Dec. 1, 2014, 6:15 a.m. UTC | #2
Joel Brobecker <brobecker@adacore.com> writes:

>> gdb/testsuite:
>> 
>> 2014-11-25  Yao Qi  <yao@codesourcery.com>
>> 
>> 	* configure.ac: Remove AC_ARG_ENABLE for gdbtk.  Don't invoke
>> 	AC_CONFIG_SUBDIRS(gdb.gdbtk).
>> 	* configure: Re-generated.
>
> FWIW, the patch looks good to me, so you can go ahead and push.

Hi Joel, thanks for the review.  Patch is pushed in.
  
Pedro Alves Dec. 4, 2014, 5:06 p.m. UTC | #3
On 11/25/2014 08:10 AM, Yao Qi wrote:
> When I skim configure.ac and Makefile.in in gdb/testsuite, I happen to
> see that directory gdb.gdbtk is added to subdirs, however it doesn't
> exist.  gdb/testsuite/gdb.gdbtk was removed by the patch below,
> 
>   [rfa] git repo fixup: delete gdb/testsuite/gdb.gdbtk
>   http://thread.gmane.org/gmane.comp.gdb.patches/61489

That patch removed it from the git repo, mirroring how CVS modules
worked.  In CVS, if you checkout the "gdb" module, you don't get
the gdbtk dirs, but if you checkout the insight module instead, you
get everything gdb, plus the insight bits: src/gdb/gdbtk subdir,
src/gdb/testsuite/gdb.gdbtk/, and maybe other bits.

So removing the testsuite support for gdbtk doesn't seem like
the right thing to do.  Particularly since we still have the
gdbtk bits in gdb/configure.ac.  IOW, I don't see how
src/gdb/testsuite/gdb.gdbtk/ not being around is different
from src/gdb/gdbtk/ not being around.  We should either keep
all support for gdbtk, or remove all of it.

Thanks,
Pedro Alves
  
Yao Qi Dec. 5, 2014, 6:55 a.m. UTC | #4
Pedro Alves <palves@redhat.com> writes:

> That patch removed it from the git repo, mirroring how CVS modules
> worked.  In CVS, if you checkout the "gdb" module, you don't get
> the gdbtk dirs, but if you checkout the insight module instead, you
> get everything gdb, plus the insight bits: src/gdb/gdbtk subdir,
> src/gdb/testsuite/gdb.gdbtk/, and maybe other bits.

Hi Pedro,
I looked at insight and the date of the last commit in
gdb/testsuite/ChangeLog is 2013-10-21.  Looks insight stops updating gdb
after gdb migrates to git so I think it should be safe to remove
testsuite gdbtk from gdb head.

>
> So removing the testsuite support for gdbtk doesn't seem like
> the right thing to do.  Particularly since we still have the
> gdbtk bits in gdb/configure.ac.  IOW, I don't see how
> src/gdb/testsuite/gdb.gdbtk/ not being around is different
> from src/gdb/gdbtk/ not being around.  We should either keep
> all support for gdbtk, or remove all of it.

It is aggressive to remove gdbtk bits from gdb/configure.ac, although
there were some "insight end-of-life" discussions on insight mail list.
I am OK to revert my patch.
  
Pedro Alves Dec. 5, 2014, 9:53 a.m. UTC | #5
On 12/05/2014 06:55 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> That patch removed it from the git repo, mirroring how CVS modules
>> worked.  In CVS, if you checkout the "gdb" module, you don't get
>> the gdbtk dirs, but if you checkout the insight module instead, you
>> get everything gdb, plus the insight bits: src/gdb/gdbtk subdir,
>> src/gdb/testsuite/gdb.gdbtk/, and maybe other bits.
> 
> Hi Pedro,
> I looked at insight and the date of the last commit in
> gdb/testsuite/ChangeLog is 2013-10-21.  Looks insight stops updating gdb
> after gdb migrates to git so I think it should be safe to remove
> testsuite gdbtk from gdb head.

insight's official repo is kind of stuck in a limbo.  CVS is dead,
so that's not where you should be looking.  There's no official git
repo yet, but AIUI, people are using this non-official repo instead:

  https://github.com/monnerat/insight

In any case, I don't see why we should treat insight bits in
gdb/testsuite/configure.ac any different from bits in gdb/configure.ac.
It's exactly the same issue.

>> So removing the testsuite support for gdbtk doesn't seem like
>> the right thing to do.  Particularly since we still have the
>> gdbtk bits in gdb/configure.ac.  IOW, I don't see how
>> src/gdb/testsuite/gdb.gdbtk/ not being around is different
>> from src/gdb/gdbtk/ not being around.  We should either keep
>> all support for gdbtk, or remove all of it.
> 
> It is aggressive to remove gdbtk bits from gdb/configure.ac, although
> there were some "insight end-of-life" discussions on insight mail list.

That topic was raised, but from those discussions it seemed clear
to me that there is still interest in it.  E.g.,

 https://sourceware.org/ml/insight/2014-q4/msg00007.html
 https://sourceware.org/ml/insight/2014-q3/msg00010.html

Personally, I wouldn't be adverse to importing gdbtk into the official
gdb repo.  I don't see how that could hurt -- we tend to worry
about insight anyway when we touch code that might affect it, like
most the deprecated_foo hooks.

> I am OK to revert my patch.

I think we should do that.

Thanks,
Pedro Alves
  
Joel Brobecker Dec. 5, 2014, 10:05 a.m. UTC | #6
> Personally, I wouldn't be adverse to importing gdbtk into the official
> gdb repo.  I don't see how that could hurt -- we tend to worry
> about insight anyway when we touch code that might affect it, like
> most the deprecated_foo hooks.

FWIW - I was hoping insight would be dead by now, but if people
still use it and make updates, then I wouldn't be opposed either.
The one thing to be careful of, I think, is handling of their
commits. Probably just a third email list for insight-specific
files?
  
Yao Qi Dec. 5, 2014, 12:04 p.m. UTC | #7
Pedro Alves <palves@redhat.com> writes:

>> I am OK to revert my patch.
>
> I think we should do that.

My patch is reverted.
  

Patch

diff --git a/gdb/testsuite/configure b/gdb/testsuite/configure
index 15944ed..51dc10e 100755
--- a/gdb/testsuite/configure
+++ b/gdb/testsuite/configure
@@ -552,7 +552,6 @@  PACKAGE_BUGREPORT=
 PACKAGE_URL=
 
 ac_unique_file="gdb.base"
-enable_option_checking=no
 # Factoring default headers for most tests.
 ac_includes_default="\
 #include <stdio.h>
@@ -603,7 +602,6 @@  LDFLAGS
 CFLAGS
 CC
 RPATH_ENVVAR
-subdirs
 SET_MAKE
 GMAKE_FALSE
 GMAKE_TRUE
@@ -662,7 +660,6 @@  SHELL'
 ac_subst_files=''
 ac_user_opts='
 enable_option_checking
-enable_gdbtk
 enable_shared
 '
       ac_precious_vars='build_alias
@@ -674,7 +671,7 @@  LDFLAGS
 LIBS
 CPPFLAGS
 CPP'
-ac_subdirs_all='gdb.gdbtk'
+
 
 # Initialize some variables set by options.
 ac_init_help=
@@ -1287,7 +1284,6 @@  Optional Features:
   --disable-option-checking  ignore unrecognized --enable/--with options
   --disable-FEATURE       do not include FEATURE (same as --enable-FEATURE=no)
   --enable-FEATURE[=ARG]  include FEATURE [ARG=yes]
-  --enable-gtk            enable gdbtk graphical user interface (GUI)
   --enable-shared         build shared libraries deault=yes
 
 Some influential environment variables:
@@ -2212,32 +2208,6 @@  $as_echo "no" >&6; }
 fi
 
 
-# Enable gdbtk.
-# Check whether --enable-gdbtk was given.
-if test "${enable_gdbtk+set}" = set; then :
-  enableval=$enable_gdbtk;
-else
-  if test -d $srcdir/../gdbtk && test -d $srcdir/gdb.gdbtk; then
-    enable_gdbtk=yes
-  else
-    enable_gdbtk=no
-  fi
-fi
-
-# We unconditionally disable gdbtk tests on selected platforms.
-case $host_os in
-  go32* | windows*)
-    enable_gdbtk=no ;;
-esac
-
-# Add gdbtk tests when appropriate.
-if test $enable_gdbtk = yes; then
-
-
-subdirs="$subdirs gdb.gdbtk"
-
-fi
-
 # Enable shared libraries.
 # Check whether --enable-shared was given.
 if test "${enable_shared+set}" = set; then :
@@ -4629,151 +4599,6 @@  if test "$no_create" != yes; then
   # would make configure fail if this is the last instruction.
   $ac_cs_success || as_fn_exit $?
 fi
-
-#
-# CONFIG_SUBDIRS section.
-#
-if test "$no_recursion" != yes; then
-
-  # Remove --cache-file, --srcdir, and --disable-option-checking arguments
-  # so they do not pile up.
-  ac_sub_configure_args=
-  ac_prev=
-  eval "set x $ac_configure_args"
-  shift
-  for ac_arg
-  do
-    if test -n "$ac_prev"; then
-      ac_prev=
-      continue
-    fi
-    case $ac_arg in
-    -cache-file | --cache-file | --cache-fil | --cache-fi \
-    | --cache-f | --cache- | --cache | --cach | --cac | --ca | --c)
-      ac_prev=cache_file ;;
-    -cache-file=* | --cache-file=* | --cache-fil=* | --cache-fi=* \
-    | --cache-f=* | --cache-=* | --cache=* | --cach=* | --cac=* | --ca=* \
-    | --c=*)
-      ;;
-    --config-cache | -C)
-      ;;
-    -srcdir | --srcdir | --srcdi | --srcd | --src | --sr)
-      ac_prev=srcdir ;;
-    -srcdir=* | --srcdir=* | --srcdi=* | --srcd=* | --src=* | --sr=*)
-      ;;
-    -prefix | --prefix | --prefi | --pref | --pre | --pr | --p)
-      ac_prev=prefix ;;
-    -prefix=* | --prefix=* | --prefi=* | --pref=* | --pre=* | --pr=* | --p=*)
-      ;;
-    --disable-option-checking)
-      ;;
-    *)
-      case $ac_arg in
-      *\'*) ac_arg=`$as_echo "$ac_arg" | sed "s/'/'\\\\\\\\''/g"` ;;
-      esac
-      as_fn_append ac_sub_configure_args " '$ac_arg'" ;;
-    esac
-  done
-
-  # Always prepend --prefix to ensure using the same prefix
-  # in subdir configurations.
-  ac_arg="--prefix=$prefix"
-  case $ac_arg in
-  *\'*) ac_arg=`$as_echo "$ac_arg" | sed "s/'/'\\\\\\\\''/g"` ;;
-  esac
-  ac_sub_configure_args="'$ac_arg' $ac_sub_configure_args"
-
-  # Pass --silent
-  if test "$silent" = yes; then
-    ac_sub_configure_args="--silent $ac_sub_configure_args"
-  fi
-
-  # Always prepend --disable-option-checking to silence warnings, since
-  # different subdirs can have different --enable and --with options.
-  ac_sub_configure_args="--disable-option-checking $ac_sub_configure_args"
-
-  ac_popdir=`pwd`
-  for ac_dir in : $subdirs; do test "x$ac_dir" = x: && continue
-
-    # Do not complain, so a configure script can configure whichever
-    # parts of a large source tree are present.
-    test -d "$srcdir/$ac_dir" || continue
-
-    ac_msg="=== configuring in $ac_dir (`pwd`/$ac_dir)"
-    $as_echo "$as_me:${as_lineno-$LINENO}: $ac_msg" >&5
-    $as_echo "$ac_msg" >&6
-    as_dir="$ac_dir"; as_fn_mkdir_p
-    ac_builddir=.
-
-case "$ac_dir" in
-.) ac_dir_suffix= ac_top_builddir_sub=. ac_top_build_prefix= ;;
-*)
-  ac_dir_suffix=/`$as_echo "$ac_dir" | sed 's|^\.[\\/]||'`
-  # A ".." for each directory in $ac_dir_suffix.
-  ac_top_builddir_sub=`$as_echo "$ac_dir_suffix" | sed 's|/[^\\/]*|/..|g;s|/||'`
-  case $ac_top_builddir_sub in
-  "") ac_top_builddir_sub=. ac_top_build_prefix= ;;
-  *)  ac_top_build_prefix=$ac_top_builddir_sub/ ;;
-  esac ;;
-esac
-ac_abs_top_builddir=$ac_pwd
-ac_abs_builddir=$ac_pwd$ac_dir_suffix
-# for backward compatibility:
-ac_top_builddir=$ac_top_build_prefix
-
-case $srcdir in
-  .)  # We are building in place.
-    ac_srcdir=.
-    ac_top_srcdir=$ac_top_builddir_sub
-    ac_abs_top_srcdir=$ac_pwd ;;
-  [\\/]* | ?:[\\/]* )  # Absolute name.
-    ac_srcdir=$srcdir$ac_dir_suffix;
-    ac_top_srcdir=$srcdir
-    ac_abs_top_srcdir=$srcdir ;;
-  *) # Relative name.
-    ac_srcdir=$ac_top_build_prefix$srcdir$ac_dir_suffix
-    ac_top_srcdir=$ac_top_build_prefix$srcdir
-    ac_abs_top_srcdir=$ac_pwd/$srcdir ;;
-esac
-ac_abs_srcdir=$ac_abs_top_srcdir$ac_dir_suffix
-
-
-    cd "$ac_dir"
-
-    # Check for guested configure; otherwise get Cygnus style configure.
-    if test -f "$ac_srcdir/configure.gnu"; then
-      ac_sub_configure=$ac_srcdir/configure.gnu
-    elif test -f "$ac_srcdir/configure"; then
-      ac_sub_configure=$ac_srcdir/configure
-    elif test -f "$ac_srcdir/configure.in"; then
-      # This should be Cygnus configure.
-      ac_sub_configure=$ac_aux_dir/configure
-    else
-      { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: no configuration information is in $ac_dir" >&5
-$as_echo "$as_me: WARNING: no configuration information is in $ac_dir" >&2;}
-      ac_sub_configure=
-    fi
-
-    # The recursion is here.
-    if test -n "$ac_sub_configure"; then
-      # Make the cache file name correct relative to the subdirectory.
-      case $cache_file in
-      [\\/]* | ?:[\\/]* ) ac_sub_cache_file=$cache_file ;;
-      *) # Relative name.
-	ac_sub_cache_file=$ac_top_build_prefix$cache_file ;;
-      esac
-
-      { $as_echo "$as_me:${as_lineno-$LINENO}: running $SHELL $ac_sub_configure $ac_sub_configure_args --cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir" >&5
-$as_echo "$as_me: running $SHELL $ac_sub_configure $ac_sub_configure_args --cache-file=$ac_sub_cache_file --srcdir=$ac_srcdir" >&6;}
-      # The eval makes quoting arguments work.
-      eval "\$SHELL \"\$ac_sub_configure\" $ac_sub_configure_args \
-	   --cache-file=\"\$ac_sub_cache_file\" --srcdir=\"\$ac_srcdir\"" ||
-	as_fn_error "$ac_sub_configure failed for $ac_dir" "$LINENO" 5
-    fi
-
-    cd "$ac_popdir"
-  done
-fi
 if test -n "$ac_unrecognized_opts" && test "$enable_option_checking" != no; then
   { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: unrecognized options: $ac_unrecognized_opts" >&5
 $as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;}
diff --git a/gdb/testsuite/configure.ac b/gdb/testsuite/configure.ac
index 9dccc9f..a1efc31 100644
--- a/gdb/testsuite/configure.ac
+++ b/gdb/testsuite/configure.ac
@@ -36,25 +36,6 @@  esac
 AM_CONDITIONAL(GMAKE, test "$MAKE_IS_GNU" = yes)
 AC_PROG_MAKE_SET
 
-# Enable gdbtk.
-AC_ARG_ENABLE(gdbtk,
-[  --enable-gtk            enable gdbtk graphical user interface (GUI)],,
-  [if test -d $srcdir/../gdbtk && test -d $srcdir/gdb.gdbtk; then
-    enable_gdbtk=yes
-  else
-    enable_gdbtk=no
-  fi])
-# We unconditionally disable gdbtk tests on selected platforms.
-case $host_os in
-  go32* | windows*)
-    enable_gdbtk=no ;;
-esac
-
-# Add gdbtk tests when appropriate.
-if test $enable_gdbtk = yes; then
-   AC_CONFIG_SUBDIRS(gdb.gdbtk)
-fi
-
 # Enable shared libraries.
 AC_ARG_ENABLE(shared,
 [  --enable-shared         build shared libraries [deault=yes]],,