Kaarle Ritvanen: 2 abuild: fix dependency checking abuild: enable -e shell option 2 files changed, 64 insertions(+), 52 deletions(-)
Hi, thanks for your patches! Could you please open a pull request in https://github.com/alpinelinux/abuild, so we can reasonably review it? Have you tested that whether it really aborts the script when some command inside build or other functions returns non-zero status? There are some quite weird limitations in -e option about in which context it is applied. Thus I thought that it’d be harder than this to get it work correctly (and I’d be very glad if I was wrong). Jakub
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.alpinelinux.org/~alpine/devel/patches/996/mbox | git am -3Learn more about email & git
- versioned dependencies - dependencies on 'provides' tags or library names --- abuild.in | 47 ++++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 25 deletions(-)
Natanael Copa <ncopa@alpinelinux.org>more deletetions than insertions. I like that :)
diff --git a/abuild.in b/abuild.in index 22bd454..90da90d 100644 --- a/abuild.in +++ b/abuild.in @@ -1855,10 +1855,26 @@ calcdeps() { fi } +get_missing_deps() { + local cmd="$APK info --quiet --installed $1" + shift + + while [ "$1" ]; do + if [ ${1:0:1} = "!" ]; then + if $cmd ${1:1}; then + error "Conflicting package installed: ${1:1}" + return 1 + fi
Natanael Copa <ncopa@alpinelinux.org>busybox ash does support the bashism ${foo:0:1} but i think we should try avoid it, since we can use ${foo#\!} here. LGTM otherwise. I haven't tested it though. -nc --- Unsubscribe: alpine-devel+unsubscribe@lists.alpinelinux.org Help: alpine-devel+help@lists.alpinelinux.org ---
+ elif [ "$upgrade" ] || ! $cmd $1; then + echo $1 + fi + shift + done +} + # build and install dependencies builddeps() { - local pkg= i= missing= BUILD_BASE= - local installed_hostdeps= installed_builddeps= + local pkg= i= BUILD_BASE= [ -n "$nodeps" ] && return 0 msg "Analyzing dependencies..." @@ -1872,30 +1888,11 @@ builddeps() { esac calcdeps "$BUILD_BASE" - installed_builddeps=$($APK info --installed $builddeps) - [ -n "$CBUILDROOT" -a -n "$hostdeps" ] && installed_hostdeps=$($APK info --root "$CBUILDROOT" --arch "$CTARGET_ARCH" --installed $hostdeps) - # find which deps are missing - for i in $builddeps; do - if [ "${i#\!}" != "$i" ]; then - if $APK info --quiet --installed "${i#\!}"; then - error "Conflicting package installed: ${i#\!}" - return 1 - fi - elif ! deplist_has $i $installed_builddeps || [ -n "$upgrade" ]; then - missing="$missing $i" - fi - done - for i in $hostdeps; do - if [ "${i#\!}" != "$i" ]; then - if $APK info --quiet --installed --root "$CBUILDROOT" --arch "$CTARGET_ARCH" "${i#\!}"; then - error "Conflicting package installed: ${i#\!}" - return 1 - fi - elif ! deplist_has $i $installed_hostdeps || [ -n "$upgrade" ]; then - missing="$missing $i" - fi - done + local mbd mhd missing + mbd=$(get_missing_deps "" $builddeps) || return 1 + mhd=$(get_missing_deps "--root $CBUILDROOT --arch $CTARGET_ARCH" $hostdeps) || return 1 + missing=$(echo $mbd $mhd) if [ -z "$install_deps" ] && [ -z "$recursive" ]; then # if we dont have any missing deps we are done now -- 2.9.3 --- Unsubscribe: alpine-devel+unsubscribe@lists.alpinelinux.org Help: alpine-devel+help@lists.alpinelinux.org ---
--- This patch has the potential to break quite a number of APKBUILD files. However, it would make them simpler and more reliable by removing the need to check command exit codes in most places. An alternative would be to add an APKBUILD option to control the automatic exit code checking behavior. abuild.in | 69 ++++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/abuild.in b/abuild.in index 90da90d..6fd0670 100644 --- a/abuild.in +++ b/abuild.in @@ -1,4 +1,4 @@ -#!/bin/ash +#!/bin/ash -e # abuild - build apk packages (light version of makepkg) # Copyright (c) 2008-2015 Natanael Copa <ncopa@alpinelinux.org> @@ -240,8 +240,7 @@ sumcheck() { endreturnval=0 for src in $sums; do origin=$1; shift - echo "$src" | ${algo}sum -c - if [ $? -ne 0 ]; then + if ! echo "$src" | ${algo}sum -c; then endreturnval=1 is_remote $origin || continue @@ -567,7 +566,9 @@ update_config_guess() { runpart() { local part=$1 [ -n "$DEBUG" ] && msg "$part" - $part || die "$part failed" + abuild_function=$part "$abuild_path" \ + $color_opt $nodeps $force $forceroot $keep $quiet \ + $install_deps $recursive $upgrade || die "$part failed" } # override those in your build script @@ -1409,12 +1410,19 @@ build_abuildrepo() { fi if ! apk_up2date || [ -n "$force" ]; then # check early if we have abuild key - abuild-sign --installed || return 1 + abuild-sign --installed logcmd "building $repo/$pkgname-$pkgver-r$pkgrel" - sanitycheck && builddeps && clean && fetch && unpack \ - && prepare && mkusers && $_build && $_check \ - && rootpkg && cleanup $CLEANUP \ - || return 1 + sanitycheck + builddeps + clean + fetch + unpack + prepare + mkusers + $_build + $_check + rootpkg + cleanup $CLEANUP fi update_abuildrepo_index } @@ -1515,9 +1523,7 @@ default_doc() { rm -f "$subpkgdir/usr/share/info/dir" # remove if empty, ignore error (not empty) - rmdir "$pkgdir/usr/share" "$pkgdir/usr" 2>/dev/null - - return 0 + rmdir "$pkgdir/usr/share" "$pkgdir/usr" 2>/dev/null || : } doc() { @@ -1573,7 +1579,7 @@ default_dev() { d="$subpkgdir/${i%/*}" # dirname $i mkdir -p "$d" mv "$pkgdir/$i" "$d" - rmdir "$pkgdir/${i%/*}" 2>/dev/null + rmdir "$pkgdir/${i%/*}" 2>/dev/null || : fi done # move *.so links needed when linking the apps to -dev packages @@ -1995,7 +2001,9 @@ stripbin() { | while read filename; do local XATTR=$(getfattr --match="" --dump "${filename}") "${stripcmd}" "${filename}" - [ -n "$XATTR" ] && { echo "$XATTR" | setfattr --restore=-; } + if [ -n "$XATTR" ]; then + echo "$XATTR" | setfattr --restore=- + fi done } @@ -2098,12 +2106,11 @@ deps() { undeps() { local _quiet="$@" - $SUDO_APK del $_quiet $apk_opt_wait .makedepends-$pkgname + $SUDO_APK del $_quiet $apk_opt_wait .makedepends-$pkgname || : if [ -n "$CBUILDROOT" ]; then $SUDO_APK del $_quiet --root "$CBUILDROOT" --arch "$CTARGET_ARCH" $apk_opt_wait \ - --no-scripts .makedepends-$pkgname + --no-scripts .makedepends-$pkgname || : fi - return 0 } # compat @@ -2247,7 +2254,7 @@ while getopts "AcdfFhkKimnp:P:qrRs:u" opt; do 'A') echo "$CARCH"; exit 0;; 'c') enable_colors color_opt="-c";; - 'd') nodeps=1;; + 'd') nodeps="-d";; 'f') force="-f";; 'F') forceroot="-F";; 'h') usage;; @@ -2303,7 +2310,7 @@ if [ -z "$REPODEST" ]; then fi # for recursive action -export REPODEST +export REPODEST SRCDEST # if we want build debug package if [ -n "$DEBUG" ] || subpackage_types_has "dbg"; then @@ -2331,15 +2338,23 @@ fi controldir="$pkgbasedir"/.control.${subpkgname:-$pkgname} trap 'die "Aborted by user"' INT -[ -z "$subpkgdir" ] && set_xterm_title "abuild${CROSS_COMPILE+-$CARCH}: $pkgname" -if [ -z "$1" ]; then - set all -fi +if [ "$abuild_function" ]; then + _function=$abuild_function + abuild_function= + $_function -while [ $# -gt 0 ]; do - runpart $1 - shift -done +else + [ -z "$subpkgdir" ] && set_xterm_title "abuild${CROSS_COMPILE+-$CARCH}: $pkgname" + + if [ -z "$1" ]; then + set all + fi + + while [ $# -gt 0 ]; do + runpart $1 + shift + done +fi cleanup -- 2.9.3 --- Unsubscribe: alpine-devel+unsubscribe@lists.alpinelinux.org Help: alpine-devel+help@lists.alpinelinux.org ---
Jakub Jirutka <jakub@jirutka.cz>--- Unsubscribe: alpine-devel+unsubscribe@lists.alpinelinux.org Help: alpine-devel+help@lists.alpinelinux.org ---
Jakub Jirutka <jakub@jirutka.cz>Hi, thanks for your patches! Could you please open a pull request in https://github.com/alpinelinux/abuild, so we can reasonably review it? Have you tested that whether it really aborts the script when some command inside build or other functions returns non-zero status? There are some quite weird limitations in -e option about in which context it is applied. Thus I thought that it’d be harder than this to get it work correctly (and I’d be very glad if I was wrong). Jakub