[Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in
Thomas Petazzoni
thomas.petazzoni at free-electrons.com
Sun Jun 29 10:36:18 UTC 2014
Arnout, Pascal,
On Wed, 25 Jun 2014 22:31:20 +0200, Arnout Vandecappelle wrote:
> > +config BR2_GOOGLE_BREAKPAD_INCLUDE_FILES
>
> I don't really like the name of this option. What do you think of
> BR2_GOOGLE_BREAKPAD_GENSYMS_PATTERNS ?
I think this explains some of your comments below: Pascal's patch only
support listing complete file names in
BR2_GOOGLE_BREAKPAD_INCLUDE_FILES, while you apparently thought his
intention was to support file patterns. It indeed seems like a good
idea, but it's clearly a different thing.
> > + string "executables and libraries to be used by google-breakpad"
> > + depends on BR2_PACKAGE_GOOGLE_BREAKPAD
>
> Hm, this is counter-intuitive, though honestly I don't know how to improve it.
> The user would first have to go to the target packages and select google
> breakpad, and then return to the Build options menu at the top to set the patterns.
>
> Actually, this symbol could be defined inside the google-breakpad/Config.in,
> no? Though you probably got a comment before that that is not the right place :-)
Yeah, that's tricky. We could turn it into a "select", but then
everybody will always see this option related to google-breakpad in the
Build options. So I believe the solution of using a 'depends on' is
still the most appropriate solution. Maybe a little piece of
documentation should be added in the Buildroot manual to explain how
the Google Breakpad integration works?
However, I don't really like the prompt. Maybe it should be:
[ ] Enable Google Breakpad support
() List of binaries to extract symbols from
> > +ifeq ($(BR2_PACKAGE_GOOGLE_BREAKPAD),y)
> > + $(EXTRA_ENV) package/google-breakpad/gen-syms.sh $(STAGING_DIR) \
> > + $(TARGET_DIR) $(call qstrip,$(BR2_GOOGLE_BREAKPAD_INCLUDE_FILES))
>
> To be more compatible with Fabio's series that reworks this part of the
> infrastructure, it's better to define this as a new variable which is called
> here. That will make it easier to resolve the conflict between these two series.
> Fabio's series will also make it possible to define this completely inside the
> google-breakpad.mk.
Yes, having it all in the google-breakpad.mk seems like a good idea.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
More information about the buildroot
mailing list