[gentoo-portage-dev] [PATCH] _expand_new_virtuals: constrain output for dep_zapdeps (bug 597752)

[gentoo-portage-dev] [PATCH] _expand_new_virtuals: constrain output for dep_zapdeps (bug 597752)

[Zac Medico]

Constrain _expand_new_virtuals output in order to avoid incorrect
re-ordering of || deps in the dep_zapdeps function, as reported in
bug 597752.

The incorrect dep_zapdeps behavior involved a problem in the
construction of the _dep_choice.cp_map dictionary inside the
dep_zapdeps function, with this input:

|| ( dev-java/icedtea-bin:8 =virtual/jdk-1.8.0-r3 >=virtual/jdk-1.5 )
( dev-java/icedtea-bin:7 =virtual/jdk-1.7.0-r2 >=virtual/jdk-1.5 )

The cp_map for virtual/jdk-1.7.0-r2 erroneously contained
virtual/jdk-1.8.0-r3 because that was the highest virtual/jdk matched
by the >=virtual/jdk-1.5 atom. This patch removes the >=virtual/jdk-1.5
atom from the _expand_new_virtuals output, in order to avoid triggering
the erroneous dep_zapdeps behavior. The >=virtual/jdk-1.5 atom is not
completely discarded, since the depgraph is able to access it via the
virt_atom._orig_atom attribute.

In order to demonstrate that this patch solves the problem, run the
command `emerge ant-core` from inside a stage3 chroot, and see that
virtual/jdk:1.8 is favored over virtual/jdk:1.7.

X-Gentoo-Bug: 597752
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=597752
---
 pym/_emerge/depgraph.py      |  6 +++++-
 pym/portage/dep/dep_check.py | 13 +++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index 26037ad..9161914 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -4453,7 +4453,11 @@ class depgraph(object):
 				# _UNREACHABLE_DEPTH for complete mode.
 				virt_depth = parent.depth
 
-			chosen_atom_ids = frozenset(id(atom) for atom in mycheck[1])
+			chosen_atom_ids = frozenset(chain(
+				(id(atom) for atom in mycheck[1]),
+				(id(atom._orig_atom) for atom in mycheck[1]
+					if hasattr(atom, '_orig_atom')),
+			))
 			selected_atoms = OrderedDict()
 			node_stack = [(parent, None, None)]
 			traversed_nodes = set()
diff --git a/pym/portage/dep/dep_check.py b/pym/portage/dep/dep_check.py
index 9af4e65..9d2ca4b 100644
--- a/pym/portage/dep/dep_check.py
+++ b/pym/portage/dep/dep_check.py
@@ -188,13 +188,14 @@ def _expand_new_virtuals(mysplit, edebug, mydbapi, mysettings, myroot="/",
 				raise ParseError("%s: %s '%s'" % \
 					(pkg, mycheck[1], depstring))
 
-			# Pull in virt_atom which refers to the specific version
-			# of the virtual whose deps we're expanding. Also pull
-			# in the original input atom, so that callers can reliably
-			# check to see if a given input atom has been selected,
-			# as in depgraph._slot_operator_update_probe.
+			# Replace the original atom "x" with "virt_atom" which refers
+			# to the specific version of the virtual whose deps we're
+			# expanding. The virt_atom._orig_atom attribute is used
+			# by depgraph to map virt_atom back to the original atom.
+			# We specifically exclude the original atom "x" from the
+			# the expanded output here, since otherwise it could trigger
+			# incorrect dep_zapdeps behavior (see bug #597752).
 			mycheck[1].append(virt_atom)
-			mycheck[1].append(x)
 			a.append(mycheck[1])
 			if atom_graph is not None:
 				virt_atom_node = (virt_atom, id(virt_atom))
-- 
2.7.4

[gentoo-portage-dev] Re: [PATCH] _expand_new_virtuals: constrain output for dep_zapdeps (bug 597752)

[Zac Medico]

On 10/30/2016 11:15 PM, Zac Medico wrote:
> Constrain _expand_new_virtuals output in order to avoid incorrect
> re-ordering of || deps in the dep_zapdeps function, as reported in
> bug 597752.
> 
> The incorrect dep_zapdeps behavior involved a problem in the
> construction of the _dep_choice.cp_map dictionary inside the
> dep_zapdeps function, with this input:
> 
> || ( dev-java/icedtea-bin:8 =virtual/jdk-1.8.0-r3 >=virtual/jdk-1.5 )
> ( dev-java/icedtea-bin:7 =virtual/jdk-1.7.0-r2 >=virtual/jdk-1.5 )
> 
> The cp_map for virtual/jdk-1.7.0-r2 erroneously contained
> virtual/jdk-1.8.0-r3 because that was the highest virtual/jdk matched
> by the >=virtual/jdk-1.5 atom. This patch removes the >=virtual/jdk-1.5
> atom from the _expand_new_virtuals output, in order to avoid triggering
> the erroneous dep_zapdeps behavior. The >=virtual/jdk-1.5 atom is not
> completely discarded, since the depgraph is able to access it via the
> virt_atom._orig_atom attribute.
> 
> In order to demonstrate that this patch solves the problem, run the
> command `emerge ant-core` from inside a stage3 chroot, and see that
> virtual/jdk:1.8 is favored over virtual/jdk:1.7.
> 
> X-Gentoo-Bug: 597752
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=597752

Any feedback?
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] Re: [PATCH] _expand_new_virtuals: constrain output for dep_zapdeps (bug 597752)

[Brian Dolbec]

On Thu, 3 Nov 2016 13:34:22 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> On 10/30/2016 11:15 PM, Zac Medico wrote:
> > Constrain _expand_new_virtuals output in order to avoid incorrect
> > re-ordering of || deps in the dep_zapdeps function, as reported in
> > bug 597752.
> > 
> > The incorrect dep_zapdeps behavior involved a problem in the
> > construction of the _dep_choice.cp_map dictionary inside the
> > dep_zapdeps function, with this input:
> > 
> > || ( dev-java/icedtea-bin:8 =virtual/jdk-1.8.0-r3 >=virtual/jdk-1.5
> > ) ( dev-java/icedtea-bin:7 =virtual/jdk-1.7.0-r2 >=virtual/jdk-1.5 )
> > 
> > The cp_map for virtual/jdk-1.7.0-r2 erroneously contained
> > virtual/jdk-1.8.0-r3 because that was the highest virtual/jdk
> > matched by the >=virtual/jdk-1.5 atom. This patch removes the
> > >=virtual/jdk-1.5 atom from the _expand_new_virtuals output, in
> > >order to avoid triggering
> > the erroneous dep_zapdeps behavior. The >=virtual/jdk-1.5 atom is
> > not completely discarded, since the depgraph is able to access it
> > via the virt_atom._orig_atom attribute.
> > 
> > In order to demonstrate that this patch solves the problem, run the
> > command `emerge ant-core` from inside a stage3 chroot, and see that
> > virtual/jdk:1.8 is favored over virtual/jdk:1.7.
> > 
> > X-Gentoo-Bug: 597752
> > X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=597752  
> 
> Any feedback?


Sorry, I was behind on emails from the weekend.

This stuff is dabbling into the dep calc voodo ;)  But I ddin't see any
typo's :)   I trust you got it right.
-- 
Brian Dolbec <dolsen>

Re: [gentoo-portage-dev] Re: [PATCH] _expand_new_virtuals: constrain output for dep_zapdeps (bug 597752)

[Zac Medico]

On 11/03/2016 03:46 PM, Brian Dolbec wrote:
> On Thu, 3 Nov 2016 13:34:22 -0700
> Zac Medico <zmedico@gentoo.org> wrote:
> 
>> On 10/30/2016 11:15 PM, Zac Medico wrote:
>>> Constrain _expand_new_virtuals output in order to avoid incorrect
>>> re-ordering of || deps in the dep_zapdeps function, as reported in
>>> bug 597752.
>>>
>>> The incorrect dep_zapdeps behavior involved a problem in the
>>> construction of the _dep_choice.cp_map dictionary inside the
>>> dep_zapdeps function, with this input:
>>>
>>> || ( dev-java/icedtea-bin:8 =virtual/jdk-1.8.0-r3 >=virtual/jdk-1.5
>>> ) ( dev-java/icedtea-bin:7 =virtual/jdk-1.7.0-r2 >=virtual/jdk-1.5 )
>>>
>>> The cp_map for virtual/jdk-1.7.0-r2 erroneously contained
>>> virtual/jdk-1.8.0-r3 because that was the highest virtual/jdk
>>> matched by the >=virtual/jdk-1.5 atom. This patch removes the
>>>> =virtual/jdk-1.5 atom from the _expand_new_virtuals output, in
>>>> order to avoid triggering
>>> the erroneous dep_zapdeps behavior. The >=virtual/jdk-1.5 atom is
>>> not completely discarded, since the depgraph is able to access it
>>> via the virt_atom._orig_atom attribute.
>>>
>>> In order to demonstrate that this patch solves the problem, run the
>>> command `emerge ant-core` from inside a stage3 chroot, and see that
>>> virtual/jdk:1.8 is favored over virtual/jdk:1.7.
>>>
>>> X-Gentoo-Bug: 597752
>>> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=597752  
>>
>> Any feedback?
> 
> 
> Sorry, I was behind on emails from the weekend.
> 
> This stuff is dabbling into the dep calc voodo ;)  But I ddin't see any
> typo's :)   I trust you got it right.
> 

Thanks, pushed:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=cd864267a463769cbd40e058611d2488cc15bf70
-- 
Thanks,
Zac