Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH V4 0/2] google-breakpad: new package
@ 2014-06-04 11:32 Pascal Huerst
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 1/2] " Pascal Huerst
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 2/2] google-breakpad: integration into makefile and Config.in Pascal Huerst
  0 siblings, 2 replies; 8+ messages in thread
From: Pascal Huerst @ 2014-06-04 11:32 UTC (permalink / raw)
  To: buildroot

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                                          | 10 ++++++
 Makefile                                           |  7 ++++
 package/Config.in                                  |  2 ++
 package/google-breakpad/Config.in                  | 21 ++++++++++++
 .../google-breakpad/google-breakpad-gen-syms.sh    | 37 ++++++++++++++++++++++
 package/google-breakpad/google-breakpad.mk         | 17 ++++++++++
 6 files changed, 94 insertions(+)
 create mode 100644 package/google-breakpad/Config.in
 create mode 100755 package/google-breakpad/google-breakpad-gen-syms.sh
 create mode 100644 package/google-breakpad/google-breakpad.mk

-- 
1.9.3

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH V4 1/2] google-breakpad: new package
  2014-06-04 11:32 [Buildroot] [PATCH V4 0/2] google-breakpad: new package Pascal Huerst
@ 2014-06-04 11:32 ` Pascal Huerst
  2014-06-04 19:53   ` Yann E. MORIN
  2014-06-09 17:41   ` Samuel Martin
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 2/2] google-breakpad: integration into makefile and Config.in Pascal Huerst
  1 sibling, 2 replies; 8+ messages in thread
From: Pascal Huerst @ 2014-06-04 11:32 UTC (permalink / raw)
  To: buildroot

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.

Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
---
 package/Config.in                                  |  1 +
 package/google-breakpad/Config.in                  | 21 ++++++++++++
 .../google-breakpad/google-breakpad-gen-syms.sh    | 37 ++++++++++++++++++++++
 package/google-breakpad/google-breakpad.mk         | 17 ++++++++++
 4 files changed, 76 insertions(+)
 create mode 100644 package/google-breakpad/Config.in
 create mode 100755 package/google-breakpad/google-breakpad-gen-syms.sh
 create mode 100644 package/google-breakpad/google-breakpad.mk

diff --git a/package/Config.in b/package/Config.in
index 1706197..ea94f01 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -54,6 +54,7 @@ source "package/dstat/Config.in"
 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..5c9e18b
--- /dev/null
+++ b/package/google-breakpad/Config.in
@@ -0,0 +1,21 @@
+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. 	 
+
+	  Add all binaries and libraries you want to debug by google-breakpad to
+	  BR2_GOOGLE_BREAKPAD_INCLUDE_FILES
+
+	  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-gen-syms.sh b/package/google-breakpad/google-breakpad-gen-syms.sh
new file mode 100755
index 0000000..e082875
--- /dev/null
+++ b/package/google-breakpad/google-breakpad-gen-syms.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+STAGING_DIR=$1
+HOST_DIR=$2
+INCLUDE_FILES=$3
+
+# Search for files we want to dump symbols of
+FIND_CMD="find $STAGING_DIR -name \"\""
+
+for INCLUDE_FILE in $INCLUDE_FILES; do
+	FIND_CMD="$FIND_CMD -o -name \"$INCLUDE_FILE\""
+done
+
+# Create directory structure
+SYMBOLS_DIR=$STAGING_DIR/usr/share/google-breakpad-symbols
+
+mkdir -p $SYMBOLS_DIR/tmp
+
+for BIN_PATH in $(eval $FIND_CMD); do
+	BIN=$(basename $BIN_PATH);
+	SYM=$BIN.sym;
+
+	$HOST_DIR/usr/bin/dump_syms $BIN_PATH > $SYMBOLS_DIR/tmp/$SYM 2>/dev/null;
+
+	if [ $? -eq 0 ]; then
+		HASH=$(head -n1 $SYMBOLS_DIR/tmp/$SYM | cut -d ' ' -f 4);
+		mkdir -p $SYMBOLS_DIR/$BIN/$HASH
+		mv $SYMBOLS_DIR/tmp/$SYM $SYMBOLS_DIR/$BIN/$HASH;
+	else
+		rm -f $SYMBOLS_DIR/tmp/$SYM
+		echo "Can not dump symbols of $BIN for google-breakpad!"
+	fi
+done
+
+rm -Rf $SYMBOLS_DIR/tmp
+
+exit 0
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] 8+ messages in thread

* [Buildroot] [PATCH V4 2/2] google-breakpad: integration into makefile and Config.in
  2014-06-04 11:32 [Buildroot] [PATCH V4 0/2] google-breakpad: new package Pascal Huerst
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 1/2] " Pascal Huerst
@ 2014-06-04 11:32 ` Pascal Huerst
  2014-06-04 19:59   ` Yann E. MORIN
  1 sibling, 1 reply; 8+ messages in thread
From: Pascal Huerst @ 2014-06-04 11:32 UTC (permalink / raw)
  To: buildroot

This adds the symbol generation for google-breakpad to
the Makefile and adds a list were users can insert libraries
and binaries that should be symbol-dumped.

Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
---
 Config.in         | 10 ++++++++++
 Makefile          |  7 +++++++
 package/Config.in |  1 +
 3 files changed, 18 insertions(+)

diff --git a/Config.in b/Config.in
index 0799cb7..0849146 100644
--- a/Config.in
+++ b/Config.in
@@ -457,6 +457,16 @@ 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-seperated list of binaries and libraries
+	  here of which debug symbols should be dumped for google breakpad.
+	  Debug symbols will be stored as google_breakpad_symbols in: 
+	  staging/usr/shared/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 0b4264a..2650a93 100644
--- a/Makefile
+++ b/Makefile
@@ -553,6 +553,13 @@ 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)
+	package/google-breakpad/google-breakpad-gen-syms.sh \
+		$(STAGING_DIR) \
+		$(HOST_DIR) \
+		"$(call qstrip,$(BR2_GOOGLE_BREAKPAD_INCLUDE_FILES))" 
+endif
+
 	rm -rf $(TARGET_DIR)/usr/lib/luarocks
 	$(STRIP_FIND_CMD) | xargs $(STRIPCMD) 2>/dev/null || true
 	if test -d $(TARGET_DIR)/lib/modules; then \
diff --git a/package/Config.in b/package/Config.in
index ea94f01..47cb483 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -786,6 +786,7 @@ source "package/fftw/Config.in"
 source "package/flann/Config.in"
 source "package/glibmm/Config.in"
 source "package/gmp/Config.in"
+source "package/google-breakpad/Config.in"
 source "package/gsl/Config.in"
 source "package/gtest/Config.in"
 source "package/libargtable2/Config.in"
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH V4 1/2] google-breakpad: new package
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 1/2] " Pascal Huerst
@ 2014-06-04 19:53   ` Yann E. MORIN
  2014-06-05 20:21     ` Pascal Hürst
  2014-06-09 17:41   ` Samuel Martin
  1 sibling, 1 reply; 8+ messages in thread
