Centralize all SVN access into 'vcs_*' functions in the 'db-functions-svn' file

Moving all SVN code in to a separate file means both that:
 1. It is easier to identify the interactions with SVN, when considering a
    replacement.
 2. It is easier to swap out the one file if/when replacing SVN with
    something else.

Put another way: try to be less tightly coupled with SVN.

Some of these functions could stand to be cleaned up a touch--for
obviousness' sake, to the extent possible, the code is copied verbatim
from where it was inline before, and modified as little as possible.

5 files changed, 138 insertions, 44 deletions

diff --git a/cron-jobs/sourceballs b/cron-jobs/sourceballs
index e24102b..5297a54 100755
--- a/cron-jobs/sourceballs
+++ b/cron-jobs/sourceballs
@@ -74,8 +74,8 @@ for repo in "${PKGREPOS[@]}"; do
 
 			# Get the sources from svn
 			mkdir -p -m0770 "${WORKDIR}/pkgbuilds/${repo}-${pkgarch}"
-			arch_svn export -q "${SVNREPO}/${pkgbase}/repos/${repo}-${pkgarch}" \
-				"${WORKDIR}/pkgbuilds/${repo}-${pkgarch}/${pkgbase}" >/dev/null 2>&1
+			vcs_export "$repo" "$pkgarch" "$pkgbase" \
+				"${WORKDIR}/pkgbuilds/${repo}-${pkgarch}/${pkgbase}"
 			if (( $? >= 1 )); then
 				failedpkgs+=("${pkgbase}-${pkgver}${SRCEXT}")
 				continue
diff --git a/db-functions b/db-functions
index f7fffeb..d89382d 100644
--- a/db-functions
+++ b/db-functions
@@ -308,16 +308,16 @@ check_pkgsvn() {
 
 	in_array "${repo}" "${PKGREPOS[@]}" || return 1
 
-	if [[ ! -f ${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase} ]]; then
+	if [[ ! -f ${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}/PKGBUILD ]]; then
 		mkdir -p "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}"
-		arch_svn export -q "${SVNREPO}/${_pkgbase}/repos/${repo}-${_pkgarch}/PKGBUILD" \
+		vcs_export "$repo" "$_pkgarch" "$_pkgbase" \
 			"${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}" >/dev/null || return 1
 	fi
 
-	local svnver="$(. "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}"; get_full_version)"
+	local svnver="$(. "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}/PKGBUILD"; get_full_version)"
 	[[ "${svnver}" = "${_pkgver}" ]] || return 1
 
-	local svnnames=($(. "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}"; echo "${pkgname[@]}"))
+	local svnnames=($(. "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}/PKGBUILD"; echo "${pkgname[@]}"))
 	in_array "${_pkgname}" "${svnnames[@]}" || return 1
 
 	return 0
