Fix use_reduce() to correctly handle "|| ( ( A B ) C )", and also

fix some test cases that had erroneous expected_result values.

2 files changed, 33 insertions, 7 deletions

diff --git a/pym/portage/dep/__init__.py b/pym/portage/dep/__init__.py
index 84cc57526..643bc56d7 100644
--- a/pym/portage/dep/__init__.py
+++ b/pym/portage/dep/__init__.py
@@ -372,9 +372,12 @@ def use_reduce(depstr, uselist=[], masklist=[], matchall=False, excludeall=[], i
 				if l and not ignore:
 					#The current list is not empty and we don't want to ignore it because
 					#of an inactive use conditional.
-					if not stack[level] or stack[level][-1] != "||":
+					if not (level>0 and stack[level-1] and stack[level-1][-1] == "||") \
+						and (not stack[level] or stack[level][-1] != "||"):
 						#Optimize: ( ( ... ) ) -> ( ... )
 						stack[level].extend(l)
+					elif not stack[level]:
+						stack[level].append(l)
 					elif len(l) == 1 and stack[level][-1] == "||":
 						#Optimize: || ( A ) -> A
 						stack[level].pop()
@@ -391,6 +394,18 @@ def use_reduce(depstr, uselist=[], masklist=[], matchall=False, excludeall=[], i
 							stack[level].append(["||"] + l)
 						else:
 							stack[level].append(l)
+
+				if level > 0 and stack[level-1] and stack[level-1][-1] == "||":
+					all_singles = True
+					for x in stack[level]:
+						if isinstance(x, list) and len(x) > 1:
+							all_singles =  False
+							break
+					if all_singles:
+						for i, x in enumerate(stack[level]):
+							if isinstance(x, list):
+								stack[level][i] = x[0]
+
 			else:
 				raise portage.exception.InvalidDependString(
 					_("no matching '%s' for '%s' in '%s', token %s") % ("(", ")", depstr, pos+1))
@@ -453,6 +468,12 @@ def use_reduce(depstr, uselist=[], masklist=[], matchall=False, excludeall=[], i
 		raise portage.exception.InvalidDependString(
 			_("Missing file name at end of string: '%s'") % (depstr,))
 
+	if len(stack[0]) == 1 and isinstance(stack[0][0], list):
+		if opconvert and stack[0][0] and stack[0][0][0] == "||":
+			pass
+		else:
+			stack[0] = stack[0][0]
+
 	return stack[0]
 
 def dep_opconvert(deplist):
diff --git a/pym/portage/tests/dep/test_use_reduce.py b/pym/portage/tests/dep/test_use_reduce.py
index 3d89dbebd..4cb778a5c 100644
--- a/pym/portage/tests/dep/test_use_reduce.py
+++ b/pym/portage/tests/dep/test_use_reduce.py
@@ -169,9 +169,9 @@ class UseReduce(TestCase):
 			UseReduceTestCase(
 				"|| ( A B )",
 				expected_result = [ "||", ["A", "B"] ]),
-			#UseReduceTestCase(
-			#	"|| ( ( A B ) C )",
-			#	expected_result = [ "||", [ ["A", "B"], "C"] ]),
+			UseReduceTestCase(
+				"|| ( ( A B ) C )",
+				expected_result = [ "||", [ ["A", "B"], "C"] ]),
 			UseReduceTestCase(
 				"|| ( A || ( B C ) )",
 				expected_result = [ "||", ["A", "||", ["B", "C"]]]),
@@ -183,7 +183,7 @@ class UseReduce(TestCase):
 				expected_result = [ "||", ["A", "||", ["B", "||", ["C", "D"], "E"]] ]),
 			UseReduceTestCase(
 				"( || ( ( ( A ) B ) ) )",
-				expected_result = [ "||", ["A", "B"] ] ),
+				expected_result = ["A", "B"] ),
 			UseReduceTestCase(
 				"( || ( || ( ( A ) B ) ) )",
 				expected_result = [ "||", ["A", "B"] ]),
@@ -285,7 +285,7 @@ class UseReduce(TestCase):
 			UseReduceTestCase(
 				"( || ( ( ( A ) B ) ) )",
 				opconvert = True,
-				expected_result = [ ["||", "A", "B"] ] ),
+				expected_result = [ "A", "B" ] ),
 			UseReduceTestCase(
 				"( || ( || ( ( A ) B ) ) )",
 				opconvert = True,
@@ -496,7 +496,12 @@ class UseReduce(TestCase):
 		)
 
 		for test_case in test_cases:
-			self.assertEqual(test_case.run(), test_case.expected_result)
+			# If it fails then show the input, since lots of our
+			# test cases have the same output but different input,
+			# making it difficult deduce which test has failed.
+			self.assertEqual(test_case.run(), test_case.expected_result,
+				"input: '%s' result: %s != %s" % (test_case.deparray,
+				test_case.run(), test_case.expected_result))
 
 		for test_case in test_cases_xfail:
 			self.assertRaisesMsg(test_case.deparray, (InvalidDependString, ValueError), test_case.run)