public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out
@ 2012-05-23 19:21 Mike Frysinger
  2012-05-23 19:52 ` Zac Medico
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Mike Frysinger @ 2012-05-23 19:21 UTC (permalink / raw
  To: gentoo-portage-dev

Rather than copying & pasting the same behavior for the different eclass
checks, add a common class for them to extend.  This makes adding more
eclass checks trivial, and keeps down bitrot.

This does abuse the checking interface slightly -- the eclass will change
its category between unused and missing based on the checks.

URL: https://bugs.gentoo.org/417159
URL: https://bugs.gentoo.org/417231
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 bin/repoman           |    6 +-
 pym/repoman/checks.py |  143 ++++++++++++++++++++++++++++++++-----------------
 pym/repoman/errors.py |    1 -
 3 files changed, 97 insertions(+), 53 deletions(-)

diff --git a/bin/repoman b/bin/repoman
index 3697403..d7ffcdd 100755
--- a/bin/repoman
+++ b/bin/repoman
@@ -315,8 +315,9 @@ qahelp={
 	"file.size.fatal":"Files in the files directory must be under 60 KiB",
 	"file.name":"File/dir name must be composed of only the following chars: %s " % allowed_filename_chars,
 	"file.UTF8":"File is not UTF8 compliant",
-	"inherit.autotools":"Ebuild inherits autotools but does not call eautomake, eautoconf or eautoreconf",
 	"inherit.deprecated":"Ebuild inherits a deprecated eclass",
+	"inherit.missing":"Ebuild uses functions from an eclass but does not inherit it",
+	"inherit.unused":"Ebuild inherits an eclass but does not use it",
 	"java.eclassesnotused":"With virtual/jdk in DEPEND you must inherit a java eclass",
 	"wxwidgets.eclassnotused":"Ebuild DEPENDs on x11-libs/wxGTK without inheriting wxwidgets.eclass",
 	"KEYWORDS.dropped":"Ebuilds that appear to have dropped KEYWORDS for some arch",
@@ -382,7 +383,6 @@ qahelp={
 	"ebuild.majorsyn":"This ebuild has a major syntax error that may cause the ebuild to fail partially or fully",
 	"ebuild.minorsyn":"This ebuild has a minor syntax error that contravenes gentoo coding style",
 	"ebuild.badheader":"This ebuild has a malformed header",
-	"eprefixify.defined":"The ebuild uses eprefixify, but does not inherit the prefix eclass",
 	"manifest.bad":"Manifest has missing or incorrect digests",
 	"metadata.missing":"Missing metadata.xml files",
 	"metadata.bad":"Bad metadata.xml files",
@@ -425,7 +425,7 @@ qawarnings = set((
 "ebuild.badheader",
 "ebuild.patches",
 "file.size",
-"inherit.autotools",
+"inherit.unused",
 "inherit.deprecated",
 "java.eclassesnotused",
 "wxwidgets.eclassnotused",
diff --git a/pym/repoman/checks.py b/pym/repoman/checks.py
index 77df603..088a916 100644
--- a/pym/repoman/checks.py
+++ b/pym/repoman/checks.py
@@ -331,24 +331,6 @@ class EbuildQuotedA(LineCheck):
 		if match:
 			return "Quoted \"${A}\" on line: %d"
 
-class EprefixifyDefined(LineCheck):
-	""" Check that prefix.eclass is inherited if needed"""
-
-	repoman_check_name = 'eprefixify.defined'
-
-	_eprefixify_re = re.compile(r'\beprefixify\b')
-	_inherit_prefix_re = re.compile(r'^\s*inherit\s(.*\s)?prefix\b')
-
-	def new(self, pkg):
-		self._prefix_inherited = False
-
-	def check(self, num, line):
-		if self._eprefixify_re.search(line) is not None:
-			if not self._prefix_inherited:
-				return errors.EPREFIXIFY_MISSING_INHERIT
-		elif self._inherit_prefix_re.search(line) is not None:
-			self._prefix_inherited = True
-
 class NoOffsetWithHelpers(LineCheck):
 	""" Check that the image location, the alternate root offset, and the
 	offset prefix (D, ROOT, ED, EROOT and EPREFIX) are not used with
@@ -464,43 +446,104 @@ class InheritDeprecated(LineCheck):
 					(eclass, replacement)
 		del self._indirect_deprecated
 
-class InheritAutotools(LineCheck):
-	"""
-	Make sure appropriate functions are called in
-	ebuilds that inherit autotools.eclass.
+class InheritEclass(LineCheck):
 	"""
+	Base class for checking for missing inherits, as well as excess inherits.
 
-	repoman_check_name = 'inherit.autotools'
-	_inherit_autotools_re = re.compile(r'^\s*inherit\s(.*\s)?autotools(\s|$)')
-	_autotools_funcs = (
-		"eaclocal", "eautoconf", "eautoheader",
-		"eautomake", "eautoreconf", "_elibtoolize")
-	_autotools_func_re = re.compile(r'\b(' + \
-		"|".join(_autotools_funcs) + r')\b')
-	# Exempt eclasses:
-	# git - An EGIT_BOOTSTRAP variable may be used to call one of
-	#       the autotools functions.
-	# subversion - An ESVN_BOOTSTRAP variable may be used to call one of
-	#       the autotools functions.
-	_exempt_eclasses = frozenset(["git", "subversion"])
+	Args:
+		_eclass: Set to the name of your eclass.
+		_funcs: A tuple of functions that this eclass provides.
+		_comprehensive: Is the list of functions complete?
+		_exempt_eclasses: If these eclasses are inherited, disable the missing
+		                  inherit check.
+	"""
 
 	def new(self, pkg):
-		self._inherit_autotools = None
-		self._autotools_func_call = None
-		self._disabled = self._exempt_eclasses.intersection(pkg.inherited)
+		self.repoman_check_name = 'inherit.missing'
+		self._inherit_re = re.compile(r'^\s*inherit\s(.*\s)?%s(\s|$)' % self._eclass)
+		self._func_re = re.compile(r'\b(' + '|'.join(self._funcs) + r')\b')
+		# We can't use pkg.inherited because that tells us all the eclass that
+		# have been inherited and not just the ones we inherit directly.
+		self._inherit = False
+		self._func_call = False
+		if '_exempt_eclasses' in dir(self):
+			self._disabled = self._exempt_eclasses.intersection(pkg.inherited)
+		else:
+			self._disabled = False
 
 	def check(self, num, line):
-		if self._disabled:
-			return
-		if self._inherit_autotools is None:
-			self._inherit_autotools = self._inherit_autotools_re.match(line)
-		if self._inherit_autotools is not None and \
-			self._autotools_func_call is None:
-			self._autotools_func_call = self._autotools_func_re.search(line)
+		if not self._inherit:
+			self._inherit = self._inherit_re.match(line)
+		if not self._inherit:
+			if self._disabled:
+				return
+			s = self._func_re.search(line)
+			if s:
+				self._func_call = True
+				return '%s.eclass is not inherited, but "%s" found at line: %s' % \
+					(self._eclass, s.group(0), '%d')
+		elif not self._func_call:
+			self._func_call = self._func_re.search(line)
 
 	def end(self):
-		if self._inherit_autotools and self._autotools_func_call is None:
-			yield 'no eauto* function called'
+		if self._comprehensive and self._inherit and not self._func_call:
+			self.repoman_check_name = 'inherit.unused'
+			yield 'no function called from %s.eclass; please drop' % self._eclass
+
+class InheritAutotools(InheritEclass):
+	_eclass = 'autotools'
+	_funcs = (
+		'eaclocal', 'eautoconf', 'eautoheader',
+		'eautomake', 'eautoreconf', '_elibtoolize',
+		'eautopoint'
+	)
+	_comprehensive = True
+
+	# Exempt eclasses:
+	# git - An EGIT_BOOTSTRAP variable may be used to call one of
+	#       the autotools functions.
+	# subversion - An ESVN_BOOTSTRAP variable may be used to call one of
+	#       the autotools functions.
+	_exempt_eclasses = frozenset(['git', 'subversion'])
+
+class InheritEutils(InheritEclass):
+	_eclass = 'eutils'
+	_funcs = (
+		'estack_push', 'estack_pop', 'eshopts_push', 'eshopts_pop',
+		'eumask_push', 'eumask_pop', 'epatch', 'epatch_user',
+		'emktemp', 'edos2unix', 'in_iuse', 'use_if_iuse', 'usex',
+		'makeopts_jobs'
+	)
+	_comprehensive = False
+
+class InheritLibtool(InheritEclass):
+	_eclass = 'libtool'
+	_funcs = (
+		'elibtoolize',
+	)
+	_comprehensive = True
+
+class InheritPrefix(InheritEclass):
+	_eclass = 'prefix'
+	_funcs = (
+		'eprefixify',
+	)
+	_comprehensive = True
+
+class InheritToolchainFuncs(InheritEclass):
+	_eclass = 'toolchain-funcs'
+	_funcs = (
+		'gen_usr_ldscript',
+	)
+	_comprehensive = True
+
+class InheritUser(InheritEclass):
+	_eclass = 'user'
+	_funcs = (
+		'enewuser', 'enewgroup',
+		'egetent', 'egethome', 'egetshell'
+	)
+	_comprehensive = True
 
 class IUseUndefined(LineCheck):
 	"""
@@ -679,8 +722,10 @@ _constant_checks = tuple((c() for c in (
 	EbuildHeader, EbuildWhitespace, EbuildBlankLine, EbuildQuote,
 	EbuildAssignment, Eapi3EbuildAssignment, EbuildUselessDodoc,
 	EbuildUselessCdS, EbuildNestedDie,
-	EbuildPatches, EbuildQuotedA, EapiDefinition, EprefixifyDefined,
-	ImplicitRuntimeDeps, InheritAutotools, InheritDeprecated, IUseUndefined,
+	EbuildPatches, EbuildQuotedA, EapiDefinition,
+	ImplicitRuntimeDeps, InheritAutotools, InheritDeprecated, InheritEutils,
+	InheritLibtool, InheritPrefix, InheritToolchainFuncs, InheritUser,
+	IUseUndefined,
 	EMakeParallelDisabled, EMakeParallelDisabledViaMAKEOPTS, NoAsNeeded,
 	DeprecatedBindnowFlags, SrcUnpackPatches, WantAutoDefaultValue,
 	SrcCompileEconf, Eapi3DeprecatedFuncs, NoOffsetWithHelpers,
diff --git a/pym/repoman/errors.py b/pym/repoman/errors.py
index 3209243..c515502 100644
--- a/pym/repoman/errors.py
+++ b/pym/repoman/errors.py
@@ -19,7 +19,6 @@ EAPI_DEFINED_AFTER_INHERIT = 'EAPI defined after inherit on line: %d'
 NO_AS_NEEDED = 'Upstream asneeded linking bug (no-as-needed on line: %d)'
 PRESERVE_OLD_LIB = 'Upstream ABI change workaround on line: %d'
 BUILT_WITH_USE = 'built_with_use on line: %d'
-EPREFIXIFY_MISSING_INHERIT = "prefix.eclass is not inherited, but eprefixify is used on line: %d"
 NO_OFFSET_WITH_HELPERS = "Helper function is used with D, ROOT, ED, EROOT or EPREFIX on line :%d"
 SANDBOX_ADDPREDICT = 'Ebuild calls addpredict on line: %d'
 USEQ_ERROR = 'Ebuild calls deprecated useq function on line: %d'
-- 
1.7.8.6




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-23 19:21 [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out Mike Frysinger
@ 2012-05-23 19:52 ` Zac Medico
  2012-05-24 19:11   ` Mike Frysinger
  2012-05-24  3:15 ` Mike Frysinger
  2012-05-24 19:20 ` [gentoo-portage-dev] [PATCH v2] " Mike Frysinger
  2 siblings, 1 reply; 12+ messages in thread
From: Zac Medico @ 2012-05-23 19:52 UTC (permalink / raw
  To: gentoo-portage-dev

On 05/23/2012 12:21 PM, Mike Frysinger wrote:
> Rather than copying & pasting the same behavior for the different eclass
> checks, add a common class for them to extend.  This makes adding more
> eclass checks trivial, and keeps down bitrot.
> 
> This does abuse the checking interface slightly -- the eclass will change
> its category between unused and missing based on the checks.
> 
> URL: https://bugs.gentoo.org/417159
> URL: https://bugs.gentoo.org/417231

It's fragile to keep all of these eclass interface definitions hardcoded
in python. For example, the _comprehensive checks are going to start
triggering repoman warnings every time that we add a new function to an
eclass. If we keep the eclass interface definitions in the tree with the
eclasses, then it will solve this problem, and it will also be possible
for overlays to fork eclasses and tweak interfaces.

>  	def new(self, pkg):
> -		self._inherit_autotools = None
> -		self._autotools_func_call = None
> -		self._disabled = self._exempt_eclasses.intersection(pkg.inherited)
> +		self.repoman_check_name = 'inherit.missing'
> +		self._inherit_re = re.compile(r'^\s*inherit\s(.*\s)?%s(\s|$)' % self._eclass)
> +		self._func_re = re.compile(r'\b(' + '|'.join(self._funcs) + r')\b')


The _inherit_re and _func_re regular expressions do not change from one
package to the next, so we can compile them inside the __init__
constructor instead, which is only called once in global scope.

> +		# We can't use pkg.inherited because that tells us all the eclass that
> +		# have been inherited and not just the ones we inherit directly.
> +		self._inherit = False
> +		self._func_call = False
> +		if '_exempt_eclasses' in dir(self):
> +			self._disabled = self._exempt_eclasses.intersection(pkg.inherited)
> +		else:
> +			self._disabled = False

It's more efficient to use hasattr(self, '_exempt_eclasses') than to use
'_exempt_eclasses' in dir(self).

-- 
Thanks,
Zac



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-23 19:21 [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out Mike Frysinger
  2012-05-23 19:52 ` Zac Medico
