[devtools] Revert "makechrootpkg: sync_chroot: Make more general."

Message ID 20190510025826.19515-1-eschwartz@archlinux.org
State Accepted
Headers show
Series [devtools] Revert "makechrootpkg: sync_chroot: Make more general." | expand

Commit Message

Emil Velikov via arch-projects May 10, 2019, 2:58 a.m. UTC
This reverts commit 6d1992909cc46e293027ff488ae2632047603e66.

It has never worked. In commit c86823a2d4a4152c71faa1c3bab227756232996f
it was noted that it compared the device numbers for [[ $1 = $1 ]] which
was a useless check and always returned true, for *any* btrfs
filesystem. Now that the function is corrected to compare [[ $1 = $2 ]]
the check is still useless, but this time because it always returns
false -- btrfs subvolumes on the same filesystem do *not* share device
numbers.

So let's go back to the original working implementation that only
matters in terms of makechrootpkg, and just checks if makechrootpkg's
root working directory is btrfs (in which case we know it will be a
subvolume because mkarchroot will create it that way).

This restores our special support for the btrfs filesystem.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---
 lib/archroot.sh  |  8 --------
 makechrootpkg.in | 32 +++++++++++++-------------------
 2 files changed, 13 insertions(+), 27 deletions(-)

Patch

diff --git a/lib/archroot.sh b/lib/archroot.sh
index 2c03c82..06d4519 100644
--- a/lib/archroot.sh
+++ b/lib/archroot.sh
@@ -37,14 +37,6 @@  is_subvolume() {
 	[[ -e "$1" && "$(stat -f -c %T "$1")" == btrfs && "$(stat -c %i "$1")" == 256 ]]
 }
 
-##
-#  usage : is_same_fs( $path_a, $path_b )
-# return : whether $path_a and $path_b are on the same filesystem
-##
-is_same_fs() {
-	[[ "$(stat -c %d "$1")" == "$(stat -c %d "$2")" ]]
-}
-
 ##
 #  usage : subvolume_delete_recursive( $path )
 #
diff --git a/makechrootpkg.in b/makechrootpkg.in
index dc647b3..52e834b 100644
--- a/makechrootpkg.in
+++ b/makechrootpkg.in
@@ -75,37 +75,31 @@  load_vars() {
 	return 0
 }
 
-# Usage: sync_chroot $rootdir $copydir [$copy]
+# Usage: sync_chroot $chrootdir $copydir [$copy]
 sync_chroot() {
-	local rootdir=$1
+	local chrootdir=$1
 	local copydir=$2
 	local copy=${3:-$2}
 
-	if [[ "$rootdir" -ef "$copydir" ]]; then
+	if [[ "$chrootdir/root" -ef "$copydir" ]]; then
 		error 'Cannot sync copy with itself: %s' "$copydir"
 		return 1
 	fi
 
 	# Get a read lock on the root chroot to make
 	# sure we don't clone a half-updated chroot
-	slock 8 "$rootdir.lock" \
-		"Locking clean chroot [%s]" "$rootdir"
-
-	stat_busy "Synchronizing chroot copy [%s] -> [%s]" "$rootdir" "$copy"
-	if is_subvolume "$rootdir" && is_same_fs "$rootdir" "$(dirname -- "$copydir")" && ! mountpoint -q "$copydir"; then
-		if is_subvolume "$copydir"; then
-			subvolume_delete_recursive "$copydir" ||
-				die "Unable to delete subvolume %s" "$copydir"
-		else
-			# avoid change of filesystem in case of an umount failure
-			rm --recursive --force --one-file-system "$copydir" ||
-				die "Unable to delete %s" "$copydir"
-		fi
-		btrfs subvolume snapshot "$rootdir" "$copydir" >/dev/null ||
+	slock 8 "$chrootdir/root.lock" \
+		"Locking clean chroot [%s]" "$chrootdir/root"
+
+	stat_busy "Synchronizing chroot copy [%s] -> [%s]" "$chrootdir/root" "$copy"
+	if is_btrfs "$chrootdir" && ! mountpoint -q "$copydir"; then
+		subvolume_delete_recursive "$copydir" ||
+			die "Unable to delete subvolume %s" "$copydir"
+		btrfs subvolume snapshot "$chrootdir/root" "$copydir" >/dev/null ||
 			die "Unable to create subvolume %s" "$copydir"
 	else
 		mkdir -p "$copydir"
-		rsync -a --delete -q -W -x "$rootdir/" "$copydir"
+		rsync -a --delete -q -W -x "$chrootdir/root/" "$copydir"
 	fi
 	stat_done
 
@@ -388,7 +382,7 @@  main() {
 	lock 9 "$copydir.lock" "Locking chroot copy [%s]" "$copy"
 
 	if [[ ! -d $copydir ]] || $clean_first; then
-		sync_chroot "$chrootdir/root" "$copydir" "$copy"
+		sync_chroot "$chrootdir" "$copydir" "$copy"
 	fi
 
 	$update_first && arch-nspawn "$copydir" \