From: Yann E. MORIN @ 2014-06-04 19:53 UTC (permalink / raw)
  To: buildroot

Pascal, All,

Some comments below...

On 2014-06-04 13:32 +0200, Pascal Huerst spake thusly:
> 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.
> 
> Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
> ---
>  package/Config.in                                  |  1 +
>  package/google-breakpad/Config.in                  | 21 ++++++++++++
>  .../google-breakpad/google-breakpad-gen-syms.sh    | 37 ++++++++++++++++++++++
>  package/google-breakpad/google-breakpad.mk         | 17 ++++++++++
>  4 files changed, 76 insertions(+)
>  create mode 100644 package/google-breakpad/Config.in
>  create mode 100755 package/google-breakpad/google-breakpad-gen-syms.sh
>  create mode 100644 package/google-breakpad/google-breakpad.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index 1706197..ea94f01 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -54,6 +54,7 @@ source "package/dstat/Config.in"
>  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..5c9e18b
> --- /dev/null
> +++ b/package/google-breakpad/Config.in
> @@ -0,0 +1,21 @@
> +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
> +

No empty line here.

> +	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. 	 
> +
> +	  Add all binaries and libraries you want to debug by google-breakpad to
> +	  BR2_GOOGLE_BREAKPAD_INCLUDE_FILES

These two lines should have been part of the following patch, since you
introduce that variable only then.

