[gentoo-portage-dev] [PATCH v2] Replace implicit {FEATURES->USE}=test forcing with USE default

[Michał Górny]

Use an explicit USE_ORDER entry to control mapping FEATURES=test into
default-enabled USE=test, rather than forcing/masking it depending
on the state of FEATURES.

This makes it possible for users to enable (or disable) USE=test
independently of FEATURES.  An example use case is installing test
dependencies and building test cases without actually running tests
at a particular moment which is something I've been doing quite
frequently with LLVM.
---
 cnf/make.globals                              |   2 +-
 lib/portage/eapi.py                           |   2 +-
 lib/portage/package/ebuild/config.py          |  52 +++++----
 .../tests/resolver/test_features_test_use.py  | 108 +++++++++++-------
 man/make.conf.5                               |   6 +-
 5 files changed, 102 insertions(+), 68 deletions(-)

diff --git a/cnf/make.globals b/cnf/make.globals
index 08a37a534..04a708af8 100644
--- a/cnf/make.globals
+++ b/cnf/make.globals
@@ -107,7 +107,7 @@ CONFIG_PROTECT="/etc"
 CONFIG_PROTECT_MASK="/etc/env.d"
 
 # Disable auto-use
-USE_ORDER="env:pkg:conf:defaults:pkginternal:repo:env.d"
+USE_ORDER="env:pkg:conf:defaults:pkginternal:features:repo:env.d"
 
 # Mode bits for ${WORKDIR} (see ebuild.5).
 PORTAGE_WORKDIR_MODE="0700"
diff --git a/lib/portage/eapi.py b/lib/portage/eapi.py
index 158d58243..5e12e976d 100644
--- a/lib/portage/eapi.py
+++ b/lib/portage/eapi.py
@@ -170,7 +170,7 @@ def _get_eapi_attrs(eapi):
 		exports_EBUILD_PHASE_FUNC = (eapi is None or eapi_exports_EBUILD_PHASE_FUNC(eapi)),
 		exports_PORTDIR = (eapi is None or eapi_exports_PORTDIR(eapi)),
 		exports_ECLASSDIR = (eapi is not None and eapi_exports_ECLASSDIR(eapi)),
-		feature_flag_test = True,
+		feature_flag_test = False,
 		feature_flag_targetroot = (eapi is not None and eapi_has_targetroot(eapi)),
 		hdepend = (eapi is not None and eapi_has_hdepend(eapi)),
 		iuse_defaults = (eapi is None or eapi_has_iuse_defaults(eapi)),
diff --git a/lib/portage/package/ebuild/config.py b/lib/portage/package/ebuild/config.py
index 320d9f6c0..37c5c6656 100644
--- a/lib/portage/package/ebuild/config.py
+++ b/lib/portage/package/ebuild/config.py
@@ -294,6 +294,7 @@ class config(object):
 			self.configlist = [
 				self.configdict['env.d'],
 				self.configdict['repo'],
+				self.configdict['features'],
 				self.configdict['pkginternal'],
 				self.configdict['globals'],
 				self.configdict['defaults'],
@@ -461,13 +462,16 @@ class config(object):
 			# back up our incremental variables:
 			self.configdict={}
 			self._use_expand_dict = {}
-			# configlist will contain: [ env.d, globals, defaults, conf, pkg, backupenv, env ]
+			# configlist will contain: [ env.d, globals, features, defaults, conf, pkg, backupenv, env ]
 			self.configlist.append({})
 			self.configdict["env.d"] = self.configlist[-1]
 
 			self.configlist.append({})
 			self.configdict["repo"] = self.configlist[-1]
 
+			self.configlist.append({})
+			self.configdict["features"] = self.configlist[-1]
+
 			self.configlist.append({})
 			self.configdict["pkginternal"] = self.configlist[-1]
 
@@ -868,7 +872,7 @@ class config(object):
 			# reasonable defaults; this is important as without USE_ORDER,
 			# USE will always be "" (nothing set)!
 			if "USE_ORDER" not in self:
-				self["USE_ORDER"] = "env:pkg:conf:defaults:pkginternal:repo:env.d"
+				self["USE_ORDER"] = "env:pkg:conf:defaults:pkginternal:features:repo:env.d"
 				self.backup_changes("USE_ORDER")
 
 			if "CBUILD" not in self and "CHOST" in self:
@@ -1292,6 +1296,7 @@ class config(object):
 			del self._penv[:]
 			self.configdict["pkg"].clear()
 			self.configdict["pkginternal"].clear()
+			self.configdict["features"].clear()
 			self.configdict["repo"].clear()
 			self.configdict["defaults"]["USE"] = \
 				" ".join(self.make_defaults_use)
@@ -1452,6 +1457,7 @@ class config(object):
 		cp = cpv_getkey(mycpv)
 		cpv_slot = self.mycpv
 		pkginternaluse = ""
+		feature_use = []
 		iuse = ""
 		pkg_configdict = self.configdict["pkg"]
 		previous_iuse = pkg_configdict.get("IUSE")
@@ -1650,6 +1656,24 @@ class config(object):
 		if has_changed:
 			self.reset(keeping_pkg=1)
 
+		if explicit_iuse is None:
+			explicit_iuse = frozenset(x.lstrip("+-") for x in iuse.split())
+		if eapi_attrs.iuse_effective:
+			iuse_implicit_match = self._iuse_effective_match
+		else:
+			iuse_implicit_match = self._iuse_implicit_match
+
+		if "test" in explicit_iuse or iuse_implicit_match("test"):
+			if "test" in self.features:
+				feature_use.append("test")
+
+		feature_use = " ".join(feature_use)
+		if feature_use != self.configdict["features"].get("USE", ""):
+			self.configdict["features"]["USE"] = feature_use
+			# TODO: can we avoid that?
+			self.reset(keeping_pkg=1)
+			has_changed = True
+
 		env_configdict = self.configdict['env']
 
 		# Ensure that "pkg" values are always preferred over "env" values.
@@ -1677,11 +1701,8 @@ class config(object):
 		# package has different IUSE.
 		use = set(self["USE"].split())
 		unfiltered_use = frozenset(use)
-		if explicit_iuse is None:
-			explicit_iuse = frozenset(x.lstrip("+-") for x in iuse.split())
 
 		if eapi_attrs.iuse_effective:
-			iuse_implicit_match = self._iuse_effective_match
 			portage_iuse = set(self._iuse_effective)
 			portage_iuse.update(explicit_iuse)
 			if built_use is not None:
@@ -1693,7 +1714,6 @@ class config(object):
 			self.configdict["pkg"]["IUSE_EFFECTIVE"] = \
 				" ".join(sorted(portage_iuse))
 		else:
-			iuse_implicit_match = self._iuse_implicit_match
 			portage_iuse = self._get_implicit_iuse()
 			portage_iuse.update(explicit_iuse)
 
@@ -1729,21 +1749,17 @@ class config(object):
 			self.get("EBUILD_FORCE_TEST") == "1"
 
 		if "test" in explicit_iuse or iuse_implicit_match("test"):
-			if "test" not in self.features:
-				use.discard("test")
-			elif restrict_test or \
+			if "test" in self.features:
+				if ebuild_force_test and "test" in self.usemask:
+					self.usemask = \
+						frozenset(x for x in self.usemask if x != "test")
+			if restrict_test or \
 				("test" in self.usemask and not ebuild_force_test):
 				# "test" is in IUSE and USE=test is masked, so execution
 				# of src_test() probably is not reliable. Therefore,
 				# temporarily disable FEATURES=test just for this package.
 				self["FEATURES"] = " ".join(x for x in self.features \
 					if x != "test")
-				use.discard("test")
-			else:
-				use.add("test")
-				if ebuild_force_test and "test" in self.usemask:
-					self.usemask = \
-						frozenset(x for x in self.usemask if x != "test")
 
 		if eapi_attrs.feature_flag_targetroot and \
 			("targetroot" in explicit_iuse or iuse_implicit_match("targetroot")):
@@ -1900,12 +1916,6 @@ class config(object):
 		iuse_implicit.add("build")
 		iuse_implicit.add("bootstrap")
 
-		# Controlled by FEATURES=test. Make this implicit, so handling
-		# of FEATURES=test is consistent regardless of explicit IUSE.
-		# Users may use use.mask/package.use.mask to control
-		# FEATURES=test for all ebuilds, regardless of explicit IUSE.
-		iuse_implicit.add("test")
-
 		return iuse_implicit
 
 	def _getUseMask(self, pkg, stable=None):
diff --git a/lib/portage/tests/resolver/test_features_test_use.py b/lib/portage/tests/resolver/test_features_test_use.py
index bdd179d7a..da7172c17 100644
--- a/lib/portage/tests/resolver/test_features_test_use.py
+++ b/lib/portage/tests/resolver/test_features_test_use.py
@@ -1,68 +1,88 @@
-# Copyright 2012 Gentoo Foundation
+# Copyright 2012-2018 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 FeaturesTestUse(TestCase):
 
-	def testFeaturesTestUse(self):
-		ebuilds = {
-			"dev-libs/A-1" : {
-				"IUSE": "test"
-			},
-			"dev-libs/B-1" : {
-				"IUSE": "test foo"
-			},
-		}
+class TestDepend(TestCase):
+	ebuilds = {
+		"dev-libs/A-1" : {
+			"IUSE": "test",
+			"DEPEND": "test? ( dev-libs/B )",
+		},
+		"dev-libs/B-1" : {
+		},
+	}
 
-		installed = {
-			"dev-libs/A-1" : {
-				"USE": "",
-				"IUSE": "test"
-			},
-			"dev-libs/B-1" : {
-				"USE": "foo",
-				"IUSE": "test foo"
-			},
-		}
+	installed = {
+		"dev-libs/A-1" : {
+			"USE": "",
+			"IUSE": "test",
+			"DEPEND": "test? ( dev-libs/B )",
+		},
+	}
 
+	def test_default_use_test(self):
+		"""
+		Test that FEATURES=test enables USE=test by default.
+		"""
 		user_config = {
-			"make.conf" : ("FEATURES=test", "USE=\"-test -foo\"")
+			"make.conf" : ("FEATURES=test", "USE=\"\"")
 		}
-
-		test_cases = (
-
-			# USE=test state should not trigger --newuse rebuilds, as
-			# specified in bug #373209, comment #3.
-			ResolverPlaygroundTestCase(
+		test_case = ResolverPlaygroundTestCase(
 				["dev-libs/A"],
-				options = {"--newuse": True, "--selective": True},
+				options = {},
 				success = True,
-				mergelist = []),
+				mergelist = ["dev-libs/B-1", "dev-libs/A-1"])
+
+		playground = ResolverPlayground(ebuilds=self.ebuilds,
+			user_config=user_config, debug=False)
+		try:
+			playground.run_TestCase(test_case)
+			self.assertEqual(test_case.test_success, True, test_case.fail_msg)
+		finally:
+			playground.cleanup()
 
-			# USE=-test -> USE=test, with USE=test forced by FEATURES=test
-			ResolverPlaygroundTestCase(
+	def test_no_forced_use_test(self):
+		"""
+		Test that FEATURES=test no longer forces USE=test.
+		"""
+		user_config = {
+			"make.conf" : ("FEATURES=test", "USE=\"-test\"")
+		}
+		test_case = ResolverPlaygroundTestCase(
 				["dev-libs/A"],
 				options = {},
 				success = True,
-				mergelist = ["dev-libs/A-1"]),
+				mergelist = ["dev-libs/A-1"])
+
+		playground = ResolverPlayground(ebuilds=self.ebuilds,
+			user_config=user_config, debug=False)
+		try:
+			playground.run_TestCase(test_case)
+			self.assertEqual(test_case.test_success, True, test_case.fail_msg)
+		finally:
+			playground.cleanup()
 
-			# USE=foo -> USE=-foo, with USE=test forced by FEATURES=test
-			ResolverPlaygroundTestCase(
-				["dev-libs/B"],
+	def test_newuse(self):
+		"""
+		Test that --newuse now detects USE=test changes.
+		"""
+		user_config = {
+			"make.conf" : ("FEATURES=test", "USE=\"\"")
+		}
+		test_case = ResolverPlaygroundTestCase(
+				["dev-libs/A"],
 				options = {"--newuse": True, "--selective": True},
 				success = True,
-				mergelist = ["dev-libs/B-1"]),
-		)
+				mergelist = ["dev-libs/B-1", "dev-libs/A-1"])
 
-		playground = ResolverPlayground(ebuilds=ebuilds,
-			installed=installed, user_config=user_config, debug=False)
+		playground = ResolverPlayground(ebuilds=self.ebuilds,
+			user_config=user_config, 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)
+			playground.run_TestCase(test_case)
+			self.assertEqual(test_case.test_success, True, test_case.fail_msg)
 		finally:
 			playground.cleanup()
-
diff --git a/man/make.conf.5 b/man/make.conf.5
index cb0f00237..a4e33923c 100644
--- a/man/make.conf.5
+++ b/man/make.conf.5
@@ -1138,7 +1138,7 @@ This variable contains options that control the build behavior of several
 packages.  More information in \fBebuild\fR(5).  Possible USE values
 can be found in \fI/usr/portage/profiles/use.desc\fR.
 .TP
-\fBUSE_ORDER\fR = \fI"env:pkg:conf:defaults:pkginternal:repo:env.d"\fR
+\fBUSE_ORDER\fR = \fI"env:pkg:conf:defaults:pkginternal:features:repo:env.d"\fR
 Determines the precedence of layers in the incremental stacking of the USE
 variable. Precedence decreases from left to right such that env overrides
 pkg, pkg overrides conf, and so forth.
@@ -1167,6 +1167,10 @@ USE from make.defaults and package.use in the profile
 .B pkginternal
 USE from \fBebuild\fR(5) IUSE defaults
 .TP
+.B features
+Flags implied by FEATURES.  Currently includes USE=\fBtest\fR
+for FEATURES=\fBtest\fR.
+.TP
 .B repo
 USE from make.defaults and package.use in the repo's profiles/ top dir
 (e.g. /usr/portage/profiles/package.use) (see \fBportage\fR(5))
-- 
2.18.0