[Buildroot] [PATCH] gdb: convert to the package infrastructure

Thomas Petazzoni thomas.petazzoni at free-electrons.com
Thu Feb 28 08:56:45 UTC 2013


Dear Arnout Vandecappelle,

On Thu, 28 Feb 2013 09:00:45 +0100, Arnout Vandecappelle wrote:

>   This looks like a great idea!

Thanks :-)

>   Just a couple of generic remarks, and then a few specific ones
> below. The generic remarks are probably for follow-up patches.
> 
> * It really doesn't make sense to build host-gdb without a gdbserver
> for the target. So I would auto-select gdbserver from host-gdb.

See the point made by James Hogan regarding kgdb. I don't have a strong
opinion on this one, and we could decide that gdbserver is lightweight
enough that even with James use case of kgdb, it still makes sense to
have gdbserver on the target when host-gdb is enabled.

> * Does it really make sense to keep options for four different gdb 
> versions? Can't we just remove the user-selectable version completely?

I just kept as it was.

> * Do gdb-7.2+ actually work for avr32? If yes, why keep the 6.7.1
> around?

I also just kept it as it was. If we want to upgrade gdb for avr32,
then let's do it as a follow-up. People having access to avr32 hardware
should test that.

> > ---
> > It would be good to have a few people testing this before
> > committing.
> 
>   Nah, just let the autobuilders do it :-)

Autobuilders are nice, but they are not yet capable of starting
gdbserver on the target, connect gdb, and verify that it works :)

> > +	  For embedded development, the most common solution is to
> > +	  build only 'gdbserver' for the target, and use a
> > cross-gdb
> > +	  on the host. See BR2_PACKAGE_HOST_GDB to enable one.
> 
>   "See BR2_PACKAGE_HOST_GDB in the Toolchain menu to enable one."
> 
>   It could be useful to add a comment that external toolchains often 
> provide gdb/gdbserver.

Indeed. I am not sure what do with external toolchains here. In the
past, I was hiding the option to build host-gdb and gdbserver for
the target, making the assumption that the external toolchain already
have those, and instead there was the option to copy the toolchain's
gdbserver to the target. Now, everything is available: host-gdb built
by Buildroot, gdbserver built by Buildroot, and gdbserver copied from
the external toolchain. Should I keep all this?

Also, the "gdbserver build by Buildroot" thing is in Packages ->
Debugging, but the "copy gdbserver from toolchain" thing is in
Toolchain. But it's not easy to put it in a package, as the code needed
to copy gdbserver from the toolchain really relates to all the complex
external toolchain logic.

> > +	  http://www.gnu.org/software/gdb/
> > +
> > +if BR2_PACKAGE_GDB
> > +
> > +config BR2_PACKAGE_GDB_SERVER
> > +	bool "gdbserver"
> > +	help
> > +	  Build the gdbserver stub to run on the target.
> > +	  A full gdb is needed to debug the progam.
> > +
> > +config BR2_PACKAGE_GDB_DEBUGGER
> > +	bool "full debugger"
> > +	select BR2_PACKAGE_NCURSES
> > +	depends on BR2_USE_WCHAR
> > +	depends on BR2_TOOLCHAIN_HAS_THREADS
> > +	depends on !BR2_sh && !BR2_sh64 && !BR2_avr32
> > && !BR2_microblaze && !BR2_bfin && !BR2_aarch64
> > +	depends on BR2_TOOLCHAIN_HAS_THREADS_DEBUG_IF_NEEDED
> > +	select BR2_PTHREAD_DEBUG if (BR2_TOOLCHAIN_BUILDROOT
> > && !BR2_PTHREADS_NONE)
> 
>   It feels pretty strange to modify a toolchain option from a package 
> Config.in. Remember, the user may add gdb in a second run and try to
> do an incremental build... So I would say: move it to a depends like
> we usually do.

Ok.