> +	  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-gen-syms.sh b/package/google-breakpad/google-breakpad-gen-syms.sh
> new file mode 100755
> index 0000000..e082875
> --- /dev/null
> +++ b/package/google-breakpad/google-breakpad-gen-syms.sh

This script should have been part of your next patch.

> @@ -0,0 +1,37 @@
> +#!/bin/sh
> +
> +STAGING_DIR=$1
> +HOST_DIR=$2

Passing HOST_DIR is not needed. You should call the script with the
EXTRA_ENV which contains the PATH variable, in the Makefile:

    $(EXTRA_ENV) your-script your-args

> +INCLUDE_FILES=$3
> +
> +# Search for files we want to dump symbols of
> +FIND_CMD="find $STAGING_DIR -name \"\""

Ugly hack. It took me a moment to understand what you wanted.
See below for a suggested better way to achieve the same result.

Besides, not all package install their files in staging. You should use
TARGET_DIR instead.

> +for INCLUDE_FILE in $INCLUDE_FILES; do
> +	FIND_CMD="$FIND_CMD -o -name \"$INCLUDE_FILE\""
> +done

This does not handle the case where two files would have the same
basename but reside in different directories. For example:
    /usr/bin/my-exec        <- ELF file
    /data/config/my-exec    <- database

Please state that INCLUDE_FILES should be relative to $(TARGET_DIR).
That way, we guarantee breakpad is not run against unwanted files, which
would otherwise confuse the unsuspecting user.

> +# Create directory structure
> +SYMBOLS_DIR=$STAGING_DIR/usr/share/google-breakpad-symbols
> +
> +mkdir -p $SYMBOLS_DIR/tmp
> +
> +for BIN_PATH in $(eval $FIND_CMD); do
> +	BIN=$(basename $BIN_PATH);
> +	SYM=$BIN.sym;
> +
> +	$HOST_DIR/usr/bin/dump_syms $BIN_PATH > $SYMBOLS_DIR/tmp/$SYM 2>/dev/null;
> +
> +	if [ $? -eq 0 ]; then
> +		HASH=$(head -n1 $SYMBOLS_DIR/tmp/$SYM | cut -d ' ' -f 4);
> +		mkdir -p $SYMBOLS_DIR/$BIN/$HASH
> +		mv $SYMBOLS_DIR/tmp/$SYM $SYMBOLS_DIR/$BIN/$HASH;
> +	else
> +		rm -f $SYMBOLS_DIR/tmp/$SYM
> +		echo "Can not dump symbols of $BIN for google-breakpad!"
> +	fi
> +done
> +
> +rm -Rf $SYMBOLS_DIR/tmp
> +
> +exit 0

'exit 0' is not needed.

What about the following (untested)?

    #!/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"

    cd "${TARGET_DIR}"
    for file in $(ls -1d "${@}"); 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.syms"; then
            sha1="$( printf "%s" "${file}" |sha1sum |awk '{print $1}' )"
            mv "${symbols_dir}/tmp/tmp.syms" "${symbols_dir}/${sha1}.sym"
            printf "%s %s\n" "${sha1}" "${file}" >>"${symbols_dir}/hashes.list"
        else
            printf "Error dumping symbols for: '%s'\n" "${file}" >&2
            exit 1
        fi
    done
    rm -rf "${symbols_dir}/tmp"

It is more concise, and handles mutilple files with the same name, and
packages which do not install their files in STAGING_DIR.

${1} and ${2} are the staging and target dirs, so we store them and shift,
so the remaining arguments are all the paths to files we want to extract
symbols from.

We $(ls -1d) the args, in case one of them is a glob.

A directory argument is invalid. The user should use a glob.

We can use 'dump_syms' without full path since it is guaranteed to be in
the PATH, as exported by the EXTRA_ENV variable used above.

We store the symbols in a file named after the sha1 of the filename. We
could use the symbols file's hash if it is a strong enough hash (eg sha1
or above, but since I don;t know what hash dump_syms generates, I can't
trust it to not collide). This allows to store symbols for file with the
same name in different directories.

We store the correspondence {sha1,file} in a text file, so it is easy to
retrieve the filename from a sha1, and conversely.

We exit as soon as we can't extract symbols from a file, as we want to
catch it as a failure, instead of ignoring it.

