[gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[SebastianLuther]

From: Sebastian Luther <SebastianLuther@gmx.de>

This time rebuilds are scheduled properly, but we
might still forget to install the package that caused
the rebuild.

URL: https://bugs.gentoo.org/486580
---
 pym/_emerge/depgraph.py                            | 30 ++++++++++-
 .../tests/resolver/test_slot_conflict_rebuild.py   | 63 ++++++++++++++++++++++
 2 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index da2e604..0f8f76d 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -2356,6 +2356,18 @@ class depgraph(object):
 			# discarded here. Try to discard as few as possible since
 			# discarded dependencies reduce the amount of information
 			# available for optimization of merge order.
+			# Don't ignore dependencies if pkg as a slot operator dependency on the child
+			# and the child has changed slot/sub_slot
+			slot_operator_rebuild = False
+			if atom.slot_operator == '=' and \
+				(pkg.root, pkg.slot_atom) in self._dynamic_config._slot_operator_replace_installed and \
+				mypriority.satisfied and \
+				mypriority.satisfied is not child and \
+				mypriority.satisfied.installed and \
+				not child.installed and \
+				(child.slot != mypriority.satisfied.slot or child.sub_slot != mypriority.satisfied.sub_slot):
+				slot_operator_rebuild = True
+
 			ignored = False
 			if not atom.blocker and \
 				not recurse_satisfied and \
@@ -2364,7 +2376,8 @@ class depgraph(object):
 				dep.child is not None and \
 				not dep.child.installed and \
 				self._dynamic_config._slot_pkg_map[dep.child.root].get(
-				dep.child.slot_atom) is None:
+				dep.child.slot_atom) is None and \
+				not slot_operator_rebuild:
 				myarg = None
 				try:
 					myarg = next(self._iter_atoms_for_pkg(dep.child), None)
@@ -2466,6 +2479,18 @@ class depgraph(object):
 					parent=virt_pkg, priority=mypriority, root=dep_root,
 					collapsed_parent=pkg, collapsed_priority=dep_priority)
 
+				# Don't ignore dependencies if pkg as a slot operator dependency on the child
+				# and the child has changed slot/sub_slot
+				slot_operator_rebuild = False
+				if atom.slot_operator == '=' and \
+					(pkg.root, pkg.slot_atom) in self._dynamic_config._slot_operator_replace_installed and \
+					mypriority.satisfied and \
+					mypriority.satisfied is not child and \
+					mypriority.satisfied.installed and \
+					not child.installed and \
+					(child.slot != mypriority.satisfied.slot or child.sub_slot != mypriority.satisfied.sub_slot):
+					slot_operator_rebuild = True
+
 				ignored = False
 				if not atom.blocker and \
 					not recurse_satisfied and \
@@ -2474,7 +2499,8 @@ class depgraph(object):
 					dep.child is not None and \
 					not dep.child.installed and \
 					self._dynamic_config._slot_pkg_map[dep.child.root].get(
-					dep.child.slot_atom) is None:
+					dep.child.slot_atom) is None and \
+					not slot_operator_rebuild:
 					myarg = None
 					try:
 						myarg = next(self._iter_atoms_for_pkg(dep.child), None)
diff --git a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
index 74f5cc1..e3c517d 100644
--- a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
+++ b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
@@ -181,6 +181,69 @@ class SlotConflictRebuildTestCase(TestCase):
 		finally:
 			playground.cleanup()
 
