* [Buildroot] [PATCH V5 0/2] google-breakpad: new package
@ 2014-06-25 13:16 Pascal Huerst
2014-06-25 13:16 ` [Buildroot] [PATCH V5 1/2] " Pascal Huerst
2014-06-25 13:16 ` [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in Pascal Huerst
0 siblings, 2 replies; 11+ messages in thread
From: Pascal Huerst @ 2014-06-25 13:16 UTC (permalink / raw)
To: buildroot
Changes v4 -> v5 (All suggested by Yann E. MORIN)
Fixed some typo
Reorganized patch structure
which parts belong to 'new package' and which parts belong to
'intergration into Makefile and Config.in'
Changed and Rewrote gen-symy.sh - script
Passing $(EXTRA_ENV)
Adapted a proposed solution from Yann E. MORIN, which
uses ls instead of find commands
Made better descriptions for users in:
Config.in
package/google-breakpad/Config.in
Changes v3 -> v4
Fixed minor issue in google-breakpad_gen-syms.sh
Had to remove -print from find command
Changes v2 -> v3 (All suggested by thomas.petazzoni at free-electrons.com)
Removed dependency of BR2_ENABLE_DEBUG, but added comment,
that this flag might have to be set in order to use breakpad
properly.
Removed "find -name ..." for libs and binaries by generic patterns,
such as "*.so" and so forth. This will never work properly. Instead
I added a list to insert libs and binaries, that will be symbol-
dumped and prepared for breakpad (see Config.in patch v3 2/2)
Changed storage path for dumps from: $(TARGET_DIR).. to
$(STAGING_DIR)/usr/share/google-breakpad-symbols/
Removed most logic from Makefile. Added script instead, which is
called by Makefile
Added dependency for compatible targets: ARM, i386, MIPS...
Set fixed version for checkout (instead of head)
Fixed typos, and some minor mistakes
Changes v1 -> v2 (All suggested by maxime.hadjinlian at gmail.com)
Added dependency from BR2_ENABLE_DEBUG
Removed symbolstore.py -> Its all done in Makefile now
Removed new Target -> It in target-finalize before stripping now
Added LICENSE and LICENSE_FILE to *.mk
Pascal Huerst (2):
google-breakpad: new package
google-breakpad: integration into Makefile and Config.in
Config.in | 14 ++++++++++++++
Makefile | 5 +++++
package/Config.in | 1 +
package/google-breakpad/Config.in | 22 ++++++++++++++++++++++
package/google-breakpad/gen-syms.sh | 25 +++++++++++++++++++++++++
package/google-breakpad/google-breakpad.mk | 17 +++++++++++++++++
6 files changed, 84 insertions(+)
create mode 100644 package/google-breakpad/Config.in
create mode 100755 package/google-breakpad/gen-syms.sh
create mode 100644 package/google-breakpad/google-breakpad.mk
--
1.9.3
^ permalink raw reply [flat|nested] 11+ messages in thread* [Buildroot] [PATCH V5 1/2] google-breakpad: new package 2014-06-25 13:16 [Buildroot] [PATCH V5 0/2] google-breakpad: new package Pascal Huerst @ 2014-06-25 13:16 ` Pascal Huerst 2014-06-25 19:16 ` Arnout Vandecappelle 2014-06-29 10:30 ` Thomas Petazzoni 2014-06-25 13:16 ` [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in Pascal Huerst 1 sibling, 2 replies; 11+ messages in thread From: Pascal Huerst @ 2014-06-25 13:16 UTC (permalink / raw) To: buildroot Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com> --- package/Config.in | 1 + package/google-breakpad/Config.in | 19 +++++++++++++++++++ package/google-breakpad/google-breakpad.mk | 17 +++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 package/google-breakpad/Config.in create mode 100644 package/google-breakpad/google-breakpad.mk diff --git a/package/Config.in b/package/Config.in index c46c0ec..392ae40 100644 --- a/package/Config.in +++ b/package/Config.in @@ -54,6 +54,7 @@ menu "Debugging, profiling and benchmark" source "package/duma/Config.in" source "package/fio/Config.in" source "package/gdb/Config.in" + source "package/google-breakpad/Config.in" source "package/iozone/Config.in" source "package/kexec/Config.in" source "package/ktap/Config.in" diff --git a/package/google-breakpad/Config.in b/package/google-breakpad/Config.in new file mode 100644 index 0000000..fa1e8d9 --- /dev/null +++ b/package/google-breakpad/Config.in @@ -0,0 +1,19 @@ +config BR2_PACKAGE_GOOGLE_BREAKPAD + bool "google-breakpad" + depends on BR2_INSTALL_LIBSTDCPP + depends on BR2_TOOLCHAIN_USES_GLIBC + depends on BR2_i386 || BR2_x86_64 || BR2_arm || BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el + help + Google-Breakpad is a library and tool suite that allows you to distribute + an application to users with compiler-provided debugging information + removed, record crashes in compact "minidump" files, send them back to + your server, and produce C and C++ stack traces from these minidumps. + Breakpad can also write minidumps on request for programs that have not + crashed. + + You may want to set BR2_ENABLE_DEBUG, in order to get useful results. + + http://code.google.com/p/google-breakpad/ + +comment "google-breakpad requires an (e)glibc toolchain w/ C++ enabled" + depends on !(BR2_INSTALL_LIBSTDCPP && BR2_TOOLCHAIN_USES_GLIBC) diff --git a/package/google-breakpad/google-breakpad.mk b/package/google-breakpad/google-breakpad.mk new file mode 100644 index 0000000..e5b69c0 --- /dev/null +++ b/package/google-breakpad/google-breakpad.mk @@ -0,0 +1,17 @@ +################################################################################ +# +# google-breakpad +# +################################################################################ + +GOOGLE_BREAKPAD_VERSION = 1320 +GOOGLE_BREAKPAD_SITE = http://google-breakpad.googlecode.com/svn/trunk +GOOGLE_BREAKPAD_SITE_METHOD = svn +GOOGLE_BREAKPAD_CONF_OPT = --disable-processor --disable-tools +GOOGLE_BREAKPAD_DEPENDENCIES = host-google-breakpad +GOOGLE_BREAKPAD_INSTALL_STAGING = YES +GOOGLE_BREAKPAD_LICENSE = BSD-3c +GOOGLE_BREAKPAD_LICENSE_FILES = LICENSE + +$(eval $(host-autotools-package)) +$(eval $(autotools-package)) -- 1.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 1/2] google-breakpad: new package 2014-06-25 13:16 ` [Buildroot] [PATCH V5 1/2] " Pascal Huerst @ 2014-06-25 19:16 ` Arnout Vandecappelle 2014-06-29 10:30 ` Thomas Petazzoni 1 sibling, 0 replies; 11+ messages in thread From: Arnout Vandecappelle @ 2014-06-25 19:16 UTC (permalink / raw) To: buildroot On 25/06/14 15:16, Pascal Huerst wrote: > Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com> > --- > package/Config.in | 1 + > package/google-breakpad/Config.in | 19 +++++++++++++++++++ > package/google-breakpad/google-breakpad.mk | 17 +++++++++++++++++ > 3 files changed, 37 insertions(+) > create mode 100644 package/google-breakpad/Config.in > create mode 100644 package/google-breakpad/google-breakpad.mk > > diff --git a/package/Config.in b/package/Config.in > index c46c0ec..392ae40 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -54,6 +54,7 @@ menu "Debugging, profiling and benchmark" > source "package/duma/Config.in" > source "package/fio/Config.in" > source "package/gdb/Config.in" > + source "package/google-breakpad/Config.in" > source "package/iozone/Config.in" > source "package/kexec/Config.in" > source "package/ktap/Config.in" > diff --git a/package/google-breakpad/Config.in b/package/google-breakpad/Config.in > new file mode 100644 > index 0000000..fa1e8d9 > --- /dev/null > +++ b/package/google-breakpad/Config.in > @@ -0,0 +1,19 @@ > +config BR2_PACKAGE_GOOGLE_BREAKPAD > + bool "google-breakpad" > + depends on BR2_INSTALL_LIBSTDCPP > + depends on BR2_TOOLCHAIN_USES_GLIBC > + depends on BR2_i386 || BR2_x86_64 || BR2_arm || BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el Too long, the line should be split after mips. > + help > + Google-Breakpad is a library and tool suite that allows you to distribute > + an application to users with compiler-provided debugging information > + removed, record crashes in compact "minidump" files, send them back to > + your server, and produce C and C++ stack traces from these minidumps. > + Breakpad can also write minidumps on request for programs that have not > + crashed. I think these lines are also too long to display on an 80-char display (it should be wrapped at 72 chars IIRC, where the tab counts for 8 chars). Also, there should be no whitespace at the end of the lines. I would also mention in the help text that this installs the host tools in $(HOST_DIR)/usr/bin and list the host tools. If I understand correctly, for the target, we just build the (static) client library, which should be linked and enabled explicitly by applications that want to make use of breakpad. Is that correct? If yes, perhaps it's better to clarify that in the help text as well. > + > + You may want to set BR2_ENABLE_DEBUG, in order to get useful results. > + > + http://code.google.com/p/google-breakpad/ > + > +comment "google-breakpad requires an (e)glibc toolchain w/ C++ enabled" > + depends on !(BR2_INSTALL_LIBSTDCPP && BR2_TOOLCHAIN_USES_GLIBC) Here you should repeat the arch dependencies. > diff --git a/package/google-breakpad/google-breakpad.mk b/package/google-breakpad/google-breakpad.mk > new file mode 100644 > index 0000000..e5b69c0 > --- /dev/null > +++ b/package/google-breakpad/google-breakpad.mk > @@ -0,0 +1,17 @@ > +################################################################################ > +# > +# google-breakpad > +# > +################################################################################ > + > +GOOGLE_BREAKPAD_VERSION = 1320 Any particular reason to take a 2-month old commit? > +GOOGLE_BREAKPAD_SITE = http://google-breakpad.googlecode.com/svn/trunk > +GOOGLE_BREAKPAD_SITE_METHOD = svn > +GOOGLE_BREAKPAD_CONF_OPT = --disable-processor --disable-tools > +GOOGLE_BREAKPAD_DEPENDENCIES = host-google-breakpad > +GOOGLE_BREAKPAD_INSTALL_STAGING = YES There is nothing to install on the target, right? So I'd expect GOOGLE_BREAKPAD_INSTALL_TARGET = NO > +GOOGLE_BREAKPAD_LICENSE = BSD-3c > +GOOGLE_BREAKPAD_LICENSE_FILES = LICENSE > + > +$(eval $(host-autotools-package)) > +$(eval $(autotools-package)) We normally put the host-package after the target-package. 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 1/2] google-breakpad: new package 2014-06-25 13:16 ` [Buildroot] [PATCH V5 1/2] " Pascal Huerst 2014-06-25 19:16 ` Arnout Vandecappelle @ 2014-06-29 10:30 ` Thomas Petazzoni 2014-07-02 12:59 ` Pascal Huerst 1 sibling, 1 reply; 11+ messages in thread From: Thomas Petazzoni @ 2014-06-29 10:30 UTC (permalink / raw) To: buildroot Dear Pascal Huerst, On Wed, 25 Jun 2014 15:16:13 +0200, Pascal Huerst wrote: > Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com> > --- > package/Config.in | 1 + > package/google-breakpad/Config.in | 19 +++++++++++++++++++ > package/google-breakpad/google-breakpad.mk | 17 +++++++++++++++++ > 3 files changed, 37 insertions(+) > create mode 100644 package/google-breakpad/Config.in > create mode 100644 package/google-breakpad/google-breakpad.mk Thanks, I've applied your patch, after making some changes according to Arnout comments. From the commit log: [Thomas: - Introduce a BR2_PACKAGE_GOOGLE_BREAKPAD_ARCH_SUPPORTS Config.in symbol to be able to easily propagate the architecture dependencies. - Wrap the help text, add some more details as suggested by Arnout. - Propagate the architecture dependencies to the comment, as suggested by Arnout. - Remove the dependency of google-breakpad on host-google-breakpad, since it's not needed. - Add <pkg>_TARGET = NO, because google-breakpad only installs a static library, so installation to staging is sufficient. - Reorder autotools-package/host-autotools-package invocations, as suggested by Arnout.] Also, could you explain why you're passing --disable-processor when building the target package google-breakpad? For the second patch adding the core support for google-breakpad, there is still some on-going discussion, so I believe it's not ready to be applied yet. Thanks, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 1/2] google-breakpad: new package 2014-06-29 10:30 ` Thomas Petazzoni @ 2014-07-02 12:59 ` Pascal Huerst 0 siblings, 0 replies; 11+ messages in thread From: Pascal Huerst @ 2014-07-02 12:59 UTC (permalink / raw) To: buildroot Hey Thomas, all On 29.06.2014 12:30, Thomas Petazzoni wrote: > On Wed, 25 Jun 2014 15:16:13 +0200, Pascal Huerst wrote: >> Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com> >> --- >> package/Config.in | 1 + >> package/google-breakpad/Config.in | 19 +++++++++++++++++++ >> package/google-breakpad/google-breakpad.mk | 17 +++++++++++++++++ >> 3 files changed, 37 insertions(+) >> create mode 100644 package/google-breakpad/Config.in >> create mode 100644 package/google-breakpad/google-breakpad.mk > > Thanks, I've applied your patch, after making some changes according to > Arnout comments. From the commit log: > > [Thomas: > - Introduce a BR2_PACKAGE_GOOGLE_BREAKPAD_ARCH_SUPPORTS Config.in > symbol to be able to easily propagate the architecture > dependencies. > - Wrap the help text, add some more details as suggested by Arnout. > - Propagate the architecture dependencies to the comment, as > suggested by Arnout. > - Remove the dependency of google-breakpad on host-google-breakpad, > since it's not needed. > - Add <pkg>_TARGET = NO, because google-breakpad only installs a > static library, so installation to staging is sufficient. > - Reorder autotools-package/host-autotools-package invocations, as > suggested by Arnout.] Glad to hear that! And thank you for fixing up some Issues! > Also, could you explain why you're passing --disable-processor when > building the target package google-breakpad? I made the assumption, that one will always do the debugging on the host and that processor-tools such as minidump-stackwalk are not needed on the target. > For the second patch adding the core support for google-breakpad, there > is still some on-going discussion, so I believe it's not ready to be > applied yet. Yes. I will have a look into that afterwards. Thanks, Pascal ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in 2014-06-25 13:16 [Buildroot] [PATCH V5 0/2] google-breakpad: new package Pascal Huerst 2014-06-25 13:16 ` [Buildroot] [PATCH V5 1/2] " Pascal Huerst @ 2014-06-25 13:16 ` Pascal Huerst 2014-06-25 20:31 ` Arnout Vandecappelle 1 sibling, 1 reply; 11+ messages in thread From: Pascal Huerst @ 2014-06-25 13:16 UTC (permalink / raw) To: buildroot Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com> --- Config.in | 14 ++++++++++++++ Makefile | 5 +++++ package/google-breakpad/Config.in | 5 ++++- package/google-breakpad/gen-syms.sh | 25 +++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 1 deletion(-) create mode 100755 package/google-breakpad/gen-syms.sh diff --git a/Config.in b/Config.in index bf1ca86..4f2bd32 100644 --- a/Config.in +++ b/Config.in @@ -480,6 +480,20 @@ config BR2_OPTIMIZE_S endchoice +config BR2_GOOGLE_BREAKPAD_INCLUDE_FILES + string "executables and libraries to be used by google-breakpad" + depends on BR2_PACKAGE_GOOGLE_BREAKPAD + default "" + help + You may specify a space-separated list of binaries and libraries + with full paths relative to $(TARGET_DIR) of which debug symbols + will be dumped for further use with google breakpad. + + A directory structure that can be used by minidump-stackwalk will + be created at: + + $(STAGING_DIR)/usr/share/google-breakpad-symbols + config BR2_ENABLE_SSP bool "build code with Stack Smashing Protection" depends on BR2_TOOLCHAIN_HAS_SSP diff --git a/Makefile b/Makefile index 4fe370a..300e86e 100644 --- a/Makefile +++ b/Makefile @@ -553,6 +553,11 @@ endif ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PYC_ONLY),y) find $(TARGET_DIR)/usr/lib/ -name '*.py' -print0 | xargs -0 rm -f endif +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)) +endif + rm -rf $(TARGET_DIR)/usr/lib/luarocks rm -rf $(TARGET_DIR)/usr/lib/perl5/$(PERL_VERSION)/$(PERL_ARCHNAME)/CORE find $(TARGET_DIR)/usr/lib/perl5/ -name '*.bs' -print0 | xargs -0 rm -f diff --git a/package/google-breakpad/Config.in b/package/google-breakpad/Config.in index fa1e8d9..99dc7a1 100644 --- a/package/google-breakpad/Config.in +++ b/package/google-breakpad/Config.in @@ -11,7 +11,10 @@ config BR2_PACKAGE_GOOGLE_BREAKPAD Breakpad can also write minidumps on request for programs that have not crashed. - You may want to set BR2_ENABLE_DEBUG, in order to get useful results. + Add all binaries and libraries you want to debug by google-breakpad to + BR2_GOOGLE_BREAKPAD_INCLUDE_FILES. + + You may also want to set BR2_ENABLE_DEBUG, in order to get useful results. http://code.google.com/p/google-breakpad/ diff --git a/package/google-breakpad/gen-syms.sh b/package/google-breakpad/gen-syms.sh new file mode 100755 index 0000000..d890752 --- /dev/null +++ b/package/google-breakpad/gen-syms.sh @@ -0,0 +1,25 @@ +#!/bin/sh +STAGING_DIR="${1}" +TARGET_DIR="${2}" +shift 2 + +SYMBOLS_DIR="${STAGING_DIR}/usr/share/google-breakpad-symbols" +rm -rf "${SYMBOLS_DIR}" +mkdir -p "${SYMBOLS_DIR}/tmp" + +for FILE in $(eval ls "${TARGET_DIR}/${@}"); do + if [ -d "${FILE}" ]; then + printf "Error: '%s' is a directory\n" "${FILE}" >&2 + exit 1 + fi + if dump_syms "${FILE}" > "${SYMBOLS_DIR}/tmp/tmp.sym" 2>/dev/null; then + HASH=$(head -n1 "${SYMBOLS_DIR}/tmp/tmp.sym" | cut -d ' ' -f 4); + FILENAME=$(basename "$FILE"); + mkdir -p "${SYMBOLS_DIR}/${FILENAME}/${HASH}" + mv "${SYMBOLS_DIR}/tmp/tmp.sym" "${SYMBOLS_DIR}/${FILENAME}/${HASH}/${FILENAME}.sym"; + else + printf "Error dumping symbols for: '%s'\n" "${FILE}" >&2 + exit 1 + fi +done +rm -rf "${SYMBOLS_DIR}/tmp" -- 1.9.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in 2014-06-25 13:16 ` [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in Pascal Huerst @ 2014-06-25 20:31 ` Arnout Vandecappelle 2014-06-29 10:36 ` Thomas Petazzoni 0 siblings, 1 reply; 11+ messages in thread From: Arnout Vandecappelle @ 2014-06-25 20:31 UTC (permalink / raw) To: buildroot On 25/06/14 15:16, Pascal Huerst wrote: > Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com> > --- > Config.in | 14 ++++++++++++++ > Makefile | 5 +++++ > package/google-breakpad/Config.in | 5 ++++- > package/google-breakpad/gen-syms.sh | 25 +++++++++++++++++++++++++ > 4 files changed, 48 insertions(+), 1 deletion(-) > create mode 100755 package/google-breakpad/gen-syms.sh > > diff --git a/Config.in b/Config.in > index bf1ca86..4f2bd32 100644 > --- a/Config.in > +++ b/Config.in > @@ -480,6 +480,20 @@ config BR2_OPTIMIZE_S > > endchoice > > +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 ? > + 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 :-) > + default "" > + help > + You may specify a space-separated list of binaries and libraries > + with full paths relative to $(TARGET_DIR) of which debug symbols Again trailing whitespace and line-wrapping. I think this text is slightly better: You may specify a space-separated list of glob patterns of binaries and libraries of which debug symbols will be dumped for further use with google breakpad. Th patterns are evaluated relative to $(TARGET_DIR). > + will be dumped for further use with google breakpad. > + > + A directory structure that can be used by minidump-stackwalk will > + be created at: > + > + $(STAGING_DIR)/usr/share/google-breakpad-symbols > + > config BR2_ENABLE_SSP > bool "build code with Stack Smashing Protection" > depends on BR2_TOOLCHAIN_HAS_SSP > diff --git a/Makefile b/Makefile > index 4fe370a..300e86e 100644 > --- a/Makefile > +++ b/Makefile > @@ -553,6 +553,11 @@ endif > ifeq ($(BR2_PACKAGE_PYTHON_PYC_ONLY)$(BR2_PACKAGE_PYTHON3_PYC_ONLY),y) > find $(TARGET_DIR)/usr/lib/ -name '*.py' -print0 | xargs -0 rm -f > endif > +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. If the config variable contains a glob pattern, then this will be evaluated here (relative to the buildroot directory). Usually that will fail so it will be passed unexpanded, but perhaps it's safer to explicitly turn off globbing. Together with my other suggestions, this would become: ifeq ($(BR2_PACKAGE_GOOGLE_BREAKPAD),y) GOOGLE_BREAKPAD_INCLUDE_FILES = $(call qstrip,$(BR2_GOOGLE_BREAKPAD_INCLUDE_FILES)) ifneq ($(GOOGLE_BREAKPAD_INCLUDE_FILES),) # BR2_GOOGLE_BREAKPAD_INCLUDE_FILES may include glob files, so turn off globbing define GOOGLE_BREAKPAD_GEN_SYMS $(Q)set -o noglob; \ $(EXTRA_ENV) package/google-breakpad/gen-syms.sh \ $(STAGING_DIR) $(TARGET_DIR) $(GOOGLE_BREAKPAD_INCLUDE_FILES) endef endif endif > +endif > + > rm -rf $(TARGET_DIR)/usr/lib/luarocks > rm -rf $(TARGET_DIR)/usr/lib/perl5/$(PERL_VERSION)/$(PERL_ARCHNAME)/CORE > find $(TARGET_DIR)/usr/lib/perl5/ -name '*.bs' -print0 | xargs -0 rm -f > diff --git a/package/google-breakpad/Config.in b/package/google-breakpad/Config.in > index fa1e8d9..99dc7a1 100644 > --- a/package/google-breakpad/Config.in > +++ b/package/google-breakpad/Config.in > @@ -11,7 +11,10 @@ config BR2_PACKAGE_GOOGLE_BREAKPAD > Breakpad can also write minidumps on request for programs that have not > crashed. > > - You may want to set BR2_ENABLE_DEBUG, in order to get useful results. > + Add all binaries and libraries you want to debug by google-breakpad to Trailing whitespace again. Also, I'd write Add all binaries and libraries that link with the google-breakpad client library and that you want to debug with minidump-stackwalk to BR2_GOOGLE_BREAKPAD_INCLUDE_FILES. > + BR2_GOOGLE_BREAKPAD_INCLUDE_FILES. > + > + You may also want to set BR2_ENABLE_DEBUG, in order to get useful results. > > http://code.google.com/p/google-breakpad/ > > diff --git a/package/google-breakpad/gen-syms.sh b/package/google-breakpad/gen-syms.sh > new file mode 100755 > index 0000000..d890752 > --- /dev/null > +++ b/package/google-breakpad/gen-syms.sh > @@ -0,0 +1,25 @@ > +#!/bin/sh > +STAGING_DIR="${1}" > +TARGET_DIR="${2}" > +shift 2 > + > +SYMBOLS_DIR="${STAGING_DIR}/usr/share/google-breakpad-symbols" > +rm -rf "${SYMBOLS_DIR}" > +mkdir -p "${SYMBOLS_DIR}/tmp" > + > +for FILE in $(eval ls "${TARGET_DIR}/${@}"); do This does not really work: if one of the patterns matches a directory, the directory will still be listed. So you have to use ls -d. However, I think the following is easier to understand, even if it's more wordy: for PATTERN in "$@"; do for FILE in ${TARGET_DIR}/${PATTERN}; do ... done done > + if [ -d "${FILE}" ]; then > + printf "Error: '%s' is a directory\n" "${FILE}" >&2 > + exit 1 > + fi > + if dump_syms "${FILE}" > "${SYMBOLS_DIR}/tmp/tmp.sym" 2>/dev/null; then Do we really want to redirect stderr here? Why do you put tmp.sym in a subdirectory, and not straignt into ${SYMBOLS_DIR}? > + HASH=$(head -n1 "${SYMBOLS_DIR}/tmp/tmp.sym" | cut -d ' ' -f 4); > + FILENAME=$(basename "$FILE"); I don't know if it is the official buildroot policy, but recently I've seen most scripts use "${FILE##*/}" instead of basename. Regards, Arnout > + mkdir -p "${SYMBOLS_DIR}/${FILENAME}/${HASH}" > + mv "${SYMBOLS_DIR}/tmp/tmp.sym" "${SYMBOLS_DIR}/${FILENAME}/${HASH}/${FILENAME}.sym"; > + else > + printf "Error dumping symbols for: '%s'\n" "${FILE}" >&2 > + exit 1 > + fi > +done > +rm -rf "${SYMBOLS_DIR}/tmp" > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in 2014-06-25 20:31 ` Arnout Vandecappelle @ 2014-06-29 10:36 ` Thomas Petazzoni 2014-07-01 6:19 ` Arnout Vandecappelle 2014-07-09 9:39 ` Pascal Huerst 0 siblings, 2 replies; 11+ messages in thread From: Thomas Petazzoni @ 2014-06-29 10:36 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in 2014-06-29 10:36 ` Thomas Petazzoni @ 2014-07-01 6:19 ` Arnout Vandecappelle 2014-07-09 9:48 ` Pascal Huerst 2014-07-09 9:39 ` Pascal Huerst 1 sibling, 1 reply; 11+ messages in thread From: Arnout Vandecappelle @ 2014-07-01 6:19 UTC (permalink / raw) To: buildroot On 29/06/14 12:36, Thomas Petazzoni wrote: > 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. My bad, I was reviewing this patch side-by-side with the comments on v4 and the script looked a bit like what was proposed by Yann in his review so I assumed that it would include support for glob patterns. Personally I don't really see this as a requirement. > >>> + 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 If it is done in that way, then it really should be in the google breakpad package and not in the toolchain menu... However, thinking a bit more about this: the dependency on the target package is not correct. There's a dependency on the host package because the host tools are used, but it's the responsibility of whatever package is linking against the static library to also depend on the target package. So in that case, it is indeed best to keep it in the toolchain menu, remove the dependency on the target package, but add a boolean config option to enable breakpad to begin with. Regards, Arnout > >>> +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 > -- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in 2014-07-01 6:19 ` Arnout Vandecappelle @ 2014-07-09 9:48 ` Pascal Huerst 0 siblings, 0 replies; 11+ messages in thread From: Pascal Huerst @ 2014-07-09 9:48 UTC (permalink / raw) To: buildroot Hey Arnout, Thomas, all -- snip -- On 01.07.2014 08:19, Arnout Vandecappelle wrote: > On 29/06/14 12:36, Thomas Petazzoni wrote: >> On Wed, 25 Jun 2014 22:31:20 +0200, Arnout Vandecappelle wrote: >>> 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 > > If it is done in that way, then it really should be in the google breakpad > package and not in the toolchain menu... > > > However, thinking a bit more about this: the dependency on the target package > is not correct. There's a dependency on the host package because the host tools > are used, but it's the responsibility of whatever package is linking against the > static library to also depend on the target package. > > So in that case, it is indeed best to keep it in the toolchain menu, remove the > dependency on the target package, but add a boolean config option to enable > breakpad to begin with. Does this look reasonable? config BR2_GOOGLE_BREAKPAD_ENABLE bool "Enable google-breakpad support" select BR2_PACKAGE_GOOGLE_BREAKPAD help This option will enable the use of google breakpad, a library and tool suite that allows you to distribute an application to users with compiler-provided debugging information removed, record crashes in compact "minidump" files, send them back to your server and produce C and C++ stack traces from these minidumps. Breakpad can also write minidumps on request for programs that have not crashed. if BR2_GOOGLE_BREAKPAD_ENABLE config BR2_GOOGLE_BREAKPAD_INCLUDE_FILES string "List of executables and libraries to extract symbols from" default "" help You may specify a space-separated list of binaries and libraries with full paths relative to $(TARGET_DIR) of which debug symbols will be dumped for further use with google breakpad. A directory structure that can be used by minidump-stackwalk will be created at: $(STAGING_DIR)/usr/share/google-breakpad-symbols endif regards pascal ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in 2014-06-29 10:36 ` Thomas Petazzoni 2014-07-01 6:19 ` Arnout Vandecappelle @ 2014-07-09 9:39 ` Pascal Huerst 1 sibling, 0 replies; 11+ messages in thread From: Pascal Huerst @ 2014-07-09 9:39 UTC (permalink / raw) To: buildroot Hey Thomas, Arnout, all -- snip -- > 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. Can you give me a hint, on how this should be done then? > Yes, having it all in the google-breakpad.mk seems like a good idea. > > Thomas > Pascal ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-07-09 9:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-25 13:16 [Buildroot] [PATCH V5 0/2] google-breakpad: new package Pascal Huerst 2014-06-25 13:16 ` [Buildroot] [PATCH V5 1/2] " Pascal Huerst 2014-06-25 19:16 ` Arnout Vandecappelle 2014-06-29 10:30 ` Thomas Petazzoni 2014-07-02 12:59 ` Pascal Huerst 2014-06-25 13:16 ` [Buildroot] [PATCH V5 2/2] google-breakpad: integration into Makefile and Config.in Pascal Huerst 2014-06-25 20:31 ` Arnout Vandecappelle 2014-06-29 10:36 ` Thomas Petazzoni 2014-07-01 6:19 ` Arnout Vandecappelle 2014-07-09 9:48 ` Pascal Huerst 2014-07-09 9:39 ` Pascal Huerst
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.