But this script should go in the second patch of yours in any case.

> 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

Do we really want to disable tools even for the host variant?
Isn't dump_symc a tool that gets disabled in this case?

> +GOOGLE_BREAKPAD_DEPENDENCIES = host-google-breakpad

Does the target variant have a build-time dependency on the host tools?

If not, then you should do, in Config.in:

    config BR2_PACKAGE_GOOGLE_BREAKPAD
        bool "google breakpad"
        select BR2_PACKAGE_HOST_GOOGLE_BREAKPAD

    config BR2_PACKAGE_HOST_GOOGLE_BREAKPAD
        bool

(with the other depends you need, of course.)

Regards,
Yann E. MORIN.

> +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
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH V4 2/2] google-breakpad: integration into makefile and Config.in
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 2/2] google-breakpad: integration into makefile and Config.in Pascal Huerst
@ 2014-06-04 19:59   ` Yann E. MORIN
  0 siblings, 0 replies; 8+ messages in thread
From: Yann E. MORIN @ 2014-06-04 19:59 UTC (permalink / raw)
  To: buildroot

Pascal, All,

On 2014-06-04 13:32 +0200, Pascal Huerst spake thusly:
> This adds the symbol generation for google-breakpad to
> the Makefile and adds a list were users can insert libraries
> and binaries that should be symbol-dumped.
> 
> Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
> ---
>  Config.in         | 10 ++++++++++
>  Makefile          |  7 +++++++
>  package/Config.in |  1 +
>  3 files changed, 18 insertions(+)
> 
> diff --git a/Config.in b/Config.in
> index 0799cb7..0849146 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -457,6 +457,16 @@ 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-seperated list of binaries and libraries

"separated"

> +	  here of which debug symbols should be dumped for google breakpad.
> +	  Debug symbols will be stored as google_breakpad_symbols in: 
> +	  staging/usr/shared/google-breakpad-symbols

Replace 'staging' with '$(STAGING_DIR)'

Also, state that the files should be full paths relative to $(TARGET_DIR)

>  config BR2_ENABLE_SSP
>  	bool "build code with Stack Smashing Protection"
>  	depends on BR2_TOOLCHAIN_HAS_SSP
> diff --git a/Makefile b/Makefile
> index 0b4264a..2650a93 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -553,6 +553,13 @@ 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)
> +	package/google-breakpad/google-breakpad-gen-syms.sh \

As said in my review on your previous patch, you should use EXTRA_ENV
here:

    $(EXTRA_ENV) package/google-breakpad/gen-syms.sh $(STAGING_DIR) \
        $(TARGET_DIR) $(call qstrip,$(BR2_GOOGLE_BREAKPAD_INCLUDE_FILES))

We do not quote BR2_GOOGLE_BREAKPAD_INCLUDE_FILES since we want to pass
each entry as its own arg on the command line.

Also, maybe name the script just 'gen-syms.sh', it's shorter, so you can
fit more on a line. ;-)

> +		$(STAGING_DIR) \
> +		$(HOST_DIR) \
> +		"$(call qstrip,$(BR2_GOOGLE_BREAKPAD_INCLUDE_FILES))" 
> +endif
> +
>  	rm -rf $(TARGET_DIR)/usr/lib/luarocks
>  	$(STRIP_FIND_CMD) | xargs $(STRIPCMD) 2>/dev/null || true
>  	if test -d $(TARGET_DIR)/lib/modules; then \
> diff --git a/package/Config.in b/package/Config.in
> index ea94f01..47cb483 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -786,6 +786,7 @@ source "package/fftw/Config.in"
>  source "package/flann/Config.in"
>  source "package/glibmm/Config.in"
>  source "package/gmp/Config.in"
> +source "package/google-breakpad/Config.in"

This should be part of your previous patch. And it should be in
the "Debugging, profiling and benchmark" sub-menu, instead of
"Libraries/Others".

Regards,
Yann E. MORIN.

>  source "package/gsl/Config.in"
>  source "package/gtest/Config.in"
>  source "package/libargtable2/Config.in"
> -- 
> 1.9.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH V4 1/2] google-breakpad: new package
  2014-06-04 19:53   ` Yann E. MORIN
@ 2014-06-05 20:21     ` Pascal Hürst
  2014-06-05 20:28       ` Yann E. MORIN
  0 siblings, 1 reply; 8+ messages in thread