+	def testSlotConflictForgottenChild(self):
+		"""
+		Similar to testSlotConflictMassRebuild above, but this time the rebuilds are scheduled,
+		but the package causing the rebuild (the child) is not installed.
+		"""
+		ebuilds = {
+
+			"app-misc/A-2" : {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:= app-misc/C",
+				"RDEPEND": "app-misc/B:= app-misc/C",
+			},
+
+			"app-misc/B-2" : {
+				"EAPI": "5",
+				"SLOT": "2"
+			},
+
+			"app-misc/C-1": {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:=",
+				"RDEPEND": "app-misc/B:="
+			},
+		}
+
+		installed = {
+			"app-misc/A-1" : {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:1/1= app-misc/C",
+				"RDEPEND": "app-misc/B:1/1= app-misc/C",
+			},
+
+			"app-misc/B-1" : {
+				"EAPI": "5",
+				"SLOT": "1"
+			},
+
+			"app-misc/C-1": {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:1/1=",
+				"RDEPEND": "app-misc/B:1/1="
+			},
+		}
+
+		test_cases = (
+			ResolverPlaygroundTestCase(
+				["app-misc/A"],
+				success = True,
+				mergelist = ['app-misc/B-2', 'app-misc/C-1', 'app-misc/A-2']),
+		)
+
+		world = []
+
+		playground = ResolverPlayground(ebuilds=ebuilds,
+			installed=installed, world=world, debug=False)
+		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.cleanup()
+
+
 	def testSlotConflictDepChange(self):
 		"""
 		Bug 490362
-- 
1.8.1.5

[gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[SebastianLuther]

From: Sebastian Luther <SebastianLuther@gmx.de>

This time rebuilds are scheduled properly, but we
might still forget to install the package that caused
the rebuild.

URL: https://bugs.gentoo.org/486580
---
 pym/_emerge/depgraph.py                            | 48 +++++++++++------
 .../tests/resolver/test_slot_conflict_rebuild.py   | 63 ++++++++++++++++++++++
 2 files changed, 95 insertions(+), 16 deletions(-)

diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index da2e604..0a998b5 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -2268,6 +2268,36 @@ class depgraph(object):
 		finally:
 			self._dynamic_config._autounmask = _autounmask_backup
 
+	def _ignore_dependency(self, atom, pkg, child, dep, mypriority, recurse_satisfied):
+		"""
+		In some cases, dep_check will return deps that shouldn't
+		be proccessed any further, so they are identified and
+		discarded here. Try to discard as few as possible since
+		discarded dependencies reduce the amount of information
+		available for optimization of merge order.
+		Don't ignore dependencies if pkg has a slot operator dependency on the child
+		and the child has changed slot/sub_slot.
+		"""
+		slot_operator_rebuild = False
+		if atom.slot_operator == '=' and \
+			(pkg.root, pkg.slot_atom) in self._dynamic_config._slot_operator_replace_installed and \
+			mypriority.satisfied and \
+			mypriority.satisfied is not child and \
+			mypriority.satisfied.installed and \
+			not child.installed and \
+			(child.slot != mypriority.satisfied.slot or child.sub_slot != mypriority.satisfied.sub_slot):
+			slot_operator_rebuild = True
+
+		return not atom.blocker and \
+			not recurse_satisfied and \
+			mypriority.satisfied and \
+			mypriority.satisfied.visible and \
+			dep.child is not None and \
+			not dep.child.installed and \
+			self._dynamic_config._slot_pkg_map[dep.child.root].get(
+			dep.child.slot_atom) is None and \
+			not slot_operator_rebuild
+
 	def _wrapped_add_pkg_dep_string(self, pkg, dep_root, dep_priority,
 		dep_string, allow_unsatisfied):
 		depth = pkg.depth + 1
@@ -2357,14 +2387,7 @@ class depgraph(object):
 			# discarded dependencies reduce the amount of information
 			# available for optimization of merge order.
 			ignored = False
-			if not atom.blocker and \
-				not recurse_satisfied and \
-				mypriority.satisfied and \
-				mypriority.satisfied.visible and \
-				dep.child is not None and \
-				not dep.child.installed and \
-				self._dynamic_config._slot_pkg_map[dep.child.root].get(
-				dep.child.slot_atom) is None:
+			if self._ignore_dependency(atom, pkg, child, dep, mypriority, recurse_satisfied):
 				myarg = None
 				try:
 					myarg = next(self._iter_atoms_for_pkg(dep.child), None)
@@ -2467,14 +2490,7 @@ class depgraph(object):
 					collapsed_parent=pkg, collapsed_priority=dep_priority)
 
 				ignored = False
-				if not atom.blocker and \
-					not recurse_satisfied and \
-					mypriority.satisfied and \
-					mypriority.satisfied.visible and \
-					dep.child is not None and \
-					not dep.child.installed and \
-					self._dynamic_config._slot_pkg_map[dep.child.root].get(
-					dep.child.slot_atom) is None:
+				if self._ignore_dependency(atom, pkg, child, dep, mypriority, recurse_satisfied):
 					myarg = None
 					try:
 						myarg = next(self._iter_atoms_for_pkg(dep.child), None)
diff --git a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
index 74f5cc1..e3c517d 100644
--- a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
+++ b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
@@ -181,6 +181,69 @@ class SlotConflictRebuildTestCase(TestCase):
 		finally:
 			playground.cleanup()
 
+	def testSlotConflictForgottenChild(self):
+		"""
+		Similar to testSlotConflictMassRebuild above, but this time the rebuilds are scheduled,
+		but the package causing the rebuild (the child) is not installed.
+		"""
+		ebuilds = {
+
+			"app-misc/A-2" : {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:= app-misc/C",
+				"RDEPEND": "app-misc/B:= app-misc/C",
+			},
+
+			"app-misc/B-2" : {
+				"EAPI": "5",
+				"SLOT": "2"
+			},
+
+			"app-misc/C-1": {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:=",
+				"RDEPEND": "app-misc/B:="
+			},
+		}
+
+		installed = {
+			"app-misc/A-1" : {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:1/1= app-misc/C",
+				"RDEPEND": "app-misc/B:1/1= app-misc/C",
+			},
+
+			"app-misc/B-1" : {
+				"EAPI": "5",
+				"SLOT": "1"
+			},
+
+			"app-misc/C-1": {
+				"EAPI": "5",
+				"DEPEND": "app-misc/B:1/1=",
+				"RDEPEND": "app-misc/B:1/1="
+			},
+		}
+
+		test_cases = (
+			ResolverPlaygroundTestCase(
+				["app-misc/A"],
+				success = True,
+				mergelist = ['app-misc/B-2', 'app-misc/C-1', 'app-misc/A-2']),
+		)
+
+		world = []
+
+		playground = ResolverPlayground(ebuilds=ebuilds,
+			installed=installed, world=world, debug=False)
+		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.cleanup()
+
+
 	def testSlotConflictDepChange(self):
 		"""
 		Bug 490362
-- 
1.8.1.5

Re: [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[Sebastian Luther]

Please ignore this one.

I reorganized these large duplicated if's as suggested by dol-sen on IRC
in a new version of this patch.

Re: [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 6425 bytes --]

I like this one much better, thank you.

Could you rebase the 2 patches together, tis one shows removing lines
from the first.  Also see inline comments below :)

On Thu, 2013-11-28 at 11:34 +0100, SebastianLuther@gmx.de wrote:
> From: Sebastian Luther <SebastianLuther@gmx.de>
> 
> This time rebuilds are scheduled properly, but we
> might still forget to install the package that caused
> the rebuild.
> 
> URL: https://bugs.gentoo.org/486580
> ---
>  pym/_emerge/depgraph.py                            | 48 +++++++++++------
>  .../tests/resolver/test_slot_conflict_rebuild.py   | 63 ++++++++++++++++++++++
>  2 files changed, 95 insertions(+), 16 deletions(-)
> 
> diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
> index da2e604..0a998b5 100644
> --- a/pym/_emerge/depgraph.py
> +++ b/pym/_emerge/depgraph.py
> @@ -2268,6 +2268,36 @@ class depgraph(object):
>  		finally:
>  			self._dynamic_config._autounmask = _autounmask_backup
>  
> +	def _ignore_dependency(self, atom, pkg, child, dep, mypriority, recurse_satisfied):
> +		"""
> +		In some cases, dep_check will return deps that shouldn't
> +		be proccessed any further, so they are identified and
> +		discarded here. Try to discard as few as possible since
> +		discarded dependencies reduce the amount of information
> +		available for optimization of merge order.
> +		Don't ignore dependencies if pkg has a slot operator dependency on the child
> +		and the child has changed slot/sub_slot.
> +		"""
> +		slot_operator_rebuild = False
> +		if atom.slot_operator == '=' and \
> +			(pkg.root, pkg.slot_atom) in self._dynamic_config._slot_operator_replace_installed and \
> +			mypriority.satisfied and \
> +			mypriority.satisfied is not child and \
> +			mypriority.satisfied.installed and \
> +			not child.installed and \
> +			(child.slot != mypriority.satisfied.slot or child.sub_slot != mypriority.satisfied.sub_slot):
> +			slot_operator_rebuild = True
> +
> +		return not atom.blocker and \
> +			not recurse_satisfied and \
> +			mypriority.satisfied and \
> +			mypriority.satisfied.visible and \
> +			dep.child is not None and \
> +			not dep.child.installed and \
> +			self._dynamic_config._slot_pkg_map[dep.child.root].get(
> +			dep.child.slot_atom) is None and \
> +			not slot_operator_rebuild
> +

Since slot_operator_rebuild is already set either true or false, would
it be better if "not slot_operator_rebuild" is first in the return
statement.  If it resolves False, it will be an early out for the
remaining checks.  Personally I don't know which is the more common
case.  But if it resolve False more often, it will be a slight speed
improvement.  That goes for the rest of them too, they should be ordered
in the most often to resolve False order.  Same for the if as well.

/end tiny nitpick 

>  	def _wrapped_add_pkg_dep_string(self, pkg, dep_root, dep_priority,
>  		dep_string, allow_unsatisfied):
>  		depth = pkg.depth + 1
> @@ -2357,14 +2387,7 @@ class depgraph(object):
>  			# discarded dependencies reduce the amount of information
>  			# available for optimization of merge order.
>  			ignored = False
> -			if not atom.blocker and \
> -				not recurse_satisfied and \
> -				mypriority.satisfied and \
> -				mypriority.satisfied.visible and \
> -				dep.child is not None and \
> -				not dep.child.installed and \
> -				self._dynamic_config._slot_pkg_map[dep.child.root].get(
> -				dep.child.slot_atom) is None:
> +			if self._ignore_dependency(atom, pkg, child, dep, mypriority, recurse_satisfied):
>  				myarg = None
>  				try:
>  					myarg = next(self._iter_atoms_for_pkg(dep.child), None)
> @@ -2467,14 +2490,7 @@ class depgraph(object):
>  					collapsed_parent=pkg, collapsed_priority=dep_priority)
>  
>  				ignored = False
> -				if not atom.blocker and \
> -					not recurse_satisfied and \
> -					mypriority.satisfied and \
> -					mypriority.satisfied.visible and \
> -					dep.child is not None and \
> -					not dep.child.installed and \
> -					self._dynamic_config._slot_pkg_map[dep.child.root].get(
> -					dep.child.slot_atom) is None:
> +				if self._ignore_dependency(atom, pkg, child, dep, mypriority, recurse_satisfied):
>  					myarg = None
>  					try:
>  						myarg = next(self._iter_atoms_for_pkg(dep.child), None)
> diff --git a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
> index 74f5cc1..e3c517d 100644
> --- a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
> +++ b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
> @@ -181,6 +181,69 @@ class SlotConflictRebuildTestCase(TestCase):
>  		finally:
>  			playground.cleanup()
>  
> +	def testSlotConflictForgottenChild(self):
> +		"""
> +		Similar to testSlotConflictMassRebuild above, but this time the rebuilds are scheduled,
> +		but the package causing the rebuild (the child) is not installed.
> +		"""
> +		ebuilds = {
> +
> +			"app-misc/A-2" : {
> +				"EAPI": "5",
> +				"DEPEND": "app-misc/B:= app-misc/C",
> +				"RDEPEND": "app-misc/B:= app-misc/C",
> +			},
> +
> +			"app-misc/B-2" : {
> +				"EAPI": "5",
> +				"SLOT": "2"
> +			},
> +
> +			"app-misc/C-1": {
> +				"EAPI": "5",
> +				"DEPEND": "app-misc/B:=",
> +				"RDEPEND": "app-misc/B:="
> +			},
> +		}
> +
> +		installed = {
> +			"app-misc/A-1" : {
> +				"EAPI": "5",
> +				"DEPEND": "app-misc/B:1/1= app-misc/C",
> +				"RDEPEND": "app-misc/B:1/1= app-misc/C",
> +			},
> +
> +			"app-misc/B-1" : {
> +				"EAPI": "5",
> +				"SLOT": "1"
> +			},
> +
> +			"app-misc/C-1": {
> +				"EAPI": "5",
> +				"DEPEND": "app-misc/B:1/1=",
> +				"RDEPEND": "app-misc/B:1/1="
> +			},
> +		}
> +
> +		test_cases = (
> +			ResolverPlaygroundTestCase(
> +				["app-misc/A"],
> +				success = True,
> +				mergelist = ['app-misc/B-2', 'app-misc/C-1', 'app-misc/A-2']),
> +		)
> +
> +		world = []
> +
> +		playground = ResolverPlayground(ebuilds=ebuilds,
> +			installed=installed, world=world, debug=False)
> +		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.cleanup()
> +
> +
>  	def testSlotConflictDepChange(self):
>  		"""
>  		Bug 490362


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[Sebastian Luther]

Am 28.11.2013 15:47, schrieb Brian Dolbec:
> I like this one much better, thank you.
> 
> Could you rebase the 2 patches together, tis one shows removing
> lines from the first.  Also see inline comments below :)

The removed lines are another large if-block that already existed.
This has been moved into _ignore_dependency() too.

> 
> On Thu, 2013-11-28 at 11:34 +0100, SebastianLuther@gmx.de wrote:
>> From: Sebastian Luther <SebastianLuther@gmx.de>
>> 
>> This time rebuilds are scheduled properly, but we might still
>> forget to install the package that caused the rebuild.
>> 
>> URL: https://bugs.gentoo.org/486580 --- pym/_emerge/depgraph.py
>> | 48 +++++++++++------ 
>> .../tests/resolver/test_slot_conflict_rebuild.py   | 63
>> ++++++++++++++++++++++ 2 files changed, 95 insertions(+), 16
>> deletions(-)
>> 
>> diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py 
>> index da2e604..0a998b5 100644 --- a/pym/_emerge/depgraph.py +++
>> b/pym/_emerge/depgraph.py @@ -2268,6 +2268,36 @@ class
>> depgraph(object): finally: self._dynamic_config._autounmask =
>> _autounmask_backup
>> 
>> +	def _ignore_dependency(self, atom, pkg, child, dep, mypriority,
>> recurse_satisfied): +		""" +		In some cases, dep_check will
>> return deps that shouldn't +		be proccessed any further, so they
>> are identified and +		discarded here. Try to discard as few as
>> possible since +		discarded dependencies reduce the amount of
>> information +		available for optimization of merge order. +
>> Don't ignore dependencies if pkg has a slot operator dependency
>> on the child +		and the child has changed slot/sub_slot. +		""" +
>> slot_operator_rebuild = False +		if atom.slot_operator == '=' and
>> \ +			(pkg.root, pkg.slot_atom) in
>> self._dynamic_config._slot_operator_replace_installed and \ +
>> mypriority.satisfied and \ +			mypriority.satisfied is not child
>> and \ +			mypriority.satisfied.installed and \ +			not
>> child.installed and \ +			(child.slot !=
>> mypriority.satisfied.slot or child.sub_slot !=
>> mypriority.satisfied.sub_slot): +			slot_operator_rebuild = True 
>> + +		return not atom.blocker and \ +			not recurse_satisfied and
>> \ +			mypriority.satisfied and \ +			mypriority.satisfied.visible
>> and \ +			dep.child is not None and \ +			not dep.child.installed
>> and \ +
>> self._dynamic_config._slot_pkg_map[dep.child.root].get( +
>> dep.child.slot_atom) is None and \ +			not slot_operator_rebuild 
>> +
> 
> Since slot_operator_rebuild is already set either true or false,
> would it be better if "not slot_operator_rebuild" is first in the
> return statement.  If it resolves False, it will be an early out
> for the remaining checks.  Personally I don't know which is the
> more common case.  But if it resolve False more often, it will be a
> slight speed improvement.  That goes for the rest of them too, they
> should be ordered in the most often to resolve False order.  Same
> for the if as well.

This should be more often False than True. recurse_satisfied in the
second block should be more often True than False.

In fact there's some duplication between the two blocks. Feel free to
clean this up after it has been pushed.


> 
> /end tiny nitpick
> 
>> def _wrapped_add_pkg_dep_string(self, pkg, dep_root,
>> dep_priority, dep_string, allow_unsatisfied): depth = pkg.depth +
>> 1 @@ -2357,14 +2387,7 @@ class depgraph(object): # discarded
>> dependencies reduce the amount of information # available for
>> optimization of merge order. ignored = False -			if not
>> atom.blocker and \ -				not recurse_satisfied and \ -
>> mypriority.satisfied and \ -				mypriority.satisfied.visible and
>> \ -				dep.child is not None and \ -				not dep.child.installed
>> and \ -
>> self._dynamic_config._slot_pkg_map[dep.child.root].get( -
>> dep.child.slot_atom) is None: +			if
>> self._ignore_dependency(atom, pkg, child, dep, mypriority,
>> recurse_satisfied): myarg = None try: myarg =
>> next(self._iter_atoms_for_pkg(dep.child), None) @@ -2467,14
>> +2490,7 @@ class depgraph(object): collapsed_parent=pkg,
>> collapsed_priority=dep_priority)
>> 
>> ignored = False -				if not atom.blocker and \ -					not
>> recurse_satisfied and \ -					mypriority.satisfied and \ -
>> mypriority.satisfied.visible and \ -					dep.child is not None
>> and \ -					not dep.child.installed and \ -
>> self._dynamic_config._slot_pkg_map[dep.child.root].get( -
>> dep.child.slot_atom) is None: +				if
>> self._ignore_dependency(atom, pkg, child, dep, mypriority,
>> recurse_satisfied): myarg = None try: myarg =
>> next(self._iter_atoms_for_pkg(dep.child), None) diff --git
>> a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py
>> b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py index
>> 74f5cc1..e3c517d 100644 ---
>> a/pym/portage/tests/resolver/test_slot_conflict_rebuild.py +++
>> b/pym/portage/tests/resolver/test_slot_conflict_rebuild.py @@
>> -181,6 +181,69 @@ class SlotConflictRebuildTestCase(TestCase): 
>> finally: playground.cleanup()
>> 
>> +	def testSlotConflictForgottenChild(self): +		""" +		Similar to
>> testSlotConflictMassRebuild above, but this time the rebuilds are
>> scheduled, +		but the package causing the rebuild (the child) is
>> not installed. +		""" +		ebuilds = { + +			"app-misc/A-2" : { +
>> "EAPI": "5", +				"DEPEND": "app-misc/B:= app-misc/C", +
>> "RDEPEND": "app-misc/B:= app-misc/C", +			}, + +			"app-misc/B-2"
>> : { +				"EAPI": "5", +				"SLOT": "2" +			}, + +
>> "app-misc/C-1": { +				"EAPI": "5", +				"DEPEND":
>> "app-misc/B:=", +				"RDEPEND": "app-misc/B:=" +			}, +		} + +
>> installed = { +			"app-misc/A-1" : { +				"EAPI": "5", +
>> "DEPEND": "app-misc/B:1/1= app-misc/C", +				"RDEPEND":
>> "app-misc/B:1/1= app-misc/C", +			}, + +			"app-misc/B-1" : { +
>> "EAPI": "5", +				"SLOT": "1" +			}, + +			"app-misc/C-1": { +
>> "EAPI": "5", +				"DEPEND": "app-misc/B:1/1=", +				"RDEPEND":
>> "app-misc/B:1/1=" +			}, +		} + +		test_cases = ( +
>> ResolverPlaygroundTestCase( +				["app-misc/A"], +				success =
>> True, +				mergelist = ['app-misc/B-2', 'app-misc/C-1',
>> 'app-misc/A-2']), +		) + +		world = [] + +		playground =
>> ResolverPlayground(ebuilds=ebuilds, +			installed=installed,
>> world=world, debug=False) +		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.cleanup() + + def
>> testSlotConflictDepChange(self): """ Bug 490362
> 

Re: [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 1608 bytes --]

On Thu, 2013-11-28 at 20:36 +0100, Sebastian Luther wrote:
> Am 28.11.2013 15:47, schrieb Brian Dolbec:
> > I like this one much better, thank you.
> > 
> > Could you rebase the 2 patches together, this one shows removing
> > lines from the first.  Also see inline comments below :)
> 
> The removed lines are another large if-block that already existed.
> This has been moved into _ignore_dependency() too.
> 

Hey, all the more reason to move it to a function, that would have been
4 copies of the same check otherwise.  Now it can be edited in one place
to change behavior, and do it consistently the same.

> > 
> > On Thu, 2013-11-28 at 11:34 +0100, SebastianLuther@gmx.de wrote:
> >> From: Sebastian Luther <SebastianLuther@gmx.de>
...

> > Since slot_operator_rebuild is already set either true or false,
> > would it be better if "not slot_operator_rebuild" is first in the
> > return statement.  If it resolves False, it will be an early out
> > for the remaining checks.  Personally I don't know which is the
> > more common case.  But if it resolve False more often, it will be a
> > slight speed improvement.  That goes for the rest of them too, they
> > should be ordered in the most often to resolve False order.  Same
> > for the if as well.
> 
> This should be more often False than True. recurse_satisfied in the
> second block should be more often True than False.
> 
> In fact there's some duplication between the two blocks. Feel free to
> clean this up after it has been pushed.
> 

