[pacman-dev] [PATCH] Give pacman-optimize a refresher

[Dan McGee]

This patch addresses quite a few lingering issues in the pacman-optimize
script. FS#11767 provoked this look-over and the following issues were
noticed and fixed:

* If an alternate dbroot was specified, then the lockfile location was never
  updated to reflect it. The lockfile location is now set after all dbpath
  initialization.
* The inclusion of a trailing slash on dbroot was problematic and led to the
  following command being executed:
    bsdtar -xpf /tmp/pacman-optimize.p12Q4vAUWY/pacman-db.tar.gz \
	  -C /var/lib/pacman/.new/
  It is doubtful we meant to create a hidden directory like this below our
  database root, only to go and delete it a second later and then
  re-extract. Fix the whole thing by ensuring our dbpath has its trailing
  slash stripped and then appending it when necessary.
* The DB extraction was performed twice for no real apparent reason. This
  opens the door for extraction problems the second time around, leaving you
  with no original database to fall back to. Change the behavior so we only
  extract once, and then perform a directory shuffle once we verify the
  checksums are correct.
* Perform an explicit sync after we drop the new database on the disk. It
  should work better this way.
* Tighten up our check for a pacman lockfile and the time we create one.
  There is still a possible race condition but the window is shorter.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
Hey guys,

I'd really appreciate if some of you can scrutinize this patch a bit and give
it a few test runs. I've ran it on my real local DB, so I can at least offer
the assurance that it worked for me.

Any feedback or suggestions for changes are obviously welcome.

I'm not sure if we should land this on maint or master- any opinions?

-Dan

 scripts/pacman-optimize.sh.in |   55 +++++++++++++++++++++++++---------------
 1 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/scripts/pacman-optimize.sh.in b/scripts/pacman-optimize.sh.in
index 0cd108b..0ea22e4 100644
--- a/scripts/pacman-optimize.sh.in
+++ b/scripts/pacman-optimize.sh.in
@@ -25,7 +25,6 @@ export TEXTDOMAINDIR='@localedir@'
 
 myver='@PACKAGE_VERSION@'
 dbroot='@localstatedir@/lib/pacman/'
-lockfile="${dbroot}db.lck"
 
 msg() {
 	local mesg=$1; shift
@@ -98,11 +97,6 @@ if ! type diff >/dev/null 2>&1; then
 	die "$(gettext "diff tool was not found, please install diffutils.")"
 fi
 
-# make sure pacman isn't running
-if [ -f "$lockfile" ]; then
-	die "$(gettext "Pacman lock file was found. Cannot run while pacman is running.")"
-fi
-
 if [ ! -d "$dbroot" ]; then
 	die "$(gettext "%s does not exist or is not a directory.")" "$dbroot"
 fi
@@ -111,12 +105,21 @@ if [ ! -w "$dbroot" ]; then
 	die "$(gettext "You must have correct permissions to optimize the database.")"
 fi
 
-workdir=$(mktemp -d /tmp/pacman-optimize.XXXXXXXXXX) ||
-	die_r "$(gettext "ERROR: Can not create temp directory for database building.")\n" >&2
+# strip any trailing slash from our dbroot
+dbroot="${dbroot%/}"
+# form the path to our lockfile location
+lockfile="${dbroot}/db.lck"
 
+# make sure pacman isn't running
+if [ -f "$lockfile" ]; then
+	die "$(gettext "Pacman lock file was found. Cannot run while pacman is running.")"
+fi
 # do not let pacman run while we do this
 touch "$lockfile"
 
+workdir=$(mktemp -d /tmp/pacman-optimize.XXXXXXXXXX) ||
+	die_r "$(gettext "ERROR: Can not create temp directory for database building.")\n" >&2
+
 # step 1: sum the old db
 msg "$(gettext "MD5sum'ing the old database...")"
 find "$dbroot" -type f | sort | xargs md5sum > "$workdir/pacsums.old"
@@ -124,46 +127,56 @@ find "$dbroot" -type f | sort | xargs md5sum > "$workdir/pacsums.old"
 # step 2: tar it up
 msg "$(gettext "Tar'ing up %s...")" "$dbroot"
 cd "$dbroot"
-bsdtar -czf "$workdir/pacmanDB.tgz" ./
+bsdtar -czf "$workdir/pacman-db.tar.gz" ./
 if [ $? -ne 0 ]; then
 	rm -rf "$workdir"
 	die_r "$(gettext "Tar'ing up %s failed.")" "$dbroot"
 fi
 
-# step 3: make and sum the new db
-msg "$(gettext "Making and MD5sum'ing the new db...")"
+# step 3: make and sum the new db side-by-side with the old
+msg "$(gettext "Making and MD5sum'ing the new database...")"
 mkdir "$dbroot.new"
-bsdtar -zxpf "$workdir/pacmanDB.tgz" -C "$dbroot.new/"
+bsdtar -xpf "$workdir/pacman-db.tar.gz" -C "$dbroot.new"
+# immediate sync following extraction should get it written continuously on HDD
+msg "$(gettext "Syncing database to disk...")"
+sync
 if [ $? -ne 0 ]; then
        rm -rf "$workdir"
        die_r "$(gettext "Untar'ing %s failed.")" "$dbroot"
 fi
 find "$dbroot.new" -type f | sort | \
-		xargs md5sum | sed 's#.new/##' > "$workdir/pacsums.new"
+		xargs md5sum | sed 's#.new##' > "$workdir/pacsums.new"
 
 # step 4: compare the sums
 msg "$(gettext "Checking integrity...")"
 diff "$workdir/pacsums.old" "$workdir/pacsums.new" >/dev/null 2>&1
 if [ $? -ne 0 ]; then
 	# failed
-	# leave /tmp/pacsums.old and .new for checking to see what doesn't match up
+	# leave our pacman-optimize tmpdir for checking to see what doesn't match up
 	rm -rf "$dbroot.new"
 	die_r "$(gettext "Integrity check FAILED, reverting to old database.")"
 fi
 
-# step 5: remove the new temporary database and the old one
-#         and use the .tgz to replace the old one
-msg "$(gettext "Putting the new database in place...")"
-rm -rf "$dbroot.new" "$dbroot"/*
-bsdtar -zxpf "$workdir/pacmanDB.tgz" -C "$dbroot/"
+# step 5: shuffle the newly extracted DB into the proper location
+msg "$(gettext "Rotating database into place...")"
+
+fail=0
+mv "$dbroot" "$dbroot.old" || fail=1
+mv "$dbroot.new" "$dbroot" || fail=1
+chmod --reference="$dbroot.old" "$dbroot" || fail=1
+chown --reference="$dbroot.old" "$dbroot" || fail=1
+if [ $fail -ne 0 ]; then
+	# failure with our directory shuffle
+	die_r "$(gettext "New database substitution failed. Check for $dbroot,\n$dbroot.old, and $dbroot.new directories.")"
+fi
+rm -rf "$dbroot.old"
 
-# remove the lock file, sum files, and .tgz of database
+# remove the lock file and our working directory with sums and tarfile
 rm -f "$lockfile"
 rm -rf "$workdir"
 
 echo
 msg "$(gettext "Finished. Your pacman database has been optimized.")"
-msg "$(gettext "For full benefits of pacman-optimize, run 'sync' now.")"
 echo
 
 exit 0
-- 
1.6.0.2