From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] gdb: convert to the package infrastructure
Date: Fri, 01 Mar 2013 22:42:41 +0100 [thread overview]
Message-ID: <513120D1.1000201@mind.be> (raw)
In-Reply-To: <20130228095645.2cdfffd6@skate>
>> 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.
Running a remote kgdb is something you would need the most on a system
with little memory, so indeed the non-gdbserver use case is important.
So, keep it as you have it.
>> * 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.
As I said: for a follow-up patch.
James Hogan makes another good point that custom patches against a
specific gdb version are easier to maintain if the version is selectable.
However, when the generic package infrastructure is used, changing the
version number directly in gdb.mk is also easy. I think the use case of
custom patches against gdb is sufficiently rare that Kconfig
configuration is not needed.
>> * 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.
Actually, I mean: there is an avr32-specific gdb version, but the 7.x
gdb versions are also enabled for avr32. It just looks strange.
>>> ---
>>> 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 :)
True, but the change you make here is not going to affect gdb runtime.
The likelihood that there is serious build breakage is much higher than
that of a runtime regression, I think. Also if there is a runtime
regression, the chance that it will be found is much higher if it's been
committed than if the patch is just lingering on the list.
>>> + 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?
You could make the gdbserver copied from the external toolchain
mutually exclusive with the gdb package, but I'm not convinced that that
is needed.
> 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.
Well, the location of the config menu is independent from where it is
used in the .mk files, of course.
Wild idea:
In Toolchain menu, under all the rest:
[ ] Cross gdb for the host
GDB debugger version (gdb 7.5.x) -->
[ ] gdbserver on the target
[ ] Copy external toolchain gdbserver to the target
[ ] full gdb on the target
Hm, this looks very much like the current situation :-)
Admittedly, gdb's Config.in would look pretty non-standard (e.g. a
blind BR2_PACKAGE_GDB symbol that is selected by
BR2_PACKAGE_GDB_GDBSERVER and BR2_PACKAGE_GDB_DEBUGGER), but it is a
fairly special case.
[snip]
>> 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.
I guess even better would be a build-if-missing approach like for tar
and python (when the patch gets merged).
>
> 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.
Agreed. This patch is just an opportunity to review gdb in general.
>
>> [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).
Fair enough. I feel something like this is worthwhile to explain in a
comment.
Regards,
Arnout
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
prev parent reply other threads:[~2013-03-01 21:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-26 22:13 [Buildroot] [PATCH] gdb: convert to the package infrastructure Thomas Petazzoni
2013-02-28 8:00 ` Arnout Vandecappelle
2013-02-28 8:39 ` James Hogan
2013-02-28 8:47 ` Thomas Petazzoni
2013-02-28 12:11 ` James Hogan
2013-02-28 12:20 ` Thomas Petazzoni
2013-02-28 12:24 ` James Hogan
2013-02-28 8:56 ` Thomas Petazzoni
2013-03-01 21:42 ` Arnout Vandecappelle [this message]
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=513120D1.1000201@mind.be \
--to=arnout@mind.be \
--cc=buildroot@busybox.net \
/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