OK, thanks, I'll look to see if it can be optimized a little more.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [gentoo-portage-dev] [PATCH] Another slot operator bug (bug 486580, try 2)

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

On Thu, 2013-11-28 at 17:00 -0800, Brian Dolbec wrote:
> On Thu, 2013-11-28 at 20:36 +0100, Sebastian Luther wrote:
> > 
> > In fact there's some duplication between the two blocks. Feel free to
> > clean this up after it has been pushed.
> > 
> 
> OK, thanks, I'll look to see if it can be optimized a little more.

Applied along with:

The mypriority.satisfied check was common to both the if statement and
the return.  So doing that check first provides an early return if it is
False.  If mypriority.satisfied is True/not None, then it is done only
once and does not affect the final truth table returned.

diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index 622d288..343cc03 100644
@@ -2276,10 +2276,11 @@ class depgraph(object):
 		Don't ignore dependencies if pkg has a slot operator dependency on
the child
 		and the child has changed slot/sub_slot.
 		"""
+		if not mypriority.satisfied:
+			return False
 		slot_operator_rebuild = False
 		if atom.slot_operator == '=' and \
 			(pkg.root, pkg.slot_atom) in
self._dynamic_config._slot_operator_replace_installed and \
-			mypriority.satisfied and \
 			mypriority.satisfied is not child and \
 			mypriority.satisfied.installed and \
 			not child.installed and \
@@ -2288,7 +2289,6 @@ class depgraph(object):
 
 		return not atom.blocker and \
 			not recurse_satisfied and \
-			mypriority.satisfied and \
 			mypriority.satisfied.visible and \
 			dep.child is not None and \
 			not dep.child.installed and \


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 620 bytes --]