Move gdbsupport to the top level

Move gdbsupport to the top level

[Tom Tromey]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

This patch moves the gdbsupport directory to the top level.  This is
the next step in the ongoing project to move gdbserver to the top
level.

The bulk of this patch was created by "git mv gdb/gdbsupport gdbsupport".

This patch then adds a build system to gdbsupport and wires it into
the top level.  Then it changes gdb to use the top-level build.

gdbserver, on the other hand, is not yet changed.  It still does its
own build of gdbsupport.

I wasn't able to send this through the buildbot.  I did test it on
x86-64 Fedora 29.  If you want to try it, it is on the branch
submit/move-gdbsupport-to-top in my github.


This patch was also too big to send the usual way, so I have compressed
it and added it as an attachment.  Sorry about that.

Tom


[-- Attachment #2: the patch --]
[-- Type: application/x-xz, Size: 69060 bytes --]

Re: Move gdbsupport to the top level

[Andrew Pinski]

On Thu, Jul 11, 2019 at 2:03 PM Tom Tromey <tom@tromey.com> wrote:
>
> This patch moves the gdbsupport directory to the top level.  This is
> the next step in the ongoing project to move gdbserver to the top
> level.
>
> The bulk of this patch was created by "git mv gdb/gdbsupport gdbsupport".
>
> This patch then adds a build system to gdbsupport and wires it into
> the top level.  Then it changes gdb to use the top-level build.

Isn't the top-level makefile also synced with GCC's (and newlib's)?
Where GCC is techincally
the maintainer of it?

Thanks,
Andrew Pinski

>
> gdbserver, on the other hand, is not yet changed.  It still does its
> own build of gdbsupport.
>
> I wasn't able to send this through the buildbot.  I did test it on
> x86-64 Fedora 29.  If you want to try it, it is on the branch
> submit/move-gdbsupport-to-top in my github.
>
>
> This patch was also too big to send the usual way, so I have compressed
> it and added it as an attachment.  Sorry about that.
>
> Tom
>

Re: Move gdbsupport to the top level

[Tom Tromey]

>>>>> "Andrew" == Andrew Pinski <pinskia@gmail.com> writes:

Andrew> On Thu, Jul 11, 2019 at 2:03 PM Tom Tromey <tom@tromey.com> wrote:
>> 
>> This patch moves the gdbsupport directory to the top level.  This is
>> the next step in the ongoing project to move gdbserver to the top
>> level.
>> 
>> The bulk of this patch was created by "git mv gdb/gdbsupport gdbsupport".
>> 
>> This patch then adds a build system to gdbsupport and wires it into
>> the top level.  Then it changes gdb to use the top-level build.

Andrew> Isn't the top-level makefile also synced with GCC's (and newlib's)?
Andrew> Where GCC is techincally
Andrew> the maintainer of it?

Last time around I checked in the patches locally first and then I sent
them upstream.

I don't mind doing it the other way, if that's important for some
reason, but it's still better to get sign-off from the gdb side before
committing to doing it.

thanks,
Tom

Re: Move gdbsupport to the top level

[Tom Tromey]

Tom> I wasn't able to send this through the buildbot.  I did test it on
Tom> x86-64 Fedora 29.  If you want to try it, it is on the branch
Tom> submit/move-gdbsupport-to-top in my github.

Sergio and Gary made some fixes to the buildbot and I was able to send
this through.  That showed that this doesn't build on mingw; so I am
going to fix this sometime soon and then send another version of this
patch.

I don't anticipate very extensive changes, so feel free to look at this
patch if you want.

Tom

Re: Move gdbsupport to the top level

[Pedro Alves]

Hi,

On 7/11/19 10:02 PM, Tom Tromey wrote:
> This patch moves the gdbsupport directory to the top level.  This is
> the next step in the ongoing project to move gdbserver to the top
> level.
> 
> The bulk of this patch was created by "git mv gdb/gdbsupport gdbsupport".
> 
> This patch then adds a build system to gdbsupport and wires it into
> the top level.  Then it changes gdb to use the top-level build.
> 
> gdbserver, on the other hand, is not yet changed.  It still does its
> own build of gdbsupport.

(I'm looking at the branch, so I'm not sure I'm commenting on the patch
as it was posted.)

This all builds fine for me when I build normally, but,
this breaks building gdbserver standalone, without configuring 
from the top level.  I.e.:

 $ mkdir build-gdbserver
 $ cd build-gdbserver
 $ .../src/gdb/gdbserver/configure
 ...
 make: *** No rule to make target '../../gdbsupport/libgdbsupport.a', needed by 'gdbserver'.  Stop.
 make: *** Waiting for unfinished jobs....
 ...


> 
> I wasn't able to send this through the buildbot.  I did test it on
> x86-64 Fedora 29.  If you want to try it, it is on the branch
> submit/move-gdbsupport-to-top in my github.
> 
> 
> This patch was also too big to send the usual way, so I have compressed
> it and added it as an attachment.  Sorry about that.

The patch is much smaller and easier to read without the
the generated files.  It's what I did here locally to try to
make sense of what I was seeing.

> -  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) gdbsupport/selftest.o selftest-arch.o"
> -  CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) gdbsupport/selftest.c selftest-arch.c"
> +  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS)  selftest-arch.o"
> +  CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) selftest-arch.c"

Spurious double space.

So I guess the biggest change here is how/where config.h is included.

From the branch:

> --- c/gdb/nat/linux-btrace.c
> +++ w/gdb/nat/linux-btrace.c
> @@ -20,6 +20,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "gdbsupport/common-defs.h"
> +#include <config.h>