From: Pascal Hürst @ 2014-06-05 20:21 UTC (permalink / raw)
  To: buildroot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello Yann, all,

[snip]

>> diff --git a/package/google-breakpad/google-breakpad-gen-syms.sh 
>> b/package/google-breakpad/google-breakpad-gen-syms.sh new file 
>> mode 100755 index 0000000..e082875 --- /dev/null +++ 
>> b/package/google-breakpad/google-breakpad-gen-syms.sh
> 
> This script should have been part of your next patch.
> 
>> @@ -0,0 +1,37 @@ +#!/bin/sh + +STAGING_DIR=$1 +HOST_DIR=$2
> 
> Passing HOST_DIR is not needed. You should call the script with the
> EXTRA_ENV which contains the PATH variable, in the Makefile:
> 
> $(EXTRA_ENV) your-script your-args
> 
>> +INCLUDE_FILES=$3 + +# Search for files we want to dump symbols 
>> of +FIND_CMD="find $STAGING_DIR -name \"\""
> 
> Ugly hack. It took me a moment to understand what you wanted. See 
> below for a suggested better way to achieve the same result.
> 
> Besides, not all package install their files in staging. You
> should use TARGET_DIR instead.

Aren't all binaries in TARGET_DIR stripped? If so, it doesn't really
make sense to run gen_syms in TARGET_DIR.

>> +for INCLUDE_FILE in $INCLUDE_FILES; do +	FIND_CMD="$FIND_CMD -o 
>> -name \"$INCLUDE_FILE\"" +done
> 
> This does not handle the case where two files would have the same 
> basename but reside in different directories. For example: 
> /usr/bin/my-exec        <- ELF file /data/config/my-exec    <- 
> database
> 
> Please state that INCLUDE_FILES should be relative to 
> $(TARGET_DIR). That way, we guarantee breakpad is not run against 
> unwanted files, which would otherwise confuse the unsuspecting 
> user.
> 
>> +# Create directory structure 
>> +SYMBOLS_DIR=$STAGING_DIR/usr/share/google-breakpad-symbols + 
>> +mkdir -p $SYMBOLS_DIR/tmp + +for BIN_PATH in $(eval $FIND_CMD); 
>> do +	BIN=$(basename $BIN_PATH); +	SYM=$BIN.sym; + + 
>> $HOST_DIR/usr/bin/dump_syms $BIN_PATH > $SYMBOLS_DIR/tmp/$SYM 
>> 2>/dev/null; + +	if [ $? -eq 0 ]; then +		HASH=$(head -n1 
>> $SYMBOLS_DIR/tmp/$SYM | cut -d ' ' -f 4); +		mkdir -p 
>> $SYMBOLS_DIR/$BIN/$HASH +		mv $SYMBOLS_DIR/tmp/$SYM 
>> $SYMBOLS_DIR/$BIN/$HASH; +	else +		rm -f $SYMBOLS_DIR/tmp/$SYM + 
>> echo "Can not dump symbols of $BIN for google-breakpad!" +	fi 
>> +done + +rm -Rf $SYMBOLS_DIR/tmp + +exit 0
> 
> 'exit 0' is not needed.
> 
> What about the following (untested)?
> 
> #!/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"
> 
> cd "${TARGET_DIR}" for file in $(ls -1d "${@}"); 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.syms"; then sha1="$( printf "%s"
>> "${file}"
> |sha1sum |awk '{print $1}' )" mv "${symbols_dir}/tmp/tmp.syms" 
> "${symbols_dir}/${sha1}.sym" printf "%s %s\n" "${sha1}" "${file}"
>>> "${symbols_dir}/hashes.list" else printf "Error dumping
>>> symbols
> for: '%s'\n" "${file}" >&2 exit 1 fi done rm -rf 
> "${symbols_dir}/tmp"
> 
> It is more concise, and handles mutilple files with the same name, 
> and packages which do not install their files in STAGING_DIR.
> 
> ${1} and ${2} are the staging and target dirs, so we store them
> and shift, so the remaining arguments are all the paths to files
> we want to extract symbols from.
> 
> We $(ls -1d) the args, in case one of them is a glob.
> 
> A directory argument is invalid. The user should use a glob.
> 
> We can use 'dump_syms' without full path since it is guaranteed to 
> be in the PATH, as exported by the EXTRA_ENV variable used above.
> 
> We store the symbols in a file named after the sha1 of the 
> filename. We could use the symbols file's hash if it is a strong 
> enough hash (eg sha1 or above, but since I don;t know what hash 
> dump_syms generates, I can't trust it to not collide). This allows 
> to store symbols for file with the same name in different 
> directories.
> 
> We store the correspondence {sha1,file} in a text file, so it is 
> easy to retrieve the filename from a sha1, and conversely.
> 
> We exit as soon as we can't extract symbols from a file, as we
> want to catch it as a failure, instead of ignoring it.

Ok, I understand your concerns and the find-call is by far not
perfect, but the whole reason why I setup the directory structure that
way, is because googles minidump-stackwalk expects it that way. See:

https://code.google.com/p/google-breakpad/wiki/LinuxStarterGuide#Producing_symbols_for_your_application


[snip]

regards
pascal
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEUEARECAAYFAlOQ0VgACgkQo7eFcXQQ8U0B0QCfQbjF0nqWHLOcwr4AyiXLXDeL
BOIAl3P1IpV/hcSaXIEZpguemMN+RgU=
=apjR
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH V4 1/2] google-breakpad: new package
  2014-06-05 20:21     ` Pascal Hürst
@ 2014-06-05 20:28       ` Yann E. MORIN
  0 siblings, 0 replies; 8+ messages in thread
