[arch-projects] [devtools] [PATCH 2/7] makechrootpkg: Have functions be more function-y.

6 Apr 2017

From: Luke Shumaker <lukeshu@parabola.nu>

Rather than them simply being named blocks of code with braces around
them.

That is: have them take things via arguments rather than global
variables.

Specific notes:

 - create_chroot->sync_chroot:

   I pulled out locking the destination chroot; getting that lock is
   now the caller's responsibility.  It still handles locking the
   source chroot though.

   I pulled the `if [[ ! -d $copydir ]] || $clean_first;` check out; it is
   now the caller's responsibility to use that check when deciding if to
   call sync_chroot.

   However, when pulling that check out, I left it as `if true;`, to
   keep an indentation level.  This patch has had to be rebased/merged
   many times, and changing the indentation is a sure way to make that
   go less smoothly; I'm not going to re-indent this block until I see
   the check removed in the git.archlinux.org/devtools.git repository.

 - install_packages:

    1. Receive the list of packages as arguments, rather than a global
       variable.
    2. Make the caller responsible for looking at PKGBUILD.  From the
       name and arguments, one would never expect it to look at PKGBUILD.
---
 makechrootpkg.in | 88 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 75 insertions(+), 13 deletions(-)

diff --git a/makechrootpkg.in b/makechrootpkg.in
index 5329f76..bdcaa88 100644
--- a/makechrootpkg.in
+++ b/makechrootpkg.in
@@ -76,6 +76,14 @@ usage() {
 }
 
 # {{{ functions
+# Usage: load_vars $makepkg_conf
+# Globals:
+#  - SRCDEST
+#  - SRCPKGDEST
+#  - PKGDEST
+#  - LOGDEST
+#  - MAKEFLAGS
+#  - PACKAGER
 load_vars() {
 	local makepkg_conf="$1" var
 
@@ -88,11 +96,23 @@ load_vars() {
 	return 0
 }
 
-create_chroot() {
-	# Lock the chroot we want to use. We'll keep this lock until we exit.
-	lock 9 "$copydir.lock" "Locking chroot copy [%s]" "$copy"
+# Usage: sync_chroot $CHROOTDIR/$CHROOT <$CHROOTCOPY|$copydir>
+sync_chroot() {
+	local chrootdir=$1
+	local copy=$2
+	local copydir=''
+	if [[ ${copy:0:1} = / ]]; then
+		copydir=$copy
+	else
+		copydir="$chrootdir/$copy"
+	fi
+
+	if [[ "$chrootdir/root" -ef "$copydir" ]]; then
+		error 'Cannot sync copy with itself: %s' "$copydir"
+		return 1
+	fi
 
-	if [[ ! -d $copydir ]] || $clean_first; then
+	if true; then # indent for rebasing/merging purposes
 		# Get a read lock on the root chroot to make
 		# sure we don't clone a half-updated chroot
 		slock 8 "$chrootdir/root.lock" "Locking clean chroot"
@@ -117,7 +137,11 @@ create_chroot() {
 	touch "$copydir"
 }
 
-clean_temporary() {
+# Usage: delete_chroot $copydir [$copy]
+delete_chroot() {
+	local copydir=$1
+	local copy=${1:-$2}
+
 	stat_busy "Removing temporary copy [%s]" "$copy"
 	if is_btrfs "$chrootdir" && ! mountpoint -q "$copydir"; then
 		btrfs subvolume delete "$copydir" >/dev/null ||
@@ -133,7 +157,11 @@ clean_temporary() {
 	stat_done
 }
 
+# Usage: install_packages $copydir $pkgs...
 install_packages() {
+	local copydir=$1
+	local install_pkgs=("${@:2}")
+
 	local -a pkgnames
 	local ret
 
@@ -145,11 +173,19 @@ install_packages() {
 	ret=$?
 	rm -- "${pkgnames[@]/#/$copydir/root/}"
 
-	# If there is no PKGBUILD we are done
-	[[ -f PKGBUILD ]] || exit $ret
+	return $ret
 }
 
+# Usage: prepare_chroot $copydir $HOME $repack $run_namcap
+# Globals:
+#  - MAKEFLAGS
+#  - PACKAGER
 prepare_chroot() {
+	local copydir=$1
+	local USER_HOME=$2
+	local repack=$3
+	local run_namcap=$4
+
 	$repack || rm -rf "$copydir/build"
 
 	local builduser_uid="${SUDO_UID:-$UID}"
@@ -216,7 +252,14 @@ _chrootnamcap() {
 	done
 }
 
+# Usage: download_sources $copydir $src_owner
+# Globals:
+#  - SRCDEST
+#  - USER
 download_sources() {
+	local copydir=$1
+	local src_owner=$2
+
 	local builddir="$(mktemp -d)"
 	chmod 1777 "$builddir"
 
@@ -235,7 +278,15 @@ download_sources() {
 	rm -rf $builddir
 }
 
+# Usage: move_products $copydir $owner
+# Globals:
+#  - PKGDEST
+#  - LOGDEST
 move_products() {
+	local copydir=$1
+	local src_owner=$2
+
+	local pkgfile
 	for pkgfile in "$copydir"/pkgdest/*; do
 		chown "$src_owner" "$pkgfile"
 		mv "$pkgfile" "$PKGDEST"
@@ -246,6 +297,7 @@ move_products() {
 		fi
 	done
 
+	local l
 	for l in "$copydir"/logdest/*; do
 		[[ $l == */logpipe.* ]] && continue
 		chown "$src_owner" "$l"
@@ -321,17 +373,27 @@ load_vars /etc/makepkg.conf
 [[ -d $SRCPKGDEST ]] || SRCPKGDEST=$PWD
 [[ -d $LOGDEST ]]    || LOGDEST=$PWD
 
-create_chroot
+# Lock the chroot we want to use. We'll keep this lock until we exit.
+lock 9 "$copydir.lock" "Locking chroot copy [%s]" "$copy"
+
+if [[ ! -d $copydir ]] || $clean_first; then
+	sync_chroot "$chrootdir" "$copy"
+fi
 
 $update_first && arch-nspawn "$copydir" \
 		"${bindmounts_ro[@]}" "${bindmounts_rw[@]}" \
 		pacman -Syu --noconfirm
 
-[[ -n ${install_pkgs[*]} ]] && install_packages
+if [[ -n ${install_pkgs[*]} ]]; then
+	install_packages "$copydir" "${install_pkgs[@]}"
+	ret=$?
+	# If there is no PKGBUILD we have done
+	[[ -f PKGBUILD ]] || return $ret
+fi
 
-download_sources
+download_sources "$copydir" "$src_owner"
 
-prepare_chroot
+prepare_chroot "$copydir" "$USER_HOME" "$repack"
 
 if arch-nspawn "$copydir" \
 	--bind="$PWD:/startdir" \
@@ -339,12 +401,12 @@ if arch-nspawn "$copydir" \
 	"${bindmounts_ro[@]}" "${bindmounts_rw[@]}" \
 	/chrootbuild
 then
-	move_products
+	move_products "$copydir" "$src_owner"
 else
 	(( ret += 1 ))
 fi
 
-$temp_chroot && clean_temporary
+$temp_chroot && delete_chroot "$copydir" "$copy"
 
 if (( ret != 0 )); then
 	if $temp_chroot; then
-- 
2.12.1

    

[arch-projects] [devtools] [PATCH 2/7] makechrootpkg: Have functions be more function-y.

lukeshu＠lukeshu.com