- versioned dependencies
- dependencies on 'provides' tags or library names
---
 abuild.in | 47 ++++++++++++++++++++++-------------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

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
+		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
---

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@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
> @@ -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
> ---
> 



---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---

On Mon,  6 Mar 2017 18:12:38 +0200
Kaarle Ritvanen <kaarle.ritvanen@datakunkku.fi> wrote:

> - versioned dependencies
> - dependencies on 'provides' tags or library names
> ---
>  abuild.in | 47 ++++++++++++++++++++++-------------------------
>  1 file changed, 22 insertions(+), 25 deletions(-)

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

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
---

---
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
---