@ 2012-05-24  3:15 ` Mike Frysinger
  2012-05-24  4:01   ` Zac Medico
  2012-05-24 19:20 ` [gentoo-portage-dev] [PATCH v2] " Mike Frysinger
  2 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2012-05-24  3:15 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: Text/Plain, Size: 767 bytes --]

On Wednesday 23 May 2012 15:21:51 Mike Frysinger wrote:
> +		self._inherit_re = re.compile(r'^\s*inherit\s(.*\s)?%s(\s|$)' %

in scanning the whole tree, this seems to cause some issues (not new) with 
extended constructs and not detecting this ebuilds inherits an eclass 
directly.  some examples:

(1)
	foo="autotools"
	...
	inherit ${foo}
this seems pointless imo since we've already ruled that multiple `inherit` 
calls are OK

(2)
	inherit ... \
		autotools
this is annoying.  maybe we should adapt the core line code to unroll these 
before passing to the individual checks ?

(3)
	[[ ${PV} == "9999" ]] && inherit autotools
this one would require tweaking the regex like so:
	(^[[:space:]]*|[[:space:]])inherit

any opinions ?
-mike

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-24  3:15 ` Mike Frysinger
@ 2012-05-24  4:01   ` Zac Medico
  0 siblings, 0 replies; 12+ messages in thread
From: Zac Medico @ 2012-05-24  4:01 UTC (permalink / raw
  To: gentoo-portage-dev

On 05/23/2012 08:15 PM, Mike Frysinger wrote:
> On Wednesday 23 May 2012 15:21:51 Mike Frysinger wrote:
>> +		self._inherit_re = re.compile(r'^\s*inherit\s(.*\s)?%s(\s|$)' %
> 
> in scanning the whole tree, this seems to cause some issues (not new) with 
> extended constructs and not detecting this ebuilds inherits an eclass 
> directly.  some examples:
> 
> (1)
> 	foo="autotools"
> 	...
> 	inherit ${foo}
> this seems pointless imo since we've already ruled that multiple `inherit` 
> calls are OK

Seems reasonable.

> (2)
> 	inherit ... \
> 		autotools
> this is annoying.  maybe we should adapt the core line code to unroll these 
> before passing to the individual checks ?

Yeah, we could do that for all escaped newlines (not just the ones
involving inherit).

> (3)
> 	[[ ${PV} == "9999" ]] && inherit autotools
> this one would require tweaking the regex like so:
> 	(^[[:space:]]*|[[:space:]])inherit

Maybe use something like (^[[:space:]]*|[|&]+[[:space:]]*)\b(inherit)
like I did here:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=a8889947c45a9fa81ca006b333466372b64f0344

That way it's less likely to match the word "inherit" inside of a
comment or quoted string.
-- 
Thanks,
Zac



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-23 19:52 ` Zac Medico
@ 2012-05-24 19:11   ` Mike Frysinger
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Frysinger @ 2012-05-24 19:11 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: Text/Plain, Size: 1422 bytes --]

On Wednesday 23 May 2012 15:52:11 Zac Medico wrote:
> On 05/23/2012 12:21 PM, Mike Frysinger wrote:
> > Rather than copying & pasting the same behavior for the different eclass
> > checks, add a common class for them to extend.  This makes adding more
> > eclass checks trivial, and keeps down bitrot.
> > 
> > This does abuse the checking interface slightly -- the eclass will change
> > its category between unused and missing based on the checks.
> > 
> > URL: https://bugs.gentoo.org/417159
> > URL: https://bugs.gentoo.org/417231
> 
> It's fragile to keep all of these eclass interface definitions hardcoded
> in python. For example, the _comprehensive checks are going to start
> triggering repoman warnings every time that we add a new function to an
> eclass.

yes, that's why i picked only ones that are simple/stable

> If we keep the eclass interface definitions in the tree with the
> eclasses, then it will solve this problem, and it will also be possible
> for overlays to fork eclasses and tweak interfaces.

if we're going to merge them, might as well do it once rather than still 
having this problem, but reduced.  if i extended the framework to parse the 
syntax used for documenting the eclass, the problem is reduced to "maintainers 
that fail to fully document their interfaces will hit problems".  but i think 
that's a good hammer to hit eclass maintainers with.
-mike

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-23 19:21 [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out Mike Frysinger
  2012-05-23 19:52 ` Zac Medico
  2012-05-24  3:15 ` Mike Frysinger
@ 2012-05-24 19:20 ` Mike Frysinger
  2012-05-24 20:04   ` Zac Medico
  2 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2012-05-24 19:20 UTC (permalink / raw
  To: gentoo-portage-dev

Rather than copying & pasting the same behavior for the different eclass
checks, add a common class for them to extend.  This makes adding more
eclass checks trivial, and keeps down bitrot.

This does abuse the checking interface slightly -- the eclass will change
its category between unused and missing based on the checks.

URL: https://bugs.gentoo.org/417159
URL: https://bugs.gentoo.org/417231
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- fix optimization aspects

 bin/repoman           |    6 +-
 pym/repoman/checks.py |  161 ++++++++++++++++++++++++++++++++++---------------
 pym/repoman/errors.py |    1 -
 3 files changed, 116 insertions(+), 52 deletions(-)

diff --git a/bin/repoman b/bin/repoman
index fd87847..779e651 100755
--- a/bin/repoman
+++ b/bin/repoman
@@ -315,8 +315,9 @@ qahelp={
 	"file.size.fatal":"Files in the files directory must be under 60 KiB",
 	"file.name":"File/dir name must be composed of only the following chars: %s " % allowed_filename_chars,
 	"file.UTF8":"File is not UTF8 compliant",
-	"inherit.autotools":"Ebuild inherits autotools but does not call eautomake, eautoconf or eautoreconf",
 	"inherit.deprecated":"Ebuild inherits a deprecated eclass",
+	"inherit.missing":"Ebuild uses functions from an eclass but does not inherit it",
+	"inherit.unused":"Ebuild inherits an eclass but does not use it",
 	"java.eclassesnotused":"With virtual/jdk in DEPEND you must inherit a java eclass",
 	"wxwidgets.eclassnotused":"Ebuild DEPENDs on x11-libs/wxGTK without inheriting wxwidgets.eclass",
 	"KEYWORDS.dropped":"Ebuilds that appear to have dropped KEYWORDS for some arch",
@@ -382,7 +383,6 @@ qahelp={
 	"ebuild.majorsyn":"This ebuild has a major syntax error that may cause the ebuild to fail partially or fully",
 	"ebuild.minorsyn":"This ebuild has a minor syntax error that contravenes gentoo coding style",
 	"ebuild.badheader":"This ebuild has a malformed header",
-	"eprefixify.defined":"The ebuild uses eprefixify, but does not inherit the prefix eclass",
 	"manifest.bad":"Manifest has missing or incorrect digests",
 	"metadata.missing":"Missing metadata.xml files",
 	"metadata.bad":"Bad metadata.xml files",
@@ -425,7 +425,7 @@ qawarnings = set((
 "ebuild.badheader",
 "ebuild.patches",
 "file.size",
-"inherit.autotools",
+"inherit.unused",
 "inherit.deprecated",
 "java.eclassesnotused",
 "wxwidgets.eclassnotused",
diff --git a/pym/repoman/checks.py b/pym/repoman/checks.py
index 77df603..c17a0bd 100644
--- a/pym/repoman/checks.py
+++ b/pym/repoman/checks.py
@@ -331,24 +331,6 @@ class EbuildQuotedA(LineCheck):
 		if match:
 			return "Quoted \"${A}\" on line: %d"
 
-class EprefixifyDefined(LineCheck):
-	""" Check that prefix.eclass is inherited if needed"""
-
-	repoman_check_name = 'eprefixify.defined'
-
-	_eprefixify_re = re.compile(r'\beprefixify\b')
-	_inherit_prefix_re = re.compile(r'^\s*inherit\s(.*\s)?prefix\b')
-
-	def new(self, pkg):
-		self._prefix_inherited = False
-
-	def check(self, num, line):
-		if self._eprefixify_re.search(line) is not None:
-			if not self._prefix_inherited:
-				return errors.EPREFIXIFY_MISSING_INHERIT
-		elif self._inherit_prefix_re.search(line) is not None:
-			self._prefix_inherited = True
-
 class NoOffsetWithHelpers(LineCheck):
 	""" Check that the image location, the alternate root offset, and the
 	offset prefix (D, ROOT, ED, EROOT and EPREFIX) are not used with
@@ -464,43 +446,124 @@ class InheritDeprecated(LineCheck):
 					(eclass, replacement)
 		del self._indirect_deprecated
 
-class InheritAutotools(LineCheck):
+class InheritEclass(LineCheck):
 	"""
-	Make sure appropriate functions are called in
-	ebuilds that inherit autotools.eclass.
+	Base class for checking for missing inherits, as well as excess inherits.
+
+	Args:
+		_eclass: Set to the name of your eclass.
+		_funcs: A tuple of functions that this eclass provides.
+		_comprehensive: Is the list of functions complete?
+		_exempt_eclasses: If these eclasses are inherited, disable the missing
+		                  inherit check.
 	"""
 
-	repoman_check_name = 'inherit.autotools'
-	_inherit_autotools_re = re.compile(r'^\s*inherit\s(.*\s)?autotools(\s|$)')
-	_autotools_funcs = (
-		"eaclocal", "eautoconf", "eautoheader",
-		"eautomake", "eautoreconf", "_elibtoolize")
-	_autotools_func_re = re.compile(r'\b(' + \
-		"|".join(_autotools_funcs) + r')\b')
-	# Exempt eclasses:
-	# git - An EGIT_BOOTSTRAP variable may be used to call one of
-	#       the autotools functions.
-	# subversion - An ESVN_BOOTSTRAP variable may be used to call one of
-	#       the autotools functions.
-	_exempt_eclasses = frozenset(["git", "subversion"])
+	def __init__(self):
+		self._inherit_re = re.compile(r'^\s*inherit\s(.*\s)?%s(\s|$)' % self._eclass)
+		self._func_re = re.compile(r'\b(' + '|'.join(self._funcs) + r')\b')
 
 	def new(self, pkg):
-		self._inherit_autotools = None
-		self._autotools_func_call = None
-		self._disabled = self._exempt_eclasses.intersection(pkg.inherited)
+		self.repoman_check_name = 'inherit.missing'
+		# We can't use pkg.inherited because that tells us all the eclass that
+		# have been inherited and not just the ones we inherit directly.
+		self._inherit = False
+		self._func_call = False
+		if hasattr(self, '_exempt_eclasses'):
+			self._disabled = self._exempt_eclasses.intersection(pkg.inherited)
+		else:
+			self._disabled = False
 
 	def check(self, num, line):
-		if self._disabled:
-			return
-		if self._inherit_autotools is None:
-			self._inherit_autotools = self._inherit_autotools_re.match(line)
-		if self._inherit_autotools is not None and \
-			self._autotools_func_call is None:
-			self._autotools_func_call = self._autotools_func_re.search(line)
+		if not self._inherit:
+			self._inherit = self._inherit_re.match(line)
+		if not self._inherit:
+			if self._disabled:
+				return
+			s = self._func_re.search(line)
+			if s:
+				self._func_call = True
+				return '%s.eclass is not inherited, but "%s" found at line: %s' % \
+					(self._eclass, s.group(0), '%d')
+		elif not self._func_call:
+			self._func_call = self._func_re.search(line)
 
 	def end(self):
-		if self._inherit_autotools and self._autotools_func_call is None:
-			yield 'no eauto* function called'
+		if self._comprehensive and self._inherit and not self._func_call:
+			self.repoman_check_name = 'inherit.unused'
+			yield 'no function called from %s.eclass; please drop' % self._eclass
+
+class InheritAutotools(InheritEclass):
+	_eclass = 'autotools'
+	_funcs = (
+		'eaclocal', 'eautoconf', 'eautoheader',
+		'eautomake', 'eautoreconf', '_elibtoolize',
+		'eautopoint'
+	)
+	_comprehensive = True
+
+	# Exempt eclasses:
+	# git - An EGIT_BOOTSTRAP variable may be used to call one of
+	#       the autotools functions.
+	# subversion - An ESVN_BOOTSTRAP variable may be used to call one of
+	#       the autotools functions.
+	_exempt_eclasses = frozenset(['git', 'subversion', 'autotools-utils'])
+
+class InheritEutils(InheritEclass):
+	_eclass = 'eutils'
+	_funcs = (
+		'estack_push', 'estack_pop', 'eshopts_push', 'eshopts_pop',
+		'eumask_push', 'eumask_pop', 'epatch', 'epatch_user',
+		'emktemp', 'edos2unix', 'in_iuse', 'use_if_iuse', 'usex',
+		'makeopts_jobs'
+	)
+	_comprehensive = False
+
+	# These are "eclasses are the whole ebuild" type thing.
+	_exempt_eclasses = frozenset(['toolchain', 'toolchain-binutils'])
+
+class InheritFlagOMatic(InheritEclass):
+	_eclass = 'flag-o-matic'
+	_funcs = (
+		'filter-(ld)?flags', 'strip-flags', 'strip-unsupported-flags',
+		'append-((ld|c(pp|xx)?))?flags', 'append-libs',
+	)
+	_comprehensive = False
+
+class InheritLibtool(InheritEclass):
+	_eclass = 'libtool'
+	_funcs = (
+		'elibtoolize',
+	)
+	_comprehensive = True
+
+class InheritMultilib(InheritEclass):
+	_eclass = 'multilib'
+	_funcs = (
+		'get_libdir',
+	)
+	_comprehensive = False
+
+class InheritPrefix(InheritEclass):
+	_eclass = 'prefix'
+	_funcs = (
+		'eprefixify',
+	)
+	_comprehensive = True
+
+class InheritToolchainFuncs(InheritEclass):
+	_eclass = 'toolchain-funcs'
+	_funcs = (
+		'gen_usr_ldscript',
+	)
+	_comprehensive = False
+
+class InheritUser(InheritEclass):
+	_eclass = 'user'
+	_funcs = (
+		'enewuser', 'enewgroup',
+		'egetent', 'egethome', 'egetshell'
+	)
+	_comprehensive = True
 
 class IUseUndefined(LineCheck):
 	"""
@@ -679,8 +742,10 @@ _constant_checks = tuple((c() for c in (
 	EbuildHeader, EbuildWhitespace, EbuildBlankLine, EbuildQuote,
 	EbuildAssignment, Eapi3EbuildAssignment, EbuildUselessDodoc,
 	EbuildUselessCdS, EbuildNestedDie,
-	EbuildPatches, EbuildQuotedA, EapiDefinition, EprefixifyDefined,
-	ImplicitRuntimeDeps, InheritAutotools, InheritDeprecated, IUseUndefined,
+	EbuildPatches, EbuildQuotedA, EapiDefinition,
+	ImplicitRuntimeDeps, InheritAutotools, InheritDeprecated, InheritEutils,
+	InheritFlagOMatic, InheritMultilib, InheritLibtool, InheritPrefix,
+	InheritToolchainFuncs, InheritUser, IUseUndefined,
 	EMakeParallelDisabled, EMakeParallelDisabledViaMAKEOPTS, NoAsNeeded,
 	DeprecatedBindnowFlags, SrcUnpackPatches, WantAutoDefaultValue,
 	SrcCompileEconf, Eapi3DeprecatedFuncs, NoOffsetWithHelpers,
diff --git a/pym/repoman/errors.py b/pym/repoman/errors.py
index 3209243..c515502 100644
--- a/pym/repoman/errors.py
+++ b/pym/repoman/errors.py
@@ -19,7 +19,6 @@ EAPI_DEFINED_AFTER_INHERIT = 'EAPI defined after inherit on line: %d'
 NO_AS_NEEDED = 'Upstream asneeded linking bug (no-as-needed on line: %d)'
 PRESERVE_OLD_LIB = 'Upstream ABI change workaround on line: %d'
 BUILT_WITH_USE = 'built_with_use on line: %d'
-EPREFIXIFY_MISSING_INHERIT = "prefix.eclass is not inherited, but eprefixify is used on line: %d"
 NO_OFFSET_WITH_HELPERS = "Helper function is used with D, ROOT, ED, EROOT or EPREFIX on line :%d"
 SANDBOX_ADDPREDICT = 'Ebuild calls addpredict on line: %d'
 USEQ_ERROR = 'Ebuild calls deprecated useq function on line: %d'
-- 
1.7.8.6




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-24 19:20 ` [gentoo-portage-dev] [PATCH v2] " Mike Frysinger
@ 2012-05-24 20:04   ` Zac Medico
  2012-05-25 16:20     ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Zac Medico @ 2012-05-24 20:04 UTC (permalink / raw
  To: gentoo-portage-dev

On 05/24/2012 12:20 PM, Mike Frysinger wrote:
> Rather than copying & pasting the same behavior for the different eclass
> checks, add a common class for them to extend.  This makes adding more
> eclass checks trivial, and keeps down bitrot.
> 
> This does abuse the checking interface slightly -- the eclass will change
> its category between unused and missing based on the checks.
> 
> URL: https://bugs.gentoo.org/417159
> URL: https://bugs.gentoo.org/417231
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- fix optimization aspects

Looks good to me.
-- 
Thanks,
Zac



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-24 20:04   ` Zac Medico
@ 2012-05-25 16:20     ` Mike Frysinger
  2012-05-31  0:18       ` Zac Medico
  0 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2012-05-25 16:20 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: Text/Plain, Size: 604 bytes --]