> > +comment "full gdb on target needs WCHAR and threads support in
> > toolchain"
> > +	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
> > +
> > +endif
> > diff --git a/package/gdb/Config.in.host b/package/gdb/Config.in.host
> > new file mode 100644
> > index 0000000..6ee1d26
> > --- /dev/null
> > +++ b/package/gdb/Config.in.host
> > @@ -0,0 +1,56 @@
> > +config BR2_PACKAGE_HOST_GDB
> > +	bool "Build cross gdb for the host"
> > +	help
> > +	    Build a cross gdb that runs on the host machine and
> > debugs
> > +	    programs running on the target. It requires 'gdbserver'
> > +	    installed on the target, see BR2_PACKAGE_GDB_SERVER to
> > +	    enable it.
> 
>   As I said, I would just auto-select BR2_PACKAGE_GDB, which will
> select BR2_PACKAGE_GDB_SERVER

Hum, why not select BR2_PACKAGE_GDB and BR2_PACKAGE_GDB_SERVER? If you
select BR2_PACKAGE_GDB only, it still the user the possibility to build
only the target debugger, and not gdbserver.

> > +# When no version is defined, it means that cross-gdb for the host
> > has +# not been enabled, and we will only build gdbserver or gdb
> > for the +# target. In this case, use the latest available version
> > +# automatically.
> > +ifeq ($(GDB_VERSION),)
> > +ifeq ($(BR2_bfin),y)
> > +GDB_VERSION = 6.6a
> > +GDB_SITE    = $(BR2_GNU_MIRROR)/gdb
> 
>   Just define this once, either before the condition (and override it
> for avr32), or after the condition (and use ?=).

Ok.

> 
> > +else ifeq ($(BR2_avr32),y)
> > +GDB_VERSION = 6.7.1-avr32-2.1.5
> > +GDB_SITE    = ftp://www.at91.com/pub/buildroot/
> > +else
> > +GDB_VERSION = 7.5.1
> > +GDB_SITE    = $(BR2_GNU_MIRROR)/gdb
> > +endif
> > +endif
> > +
> > +GDB_SOURCE = gdb-$(GDB_VERSION).tar.bz2
> > +GDB_LICENSE = GPLv2+ LGPLv2+ GPLv3+ LGPLv3+
> > +GDB_LICENSE_FILES = COPYING COPYING.LIB COPYING3 COPYING3.LIB
> > +
> > +# We only want gdbserver and not the entire debugger.
> > +ifeq ($(BR2_PACKAGE_GDB_DEBUGGER),)
> > +GDB_SUBDIR = gdb/gdbserver
> > +HOST_GDB_SUBDIR = .
> > +else
> > +GDB_DEPENDENCIES = ncurses
> > +endif
> > +
> > +# For the host variant, we really want to build with XML support,
> > +# which is needed to read XML descriptions of target architectures.
> > +HOST_GDB_DEPENDENCIES = host-expat
> 
>   Can we rely on host-nurses being present? It is not checked in 
> dependencies.sh AFAIK. Admittedly, it is not a dependency of host-gdb 
> now, but most people have ncurses installed anyway for menuconfig...

host-ncurses wasn't a dependency, and our host-ncurses package is only
here to build a version of "tic" that works properly. Some systems had
a "tic" version causing problems when building a more recent ncurses
for the target. I think that other than for this "tic" program, we
should really on the pre-installed ncurses library, which should
already be present since the user uses menuconfig.

However, it's true we don't check it in dependencies.sh, and the user
could very well be using xconfig or gconfig, neither of which require
ncurses.

I'd say that's a separate matter really.

> [snip]
> > +define GDB_REMOVE_UNNEEDED_FILES
> > +	$(RM) -rf $(TARGET_DIR)/usr/share/gdb
> > +endef
> 
>   Perhaps a comment why this is needed? Or is it just to keep it 
> consistent with the current behaviour?

The old gdb installation logic was to manually copy the gdb binary to
the $(TARGET_DIR). In my patch, I've chosen to use 'make install'
instead, which requires removing a bunch of generally useless stuff
(Python scripts and XML descriptions of targets, which as per my
understanding, are needed only when you do remote debugging).

Thanks for the review!

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com


More information about the buildroot mailing list