I noticed that only a few nat/ files needed to include config.h,
which I found surprising/confusing at first.

What are the new rules here?  Add <config.h> on as-needed basis,
or should we have some nat.h file that is included by
all nat/ files, and same for arch/ ?  The former seems a bit
error prone, given that you could move code around and not realize
that an #ifdef is disabling something because you missed config.h.

Alternatively, I guess we could move the required bits from
gdb&gdbserver's configury to gdbsupport's, so that config.h
wasn't ever necessary in shared code.  Not sure whether that
would be a bit of an abstraction violation.

Thanks,
Pedro Alves

Re: Move gdbsupport to the top level

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> (I'm looking at the branch, so I'm not sure I'm commenting on the patch
Pedro> as it was posted.)

I think I made a few changes based on reviews, though I don't fully
recall.

Pedro> This all builds fine for me when I build normally, but,
Pedro> this breaks building gdbserver standalone, without configuring 
Pedro> from the top level.  I.e.:

I will take a look.

Meanwhile you should also look at this:

    https://sourceware.org/ml/gdb-patches/2019-08/msg00053.html

... because I think that's another blocker to this project.

That message neglected option 4, which is removing readline from the
tree.  (But making it continue to work if the sources are dropped in, as
we already do for libiconv etc.)

Pedro> What are the new rules here?  Add <config.h> on as-needed basis,
Pedro> or should we have some nat.h file that is included by
Pedro> all nat/ files, and same for arch/ ?  The former seems a bit
Pedro> error prone, given that you could move code around and not realize
Pedro> that an #ifdef is disabling something because you missed config.h.

Pedro> Alternatively, I guess we could move the required bits from
Pedro> gdb&gdbserver's configury to gdbsupport's, so that config.h
Pedro> wasn't ever necessary in shared code.  Not sure whether that
Pedro> would be a bit of an abstraction violation.

Yeah, I just did what was needed to make it work, but I didn't really
come up with a long-term plan.

Maybe moving nat and arch to gdbsupport is the another option?  That
would require some #include adjuments but otherwise maybe it's not such
a big deal.

Tom

Re: Move gdbsupport to the top level

[Tom Tromey]

Tom> That message neglected option 4, which is removing readline from the
Tom> tree.  (But making it continue to work if the sources are dropped in, as
Tom> we already do for libiconv etc.)

We discussed this at Cauldron and agreed to try this approach.  That is,
the plan is to remove the readline sources from the tree, and remove
--with-system-readline, but follow the approach of some other libraries
where one can unpack readline into the source tree and top-level
configure will arrange to build it.

Tom

Re: Move gdbsupport to the top level

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Pedro Alves <palves@redhat.com>,  gdb-patches@sourceware.org
> Date: Tue, 17 Sep 2019 10:11:39 -0600
> 
> We discussed this at Cauldron and agreed to try this approach.

Is it possible to still discuss this?  Or is the decision at Cauldron
final and cannot be appealed?

> That is, the plan is to remove the readline sources from the tree,
> and remove --with-system-readline, but follow the approach of some
> other libraries where one can unpack readline into the source tree
> and top-level configure will arrange to build it.

Would it be possible for whoever tars the release to drop the readline
into the tree and build it under some opt-in configure-time switch?
Making this opt-in might solve at least some of the problems that led
to the decision, I think/hope.

Re: Move gdbsupport to the top level

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tom Tromey <tom@tromey.com>
>> Cc: Pedro Alves <palves@redhat.com>,  gdb-patches@sourceware.org
>> Date: Tue, 17 Sep 2019 10:11:39 -0600
>> 
>> We discussed this at Cauldron and agreed to try this approach.

Eli> Is it possible to still discuss this?  Or is the decision at Cauldron
Eli> final and cannot be appealed?

Nothing is final.

>> That is, the plan is to remove the readline sources from the tree,
>> and remove --with-system-readline, but follow the approach of some
>> other libraries where one can unpack readline into the source tree
>> and top-level configure will arrange to build it.

Eli> Would it be possible for whoever tars the release to drop the readline
Eli> into the tree and build it under some opt-in configure-time switch?
Eli> Making this opt-in might solve at least some of the problems that led
Eli> to the decision, I think/hope.

Seems like a good suggestion to me.

Tom

Re: Move gdbsupport to the top level

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  palves@redhat.com,  gdb-patches@sourceware.org
> Date: Tue, 17 Sep 2019 10:34:56 -0600
> 
> >> That is, the plan is to remove the readline sources from the tree,
> >> and remove --with-system-readline, but follow the approach of some
> >> other libraries where one can unpack readline into the source tree
> >> and top-level configure will arrange to build it.
> 
> Eli> Would it be possible for whoever tars the release to drop the readline
> Eli> into the tree and build it under some opt-in configure-time switch?
> Eli> Making this opt-in might solve at least some of the problems that led
> Eli> to the decision, I think/hope.
> 
> Seems like a good suggestion to me.

Thanks, I hope it will be accepted.

My rationale is that, since GDB uses Readline in a much more
sophisticated way than many other packages, I'd expect GDB to depend
on a relatively recent version of Readline, where several important
changes were made lately, in particular those submitted by us.  Having
such a version ready in the tarball would, on the one side, allow
people not to upgrade their system Readline just to be able to build
and use GDB, and OTOH, will greatly simplify the task of finding that
specific version which GDB requires if they want to build GDB with a
version of Readline different from their installed one.