[gentoo-portage-dev] [PATCH] emerge --with-test-deps: allow circular deps

[Zac Medico]

When USE=test is not enabled, allow circular test dependencies
by treating them like PDEPEND. When USE=test is enabled, circular
dependencies are still not allowed, as shown in unit tests.

Suggested-by: Michał Górny <mgorny@g.o>
Bug: https://bugs.gentoo.org/703348
Signed-off-by: Zac Medico <zmedico@g.o>
---
 lib/_emerge/depgraph.py                       | 18 +++++++--
 lib/portage/dep/__init__.py                   | 32 ++++++++++++++-
 lib/portage/tests/dep/test_use_reduce.py      | 30 +++++++++++++-
 .../tests/resolver/test_with_test_deps.py     | 39 ++++++++++++++++++-
 4 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/lib/_emerge/depgraph.py b/lib/_emerge/depgraph.py
index 1a5448c8f..610a0fc47 100644
--- a/lib/_emerge/depgraph.py
+++ b/lib/_emerge/depgraph.py
@@ -3325,10 +3325,6 @@ class depgraph(object):
 			pkg.iuse.is_valid_flag("test") and \
 			self._is_argument(pkg)
 
-		if with_test_deps:
-			use_enabled = set(use_enabled)
-			use_enabled.add("test")
-
 		if not pkg.built and \
 			"--buildpkgonly" in self._frozen_config.myopts and \
 			"deep" not in self._dynamic_config.myparams:
@@ -3430,6 +3426,20 @@ class depgraph(object):
 						noiselevel=-1, level=logging.DEBUG)
 
 				try:
+					if with_test_deps and 'test' not in use_enabled:
+						test_deps = portage.dep.use_reduce(dep_string,
+							uselist=use_enabled | {'test'},
+							is_valid_flag=pkg.iuse.is_valid_flag,
+							opconvert=True, token_class=Atom,
+							eapi=pkg.eapi,
+							subset={'test'})
+
+						if test_deps and not self._add_pkg_dep_string(
+							pkg, dep_root, self._priority(runtime_post=True),
+							test_deps,
+							allow_unsatisfied):
+							return 0
+
 					dep_string = portage.dep.use_reduce(dep_string,
 						uselist=use_enabled,
 						is_valid_flag=pkg.iuse.is_valid_flag,
diff --git a/lib/portage/dep/__init__.py b/lib/portage/dep/__init__.py
index f08f6ba4c..eaa200751 100644
--- a/lib/portage/dep/__init__.py
+++ b/lib/portage/dep/__init__.py
@@ -405,7 +405,8 @@ def paren_enclose(mylist, unevaluated_atom=False, opconvert=False):
 	return " ".join(mystrparts)
 
 def use_reduce(depstr, uselist=(), masklist=(), matchall=False, excludeall=(), is_src_uri=False, \
-	eapi=None, opconvert=False, flat=False, is_valid_flag=None, token_class=None, matchnone=False):
+	eapi=None, opconvert=False, flat=False, is_valid_flag=None, token_class=None, matchnone=False,
+	subset=None):
 	"""
 	Takes a dep string and reduces the use? conditionals out, leaving an array
 	with subarrays. All redundant brackets are removed.
@@ -434,6 +435,8 @@ def use_reduce(depstr, uselist=(), masklist=(), matchall=False, excludeall=(), i
 	@type token_class: Class
 	@param matchnone: Treat all conditionals as inactive. Used by digestgen(). 
 	@type matchnone: Bool
+	@param subset: Select a subset of dependencies conditional on the given flags
+	@type subset: Sequence
 	@rtype: List
 	@return: The use reduced depend array
 	"""
@@ -491,6 +494,33 @@ def use_reduce(depstr, uselist=(), masklist=(), matchall=False, excludeall=(), i
 		return (flag in uselist and not is_negated) or \
 			(flag not in uselist and is_negated)
 
+	if subset:
+		def select_subset(dep_struct, disjunction, selected):
+			result = []
+			stack = list(dep_struct)
+			stack.reverse()
+			while stack:
+				token = stack.pop()
+				try:
+					conditional = token.endswith('?')
+				except AttributeError:
+					result.extend(token)
+				else:
+					if conditional:
+						children = stack.pop()
+						if is_active(token) and token[:-1] in subset:
+							if disjunction:
+								result.append(select_subset(children, True, True))
+							else:
+								result.extend(select_subset(children, False, True))
+					elif token == '||':
+						result.append(token)
+						result.append(select_subset(stack.pop(), True, selected))
+					elif selected:
+						result.append(token)
+			return result
+		depstr = paren_enclose(select_subset(paren_reduce(depstr, _deprecation_warn=False), False, False))
+
 	def missing_white_space_check(token, pos):
 		"""
 		Used to generate good error messages for invalid tokens.
diff --git a/lib/portage/tests/dep/test_use_reduce.py b/lib/portage/tests/dep/test_use_reduce.py
index 4f65567cf..3435b6116 100644
--- a/lib/portage/tests/dep/test_use_reduce.py
+++ b/lib/portage/tests/dep/test_use_reduce.py
@@ -9,7 +9,7 @@ class UseReduceTestCase(object):
 	def __init__(self, deparray, uselist=[], masklist=[],
 	             matchall=0, excludeall=[], is_src_uri=False,
 	             eapi='0', opconvert=False, flat=False, expected_result=None,
-	             is_valid_flag=None, token_class=None):
+	             is_valid_flag=None, token_class=None, subset=None):
 		self.deparray = deparray
 		self.uselist = uselist
 		self.masklist = masklist
@@ -21,13 +21,15 @@ class UseReduceTestCase(object):
 		self.flat = flat
 		self.is_valid_flag = is_valid_flag
 		self.token_class = token_class
+		self.subset = subset
 		self.expected_result = expected_result
 
 	def run(self):
 		try:
 			return use_reduce(self.deparray, self.uselist, self.masklist,
 				self.matchall, self.excludeall, self.is_src_uri, self.eapi,
-				self.opconvert, self.flat, self.is_valid_flag, self.token_class)
+				self.opconvert, self.flat, self.is_valid_flag, self.token_class,
+				subset=self.subset)
 		except InvalidDependString as e:
 			raise InvalidDependString("%s: %s" % (e, self.deparray))
 
@@ -50,6 +52,30 @@ class UseReduce(TestCase):
 				uselist=["a", "b", "c", "d"],
 				expected_result=["A", "B"]
 				),
+			UseReduceTestCase(
+				"a? ( A ) b? ( B ) !c? ( C ) !d? ( D )",
+				uselist=["a", "b", "c", "d"],
+				subset=["b"],
+				expected_result=["B"]
+				),
+			UseReduceTestCase(
+				"bar? ( || ( foo bar? ( baz ) ) )",
+				uselist=["bar"],
+				subset=["bar"],
+				expected_result=['||', ['foo', 'baz']]
+				),
+			UseReduceTestCase(
+				"bar? ( foo bar? ( baz ) foo )",
+				uselist=["bar"],
+				subset=["bar"],
+				expected_result=['foo', 'baz', 'foo']
+				),
+			UseReduceTestCase(
+				"|| ( foo bar? ( baz ) )",
+				uselist=["bar"],
+				subset=["bar"],
+				expected_result=["baz"]
+				),
 			UseReduceTestCase(
 				"a? ( A ) b? ( B ) !c? ( C ) !d? ( D )",
 				uselist=["a", "b", "c"],
diff --git a/lib/portage/tests/resolver/test_with_test_deps.py b/lib/portage/tests/resolver/test_with_test_deps.py
index 5bfc6a8a2..d88e3cb6e 100644
--- a/lib/portage/tests/resolver/test_with_test_deps.py
+++ b/lib/portage/tests/resolver/test_with_test_deps.py
@@ -21,7 +21,27 @@ class WithTestDepsTestCase(TestCase):
 			},
 			"app-misc/C-0": {
 				"EAPI": "5",
-			}
+			},
+			"app-misc/D-0": {
+				"EAPI": "5",
+				"IUSE": "test",
+				"DEPEND": "test? ( app-misc/E )"
+			},
+			"app-misc/E-0": {
+				"EAPI": "5",
+				"IUSE": "test",
+				"DEPEND": "test? ( app-misc/D )"
+			},
+			"app-misc/F-0": {
+				"EAPI": "5",
+				"IUSE": "+test",
+				"DEPEND": "test? ( app-misc/G )"
+			},
+			"app-misc/G-0": {
+				"EAPI": "5",
+				"IUSE": "+test",
+				"DEPEND": "test? ( app-misc/F )"
+			},
 		}
 
 		test_cases = (
@@ -32,6 +52,23 @@ class WithTestDepsTestCase(TestCase):
 				success = True,
 				options = { "--onlydeps": True, "--with-test-deps": True },
 				mergelist = ["app-misc/B-0"]),
+
+			# Test that --with-test-deps allows circular dependencies.
+			ResolverPlaygroundTestCase(
+				['app-misc/D'],
+				success = True,
+				options = {'--with-test-deps': True},
+				mergelist = [('app-misc/D-0', 'app-misc/E-0')],
+				ambiguous_merge_order=True),
+
+			# Test that --with-test-deps does not allow circular dependencies
+			# when USE=test is explicitly enabled.
+			ResolverPlaygroundTestCase(
+				['app-misc/F'],
+				success = False,
+				options = {'--with-test-deps': True},
+				circular_dependency_solutions = {'app-misc/G-0': {frozenset({('test', False)})}, 'app-misc/F-0': {frozenset({('test', False)})}},
+			)
 		)
 
 		playground = ResolverPlayground(ebuilds=ebuilds, debug=False)
-- 
2.21.0