[gentoo-portage-dev] [PATCH] depgraph: eval disjunctive build deps earlier (bug 639346)

[Zac Medico]

Since built-time deps tend to be a superset of run-time deps, evaluate
disjunctive build-time deps before run-time deps, so that choices for
build-time deps influence choices for run-time deps.

Also, fix OnlydepsMinimalTestCase to specify ambiguous_merge_order,
since the merge order is affected by the order of evaluation.

Bug: https://bugs.gentoo.org/639346
---
 pym/_emerge/depgraph.py                            | 12 ++-
 .../resolver/test_disjunctive_depend_order.py      | 87 ++++++++++++++++++++++
 .../tests/resolver/test_onlydeps_minimal.py        |  5 +-
 3 files changed, 98 insertions(+), 6 deletions(-)
 create mode 100644 pym/portage/tests/resolver/test_disjunctive_depend_order.py

diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index f54acdc26..6e5ca6508 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -3234,7 +3234,15 @@ class depgraph(object):
 			if ignore_hdepend_deps:
 				edepend["HDEPEND"] = ""
 
+		# Since build-time deps tend to be a superset of run-time deps, order
+		# dep processing such that build-time deps are popped from
+		# _dep_disjunctive_stack first, so that choices for build-time
+		# deps influence choices for run-time deps (bug 639346).
 		deps = (
+			(myroot, edepend["RDEPEND"],
+				self._priority(runtime=True)),
+			(myroot, edepend["PDEPEND"],
+				self._priority(runtime_post=True)),
 			(depend_root, edepend["DEPEND"],
 				self._priority(buildtime=True,
 				optional=(pkg.built or ignore_depend_deps),
@@ -3243,10 +3251,6 @@ class depgraph(object):
 				self._priority(buildtime=True,
 				optional=(pkg.built or ignore_hdepend_deps),
 				ignored=ignore_hdepend_deps)),
-			(myroot, edepend["RDEPEND"],
-				self._priority(runtime=True)),
-			(myroot, edepend["PDEPEND"],
-				self._priority(runtime_post=True))
 		)
 
 		debug = "--debug" in self._frozen_config.myopts
diff --git a/pym/portage/tests/resolver/test_disjunctive_depend_order.py b/pym/portage/tests/resolver/test_disjunctive_depend_order.py
new file mode 100644
index 000000000..88f6dac2d
--- /dev/null
+++ b/pym/portage/tests/resolver/test_disjunctive_depend_order.py
@@ -0,0 +1,87 @@
+# Copyright 2017 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+from portage.tests import TestCase
+from portage.tests.resolver.ResolverPlayground import (
+	ResolverPlayground,
+	ResolverPlaygroundTestCase,
+)
+
+class DisjunctiveDependOrderTestCase(TestCase):
+
+	def testDisjunctiveDependOrderTestCase(self):
+		ebuilds = {
+			'virtual/jre-1.8': {
+				'EAPI': '6',
+				'SLOT' : '1.8',
+				'RDEPEND' : '|| ( dev-java/oracle-jre-bin:1.8 virtual/jdk:1.8 )',
+			},
+			'virtual/jdk-1.8': {
+				'EAPI': '6',
+				'SLOT' : '1.8',
+				'RDEPEND' : '|| ( dev-java/icedtea:8 dev-java/oracle-jdk-bin:1.8 )',
+			},
+			'dev-java/icedtea-3.6': {
+				'SLOT' : '8',
+			},
+			'dev-java/oracle-jdk-bin-1.8': {
+				'SLOT' : '1.8',
+			},
+			'dev-java/oracle-jre-bin-1.8': {
+				'SLOT' : '1.8',
+			},
+			'dev-db/hsqldb-1.8'       : {
+				'DEPEND' : 'virtual/jdk',
+				'RDEPEND' : 'virtual/jre',
+			},
+		}
+
+		binpkgs = {
+			'dev-db/hsqldb-1.8'       : {
+				'DEPEND' : 'virtual/jdk',
+				'RDEPEND' : 'virtual/jre',
+			},
+		}
+
+		test_cases = (
+			# Test bug 639346, where a redundant jre implementation
+			# was pulled in because DEPEND was evaluated after
+			# RDEPEND.
+			ResolverPlaygroundTestCase(
+				['dev-db/hsqldb'],
+				success=True,
+				mergelist=[
+					'dev-java/icedtea-3.6',
+					'virtual/jdk-1.8',
+					'virtual/jre-1.8',
+					'dev-db/hsqldb-1.8',
+				],
+			),
+
+			# The jdk is not needed with --usepkg, so the jre should
+			# be preferred in this case.
+			ResolverPlaygroundTestCase(
+				['dev-db/hsqldb'],
+				options = {
+					'--usepkg': True
+				},
+				success=True,
+				mergelist=[
+					'dev-java/oracle-jre-bin-1.8',
+					'virtual/jre-1.8',
+					'[binary]dev-db/hsqldb-1.8',
+				],
+			),
+		)
+
+		playground = ResolverPlayground(debug=False,
+			binpkgs=binpkgs, ebuilds=ebuilds)
+
+		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.debug = False
+			playground.cleanup()
diff --git a/pym/portage/tests/resolver/test_onlydeps_minimal.py b/pym/portage/tests/resolver/test_onlydeps_minimal.py
index 13c79ed55..efda02c59 100644
--- a/pym/portage/tests/resolver/test_onlydeps_minimal.py
+++ b/pym/portage/tests/resolver/test_onlydeps_minimal.py
@@ -25,9 +25,10 @@ class OnlydepsMinimalTestCase(TestCase):
 				success = True,
 				options = { "--onlydeps": True,
 				            "--onlydeps-with-rdeps": "y" },
-				mergelist = ["dev-libs/B-1",
+				ambiguous_merge_order = True,
+				mergelist = [("dev-libs/B-1",
 				             "dev-libs/C-1",
-				             "dev-libs/D-1"]),
+				             "dev-libs/D-1")]),
 			ResolverPlaygroundTestCase(
 				["dev-libs/A"],
 				all_permutations = True,
-- 
2.13.6