Mail archive
alpine-devel

Re: [alpine-devel] [PATCH 2/2] abuild: enable -e shell option

From: Jakub Jirutka <jakub_at_jirutka.cz>
Date: Mon, 6 Mar 2017 17:21:37 +0100

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

> On 6. Mar 2017, at 17:12, Kaarle Ritvanen <kaarle.ritvanen_at_datakunkku.fi> wrote:
>
> ---
> 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
> _at_@ -1,4 +1,4 @@
> -#!/bin/ash
> +#!/bin/ash -e
>
> # abuild - build apk packages (light version of makepkg)
> # Copyright (c) 2008-2015 Natanael Copa <ncopa_at_alpinelinux.org>
> _at_@ -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
>
> _at_@ -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
> _at_@ -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
> }
> _at_@ -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() {
> _at_@ -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
> _at_@ -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
> }
>
> _at_@ -2098,12 +2106,11 @@ deps() {
>
> undeps() {
> local _quiet="$_at_"
> - $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
> _at_@ -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;;
> _at_@ -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
> _at_@ -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_at_lists.alpinelinux.org
> Help: alpine-devel+help_at_lists.alpinelinux.org
> ---
>



---
Unsubscribe:  alpine-devel+unsubscribe_at_lists.alpinelinux.org
Help:         alpine-devel+help_at_lists.alpinelinux.org
---
Received on Mon Mar 06 2017 - 17:21:37 GMT