Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Sergio Durigan Junior <	sergiodj@redhat.com>
To: GDB Patches <gdb-patches@sourceware.org>
Cc: Pedro Alves <palves@redhat.com>
Subject: Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
Date: Sun, 23 Sep 2018 03:56:00 -0000	[thread overview]
Message-ID: <8736u027f4.fsf@redhat.com> (raw)
In-Reply-To: <20180917202242.28583-1-sergiodj@redhat.com> (Sergio Durigan	Junior's message of "Mon, 17 Sep 2018 16:22:42 -0400")

On Monday, September 17 2018, I wrote:

> This is a follow-up of:
>
>   https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html

Ping.

> Instead of going throttle and always enabling our selftests (even in
> non-development builds), this patch is a bit more conservative and
> introduces a configure option ("--enable-unit-tests") that allows the
> user to choose whether she wants unit tests in the build or not.  Note
> that the current behaviour is retained: if no option is provided, GDB
> will have selftests included in a development build, and will *not*
> have selftests included in a non-development build.
>
> The rationale for having this option is still the same: due to the
> many racy testcases and random failures we see when running the GDB
> testsuite, it is unfortunately not possible to perform a full test
> when one is building a downstream package.  As the Fedora GDB
> maintainer and one of the Debian GDB uploaders, I feel like this
> situation could be improved by, at least, executing our selftests
> after the package has been built.
>
> This patch introduces no regressions to our build.
>
> OK?
>
> gdb/ChangeLog:
> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* README (`configure' options): Add documentation for new
> 	"--enable-unit-tests" option.
> 	* configure: Regenerate.
> 	* configure.ac: Add "--enable-unit-tests" option.
> 	* maint.c (maintenance_selftest): Update message informing
> 	that selftests have been disabled.
> 	(maintenance_info_selftests): Likewise.
>
> gdb/gdbserver/ChangeLog:
> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* configure: Regenerate.
> 	* configure.ac: Add "--enable-unit-tests" option.
> 	* configure.srv: Use "$enable_unittests" instead of
> 	"$development" when checking whether unit tests have been
> 	enabled.
> 	* server.c (captured_main): Update message informing that
> 	selftests have been disabled.
>
> gdb/testsuite/ChangeLog:
> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* gdb.gdb/unittest.exp: Update expected message informing that
> 	selftests have been disabled.
> 	* gdb.server/unittest.exp: Likewise.
> ---
>  gdb/README                            |  6 ++++++
>  gdb/configure                         | 23 ++++++++++++++++++++++-
>  gdb/configure.ac                      | 17 ++++++++++++++++-
>  gdb/gdbserver/configure               | 23 ++++++++++++++++++++++-
>  gdb/gdbserver/configure.ac            | 17 ++++++++++++++++-
>  gdb/gdbserver/configure.srv           |  2 +-
>  gdb/gdbserver/server.c                |  2 +-
>  gdb/maint.c                           |  4 ++--
>  gdb/testsuite/gdb.gdb/unittest.exp    |  2 +-
>  gdb/testsuite/gdb.server/unittest.exp |  2 +-
>  10 files changed, 88 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/README b/gdb/README
> index e43887ffcd..e6fe3b183d 100644
> --- a/gdb/README
> +++ b/gdb/README
> @@ -524,6 +524,12 @@ prefer; but you may abbreviate option names if you use `--'.
>       after being built, the location of the system-wide init file will
>       be adjusted accordingly. 
>  
> +`--enable-unit-tests[=yes|no]'
> +     Enable (i.e., include) support for unit tests when compiling GDB
> +     and GDBServer.  Note that if this option is not passed, GDB will
> +     have selftests if it is a development build, and will *not* have
> +     selftests if it a non-development build.
> +
>  `configure' accepts other options, for compatibility with configuring
>  other GNU tools recursively; but these are the only options that affect
>  GDB or its supporting libraries.
> diff --git a/gdb/configure b/gdb/configure
> index d92a256f1f..18e04c0c50 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -895,6 +895,7 @@ enable_sim
>  enable_gdbserver
>  with_babeltrace
>  with_libbabeltrace_prefix
> +enable_unit_tests
>  '
>        ac_precious_vars='build_alias
>  host_alias
> @@ -1559,6 +1560,8 @@ Optional Features:
>    --enable-sim            link gdb with simulator
>    --enable-gdbserver      automatically build gdbserver (yes/no/auto, default
>                            is auto)
> +  --enable-unit-tests     Enable the inclusion of unit tests when compiling
> +                          GDB
>  
>  Optional Packages:
>    --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
> @@ -17731,7 +17734,25 @@ ac_config_links="$ac_config_links $ac_config_links_1"
>  $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
>  
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +# Check whether --enable-unit-tests was given.
> +if test "${enable_unit_tests+set}" = set; then :
> +  enableval=$enable_unit_tests; case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
> +esac
> +else
> +  enable_unittests=$development
> +fi
> +
> +
> +if $enable_unittests; then
>  
>  $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
>  
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index e38604cb65..3ef43a95f4 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -2267,7 +2267,22 @@ dnl  At the moment, we just assume it's UTF-8.
>  AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
>            [Define to be a string naming the default host character set.])
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +AC_ARG_ENABLE(unit-tests,
> +AS_HELP_STRING([--enable-unit-tests],
> +[Enable the inclusion of unit tests when compiling GDB]),
> +[case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
> +esac], [enable_unittests=$development])
> +
> +if $enable_unittests; then
>    AC_DEFINE(GDB_SELF_TEST, 1,
>              [Define if self-testing features should be enabled])
>    CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
> diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
> index f5cbbaea78..9915e212d7 100755
> --- a/gdb/gdbserver/configure
> +++ b/gdb/gdbserver/configure
> @@ -722,6 +722,7 @@ enable_option_checking
>  enable_maintainer_mode
>  enable_largefile
>  enable_libmcheck
> +enable_unit_tests
>  with_ust
>  with_ust_include
>  with_ust_lib
> @@ -1367,6 +1368,8 @@ Optional Features:
>                            sometimes confusing) to the casual installer
>    --disable-largefile     omit support for large files
>    --enable-libmcheck      Try linking with -lmcheck if available
> +  --enable-unit-tests     Enable the inclusion of unit tests when compiling
> +                          GDB
>    --enable-werror         treat compile warnings as errors
>    --enable-build-warnings enable build-time compiler warnings if gcc is used
>    --enable-gdb-build-warnings
> @@ -5889,7 +5892,25 @@ fi
>    fi
>  
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +# Check whether --enable-unit-tests was given.
> +if test "${enable_unit_tests+set}" = set; then :
> +  enableval=$enable_unit_tests; case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
> +esac
> +else
> +  enable_unittests=$development
> +fi
> +
> +
> +if $enable_unittests; then
>    srv_selftest_objs="common/selftest.o"
>  
>  $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
> diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
> index 99bc46221c..3c6ed9143f 100644
> --- a/gdb/gdbserver/configure.ac
> +++ b/gdb/gdbserver/configure.ac
> @@ -54,7 +54,22 @@ else
>  fi
>  GDB_AC_LIBMCHECK(${libmcheck_default})
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +AC_ARG_ENABLE(unit-tests,
> +AS_HELP_STRING([--enable-unit-tests],
> +[Enable the inclusion of unit tests when compiling GDB]),
> +[case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
> +esac], [enable_unittests=$development])
> +
> +if $enable_unittests; then
>    srv_selftest_objs="common/selftest.o"
>    AC_DEFINE(GDB_SELF_TEST, 1,
>              [Define if self-testing features should be enabled])
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index 72e6a0d87f..636d830f1a 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -24,7 +24,7 @@
>  # Default hostio_last_error implementation
>  srv_hostio_err_objs="hostio-errno.o"
>  
> -if $development; then
> +if $enable_unittests; then
>     srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
>     srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
>  else
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index a491ae0257..f2f0d569ab 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -3790,7 +3790,7 @@ captured_main (int argc, char *argv[])
>  #if GDB_SELF_TEST
>        selftests::run_tests (selftest_filter);
>  #else
> -      printf (_("Selftests are not available in a non-development build.\n"));
> +      printf (_("Selftests have been disabled for this build.\n"));
>  #endif
>        throw_quit ("Quit");
>      }
> diff --git a/gdb/maint.c b/gdb/maint.c
> index 19db8a850b..01a80f5d73 100644
> --- a/gdb/maint.c
> +++ b/gdb/maint.c
> @@ -943,7 +943,7 @@ maintenance_selftest (const char *args, int from_tty)
>    selftests::run_tests (args);
>  #else
>    printf_filtered (_("\
> -Selftests are not available in a non-development build.\n"));
> +Selftests have been disabled for this build.\n"));
>  #endif
>  }
>  
> @@ -957,7 +957,7 @@ maintenance_info_selftests (const char *arg, int from_tty)
>    });
>  #else
>    printf_filtered (_("\
> -Selftests are not available in a non-development build.\n"));
> +Selftests have been disabled for this build.\n"));
>  #endif
>  }
>  
> diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
> index 1c835850b8..8e3e9a1761 100644
> --- a/gdb/testsuite/gdb.gdb/unittest.exp
> +++ b/gdb/testsuite/gdb.gdb/unittest.exp
> @@ -24,7 +24,7 @@ gdb_test_multiple $test $test {
>  	gdb_assert "$num_ran > 0" $test
>    }
>  
> -  -re "Selftests are not available in a non-development build.\r\n$gdb_prompt $" {
> +  -re "Selftests have been disabled for this build.\r\n$gdb_prompt $" {
>  	unsupported $test
>    }
>  }
> diff --git a/gdb/testsuite/gdb.server/unittest.exp b/gdb/testsuite/gdb.server/unittest.exp
> index e947ff2c30..b0a7c6ae56 100644
> --- a/gdb/testsuite/gdb.server/unittest.exp
> +++ b/gdb/testsuite/gdb.server/unittest.exp
> @@ -38,7 +38,7 @@ gdb_expect {
>  	gdb_assert "$num_ran > 0" $test
>      }
>  
> -    -re "Selftests are not available in a non-development build.\r\n$" {
> +    -re "Selftests have been disabled for this build.\r\n$" {
>  	unsupported $test
>      }
>  
> -- 
> 2.17.1

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/


  reply	other threads:[~2018-09-23  3:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-14  5:42 [RFC/PATCH] Don't disable selftests in a non-development build Sergio Durigan Junior
2018-08-14 16:28 ` Sergio Durigan Junior
2018-08-14 18:08 ` Pedro Alves
2018-08-14 18:42   ` Philippe Waroquiers
2018-08-14 20:17     ` Sergio Durigan Junior
2018-08-14 20:12   ` Sergio Durigan Junior
2018-09-17 20:22 ` [PATCH] Add parameter to allow enabling/disabling selftests via configure Sergio Durigan Junior
2018-09-23  3:56   ` Sergio Durigan Junior [this message]
2018-10-06  1:19   ` Simon Marchi
2018-10-10 20:31     ` Sergio Durigan Junior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8736u027f4.fsf@redhat.com \
    --to=sergiodj@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox