[gentoo-commits] proj/portage:master commit in: lib/_emerge/, lib/portage/tests/resolver/

[Zac Medico]

commit:     079f8c4a36ccc2ef5e25e7a57cd0707640f82592
Author:     Zac Medico <zmedico <AT> gentoo <DOT> org>
AuthorDate: Fri Feb 14 19:21:28 2020 +0000
Commit:     Zac Medico <zmedico <AT> gentoo <DOT> org>
CommitDate: Fri Feb 14 23:10:37 2020 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=079f8c4a

depclean: ensure consistency with update actions (bug 649622)

Make depclean traverse dependencies in the same order as update
actions, in order to ensure consistency in decisions which are
dependent on the order of dependency evaluation due to inconsistent
use of || preferences in different packages.

In unit tests, update test_virtual_w3m_realistic to assert that
the order of graph traversal is deterministic and consistent
between update and removal (depclean) actions.

Bug: https://bugs.gentoo.org/649622
Signed-off-by: Zac Medico <zmedico <AT> gentoo.org>

 lib/_emerge/actions.py                           | 33 +++++++---
 lib/_emerge/depgraph.py                          | 21 +++++--
 lib/portage/tests/resolver/ResolverPlayground.py | 77 +++++++++++++++---------
 lib/portage/tests/resolver/test_or_choices.py    | 12 +++-
 4 files changed, 96 insertions(+), 47 deletions(-)

diff --git a/lib/_emerge/actions.py b/lib/_emerge/actions.py
index 31252af16..4bf9ce425 100644
--- a/lib/_emerge/actions.py
+++ b/lib/_emerge/actions.py
@@ -1,8 +1,9 @@
-# Copyright 1999-2019 Gentoo Authors
+# Copyright 1999-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 from __future__ import division, print_function, unicode_literals
 
+import collections
 import errno
 import logging
 import operator
@@ -741,7 +742,19 @@ def action_depclean(settings, trees, ldpath_mtimes,
 
 	return rval
 
+
 def calc_depclean(settings, trees, ldpath_mtimes,
+	myopts, action, args_set, spinner):
+	result = _calc_depclean(settings, trees, ldpath_mtimes,
+		myopts, action, args_set, spinner)
+	return result.returncode, result.cleanlist, result.ordered, result.req_pkg_count
+
+
+_depclean_result = collections.namedtuple('_depclean_result',
+	('returncode', 'cleanlist', 'ordered', 'req_pkg_count', 'depgraph'))
+
+
+def _calc_depclean(settings, trees, ldpath_mtimes,
 	myopts, action, args_set, spinner):
 	allow_missing_deps = bool(args_set)
 
@@ -805,7 +818,7 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 		writemsg_level(_("!!! Aborting due to set configuration "
 			"errors displayed above.\n"),
 			level=logging.ERROR, noiselevel=-1)
-		return 1, [], False, 0
+		return _depclean_result(1, [], False, 0, None)
 
 	if action == "depclean":
 		emergelog(xterm_titles, " >>> depclean")
@@ -920,7 +933,7 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 	resolver.display_problems()
 
 	if not success:
-		return 1, [], False, 0
+		return _depclean_result(1, [], False, 0, resolver)
 
 	def unresolved_deps():
 
@@ -1020,7 +1033,7 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 		return False
 
 	if unresolved_deps():
-		return 1, [], False, 0
+		return _depclean_result(1, [], False, 0, resolver)
 
 	graph = resolver._dynamic_config.digraph.copy()
 	required_pkgs_total = 0
@@ -1321,7 +1334,7 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 							runtime_slot_op=True),
 						root=pkg.root)):
 						resolver.display_problems()
-						return 1, [], False, 0
+						return _depclean_result(1, [], False, 0, resolver)
 
 			writemsg_level("\nCalculating dependencies  ")
 			success = resolver._complete_graph(
@@ -1329,9 +1342,9 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 			writemsg_level("\b\b... done!\n")
 			resolver.display_problems()
 			if not success:
-				return 1, [], False, 0
+				return _depclean_result(1, [], False, 0, resolver)
 			if unresolved_deps():
-				return 1, [], False, 0
+				return _depclean_result(1, [], False, 0, resolver)
 
 			graph = resolver._dynamic_config.digraph.copy()
 			required_pkgs_total = 0
@@ -1340,7 +1353,7 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 					required_pkgs_total += 1
 			cleanlist = create_cleanlist()
 			if not cleanlist:
-				return 0, [], False, required_pkgs_total
+				return _depclean_result(0, [], False, required_pkgs_total, resolver)
 			clean_set = set(cleanlist)
 
 	if clean_set:
@@ -1458,8 +1471,8 @@ def calc_depclean(settings, trees, ldpath_mtimes,
 					graph.remove(node)
 					cleanlist.append(node.cpv)
 
-		return 0, cleanlist, ordered, required_pkgs_total
-	return 0, [], False, required_pkgs_total
+		return _depclean_result(0, cleanlist, ordered, required_pkgs_total, resolver)
+	return _depclean_result(0, [], False, required_pkgs_total, resolver)
 
 def action_deselect(settings, trees, opts, atoms):
 	enter_invalid = '--ask-enter-invalid' in opts

diff --git a/lib/_emerge/depgraph.py b/lib/_emerge/depgraph.py
index 8e0d79e29..27696ad40 100644
--- a/lib/_emerge/depgraph.py
+++ b/lib/_emerge/depgraph.py
@@ -6955,9 +6955,18 @@ class depgraph(object):
 				# Removal actions may override sets with temporary
 				# replacements that have had atoms removed in order
 				# to implement --deselect behavior.
-				required_set_names = set(required_sets[root])
 				depgraph_sets.sets.clear()
 				depgraph_sets.sets.update(required_sets[root])
+				if 'world' in depgraph_sets.sets:
+					# For consistent order of traversal for both update
+					# and removal (depclean) actions, sets other that
+					# world are always nested under the world set.
+					world_atoms = list(depgraph_sets.sets['world'])
+					world_atoms.extend(SETPREFIX + s for s in required_sets[root] if s != 'world')
+					depgraph_sets.sets['world'] = InternalPackageSet(initial_atoms=world_atoms)
+					required_set_names = {'world'}
+				else:
+					required_set_names = set(required_sets[root])
 			if "remove" not in self._dynamic_config.myparams and \
 				root == self._frozen_config.target_root and \
 				already_deep:
@@ -6967,7 +6976,7 @@ class depgraph(object):
 				not self._dynamic_config._dep_stack:
 				continue
 			root_config = self._frozen_config.roots[root]
-			for s in required_set_names:
+			for s in sorted(required_set_names):
 				pset = depgraph_sets.sets.get(s)
 				if pset is None:
 					pset = root_config.sets[s]
@@ -6977,10 +6986,10 @@ class depgraph(object):
 
 		self._set_args(args)
 		for arg in self._expand_set_args(args, add_to_digraph=True):
-			for atom in sorted(arg.pset.getAtoms(), reverse=True):
-				self._dynamic_config._dep_stack.append(
-					Dependency(atom=atom, root=arg.root_config.root,
-						parent=arg, depth=self._UNREACHABLE_DEPTH))
+			for atom in sorted(arg.pset.getAtoms()):
+				if not self._add_dep(Dependency(atom=atom, root=arg.root_config.root,
+					parent=arg, depth=self._UNREACHABLE_DEPTH), allow_unsatisfied=True):
+					return 0
 
 		if True:
 			if self._dynamic_config._ignored_deps:

diff --git a/lib/portage/tests/resolver/ResolverPlayground.py b/lib/portage/tests/resolver/ResolverPlayground.py
index cc0aa46e9..0456ce2e2 100644
--- a/lib/portage/tests/resolver/ResolverPlayground.py
+++ b/lib/portage/tests/resolver/ResolverPlayground.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2019 Gentoo Authors
+# Copyright 2010-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 import bz2
@@ -22,9 +22,10 @@ from portage.util import ensure_dirs, normalize_path
 from portage.versions import catsplit
 
 import _emerge
-from _emerge.actions import calc_depclean
+from _emerge.actions import _calc_depclean
 from _emerge.Blocker import Blocker
 from _emerge.create_depgraph_params import create_depgraph_params
+from _emerge.DependencyArg import DependencyArg
 from _emerge.depgraph import backtrack_depgraph
 from _emerge.RootConfig import RootConfig
 
@@ -593,11 +594,16 @@ class ResolverPlayground(object):
 			_emerge.emergelog._disable = True
 
 			if action in ("depclean", "prune"):
-				rval, cleanlist, ordered, req_pkg_count = \
-					calc_depclean(self.settings, self.trees, None,
+				depclean_result = _calc_depclean(self.settings, self.trees, None,
 					options, action, InternalPackageSet(initial_atoms=atoms, allow_wildcard=True), None)
 				result = ResolverPlaygroundDepcleanResult(
-					atoms, rval, cleanlist, ordered, req_pkg_count)
+					atoms,
+					depclean_result.returncode,
+					depclean_result.cleanlist,
+					depclean_result.ordered,
+					depclean_result.req_pkg_count,
+					depclean_result.depgraph,
+				)
 			else:
 				params = create_depgraph_params(options, action)
 				success, depgraph, favorites = backtrack_depgraph(
@@ -780,18 +786,46 @@ class ResolverPlaygroundTestCase(object):
 			return False
 		return True
 
+
+def _mergelist_str(x, depgraph):
+	if isinstance(x, DependencyArg):
+		mergelist_str = x.arg
+	elif isinstance(x, Blocker):
+		mergelist_str = x.atom
+	else:
+		repo_str = ""
+		if x.repo != "test_repo":
+			repo_str = _repo_separator + x.repo
+		build_id_str = ""
+		if (x.type_name == "binary" and
+			x.cpv.build_id is not None):
+			build_id_str = "-%s" % x.cpv.build_id
+		mergelist_str = x.cpv + build_id_str + repo_str
+		if x.built:
+			if x.operation == "merge":
+				desc = x.type_name
+			else:
+				desc = x.operation
+			mergelist_str = "[%s]%s" % (desc, mergelist_str)
+		if x.root != depgraph._frozen_config._running_root.root:
+			mergelist_str += "{targetroot}"
+	return mergelist_str
+
+
 class ResolverPlaygroundResult(object):
 
 	checks = (
 		"success", "mergelist", "use_changes", "license_changes",
 		"unstable_keywords", "slot_collision_solutions",
 		"circular_dependency_solutions", "needed_p_mask_changes",
-		"unsatisfied_deps", "forced_rebuilds", "required_use_unsatisfied"
+		"unsatisfied_deps", "forced_rebuilds", "required_use_unsatisfied",
+		"graph_order",
 		)
 	optional_checks = (
 		"forced_rebuilds",
 		"required_use_unsatisfied",
-		"unsatisfied_deps"
+		"unsatisfied_deps",
+		"graph_order",
 		)
 
 	def __init__(self, atoms, success, mydepgraph, favorites):
@@ -810,30 +844,12 @@ class ResolverPlaygroundResult(object):
 		self.forced_rebuilds = None
 		self.required_use_unsatisfied = None
 
+		self.graph_order = [_mergelist_str(node, self.depgraph) for node in self.depgraph._dynamic_config.digraph]
+
 		if self.depgraph._dynamic_config._serialized_tasks_cache is not None:
 			self.mergelist = []
-			host_root = self.depgraph._frozen_config._running_root.root
 			for x in self.depgraph._dynamic_config._serialized_tasks_cache:
-				if isinstance(x, Blocker):
-					self.mergelist.append(x.atom)
-				else:
-					repo_str = ""
-					if x.repo != "test_repo":
-						repo_str = _repo_separator + x.repo
-					build_id_str = ""
-					if (x.type_name == "binary" and
-						x.cpv.build_id is not None):
-						build_id_str = "-%s" % x.cpv.build_id
-					mergelist_str = x.cpv + build_id_str + repo_str
-					if x.built:
-						if x.operation == "merge":
-							desc = x.type_name
-						else:
-							desc = x.operation
-						mergelist_str = "[%s]%s" % (desc, mergelist_str)
-					if x.root != host_root:
-						mergelist_str += "{targetroot}"
-					self.mergelist.append(mergelist_str)
+				self.mergelist.append(_mergelist_str(x, self.depgraph))
 
 		if self.depgraph._dynamic_config._needed_use_config_changes:
 			self.use_changes = {}
@@ -894,14 +910,17 @@ class ResolverPlaygroundDepcleanResult(object):
 
 	checks = (
 		"success", "cleanlist", "ordered", "req_pkg_count",
+		"graph_order",
 		)
 	optional_checks = (
 		"ordered", "req_pkg_count",
+		"graph_order",
 		)
 
-	def __init__(self, atoms, rval, cleanlist, ordered, req_pkg_count):
+	def __init__(self, atoms, rval, cleanlist, ordered, req_pkg_count, depgraph):
 		self.atoms = atoms
 		self.success = rval == 0
 		self.cleanlist = cleanlist
 		self.ordered = ordered
 		self.req_pkg_count = req_pkg_count
+		self.graph_order = [_mergelist_str(node, depgraph) for node in depgraph._dynamic_config.digraph]

diff --git a/lib/portage/tests/resolver/test_or_choices.py b/lib/portage/tests/resolver/test_or_choices.py
index f31a5ff22..5c6803784 100644
--- a/lib/portage/tests/resolver/test_or_choices.py
+++ b/lib/portage/tests/resolver/test_or_choices.py
@@ -667,12 +667,16 @@ class OrChoicesTestCase(TestCase):
 
 			# Test for bug 649622 (with www-client/w3m installed via
 			# xorg-server dependency), where virtual/w3m was pulled in
-			# only to be removed by the next emerge --depclean.
+			# only to be removed by the next emerge --depclean. Note
+			# that graph_order must be deterministic in order to achieve
+			# deterministic results which are consistent between both
+			# update and removal (depclean) actions.
 			ResolverPlaygroundTestCase(
 				['@world'],
 				options = {'--update': True, '--deep': True},
 				success = True,
 				mergelist=['virtual/w3m-0'],
+				graph_order=['@world', '@system', '@selected', '@profile', '[nomerge]app-misc/neofetch-6.1.0', '[nomerge]mail-client/neomutt-20191207', '[nomerge]www-client/lynx-2.9.0_pre4', '[nomerge]x11-base/xorg-server-1.20.7', '[nomerge]app-text/xmlto-0.0.28-r1', '[nomerge]www-client/w3m-0.5.3_p20190105', 'virtual/w3m-0'],
 			),
 
 		)
@@ -702,12 +706,16 @@ class OrChoicesTestCase(TestCase):
 			# Test for bug 649622, where virtual/w3m is removed by
 			# emerge --depclean immediately after it's installed
 			# by a world update. Since virtual/w3m-0 is not removed
-			# here, this case fails to reproduce bug 649622.
+			# here, this case fails to reproduce bug 649622. Note
+			# that graph_order must be deterministic in order to achieve
+			# deterministic results which are consistent between both
+			# update and removal (depclean) actions.
 			ResolverPlaygroundTestCase(
 				[],
 				options={'--depclean': True},
 				success=True,
 				cleanlist=[],
+				graph_order=['@world', '@system', '@selected', '@profile', '@____depclean_protected_set____', '[nomerge]app-misc/neofetch-6.1.0', '[nomerge]mail-client/neomutt-20191207', '[nomerge]www-client/lynx-2.9.0_pre4', '[nomerge]x11-base/xorg-server-1.20.7', '[nomerge]app-text/xmlto-0.0.28-r1', '[nomerge]www-client/w3m-0.5.3_p20190105', '[nomerge]virtual/w3m-0'],
 			),
 
 		)