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

[Zac Medico]

It's useful to automatically enable --with-bdeps 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.

A new --with-bdeps-auto=<y|n> option is provided, making it possible to
enable or disable the program logic that causes --with-bdeps to be
automatically enabled. Use --with-bdeps-auto=n to prevent --with-bdeps
from being automatically enabled for installation actions. This is useful
for some rare cases in which --with-bdeps triggers unsolvable dependency
conflicts (and putting --with-bdeps=n in EMERGE_DEFAULT_OPTS would cause
undesirable --depclean behavior).

X-Gentoo-bug: 598444
X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=598444
---
[PATCH v2] adds a --with-bdeps-auto=<y|n> option which is useful for some
rare cases in which --with-bdeps triggers unsolvable dependency conflicts

 man/emerge.1                                     |  37 +++-
 pym/_emerge/create_depgraph_params.py            |   5 +
 pym/_emerge/depgraph.py                          |   4 +-
 pym/_emerge/main.py                              |   5 +
 pym/portage/tests/resolver/ResolverPlayground.py |   5 +
 pym/portage/tests/resolver/test_bdeps.py         | 215 +++++++++++++++++++++++
 6 files changed, 266 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..7db4271 100644
--- a/man/emerge.1
+++ b/man/emerge.1
@@ -986,13 +986,44 @@ 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 automatically enabled for
+installation actions, meaning they will be installed, and defaults to
+\(aqy\(aq for the \fB\-\-depclean\fR action, meaning they will not be
+removed. In order to prevent the \fB\-\-with\-bdeps\fR option from being
+automatically enabled for installation actions, specify
+\fB\-\-with\-bdeps\-auto=n\fR in either the command line or
+\fBEMERGE_DEFAULT_OPTS\fR.
+
+Since many users of binary packages do not want unnecessary build time
+dependencies installed, this option is not automatically enabled 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.
 .TP
+.BR "\-\-with\-bdeps\-auto < y | n >"
+This option is used to enable or disable the program logic that causes
+\fB\-\-with\-bdeps\fR is to be automatically enabled for installation
+actions. This option is enabled by default. Use
+\fB\-\-with\-bdeps\-auto=n\fR to prevent \fB\-\-with\-bdeps\fR from
+being automatically enabled for installation actions. This setting can
+be added to \fBEMERGE_DEFAULT_OPTS\fR (see make.conf(5)) and later
+overridden via the command line.
+
+\fBNOTE:\fR The program logic that causes \fB\-\-with\-bdeps\fR to be
+automatically enabled for installation actions does not affect removal
+actions such as the \fB\-\-depclean\fR action. Therefore, when
+\fB\-\-with\-bdeps\-auto=n\fR is specified in \fBEMERGE_DEFAULT_OPTS\fR,
+it does not affect the default \fB\-\-with\-bdeps=y\fR setting that
+applies to the \fB\-\-depclean\fR action. The default
+\fB\-\-with\-bdeps=y\fR setting that applies to the \fB\-\-depclean\fR
+action can be overridden only by specifying \fB\-\-with\-bdeps=n\fR.
+.TP
 .BR "\-\-with\-test\-deps [ y | n ]"
 For packages matched by arguments, this option will pull in dependencies
 that are conditional on the "test" USE flag, even if "test" is not
diff --git a/pym/_emerge/create_depgraph_params.py b/pym/_emerge/create_depgraph_params.py
index 2c64928..cdea029 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,9 @@ def create_depgraph_params(myopts, myaction):
 	bdeps = myopts.get("--with-bdeps")
 	if bdeps is not None:
 		myparams["bdeps"] = bdeps
+	elif myaction == "remove" or (
+		myopts.get("--with-bdeps-auto") != "n" and "--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/_emerge/main.py b/pym/_emerge/main.py
index a83b328..76e963a 100644
--- a/pym/_emerge/main.py
+++ b/pym/_emerge/main.py
@@ -516,6 +516,11 @@ def parse_opts(tmpcmdline, silent=False):
 			"help":"include unnecessary build time dependencies",
 			"choices":("y", "n")
 		},
+		"--with-bdeps-auto": {
+			"help":("automatically enable --with-bdeps for installation"
+				" actions, unless --usepkg is enabled"),
+			"choices":("y", "n")
+		},
 		"--reinstall": {
 			"help":"specify conditions to trigger package reinstallation",
 			"choices":["changed-use"]
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..c0d6499
--- /dev/null
+++ b/pym/portage/tests/resolver/test_bdeps.py
@@ -0,0 +1,215 @@
+# 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"),
+				]
+			),
+
+			# Use --with-bdeps-auto=n to prevent --with-bdeps
+			# from being enabled automatically.
+			ResolverPlaygroundTestCase(
+				["@world"],
+				options = {
+					"--update": True,
+					"--deep": True,
+					"--with-bdeps-auto": "n",
+				},
+				success = True,
+				mergelist = [
+					"app-misc/D-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
+			# 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. Specify --with-bdeps-auto=n, in order to
+			# demonstrate that it does not affect removal actions.
+			ResolverPlaygroundTestCase(
+				[],
+				options = {
+					"--depclean": True,
+					"--with-bdeps-auto": "n",
+				},
+				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