[gentoo-portage-dev] [PATCH] binarytree.move_ent: copy on write for package move

[Zac Medico]

Copy on write when applying package moves, and silently
skip package moves when the same move has already been
applied to the same build of the package. Since the old
package instance is preserved, it avoids the problem
of having enries for deleted packages remain in the
package index. We can simply assume that the package
will be deleted by eclean-pkg when its time comes.

Bug: https://bugs.gentoo.org/766012
Signed-off-by: Zac Medico <zmedico@g.o>
---
 lib/portage/dbapi/bintree.py              | 40 ++++++++++++++++-------
 lib/portage/emaint/modules/move/move.py   | 13 ++++++--
 lib/portage/tests/update/test_move_ent.py |  7 ++--
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/lib/portage/dbapi/bintree.py b/lib/portage/dbapi/bintree.py
index 180e48c3b..76fca5523 100644
--- a/lib/portage/dbapi/bintree.py
+++ b/lib/portage/dbapi/bintree.py
@@ -31,6 +31,7 @@ from portage.exception import AlarmSignal, InvalidPackageName, \
 	ParseError, PortageException
 from portage.localization import _
 from portage.package.ebuild.profile_iuse import iter_iuse_vars
+from portage.util.file_copy import copyfile
 from portage.util.futures import asyncio
 from portage.util.futures.compat_coroutine import coroutine
 from portage.util.futures.executor.fork import ForkExecutor
@@ -483,6 +484,17 @@ class binarytree:
 			myoldpkg = catsplit(mycpv)[1]
 			mynewpkg = catsplit(mynewcpv)[1]
 
+			# If this update has already been applied to the same
+			# package build then silently continue.
+			applied = False
+			for maybe_applied in self.dbapi.match('={}'.format(mynewcpv)):
+				if maybe_applied.build_time == mycpv.build_time:
+					applied = True
+					break
+
+			if applied:
+				continue
+
 			if (mynewpkg != myoldpkg) and self.dbapi.cpv_exists(mynewcpv):
 				writemsg(_("!!! Cannot update binary: Destination exists.\n"),
 					noiselevel=-1)
@@ -513,24 +525,30 @@ class binarytree:
 					mydata[_unicode_encode(mynewpkg + '.ebuild',
 						encoding=_encodings['repo.content'])] = ebuild_data
 
-			mytbz2.recompose_mem(portage.xpak.xpak_mem(mydata))
-
-			self.dbapi.cpv_remove(mycpv)
-			del self._pkg_paths[self.dbapi._instance_key(mycpv)]
 			metadata = self.dbapi._aux_cache_slot_dict()
 			for k in self.dbapi._aux_cache_keys:
 				v = mydata.get(_unicode_encode(k))
 				if v is not None:
 					v = _unicode_decode(v)
 					metadata[k] = " ".join(v.split())
+
+			# Create a copy of the old version of the package and
+			# apply the update to it. Leave behind the old version,
+			# assuming that it will be deleted by eclean-pkg when its
+			# time comes.
 			mynewcpv = _pkg_str(mynewcpv, metadata=metadata, db=self.dbapi)
-			new_path = self.getname(mynewcpv)
-			self._pkg_paths[
-				self.dbapi._instance_key(mynewcpv)] = new_path[len(self.pkgdir)+1:]
-			if new_path != tbz2path:
-				self._ensure_dir(os.path.dirname(new_path))
-				_movefile(tbz2path, new_path, mysettings=self.settings)
-			self.inject(mynewcpv)
+			update_path = self.getname(mynewcpv, allocate_new=True) + ".partial"
+			self._ensure_dir(os.path.dirname(update_path))
+			update_path_lock = None
+			try:
+				update_path_lock = lockfile(update_path, wantnewlockfile=True)
+				copyfile(tbz2path, update_path)
+				mytbz2 = portage.xpak.tbz2(update_path)
+				mytbz2.recompose_mem(portage.xpak.xpak_mem(mydata))
+				self.inject(mynewcpv, filename=update_path)
+			finally:
+				if update_path_lock is not None:
+					unlockfile(update_path_lock)
 
 		return moves
 
diff --git a/lib/portage/emaint/modules/move/move.py b/lib/portage/emaint/modules/move/move.py
index 8fc3269ca..2a95e99c4 100644
--- a/lib/portage/emaint/modules/move/move.py
+++ b/lib/portage/emaint/modules/move/move.py
@@ -1,4 +1,4 @@
-# Copyright 2005-2020 Gentoo Authors
+# Copyright 2005-2021 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from _emerge.Package import Package
@@ -76,7 +76,16 @@ class MoveHandler:
 						except (KeyError, InvalidData):
 							continue
 						if repo_match(cpv.repo):
-							errors.append("'%s' moved to '%s'" % (cpv, newcp))
+							build_time = getattr(cpv, 'build_time', None)
+							if build_time is not None:
+								# If this update has already been applied to the same
+								# package build then silently continue.
+								for maybe_applied in match('={}'.format(
+									cpv.replace(cpv.cp, str(newcp), 1))):
+									if maybe_applied.build_time == build_time:
+										break
+								else:
+									errors.append("'%s' moved to '%s'" % (cpv, newcp))
 				elif update_cmd[0] == "slotmove":
 					pkg, origslot, newslot = update_cmd[1:]
 					atom = pkg.with_slot(origslot)
diff --git a/lib/portage/tests/update/test_move_ent.py b/lib/portage/tests/update/test_move_ent.py
index d9647a95e..d9036db0a 100644
--- a/lib/portage/tests/update/test_move_ent.py
+++ b/lib/portage/tests/update/test_move_ent.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2013 Gentoo Foundation
+# Copyright 2012-2021 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import textwrap
@@ -93,8 +93,9 @@ class MoveEntTestCase(TestCase):
 			self.assertRaises(KeyError,
 				vardb.aux_get, "dev-libs/A-1", ["EAPI"])
 			vardb.aux_get("dev-libs/A-moved-1", ["EAPI"])
-			self.assertRaises(KeyError,
-				bindb.aux_get, "dev-libs/A-1", ["EAPI"])
+			# The original package should still exist because a binary
+			# package move is a copy on write operation.
+			bindb.aux_get("dev-libs/A-1", ["EAPI"])
 			bindb.aux_get("dev-libs/A-moved-1", ["EAPI"])
 
 			# dont_apply_updates
-- 
2.26.2