On Thursday 24 May 2012 16:04:30 Zac Medico wrote:
> On 05/24/2012 12:20 PM, Mike Frysinger wrote:
> > Rather than copying & pasting the same behavior for the different eclass
> > checks, add a common class for them to extend.  This makes adding more
> > eclass checks trivial, and keeps down bitrot.
> > 
> > This does abuse the checking interface slightly -- the eclass will change
> > its category between unused and missing based on the checks.
> 
> Looks good to me.

i'll push this now while we hammer out the "complete" solution since these get 
pretty good coverage right now
-mike

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-25 16:20     ` Mike Frysinger
@ 2012-05-31  0:18       ` Zac Medico
  2012-05-31  0:42         ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Zac Medico @ 2012-05-31  0:18 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Mike Frysinger

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/25/2012 09:20 AM, Mike Frysinger wrote:
> On Thursday 24 May 2012 16:04:30 Zac Medico wrote:
>> On 05/24/2012 12:20 PM, Mike Frysinger wrote:
>>> Rather than copying & pasting the same behavior for the
>>> different eclass checks, add a common class for them to extend.
>>> This makes adding more eclass checks trivial, and keeps down
>>> bitrot.
>>> 
>>> This does abuse the checking interface slightly -- the eclass
>>> will change its category between unused and missing based on
>>> the checks.
>> 
>> Looks good to me.
> 
> i'll push this now while we hammer out the "complete" solution
> since these get pretty good coverage right now -mike