From: Yann E. MORIN @ 2014-06-05 20:28 UTC (permalink / raw)
  To: buildroot

Pascal, All,

On 2014-06-05 22:21 +0200, Pascal H?rst spake thusly:
> > Besides, not all package install their files in staging. You
> > should use TARGET_DIR instead.
> 
> Aren't all binaries in TARGET_DIR stripped? If so, it doesn't really
> make sense to run gen_syms in TARGET_DIR.

They are stripped only just before making the filesystem images.
So, if you run your script before that, you'll get unstripped binaries
in the target/ dir.

[--SNIP alternate script proposal--]
> Ok, I understand your concerns and the find-call is by far not
> perfect, but the whole reason why I setup the directory structure that
> way, is because googles minidump-stackwalk expects it that way. See:
> https://code.google.com/p/google-breakpad/wiki/LinuxStarterGuide#Producing_symbols_for_your_application

OK (I was afraid that layout was indeed a requirement). You can keep the
layout as you created it. Just adapt my script so it generates the proper
layout.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Buildroot] [PATCH V4 1/2] google-breakpad: new package
  2014-06-04 11:32 ` [Buildroot] [PATCH V4 1/2] " Pascal Huerst
  2014-06-04 19:53   ` Yann E. MORIN
@ 2014-06-09 17:41   ` Samuel Martin
  1 sibling, 0 replies; 8+ messages in thread
From: Samuel Martin @ 2014-06-09 17:41 UTC (permalink / raw)
  To: buildroot

Pascal, all,

On Wed, Jun 4, 2014 at 1:32 PM, Pascal Huerst <pascal.huerst@gmail.com> wrote:
[...]
> +################################################################################
> +#
> +# 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))

Unfortunately, among the built host-tools, minidump-2-core is not a
cross-tool :-( (see [1]), so it might be worthwhile to not installed
it.

[1] https://code.google.com/p/google-breakpad/source/browse/trunk/src/tools/linux/md2core/minidump-2-core.cc#425

> +$(eval $(autotools-package))
> --
> 1.9.3
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,

-- 
Samuel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-06-09 17:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-04 11:32 [Buildroot] [PATCH V4 0/2] google-breakpad: new package Pascal Huerst
2014-06-04 11:32 ` [Buildroot] [PATCH V4 1/2] " Pascal Huerst
2014-06-04 19:53   ` Yann E. MORIN
2014-06-05 20:21     ` Pascal Hürst
2014-06-05 20:28       ` Yann E. MORIN
2014-06-09 17:41   ` Samuel Martin
2014-06-04 11:32 ` [Buildroot] [PATCH V4 2/2] google-breakpad: integration into makefile and Config.in Pascal Huerst
2014-06-04 19:59   ` Yann E. MORIN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox