[gentoo-portage-dev] [PATCH] emerge: auto-enable --with-bdeps if --usepkg is not enabled (bug 598444)

[Zac Medico]

It's useful to enable --with-bdeps by default so that @world updates
will update all packages that are not eligible for removal by
emerge --depclean. However, many users of binary packages do not want
unnecessary build time dependencies installed, therefore do not
auto-enable --with-bdeps for installation actions when the --usepkg
option is enabled.

X-Gentoo-bug: 598444
X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=598444
---
 man/emerge.1                                     |  18 ++-
 pym/_emerge/create_depgraph_params.py            |   4 +
 pym/_emerge/depgraph.py                          |   4 +-
 pym/portage/tests/resolver/ResolverPlayground.py |   5 +
 pym/portage/tests/resolver/test_bdeps.py         | 197 +++++++++++++++++++++++
 5 files changed, 223 insertions(+), 5 deletions(-)
 create mode 100644 pym/portage/tests/resolver/test_bdeps.py

diff --git a/man/emerge.1 b/man/emerge.1
index 5b61220..970bb5b 100644
--- a/man/emerge.1
+++ b/man/emerge.1
@@ -986,9 +986,21 @@ The default is set to "y" (on).
 .TP
 .BR "\-\-with\-bdeps < y | n >"
 In dependency calculations, pull in build time dependencies
-that are not strictly required. This defaults to \'n\' for
-installation actions, meaning they will not be installed, and
-\'y\' for the \fB\-\-depclean\fR action, meaning they will not be removed.
+that are not strictly required. This option is enabled automatically for
+installation actions, meaning they will be installed, and is also enabled
+automatically for the \fB\-\-depclean\fR action, meaning they will not be
+removed. In order to prevent this option from being enabled automatically,
+\fB\-\-with-bdeps=n\fR must be specified either in the command line or
+via \fBEMERGE_DEFAULT_OPTS\fR.
+
+Since many users of binary packages do not want unnecessary build time
+dependencies installed, this option is not enabled automatically for
+installation actions when the \fB\-\-usepkg\fR option is enabled. In
+order to pull in build time dependencies for binary packages with
+\fB\-\-usepkg\fR, \fB\-\-with-bdeps=y\fR must be specified explicitly.
+This also applies to options that enable the \fB\-\-usepkg\fR option
+implicitly, such as \fB\-\-getbinpkg\fR.
+
 This setting can be added to
 \fBEMERGE_DEFAULT_OPTS\fR (see make.conf(5)) and later overridden via the
 command line.
diff --git a/pym/_emerge/create_depgraph_params.py b/pym/_emerge/create_depgraph_params.py
index 2c64928..676aac5 100644
--- a/pym/_emerge/create_depgraph_params.py
+++ b/pym/_emerge/create_depgraph_params.py
@@ -13,6 +13,8 @@ def create_depgraph_params(myopts, myaction):
 	# deep:      go into the dependencies of already merged packages
 	# empty:     pretend nothing is merged
 	# complete:  completely account for all known dependencies
+	# bdeps:     satisfy build time dependencies of packages that are
+	#   already built, even though they are not strictly required
 	# remove:    build graph for use in removing packages
 	# rebuilt_binaries: replace installed packages with rebuilt binaries
 	# rebuild_if_new_slot: rebuild or reinstall packages when
@@ -32,6 +34,8 @@ def create_depgraph_params(myopts, myaction):
 	bdeps = myopts.get("--with-bdeps")
 	if bdeps is not None:
 		myparams["bdeps"] = bdeps
+	elif myaction == "remove" or "--usepkg" not in myopts:
+		myparams["bdeps"] = "auto"
 
 	ignore_built_slot_operator_deps = myopts.get("--ignore-built-slot-operator-deps")
 	if ignore_built_slot_operator_deps is not None:
diff --git a/pym/_emerge/depgraph.py b/pym/_emerge/depgraph.py
index ce0fde1..02a3226 100644
--- a/pym/_emerge/depgraph.py
+++ b/pym/_emerge/depgraph.py
@@ -2362,7 +2362,7 @@ class depgraph(object):
 		if ebuild is None:
 			changed = False
 		else:
-			if self._dynamic_config.myparams.get("bdeps", "n") == "y":
+			if self._dynamic_config.myparams.get("bdeps") in ("y", "auto"):
 				depvars = Package._dep_keys
 			else:
 				depvars = Package._runtime_keys
@@ -2998,7 +2998,7 @@ class depgraph(object):
 
 		ignore_build_time_deps = False
 		if pkg.built and not removal_action:
-			if self._dynamic_config.myparams.get("bdeps", "n") == "y":
+			if self._dynamic_config.myparams.get("bdeps") in ("y", "auto"):
 				# Pull in build time deps as requested, but marked them as
 				# "optional" since they are not strictly required. This allows
 				# more freedom in the merge order calculation for solving
diff --git a/pym/portage/tests/resolver/ResolverPlayground.py b/pym/portage/tests/resolver/ResolverPlayground.py
index d1434f7..d803719 100644
--- a/pym/portage/tests/resolver/ResolverPlayground.py
+++ b/pym/portage/tests/resolver/ResolverPlayground.py
@@ -558,6 +558,7 @@ class ResolverPlaygroundTestCase(object):
 	def __init__(self, request, **kwargs):
 		self.all_permutations = kwargs.pop("all_permutations", False)
 		self.ignore_mergelist_order = kwargs.pop("ignore_mergelist_order", False)
+		self.ignore_cleanlist_order = kwargs.pop("ignore_cleanlist_order", False)
 		self.ambiguous_merge_order = kwargs.pop("ambiguous_merge_order", False)
 		self.ambiguous_slot_collision_solutions = kwargs.pop("ambiguous_slot_collision_solutions", False)
 		self.check_repo_names = kwargs.pop("check_repo_names", False)
@@ -675,6 +676,10 @@ class ResolverPlaygroundTestCase(object):
 									str((node1, node2))) + \
 									", got: " + str(got))
 
+			elif key == "cleanlist" and self.ignore_cleanlist_order:
+				got = set(got)
+				expected = set(expected)
+
 			elif key == "slot_collision_solutions" and \
 				self.ambiguous_slot_collision_solutions:
 				# Tests that use all_permutations can have multiple
diff --git a/pym/portage/tests/resolver/test_bdeps.py b/pym/portage/tests/resolver/test_bdeps.py
new file mode 100644
index 0000000..19159bd
--- /dev/null
+++ b/pym/portage/tests/resolver/test_bdeps.py
@@ -0,0 +1,197 @@
+# 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 BdepsTestCase(TestCase):
+
+	def testImageMagickUpdate(self):
+
+		ebuilds = {
+			"app-misc/A-1" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/B",
+				"RDEPEND": "app-misc/C",
+			},
+
+			"app-misc/B-1" : {
+				"EAPI": "6"
+			},
+			"app-misc/B-2" : {
+				"EAPI": "6",
+			},
+
+			"app-misc/C-1" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/D",
+			},
+			"app-misc/C-2" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/D",
+			},
+
+			"app-misc/D-1" : {
+				"EAPI": "6",
+			},
+			"app-misc/D-2" : {
+				"EAPI": "6",
+			},
+		}
+
+		installed = {
+			"app-misc/A-1" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/B",
+				"RDEPEND": "app-misc/C",
+			},
+
+			"app-misc/B-1" : {
+				"EAPI": "6",
+			},
+			"app-misc/C-1" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/D",
+			},
+
+			"app-misc/D-1" : {
+				"EAPI": "6",
+			},
+		}
+
+		binpkgs = {
+			"app-misc/A-1" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/B",
+				"RDEPEND": "app-misc/C",
+			},
+
+			"app-misc/B-1" : {
+				"EAPI": "6",
+			},
+			"app-misc/B-2" : {
+				"EAPI": "6",
+			},
+
+			"app-misc/C-1" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/D",
+			},
+			"app-misc/C-2" : {
+				"EAPI": "6",
+				"DEPEND": "app-misc/D",
+			},
+
+			"app-misc/D-1" : {
+				"EAPI": "6",
+			},
+			"app-misc/D-2" : {
+				"EAPI": "6",
+			},
+		}
+
+		world = (
+			"app-misc/A",
+		)
+
+		test_cases = (
+
+			# Enable --with-bdeps automatically when
+			# --usepkg has not been specified.
+			ResolverPlaygroundTestCase(
+				["@world"],
+				options = {
+					"--update": True,
+					"--deep": True,
+				},
+				success = True,
+				ambiguous_merge_order = True,
+				mergelist = [
+					"app-misc/D-2",
+					("app-misc/B-2", "app-misc/C-2"),
+				]
+			),
+
+			# Do not enable --with-bdeps automatically when
+			# --usepkg has been specified, since many users of binary
+			# packages do not want unnecessary build time dependencies
+			# installed. In this case we miss an update to for
+			# app-misc/D-2, since DEPEND is not pulled in for
+			# the [binary]app-misc/C-2 update.
+			ResolverPlaygroundTestCase(
+				["@world"],
+				options = {
+					"--update": True,
+					"--deep": True,
+					"--usepkg": True,
+				},
+				success = True,
+				mergelist = [
+					"[binary]app-misc/C-2",
+				]
+			),
+
+			# Use --with-bdeps=y to pull in build-time dependencies of
+			# binary packages.
+			ResolverPlaygroundTestCase(
+				["@world"],
+				options = {
+					"--update": True,
+					"--deep": True,
+					"--usepkg": True,
+					"--with-bdeps": "y",
+				},
+				success = True,
+				ambiguous_merge_order = True,
+				mergelist = [
+					(
+						"[binary]app-misc/D-2",
+						"[binary]app-misc/B-2",
+						"[binary]app-misc/C-2",
+					),
+				]
+			),
+
+			# For --depclean, do not remove build-time dependencies by
+			# default.
+			ResolverPlaygroundTestCase(
+				[],
+				options = {
+					"--depclean": True,
+				},
+				success = True,
+				cleanlist = [],
+			),
+
+			# For --depclean, remove build-time dependencies if
+			# --with-bdeps=n has been specified.
+			ResolverPlaygroundTestCase(
+				[],
+				options = {
+					"--depclean": True,
+					"--with-bdeps": "n",
+				},
+				success = True,
+				ignore_cleanlist_order = True,
+				cleanlist = [
+					"app-misc/D-1",
+					"app-misc/B-1",
+				],
+			),
+		)
+
+		playground = ResolverPlayground(debug=False,
+			ebuilds=ebuilds, installed=installed,
+			binpkgs=binpkgs, world=world)
+		try:
+			for test_case in test_cases:
+				playground.run_TestCase(test_case)
+				self.assertEqual(test_case.test_success, True,
+					test_case.fail_msg)
+		finally:
+			# Disable debug so that cleanup works.
+			playground.debug = False
+			playground.cleanup()
-- 
2.10.2