Do you want to support EGIT_BOOTSTRAP settings prior to inherit?

For example, dev-libs/polylib/polylib-9999.ebuild triggers
inherit.missing because InheritEclass currently expects the inherit to
occur before the EGIT_BOOTSTRAP="eautoreconf" setting.
- -- 
Thanks,
Zac
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/GuMMACgkQ/ejvha5XGaPxZwCfQrrEmvSG19KZkObav4Kc5TPc
GOcAnRwrDMBzatViRcv2WxiAOFbIt/mC
=uC7c
-----END PGP SIGNATURE-----



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-31  0:18       ` Zac Medico
@ 2012-05-31  0:42         ` Mike Frysinger
  2012-05-31  1:00           ` Zac Medico
  0 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2012-05-31  0:42 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: Text/Plain, Size: 1221 bytes --]

On Wednesday 30 May 2012 20:18:11 Zac Medico wrote:
> On 05/25/2012 09:20 AM, Mike Frysinger wrote:
> > On Thursday 24 May 2012 16:04:30 Zac Medico wrote:
> >> On 05/24/2012 12:20 PM, Mike Frysinger wrote:
> >>> Rather than copying & pasting the same behavior for the
> >>> different eclass checks, add a common class for them to extend.
> >>> This makes adding more eclass checks trivial, and keeps down
> >>> bitrot.
> >>> 
> >>> This does abuse the checking interface slightly -- the eclass
> >>> will change its category between unused and missing based on
> >>> the checks.
> >> 
> >> Looks good to me.
> > 
> > i'll push this now while we hammer out the "complete" solution
> > since these get pretty good coverage right now
> 
> Do you want to support EGIT_BOOTSTRAP settings prior to inherit?
> 
> For example, dev-libs/polylib/polylib-9999.ebuild triggers
> inherit.missing because InheritEclass currently expects the inherit to
> occur before the EGIT_BOOTSTRAP="eautoreconf" setting.

i don't think we need to support that level of detection.  it's a fairly 
extreme edge case, and we can just let the git eclass say "allow implicit 
autotools eclass inclusion" all the time.
-mike

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-31  0:42         ` Mike Frysinger
@ 2012-05-31  1:00           ` Zac Medico
  2012-05-31  1:57             ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Zac Medico @ 2012-05-31  1:00 UTC (permalink / raw
  To: gentoo-portage-dev

On 05/30/2012 05:42 PM, Mike Frysinger wrote:
> On Wednesday 30 May 2012 20:18:11 Zac Medico wrote:
>> On 05/25/2012 09:20 AM, Mike Frysinger wrote:
>>> On Thursday 24 May 2012 16:04:30 Zac Medico wrote:
>>>> On 05/24/2012 12:20 PM, Mike Frysinger wrote:
>>>>> Rather than copying & pasting the same behavior for the
>>>>> different eclass checks, add a common class for them to extend.
>>>>> This makes adding more eclass checks trivial, and keeps down
>>>>> bitrot.
>>>>>
>>>>> This does abuse the checking interface slightly -- the eclass
>>>>> will change its category between unused and missing based on
>>>>> the checks.
>>>>
>>>> Looks good to me.
>>>
>>> i'll push this now while we hammer out the "complete" solution
>>> since these get pretty good coverage right now
>>
>> Do you want to support EGIT_BOOTSTRAP settings prior to inherit?
>>
>> For example, dev-libs/polylib/polylib-9999.ebuild triggers
>> inherit.missing because InheritEclass currently expects the inherit to
>> occur before the EGIT_BOOTSTRAP="eautoreconf" setting.
> 
> i don't think we need to support that level of detection.  it's a fairly 
> extreme edge case, and we can just let the git eclass say "allow implicit 
> autotools eclass inclusion" all the time.
> -mike

Okay, this seems to do the trick:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=6d3873a690ccdf47f1d5c3f83fc8dbef92f5a9f1
-- 
Thanks,
Zac



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [gentoo-portage-dev] [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out
  2012-05-31  1:00           ` Zac Medico
