Move gdbserver to top level

From: Pedro Alves <palves@redhat.com>

  On 1/26/20 3:41 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I think this should have a NEWS entry, given that it affects
> Pedro> people building standalone gdbserver today.  Do you have
> Pedro> documentation changes queued?
> 
> Nope, I've added those now.
> 
>>> +// Host modules specific to gdbserver.
>>> +dependencies = { module=configure-gdbserver; on=all-intl; };
> 
> Pedro> This surprised me, as I don't think gdbserver actually depends on
> Pedro> intl currently?  Might still be a good idea to actually enable it,
> Pedro> but I was surprised from an "as pure as possible" perspective.
> 
> I don't recall (I wrote this patch originally in May of last year), but
> maybe I just copied gdb's dependencies.  Anyway, I've dropped them now.

Thanks.

> 
> Tom
> 
> commit 4c5c9a7a0b64c98f60c2ab79ad705d14ea568d0f
> Author: Tom Tromey <tom@tromey.com>
> Date:   Sun Dec 15 07:37:06 2019 -0700
> 
>     Move gdbserver to top level
>     
>     This patch moves gdbserver to the top level.
>     
>     This patch is as close to a pure move as possible -- gdbserver still
>     builds its own variant of gnulib and gdbsupport.  Changing this will
>     be done in a separate patch.
>     
>     [v2] Note that, per Simon's review comment, this patch changes the
>     tree so that gdbserver is not built for or1k or score.  This makes
>     sense, because there is apparently not actually a gdbserver port here.
>     
>     [v3] This version of the patch also splits out some configury into a
>     new file, gdbserver/configure.host, so that the top-level configure
>     script can simply rely on it in order to decide whether gdbserver
>     should be built.
>     
>     [v4] This version adds documentation and removes some unnecessary
>     top-level dependencies.

Thanks.

> +# Only allow gdbserver on some systems.
> +if test -d ${srcdir}/gdbserver; then
> +    if test x$enable_gdbserver = x; then
> +        AC_MSG_CHECKING([for gdbserver support])
> +	. ${srcdir}/gdbserver/configure.host
> +	if test x$build_gdbserver = xyes; then
> +	    AC_MSG_RESULT([yes])
> +	else
> +            noconfigdirs="$noconfigdirs gdbserver"
> +	    AC_MSG_RESULT([no])
> +        fi
> +    fi
> +fi

Some tabs vs spaces mixup above.  (But see below.)

> --- a/gdb/gdbserver/README
> +++ b/gdbserver/README
> @@ -100,27 +100,24 @@ The supported targets as of November 2006 are:
>  	spu*-*-*
>  	x86_64-*-linux*
>  
> -Configuring GDBserver you should specify the same machine for host and
> -target (which are the machine that GDBserver is going to run on.  This
> -is not the same as the machine that GDB is going to run on; building
> -GDBserver automatically as part of building a whole tree of tools does
> -not currently work if cross-compilation is involved (we don't get the
> -right CC in the Makefile, to start with)).
> -
> -Building GDBserver for your target is very straightforward.  If you build
> -GDB natively on a target which GDBserver supports, it will be built
> +Building GDBserver for your host is very straightforward.  If you build
> +GDB natively on a host which GDBserver supports, it will be built
>  automatically when you build GDB.  You can also build just GDBserver:
>  
>  	% mkdir obj
>  	% cd obj
> -	% path-to-gdbserver-sources/configure
> +	% path-to-toplevel-sources/configure --disable-gdb
>  	% make
>  
> +(If you have a combined binutils+gdb tree, you may want to also
> +disable other directories when configuring, e.g., binutils, gas, gold,
> +gprof, and ld.)
> +

On a git checkout, configuring with:

 configure \
 --disable-binutils \
 --disable-ld \
 --disable-gas \
 --disable-gprof \
 --disable-gold \
 --disable-gdb

and compiling with '$ make', still builds a number of top level directories
other than gdbserver.  We end up with this in the top level Makefile.in:

 SUBDIRS =  intl libiberty opcodes bfd readline zlib libdecnumber libctf sim gdbserver etc

So to build just gdbserver, you'd need to add a --disable-foo for each of those above
in addition to the ones already indicated, something like:

 configure \
 --disable-binutils \
 --disable-ld \
 --disable-gas \
 --disable-gprof \
 --disable-gold \
 --disable-gdb \
 --disable-intl \
 --disable-libiberty \
 --disable-opcodes \
 --disable-bfd \
 --disable-readline \
 --disable-zlib \
 --disable-libdecnumber \
 --disable-libctf \
 --disable-sim \
 --disable-etc

I guess it's the intended design for top level to build readline, bfd,
etc.  by default even if no application is being built that depends
on them.  I don't know.

People may trip on this if they try to build gdbserver for a port for which
one of those top level dirs doesn't build, like bfd.  powerpc-lynxos is 
I think one such case, but there are probably others in the non-glibc space,
and also off tree.

However, a configure command, followed by "make all-gdbserver" builds only
gdbserver and its required dependencies (which are none at the moment).

So I'm thinking that it might be better to document "make all-gdbserver" instead
of the --disable approach.  Or at least, mention it as alternative.  WDYT?

>  If you prefer to cross-compile to your target, then you can also build
>  GDBserver that way.  In a Bourne shell, for example:
>  
>  	% export CC=your-cross-compiler
> -	% path-to-gdbserver-sources/configure your-target-name
> +	% path-to-topevel-sources/configure your-target-name --disable-gdb
>  	% make
>  
>  Using GDBreplay:

> index 00000000000..86d3a80148d
> --- /dev/null
> +++ b/gdbserver/configure.host

I noticed something about this file that made me look a bit more
and I have a couple questions, including a patch.

I noticed that several directories already contain a file named
configure.host, including gdb:

 $ find . -name configure.host
 ./gdb/configure.host
 ./ld/configure.host
 ./gdbserver/configure.host
 ./bfd/configure.host

And in gdb's ld's and bfd's case, this file is sourced
by the respective directory's configure script, not by
the top level script.  If we wanted to migrate gdb's top
configure bits to source a new file from gdb/ to determine
whether gdb is supported, like gdbserver, we wouldn't
be able to call the new "do we support gdb on this host"
configure snippet file gdb/configure.host, since that
file already exists with a different purpose.  Likewise
for ld and bfd.  So, with that in mind, should we name
gdbserver's new configure.host file to something else?

However, looking at how libatomic, liboffloadmic, etc.
handle this at the top level, I see that the top level
sources a file that is also sourced by their respective
configures.  I.e., the file sourced served a dual purpose.
From the top level, the file is sourced in subshell, which
avoids variable polluting the top level.  This has the advantage
that you only have to write support for a port once in one file,
instead of in two places.

The equivalent for gdbserver would be the patch below,
which seems to work well.  Was there a reason you didn't follow
libatomic's (etc.) model?

From 0a8e011249c5d887886b1e06d5b3b774bd6f9d10 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 27 Jan 2020 12:53:10 +0000
Subject: [PATCH] UNSUPPORTED

---
 configure                |  18 ++++----
 configure.ac             |  16 ++++---
 gdbserver/configure.host | 111 -----------------------------------------------
 gdbserver/configure.srv  |  12 +++--
 4 files changed, 27 insertions(+), 130 deletions(-)
 delete mode 100644 gdbserver/configure.host

base-commit: 4c5c9a7a0b64c98f60c2ab79ad705d14ea568d0f

Move gdbserver to top level

Commit Message

Comments

Patch