@@ -342,13 +342,13 @@ check_splitpkgs() {
 		mkdir -p "${repo}/${_pkgarch}/${_pkgbase}"
 		echo "${_pkgname}" >> "${repo}/${_pkgarch}/${_pkgbase}/staging"
 
-		if [[ ! -f ${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase} ]]; then
+		if [[ ! -f ${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}/PKGBUILD ]]; then
 			mkdir -p "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}"
-			arch_svn export -q "${SVNREPO}/${_pkgbase}/repos/${repo}-${_pkgarch}/PKGBUILD" \
+			vcs_export "$repo" "$_pkgarch" "$_pkgbase" \
 				"${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}" >/dev/null || return 1
 		fi
 
-		local svnnames=($(. "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}"; echo "${pkgname[@]}"))
+		local svnnames=($(. "${WORKDIR}/pkgbuilds/${repo}-${_pkgarch}/${_pkgbase}/PKGBUILD"; echo "${pkgname[@]}"))
 		printf '%s\n' "${svnnames[@]}" >> "${repo}/${_pkgarch}/${_pkgbase}/svn"
 	done
 	popd >/dev/null
diff --git a/db-functions-svn b/db-functions-svn
index fe043a0..e7c0830 100644
--- a/db-functions-svn
+++ b/db-functions-svn
@@ -1,5 +1,9 @@
 #!/hint/bash
 
+# Within this file (and in most of dbscripts, really):
+# - "pkgarch" refers to the package architecture; may be "any"
+# - "tarch" refers to the repo architecture; may NOT be "any"
+
 if [[ -n ${SVNUSER} ]]; then
 	setfacl -m u:"${SVNUSER}":rwx "${WORKDIR}"
 	setfacl -m d:u:"${USER}":rwx "${WORKDIR}"
@@ -13,3 +17,123 @@ arch_svn() {
 		sudo -u "${SVNUSER}" -- /usr/bin/svn --username "${USER}" "${@}"
 	fi
 }
+
+_svn_checkout() {
+	local pkgbase=$1
+	if ! [[ -d ${WORKDIR}/svn ]]; then
+		arch_svn checkout -q -N "${SVNREPO}" "${WORKDIR}/svn" >/dev/null
+	fi
+	if ! [[ -d ${WORKDIR}/svn/${pkgbase} ]]; then
+		arch_svn up -q "${WORKDIR}/svn/${pkgbase}" >/dev/null
+	fi
+}
+
+# usage: vcs_move_preflight_check repo_from pkgarch pkgbase
+#
+# Verify that $pkgbase exists in $repo_from in SVN, such that we can
+# move it to a different repo.
+vcs_move_preflight_check() {
+	local repo_from=$1
+	local pkgarch=$2
+	local pkgbase=$3
+
+	_svn_checkout "$pkgbase"
+	local svnrepo_from="${WORKDIR}/svn/${pkgbase}/repos/${repo_from}-${pkgarch}"
+	[[ -r ${svnrepo_from}/PKGBUILD ]]
+}
+
+# usage: vcs_move_start repo_from repo_to pkgbase
+#
+# Begin a VCS transaction moving $pkgbase from $repo_from to $repo_to.
+# This should be followed by a call to vcs_move_arch for each
+# architecture we're movin it for, and finally by a call to
+# vcs_move_finish.
+vcs_move_start() {
+	svn_move_repo_from=$1
+	svn_move_repo_to=$2
+	svn_move_pkgbase=$3
+
+	svn_move_tag_list=""
+}
+
+# usage: vcs_move_arch pkgarch
+#
+# Add an architecture to a VCS transaction started by vcs_move_start.
+#
+# If the "from" PKGBUILD doesn't exist, this is a no-op (not an
+# error), so that it can be run for each arch, and the invoker doesn't
+# need to worry about hoisting it out of the loop if arch=(any).  If
+# the nonexistence is such that it should be an error, we count on
+# vcs_move_preflight_check having already caught that.
+vcs_move_arch() {
+	local pkgarch=$1
+
+	local repo_from=$svn_move_repo_from
+	local repo_to=$svn_move_repo_to
+	local pkgbase=$svn_move_pkgbase
+
+	local svnrepo_from="${WORKDIR}/svn/${pkgbase}/repos/${repo_from}-${pkgarch}"
+	local svnrepo_to="${WORKDIR}/svn/${pkgbase}/repos/${repo_to}-${pkgarch}"
+	if [[ -f ${svnrepo_from}/PKGBUILD ]]; then
+		msg2 "%s (%s)" "$pkgbase" "$pkgarch"
+
+		if [[ -d ${svnrepo_to} ]]; then
+			for file in $(arch_svn ls "${svnrepo_to}"); do
+				arch_svn rm -q "${svnrepo_to}/$file@"
+			done
+		else
+			mkdir "${svnrepo_to}"
+			arch_svn add -q "${svnrepo_to}"
+		fi
+
+		for file in $(arch_svn ls "${svnrepo_from}"); do
+			arch_svn mv -q -r HEAD "${svnrepo_from}/$file@" "${svnrepo_to}/"
+		done
+		arch_svn rm --force -q "${svnrepo_from}"
+		svn_move_tag_list+=", $pkgarch"
+	fi
+}
+
+# usage: vcs_move_finish
+#
+# Commit/finalize a VCS transaction started by vcs_move_start.
+vcs_move_finish() {
+	local repo_from=$svn_move_repo_from
+	local repo_to=$svn_move_repo_to
+	local pkgbase=$svn_move_pkgbase
+
+	local tag_list="${svn_move_tag_list#, }"
+	arch_svn commit -q "${WORKDIR}/svn/${pkgbase}" -m "${0##*/}: moved ${pkgbase} from [${repo_from}] to [${repo_to}] (${tag_list})"
+}
+
+# usage: vcs_remove repo pkgarch pkgbase
+#
+# Remove the given package in VCS.
+vcs_remove() {
+	local repo=$1
+	local arch=$2
+	local pkgbase=$3
+
+	local svnrepo="$repo-$arch"
+
+	_svn_checkout "$pkgbase"
+	if [[ -d ${WORKDIR}/svn/$pkgbase/repos/$svnrepo ]]; then
+		arch_svn rm --force -q "${WORKDIR}/svn/$pkgbase/repos/$svnrepo"
+		arch_svn commit -q "${WORKDIR}/svn/$pkgbase" -m "${0##*/}: $pkgbase removed by $(id -un)"
+	else
+		warning "pkgbase '%s' not found in svn; unable to commit removal to svn" "$pkgbase"
+	fi
+}
+
+# usage: vcs_export repo pkgarch pkgbase dest
+#
+# Export the VCS files for a package to the given $dest directory.
+vcs_export() {
+	local repo=$1
+	local pkgarch=$2
+	local pkgbase=$3
+	local dest=$4
+
+	arch_svn export -q "${SVNREPO}/${pkgbase}/repos/${repo}-${pkgarch}" \
+		 "${dest}" >/dev/null 2>&1
+}
diff --git a/db-move b/db-move
index 0a00cfb..505b040 100755
--- a/db-move
+++ b/db-move
@@ -25,15 +25,12 @@ for pkgarch in "${ARCHES[@]}"; do
 done
 
 # check if packages to be moved exist in svn and ftp dir
-arch_svn checkout -q -N "${SVNREPO}" "${WORKDIR}/svn" >/dev/null
 for pkgbase in "${args[@]:2}"; do
-	arch_svn up -q "${WORKDIR}/svn/${pkgbase}" >/dev/null
 	found=false
 	for tarch in "${ARCHES[@]}"; do
 		while read -r pkgarch pkgfile; do
 
-			svnrepo_from="${WORKDIR}/svn/${pkgbase}/repos/${repo_from}-${pkgarch}"
-			if ! [[ -r ${svnrepo_from}/PKGBUILD ]]; then
+			if ! vcs_move_preflight_check "$repo_from" "$pkgarch" "$pkgbase"; then
 				die "%s not found in %s-%s" "$pkgbase" "$repo_from" "$pkgarch"
 			fi
 
@@ -54,29 +51,10 @@ for arch in "${ARCHES[@]}"; do
 	declare -a remove_pkgs_$arch
 done
 for pkgbase in "${args[@]:2}"; do
-	tag_list=""
+	vcs_move_start "$repo_from" "$repo_to" "$pkgbase"
 	for tarch in "${ARCHES[@]}"; do
 		while read -r pkgname pkgver pkgarch pkgfile; do
-			svnrepo_from="${WORKDIR}/svn/${pkgbase}/repos/${repo_from}-${pkgarch}"
-			svnrepo_to="${WORKDIR}/svn/${pkgbase}/repos/${repo_to}-${pkgarch}"
-			if [[ -f ${svnrepo_from}/PKGBUILD ]]; then
-				msg2 "%s (%s)" "$pkgbase" "$pkgarch"
-
-				if [[ -d ${svnrepo_to} ]]; then
-					for file in $(arch_svn ls "${svnrepo_to}"); do
-						arch_svn rm -q "${svnrepo_to}/$file@"
-					done
-				else
-					mkdir "${svnrepo_to}"
-					arch_svn add -q "${svnrepo_to}"
-				fi
-
-				for file in $(arch_svn ls "${svnrepo_from}"); do
-					arch_svn mv -q -r HEAD "${svnrepo_from}/$file@" "${svnrepo_to}/"
-				done
-				arch_svn rm --force -q "${svnrepo_from}"
-				tag_list+=", $pkgarch"
-			fi
+			vcs_move_arch "$pkgarch"
 
 			declare -n add_pkgs="add_pkgs_${tarch}"
 			declare -n remove_pkgs="remove_pkgs_${tarch}"
@@ -89,8 +67,7 @@ for pkgbase in "${args[@]:2}"; do
 			remove_pkgs+=("${pkgname}")
 		done < <(arch_expac_pkgbase "$repo_from" "$tarch" '%n %v %a %f' "$pkgbase")
 	done
-	tag_list="${tag_list#, }"
-	arch_svn commit -q "${WORKDIR}/svn/${pkgbase}" -m "${0##*/}: moved ${pkgbase} from [${repo_from}] to [${repo_to}] (${tag_list})"
+	vcs_move_finish
 done
 
 for tarch in "${ARCHES[@]}"; do
diff --git a/db-remove b/db-remove
index 5c83d3f..60bd8d9 100755
--- a/db-remove
+++ b/db-remove
@@ -13,7 +13,6 @@ arch="$2"
 pkgbases=("${@:3}")
 
 ftppath="$FTP_BASE/$repo/os"
-svnrepo="$repo-$arch"
 
 if ! check_repo_permission "$repo"; then
 	die "You don't have permission to remove packages from %s" "$repo"
@@ -36,13 +35,7 @@ for pkgbase in "${pkgbases[@]}"; do
 	mapfile -t pkgnames < <(arch_expac_pkgbase "$repo" "${tarches[0]}" '%n' "$pkgbase")
 	remove_pkgs+=("${pkgnames[@]}")
 
-	arch_svn checkout -q "${SVNREPO}/${pkgbase}" "${WORKDIR}/svn/${pkgbase}" >/dev/null
-	if [[ -d ${WORKDIR}/svn/$pkgbase/repos/$svnrepo ]]; then
-		arch_svn rm --force -q "${WORKDIR}/svn/$pkgbase/repos/$svnrepo"
-		arch_svn commit -q "${WORKDIR}/svn/$pkgbase" -m "${0##*/}: $pkgbase removed by $(id -un)"
-	else
-		warning "pkgbase '%s' not found in svn; unable to commit removal to svn" "$pkgbase"
-	fi
+	vcs_remove "$repo" "$arch" "$pkgbase"
 done
 
 for tarch in "${tarches[@]}"; do