@ 2012-05-31  1:57             ` Mike Frysinger
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Frysinger @ 2012-05-31  1:57 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: Text/Plain, Size: 827 bytes --]

On Wednesday 30 May 2012 21:00:33 Zac Medico wrote:
> On 05/30/2012 05:42 PM, Mike Frysinger wrote:
> > On Wednesday 30 May 2012 20:18:11 Zac Medico wrote:
> >> Do you want to support EGIT_BOOTSTRAP settings prior to inherit?
> >> 
> >> For example, dev-libs/polylib/polylib-9999.ebuild triggers
> >> inherit.missing because InheritEclass currently expects the inherit to
> >> occur before the EGIT_BOOTSTRAP="eautoreconf" setting.
> > 
> > i don't think we need to support that level of detection.  it's a fairly
> > extreme edge case, and we can just let the git eclass say "allow implicit
> > autotools eclass inclusion" all the time.
> 
> Okay, this seems to do the trick:
> 
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=6d3873
> a690ccdf47f1d5c3f83fc8dbef92f5a9f1

LGTM
-mike

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-05-31  3:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-23 19:21 [gentoo-portage-dev] [PATCH] repoman: add a mini framework for checking eclasses, and fill it out Mike Frysinger
2012-05-23 19:52 ` Zac Medico
2012-05-24 19:11   ` Mike Frysinger
2012-05-24  3:15 ` Mike Frysinger
2012-05-24  4:01   ` Zac Medico
2012-05-24 19:20 ` [gentoo-portage-dev] [PATCH v2] " Mike Frysinger
2012-05-24 20:04   ` Zac Medico
2012-05-25 16:20     ` Mike Frysinger
2012-05-31  0:18       ` Zac Medico
2012-05-31  0:42         ` Mike Frysinger
2012-05-31  1:00           ` Zac Medico
2012-05-31  1:57             ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox