[gentoo-portage-dev] [PATCH] Fix unnecessary rebuild (bug 487198)

From: Sebastian Luther <SebastianLuther@×××.de>

This one was caused by a mix of >= and < dependencies.
---
 pym/_emerge/depgraph.py                            | 92 +++++++++++++++++-----
 .../tests/resolver/test_slot_conflict_rebuild.py   | 66 ++++++++++++++++
 2 files changed, 139 insertions(+), 19 deletions(-)

diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index cd68db6..67b8881 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -1298,7 +1298,34 @@ class depgraph(object):
 
 			selected_atoms = None
 
-			for atom in replacement_parent.validated_atoms:
+			atoms = set()
+			invalid_metadata = False
+			for dep_key in ("DEPEND", "HDEPEND", "RDEPEND", "PDEPEND"):
+				dep_string = replacement_parent._metadata[dep_key]
+				if not dep_string:
+					continue
+
+				try:
+					dep_string = portage.dep.use_reduce(dep_string,
+						uselist=self._pkg_use_enabled(replacement_parent),
+						is_valid_flag=replacement_parent.iuse.is_valid_flag,
+						flat=True, token_class=Atom,
+						eapi=replacement_parent.eapi)
+				except portage.exception.InvalidDependString:
+					invalid_metadata = True
+					break
+
+				atoms.update(token for token in dep_string if isinstance(token, Atom))
+
+			if invalid_metadata:
+				continue
+
+			# List of list of child,atom pairs for each atom.
+			replacement_candidates = []
+			# Set of all packages all atoms can agree on.
+			all_candidate_pkgs = None
+
+			for atom in atoms:
 				if atom.blocker or \
 					atom.cp != dep.atom.cp:
 					continue
@@ -1316,6 +1343,8 @@ class depgraph(object):
 					# parent and search for another.
 					break
 
+				candidate_pkg_atoms = []
+				candidate_pkgs = []
 				for pkg in self._iter_similar_available(
 					dep.child, atom):
 					if pkg.slot == dep.child.slot and \
@@ -1367,26 +1396,51 @@ class depgraph(object):
 						if unevaluated_atom not in selected_atoms:
 							continue
 
-					if debug:
-						msg = []
-						msg.append("")
-						msg.append("")
-						msg.append("slot_operator_update_probe:")
-						msg.append("   existing child package:  %s" % dep.child)
-						msg.append("   existing parent package: %s" % dep.parent)
-						msg.append("   new child package:  %s" % pkg)
-						msg.append("   new parent package: %s" % replacement_parent)
-						if insignificant:
-							msg.append("insignificant changes detected")
-						msg.append("")
-						writemsg_level("\n".join(msg),
-							noiselevel=-1, level=logging.DEBUG)
+					if not insignificant:
+						candidate_pkg_atoms.append((pkg, unevaluated_atom))
+						candidate_pkgs.append(pkg)
+
+				replacement_candidates.append(candidate_pkg_atoms)
+				if all_candidate_pkgs is None:
+					all_candidate_pkgs = set(candidate_pkgs)
+				else:
+					all_candidate_pkgs.intersection_update(candidate_pkgs)
+
+			if not all_candidate_pkgs:
+				# If the atoms that connect parent and child can't agree on
+				# any replacement child, we can't do anything.
+				continue
+
+			# Now select one of the pkgs as replacement. This is as easy as
+			# selecting the highest version.
+			# The more complicated part is to choose an atom for the
+			# new Dependency object. Choose the one which ranked the selected
+			# parent highest.
+			selected = None
+			for candidate_pkg_atoms in replacement_candidates:
+				for i, (pkg, atom) in enumerate(candidate_pkg_atoms):
+					if pkg not in all_candidate_pkgs:
+						continue
+					if selected is None or \
+						selected[0] < pkg or \
+						(selected[0] is pkg and i < selected[2]):
+						selected = (pkg, atom, i)
 
-					if insignificant:
-						return None
+			if debug:
+				msg = []
+				msg.append("")
+				msg.append("")
+				msg.append("slot_operator_update_probe:")
+				msg.append("   existing child package:  %s" % dep.child)
+				msg.append("   existing parent package: %s" % dep.parent)
+				msg.append("   new child package:  %s" % selected[0])
+				msg.append("   new parent package: %s" % replacement_parent)
+				msg.append("")
+				writemsg_level("\n".join(msg),
+					noiselevel=-1, level=logging.DEBUG)
 
-					return Dependency(parent=replacement_parent,
-						child=pkg, atom=unevaluated_atom)
+			return Dependency(parent=replacement_parent,
+				child=selected[0], atom=selected[1])
 
 		if debug:
 			msg = []
diff --git a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
index c7c62dd..6599a82 100644
--- a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
+++ b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
@@ -298,3 +298,69 @@ class SlotConflictRebuildTestCase(TestCase):
 				self.assertEqual(test_case.test_success, True, test_case.fail_msg)
 		finally:
 			playground.cleanup()
+
+
+	def testSlotConflictDontKnow(self):
+		"""
+		Bug 487198
+		For parents with mixed >= and < dependencies, we scheduled rebuilds for the
+		>= atom, but in the end didn't install the child update becaue of the < atom.
+		"""
+		ebuilds = {
+			"cat/slotted-lib-1" : {
+				"EAPI": "5",
+				"SLOT": "1"
+			},
+			"cat/slotted-lib-2" : {
+				"EAPI": "5",
+				"SLOT": "2"
+			},
+			"cat/slotted-lib-3" : {
+				"EAPI": "5",
+				"SLOT": "3"
+			},
+			"cat/slotted-lib-4" : {
+				"EAPI": "5",
+				"SLOT": "4"
+			},
+			"cat/slotted-lib-5" : {
+				"EAPI": "5",
+				"SLOT": "5"
+			},
+			"cat/user-1" : {
+				"EAPI": "5",
+				"DEPEND": ">=cat/slotted-lib-2:= <cat/slotted-lib-4:=",
+				"RDEPEND": ">=cat/slotted-lib-2:= <cat/slotted-lib-4:=",
+			},
+		}
+
+		installed = {
+			"cat/slotted-lib-3" : {
+				"EAPI": "5",
+				"SLOT": "3"
+			},
+			"cat/user-1" : {
+				"EAPI": "5",
+				"DEPEND": ">=cat/slotted-lib-2:3/3= <cat/slotted-lib-4:3/3=",
+				"RDEPEND": ">=cat/slotted-lib-2:3/3= <cat/slotted-lib-4:3/3=",
+			},
+		}
+
+		test_cases = (
+			ResolverPlaygroundTestCase(
+				["cat/user"],
+				options = {"--deep": True, "--update": True},
+				success = True,
+				mergelist = []),
+		)
+
+		world = []
+
+		playground = ResolverPlayground(ebuilds=ebuilds,
+			installed=installed, world=world, debug=False)
+		try:
+			for test_case in test_cases:
+				playground.run_TestCase(test_case)
+				self.assertEqual(test_case.test_success, True, test_case.fail_msg)
+		finally:
+			playground.cleanup()
-- 
1.8.1.5