On Sat, Aug 20, 2011 at 04:07:24PM +0300, Vitaliy Gorbunov wrote:
I have read submitting_patches.txt and created git formatted patch. See it below.
Some comments about previous reviewing:
create_package() {
+ if (( SKIP_DEBUG_PACKAGE )); then
+ warning "$(gettext "No .debug files for debug package found,
skipping...")"
+ SKIP_DEBUG_PACKAGE=0
Why are we resetting the variable? Do we do this once per regular
package function? This seems misplaced, even if so.
SKIPDEBUGPACKAGE is needed to determine that debug files for package was not created (package has 'any' architecture or package was compiled without debug symbols). And we need to reset it, because the next package after debug will be regular package. I can't think of simpler way to do it.
+ pkgdesc="$pkgdesc (debugging symbols)" + depends=("${pkgname%-dbg}=$
+ restore_package_variables "dbg" pkgver-$pkgrel")
+ groups=('debug' ${groups[@]/%/-debug}) This seems very magic and arbitrary.
Why? If group consist some package, then ${group}-debug will consist appropriate debug package. It's seems fine for me.
+ if readelf -S $binary | grep "\.gnu_debuglink" &>/dev/null;
This is very Linux specific. Not sure what we think about this.
What do you mean? Why this is Linux-specific?
readelf is Linux specific, and you're assuming glibc/gcc. There's not really a portable way to read data out of executables given the array of formats for the platforms we support: PE, COFF, a.out, ELF, Mach-O... You're not going to find readelf in normal usage on cygwin or OSX. There's some other blockers I've commented on inline below. I sort of gave up with commenting on style because there's too much of it to cover and I start sounding like a broken record. I'll just leave this here: http://mywiki.wooledge.org/BashGuide d
This add support for split debug packages. The main idea is that debug packages will be added to pkgname array and for each of it package_${pkgname}-debug function will be created. So it treated like usual split packages.
Signed-off-by: Vitaly Gorbunov <vit.gorbunov@gmail.com> --- scripts/makepkg.sh.in | 85 +++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 76 insertions(+), 9 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0a514a..08c4a09 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -72,6 +72,8 @@ BUILDFUNC=0 CHECKFUNC=0 PKGFUNC=0 SPLITPKG=0 +SPLITDEBUG=0 +SKIPDEBUGPACKAGE=0 PKGLIST=() SIGNPKG=''
@@ -973,7 +975,7 @@ tidy_install() { done fi
- if [[ $(check_option zipman) = "y" && -n ${MAN_DIRS[*]} ]]; then + if [[ $(check_option zipman) != "n" && -n ${MAN_DIRS[*]} ]]; then msg2 "$(gettext "Compressing man and info pages...")" local manpage ext file link hardlinks hl find ${MAN_DIRS[@]} -type f 2>/dev/null | @@ -1009,21 +1011,52 @@ tidy_install() { done fi
+ if [ "$(check_option splitdebug)" = "y" ] || (( SPLITDEBUG )); then + local is_splitdebug=1 + local dbgdir="$pkgdir-debug" + backup_package_variables "debug" + fi if [[ $(check_option strip) = y ]]; then msg2 "$(gettext "Stripping unneeded symbols from binaries and libraries...")" # make sure library stripping variables are defined to prevent excess stripping [[ -z ${STRIP_SHARED+x} ]] && STRIP_SHARED="-S" [[ -z ${STRIP_STATIC+x} ]] && STRIP_STATIC="-S" - local binary + local binary hardlink strip_opt find . -type f -perm -u+w 2>/dev/null | while read binary ; do case "$(file -bi "$binary")" in *application/x-sharedlib*) # Libraries (.so) - strip $STRIP_SHARED "$binary";; + strip_opt=$STRIP_SHARED;; *application/x-archive*) # Libraries (.a) - strip $STRIP_STATIC "$binary";; + strip_opt=$STRIP_STATIC;; *application/x-executable*) # Binaries - strip $STRIP_BINARIES "$binary";; + strip_opt=$STRIP_BINARIES;; + *) continue ;; esac + # Strip symbols + if (( is_splitdebug )); then + # Create directory for the symbol file + mkdir -p "${dbgdir}/usr/lib/debug/${binary%/*}" + if readelf -S $binary | grep "\.gnu_debuglink" &>/dev/null; then + # An already stripped binary is a hard link + find ${STRIP_DIRS[@]} -samefile "$binary" 2>/dev/null | while read hardlink ; do
Quote the array. Null delimited output on find... If you're going to loop over find's output, please use -print0 on the find side and -d '' on the read side. There's no reason not to be safe about this.
+ # Is the symbol file for the hardlink in the right dir ? + if [ -e "${dbgdir}/usr/lib/debug/${hardlink#/}.debug" ]; then
style! we don't use [ ] anywhere else in makepkg.
+ if [ "${hardlink%/*}" != "${binary%/*}" ]; then + ln "${dbgdir}/usr/lib/debug/${hardlink#/}.debug" "${dbgdir}/usr/lib/debug/${binary%/*}" + break + fi + fi + done + else + # Strip the file + objcopy --only-keep-debug "$binary" "${dbgdir}/usr/lib/debug/${binary#/}.debug" + strip $strip_opts "$binary" + objcopy --add-gnu-debuglink="/usr/lib/debug/${binary#/}.debug" "$binary"
objcopy definitely doesn't portably support the --add-gnu-debuglink flag...
+ chmod 644 "${dbgdir}/usr/lib/debug/${binary#/}.debug" + fi + else + strip $strip_opts "$binary" + fi done fi
@@ -1332,6 +1365,33 @@ create_signature() { fi }
+add_split_debug_packages(){ + [ ${#pkgname[@]} = 1 ] && eval "function package_${pkgname}() { package;}"
We don't prefix with function.
+ + local pkg pkgname_with_debug + for pkg in ${pkgname[@]} + do
style. We always have 'do' on the same line as 'for'. quoting, too.
+ pkgname_with_debug=( ${pkgname_with_debug[@]} ${pkg} ${pkg/%/-debug} ) + eval "function package_${pkg}-debug() { package_debug; }" + done + pkgname=( ${pkgname_with_debug[@]} )
more quoting...
+} + +package_debug(){ + check_debug_files=`find $pkgdir/ -name "*.debug"` + if [ -z "$check_debug_files" ]; then + SKIPDEBUGPACKAGE=1 + return + fi + + restore_package_variables "debug" + pkgdesc="$pkgdesc (debugging symbols)" + depends=("${pkgname%-debug}=$pkgver-$pkgrel") + groups=('debug' ${groups[@]/%/-debug}) + options=(!strip !splitdebug !zipman !purge !emptydirs !upx) + unset optdepends provides conflicts replaces backup install changelog +} + create_srcpackage() { msg "$(gettext "Creating source package...")" local srclinks="$(mktemp -d "$startdir"/srclinks.XXXXXXXXX)" @@ -1776,16 +1836,18 @@ devel_update() {
backup_package_variables() { local var + local suffix=$1 for var in ${splitpkg_overrides[@]}; do - local indirect="${var}_backup" + local indirect="${var}_backup${suffix}" eval "${indirect}=(\"\${$var[@]}\")" done }
restore_package_variables() { local var + local suffix=$1 for var in ${splitpkg_overrides[@]}; do - local indirect="${var}_backup" + local indirect="${var}_backup${suffix}" if [[ -n ${!indirect} ]]; then eval "${var}=(\"\${$indirect[@]}\")" else @@ -1849,6 +1911,7 @@ usage() { printf "$(gettext " -s, --syncdeps Install missing dependencies with %s")\n" "pacman" echo "$(gettext " -S, --source Generate a source-only tarball without downloaded sources")" echo "$(gettext " --allsource Generate a source-only tarball including downloaded sources")" + echo "$(gettext " -t, --splitdebug Split debugging symbols into separate package")" printf "$(gettext " --asroot Allow %s to run as root user")\n" "makepkg" printf "$(gettext " --check Run the %s function in the %s")\n" "check()" "$BUILDSCRIPT" printf "$(gettext " --config <file> Use an alternate config file (instead of '%s')")\n" "$confdir/makepkg.conf" @@ -1892,12 +1955,12 @@ fi ARGLIST=("$@")
# Parse Command Line Options. -OPT_SHORT="AcdefFghiLmop:rRsSV" +OPT_SHORT="AcdefFghiLmop:rRsStV" OPT_LONG="allsource,asroot,ignorearch,check,clean,nodeps" OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver,skippgpcheck" OPT_LONG+=",install,key:,log,nocolor,nobuild,nocheck,nosign,pkg:,rmdeps" OPT_LONG+=",repackage,skipchecksums,skipinteg,skippgpcheck,sign,source,syncdeps" -OPT_LONG+=",version,config:" +OPT_LONG+=",splitdebug,version,config:"
# Pacman Options OPT_LONG+=",noconfirm,noprogressbar" @@ -1945,6 +2008,7 @@ while true; do --sign) SIGNPKG='y' ;; -s|--syncdeps) DEP_BIN=1 ;; -S|--source) SOURCEONLY=1 ;; + -t|--splitdebug) SPLITDEBUG=1 ;;
-h|--help) usage; exit 0 ;; # E_OK -V|--version) version; exit 0 ;; # E_OK @@ -2130,6 +2194,9 @@ check_software || exit 1 devel_check devel_update
+if [ "$(check_option splitdebug)" = "y" ] || (( SPLITDEBUG )); then + add_split_debug_packages +fi if (( ${#pkgname[@]} > 1 )); then SPLITPKG=1 fi -- 1.7.6