Content-Type: text/plain; charset=utf-8
Precedence: list
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Subject: Re: [alpine-devel] [PATCH 2/2] abuild: enable -e shell option
From: Jakub Jirutka <jakub@jirutka.cz>
In-Reply-To: <20170306161239.2317-2-kaarle.ritvanen@datakunkku.fi>
Date: Mon, 6 Mar 2017 17:21:37 +0100
Cc: alpine-devel@lists.alpinelinux.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <EF2D1F32-CBB8-40A0-904C-E9EDBECC28D8@jirutka.cz>
References: <20170306161239.2317-1-kaarle.ritvanen@datakunkku.fi> <20170306161239.2317-2-kaarle.ritvanen@datakunkku.fi>
To: Kaarle Ritvanen <kaarle.ritvanen@datakunkku.fi>

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=E2=80=99d be harder than this to get =
it work correctly (and I=E2=80=99d be very glad if I was wrong).

Jakub

> On 6. Mar 2017, at 17:12, Kaarle Ritvanen =
<kaarle.ritvanen@datakunkku.fi> wrote:
>=20
> ---
> 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.
>=20
> abuild.in | 69 =
++++++++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 42 insertions(+), 27 deletions(-)
>=20
> 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
>=20
> # abuild - build apk packages (light version of makepkg)
> # Copyright (c) 2008-2015 Natanael Copa <ncopa@alpinelinux.org>
> @@ -240,8 +240,7 @@ sumcheck() {
> 	endreturnval=3D0
> 	for src in $sums; do
> 		origin=3D$1; shift
> -		echo "$src" | ${algo}sum -c
> -		if [ $? -ne 0 ]; then
> +		if ! echo "$src" | ${algo}sum -c; then
> 			endreturnval=3D1
> 			is_remote $origin || continue
>=20
> @@ -567,7 +566,9 @@ update_config_guess() {
> runpart() {
> 	local part=3D$1
> 	[ -n "$DEBUG" ] && msg "$part"
> -	$part || die "$part failed"
> +	abuild_function=3D$part "$abuild_path" \
> +		$color_opt $nodeps $force $forceroot $keep $quiet \
> +		$install_deps $recursive $upgrade || die "$part failed"
> }
>=20
> # 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"
>=20
> 	# 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 || :
> }
>=20
> doc() {
> @@ -1573,7 +1579,7 @@ default_dev() {
> 			d=3D"$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=3D$(getfattr --match=3D"" --dump =
"${filename}")
> 		"${stripcmd}" "${filename}"
> -		[ -n "$XATTR" ] && { echo "$XATTR" | setfattr =
--restore=3D-; }
> +		if [ -n "$XATTR" ]; then
> +			echo "$XATTR" | setfattr --restore=3D-
> +		fi
> 	done
> }
>=20
> @@ -2098,12 +2106,11 @@ deps() {
>=20
> undeps() {
> 	local _quiet=3D"$@"
> -	$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
> }
>=20
> # compat
> @@ -2247,7 +2254,7 @@ while getopts "AcdfFhkKimnp:P:qrRs:u" opt; do
> 		'A') echo "$CARCH"; exit 0;;
> 		'c') enable_colors
> 		     color_opt=3D"-c";;
> -		'd') nodeps=3D1;;
> +		'd') nodeps=3D"-d";;
> 		'f') force=3D"-f";;
> 		'F') forceroot=3D"-F";;
> 		'h') usage;;
> @@ -2303,7 +2310,7 @@ if [ -z "$REPODEST" ]; then
> fi
>=20
> # for recursive action
> -export REPODEST
> +export REPODEST SRCDEST
>=20
> # if we want build debug package
> if [ -n "$DEBUG" ] || subpackage_types_has "dbg"; then
> @@ -2331,15 +2338,23 @@ fi
> controldir=3D"$pkgbasedir"/.control.${subpkgname:-$pkgname}
>=20
> trap 'die "Aborted by user"' INT
> -[ -z "$subpkgdir" ] && set_xterm_title =
"abuild${CROSS_COMPILE+-$CARCH}: $pkgname"
>=20
> -if [ -z "$1" ]; then
> -	set all
> -fi
> +if [ "$abuild_function" ]; then
> +	_function=3D$abuild_function
> +	abuild_function=3D
> +	$_function
>=20
> -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
>=20
> cleanup
> --=20
> 2.9.3
>=20
>=20
>=20
> ---
> Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
> Help:         alpine-devel+help@lists.alpinelinux.org
> ---
>=20



---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---