On Wed, Aug 10, 2011 at 1:25 PM, Vitaliy Gorbunov <vit.gorbunov@gmail.com> wrote:
--- bin/makepkg 2011-08-10 19:37:37.000000000 +0300 +++ local/bin/makepkg4 2011-08-10 19:41:15.000000000 +0300
@@ -44,7 +44,7 @@ srcdir="$startdir/src" pkgdir="$startdir/pkg"
-packaging_options=('strip' 'docs' 'libtool' 'emptydirs' 'zipman' 'purge') +packaging_options=('strip' 'splitdbg' 'docs' 'libtool' 'emptydirs' 'zipman' Let's not try to save chrs here. splitdebug would be nicer. 'purge') other_options=('ccache' 'distcc' 'buildflags' 'makeflags') splitpkg_overrides=('pkgver' 'pkgrel' 'pkgdesc' 'arch' 'license' 'groups' \ 'depends' 'optdepends' 'provides' 'conflicts' 'replaces' \ @@ -74,6 +74,8 @@ CHECKFUNC=0 PKGFUNC=0 SPLITPKG=0 +SPLIT_DBG=0 +SKIP_DEBUG_PACKAGE=0 DBG vs DEBUG- pick one please. And why do we need two different vars here? PKGLIST=()
# Forces the pkgver of the current PKGBUILD. Used by the fakeroot call @@ -902,21 +904,49 @@ done fi
+ local is_splitdbg + [ "$(check_option splitdbg)" = "y" ] || (( SPLIT_DBG )) && is_splitdbg=1 + (( is_splitdbg )) && backup_package_variables "dbg" Too much logic voodoo going on here for me to understand quickly- I'd
Allan, can you review this when you get a chance? Vitaliy, it might also be best for you to take a look at our HACKING document, as well as submitting-patches.txt, as we prefer git formatted patches with a commit message and a signoff (and it ensures you are working against git code rather than outdated 3.5.x code at this point). prefer more conventional conditional blocks.
+ + local dbgdir="$pkgdir-dbg" 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) - /usr/bin/strip $STRIP_SHARED "$binary";; + strip_opt=$STRIP_SHARED;; *application/x-archive*) # Libraries (.a) - /usr/bin/strip $STRIP_STATIC "$binary";; + strip_opt=$STRIP_STATIC;; *application/x-executable*) # Binaries - /usr/bin/strip $STRIP_BINARIES "$binary";; + strip_opt=$STRIP_BINARIES;; + *) continue ;; esac + # Strip symbols + if (( is_splitdbg )); then + # Create directory for the symbol file + mkdir -p "${dbgdir}/usr/lib/debug/${binary%/*}" + if readelf -S $binary | grep "\.gnu_debuglink" &>/dev/null; This is very Linux specific. Not sure what we think about this.
then + # An already stripped binary is a hard link + find ${STRIP_DIRS[@]} -samefile "$binary" 2>/dev/null | while read hardlink ; do + # Is the symbol file for the hardlink in the right dir ? + [ -e "${dbgdir}/usr/lib/debug/${hardlink#/}.debug" ] && \ + [ "${hardlink%/*}" != "${binary%/*}" ] && \ + ln "${dbgdir}/usr/lib/debug/${hardlink#/}.debug" "${dbgdir}/usr/lib/debug/${binary%/*}" && break && madness- not a fan.
+ done + else + # Strip the file + /usr/bin/objcopy --only-keep-debug "$binary" "${dbgdir}/usr/lib/debug/${binary#/}.debug" + /usr/bin/strip $strip_opts "$binary" + /usr/bin/objcopy --add-gnu-debuglink="${dbgdir}/usr/lib/debug/${binary#/}.debug" "$binary" + chmod 644 "${dbgdir}/usr/lib/debug/${binary#/}.debug" + fi + else + /usr/bin/strip $strip_opts "$binary" + fi done fi
@@ -1008,6 +1038,12 @@ }
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. + return + fi + if [[ ! -d $pkgdir ]]; then error "$(gettext "Missing pkg/ directory.")" plain "$(gettext "Aborting...")" @@ -1096,6 +1132,38 @@ fi }
+add_split_debug_packages(){ + [ ${#pkgname[@]} = 1 ] && eval "function package_${pkgname}() { package;}" + + local pkg pkgname_dbg + for pkg in ${pkgname[@]} + do + pkgname_dbg=( ${pkgname_dbg[@]} ${pkg} ${pkg/%/-dbg} ) + eval "function package_${pkg}-dbg() { package_dbg; }" + done + pkgname=( ${pkgname_dbg[@]} ) +} + +package_dbg(){ + check_dbg_files=`find $pkgdir/ -name "*.debug"` + if [ -z "$check_dbg_files" ]; then + SKIP_DEBUG_PACKAGE=1 + return + fi + + restore_package_variables "dbg" + pkgdesc="$pkgdesc (debugging symbols)" + depends=("${pkgname%-dbg}=$pkgver-$pkgrel") + groups=('debug' ${groups[@]/%/-debug}) This seems very magic and arbitrary. + optdepends=() + replaces=() + provides=() + conflicts=() + backup=() Why are we listing all these variables if they are empty; is there a better way? I guess it is because our top-level package variables would be there otherwise, so maybe we have to do this. However you already missed makedepends at quick glance, as well as changelog, and I worry about this duplication. + options=(!strip !splitdbg docs !zipman !purge libtool !emptydirs) + unset install +} + create_srcpackage() { cd "$startdir"
@@ -1423,17 +1491,19 @@ }
backup_package_variables() { - local var + local var suffix + 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 var suffix + 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 @@ -1582,6 +1652,7 @@ echo "$(gettext " -r, --rmdeps Remove installed dependencies after a successful build")" echo "$(gettext " -R, --repackage Repackage contents of the package without rebuilding")" echo "$(gettext " -s, --syncdeps Install missing dependencies with pacman")" + echo "$(gettext " -t, --splitdbg Split debugging symbols into separate package")" echo "$(gettext " --allsource Generate a source-only tarball including downloaded sources")" echo "$(gettext " --asroot Allow makepkg to run as root user")" printf "$(gettext " --check Run the check() function in the %s")\n" "$BUILDSCRIPT" @@ -1622,11 +1693,11 @@ ARGLIST=("$@")
# Parse Command Line Options. -OPT_SHORT="AcCdefFghiLmop:rRsV" +OPT_SHORT="AcCdefFghiLmop:rRstV" OPT_LONG="allsource,asroot,ignorearch,check,clean,cleancache,nodeps" OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" OPT_LONG+=",install,log,nocolor,nobuild,nocheck,pkg:,rmdeps" -OPT_LONG+=",repackage,skipinteg,source,syncdeps,version,config:" +OPT_LONG+=",repackage,skipinteg,source,syncdeps,splitdbg,version,config:" # Pacman Options OPT_LONG+=",noconfirm,noprogressbar" OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@" || echo 'PARSE_OPTIONS FAILED')" @@ -1671,6 +1742,7 @@ --skipinteg) SKIPINTEG=1 ;; --source) SOURCEONLY=1 ;; -s|--syncdeps) DEP_BIN=1 ;; + -t|--splitdbg) SPLIT_DBG=1 ;;
-h|--help) usage; exit 0 ;; # E_OK -V|--version) version; exit 0 ;; # E_OK @@ -1874,6 +1946,8 @@ devel_check devel_update
+[ "$(check_option splitdbg)" = "y" ] || (( SPLIT_DBG )) && add_split_debug_packages + if (( ${#pkgname[@]} > 1 )); then SPLITPKG=1 fi
On Wed, Aug 10, 2011 at 9:09 PM, Dan McGee <dpmcgee@gmail.com> wrote:
I rewrite patch from Remy Oudomphen**g Rémy Oudompheng ? Not sure what the ** was for. :) (http://bugs.archlinux.org/task/10975<; http://bugs.archlinux.org/task/10975>) in order to use generic split packaging mechanism. The main idea is that debug packages will be added to pkgname array and for each of it package_<name>-dbg function will be created. So it treated
On Wed, Aug 10, 2011 at 12:04 PM, Vitaliy Gorbunov <vit.gorbunov@gmail.com> wrote: like
usual split packages. Tested on split and non-split packages. All seems to work fine.
Please send inline rather than as an attachment so we can review it, thanks!
-Dan