gmane.linux.gentoo.portage.devel

Re: [PATCH v3] repoman: unroll escaped lines so we can check the entirety of it

Zac Medico — 2012-05-25T08:47:24

It looks good to me, except for this one part, where we should let
SystemExit raise:


+except SystemExit:
+raise

[PATCH v3] repoman: unroll escaped lines so we can check the entirety of it

Mike Frysinger — 2012-05-25T05:22:50

Sometimes people wrap long lines in their ebuilds to make it easier to
read, but this causes us issues when doing line-by-line checking.  So
automatically unroll those lines before passing the full content down
to our checkers.

Signed-off-by: Mike Frysinger <vapier< at >gentoo.org>
---
v3
- use import codecs for escaping strings

 pym/repoman/checks.py |   63 +++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/pym/repoman/checks.py b/pym/repoman/checks.py
index c17a0bd..402169e 100644
--- a/pym/repoman/checks.py
+++ b/pym/repoman/checks.py
< at >< at > -5,6 +5,7 < at >< at >
 """This module contains functions used in Repoman to ascertain the quality
 and correctness of an ebuild."""
 
+import codecs
 import re
 import time
 import repoman.errors as errors
< at >< at > -757,8 +758,11 < at >< at > _here_doc_re = re.compile(r'.*\s<<[-]?(\w+)$')
 _ignore_comment_re = re.compile(r'^\s*#')
 
 def run_checks(contents, pkg):
+unicode_escape_codec = codecs.lookup('unicode_escape')
+unicode_escape = lambda x: unicode_escape_codec.decode(x)[0]
 checks = _constant_checks
 here_doc_delim = None
+multiline = None
 
 for lc in checks:
 lc.new(pkg)
< at >< at > -772,19 +776,54 < at >< at > def run_checks(contents, pkg):
 here_doc = _here_doc_re.match(line)
 if here_doc is not None:
 here_doc_delim = re.compile(r'^\s*%s$' % here_doc.group(1))
+if here_doc_delim is not None:
+continue
+
+# Unroll multiline escaped strings so that we can check things:
+#inherit foo bar \
+#moo \
+#cow
+# This will merge these lines like so:
+#inherit foo bar moo cow
+try:
+# A normal line will end in the two bytes: <\> <\n>.  So decoding
+# that will result in python thinking the <\n> is being escaped
+# and eat the single <\> which makes it hard for us to detect.
+# Instead, strip the newline (which we know all lines have), and
+# append a <0>.  Then when python escapes it, if the line ended
+# in a <\>, we'll end up with a <\0> marker to key off of.  This
+# shouldn't be a problem with any valid ebuild ...
+line_escaped = unicode_escape(line.rstrip('\n') + '0')
+except:
+# Who knows what kind of crazy crap an ebuild will have
+# in it -- don't allow it to kill us.
+line_escaped = line
+if multiline:
+# Chop off the \ and \n bytes from the previous line.
+multiline = multiline[:-2] + line
+if not line_escaped.endswith('\0'):
+line = multiline
+num = multinum
+multiline = None
+else:
+continue
+else:
+if line_escaped.endswith('\0'):
+multinum = num
+multiline = line
+continue
 
-if here_doc_delim is None:
-# We're not in a here-document.
-is_comment = _ignore_comment_re.match(line) is not None
-for lc in checks:
-if is_comment and lc.ignore_comment:
-continue
-if lc.check_eapi(pkg.metadata['EAPI']):
-ignore = lc.ignore_line
-if not ignore or not ignore.match(line):
-e = lc.check(num, line)
-if e:
-yield lc.repoman_check_name, e % (num + 1)
+# Finally we have a full line to parse.
+is_comment = _ignore_comment_re.match(line) is not None
+for lc in checks:
+if is_comment and lc.ignore_comment:
+continue
+if lc.check_eapi(pkg.metadata['EAPI']):
+ignore = lc.ignore_line
+if not ignore or not ignore.match(line):
+e = lc.check(num, line)
+if e:
+yield lc.repoman_check_name, e % (num + 1)
 
 for lc in checks:
 i = lc.end()

Re: [RFC/PATCH] repoman: unroll escaped lines so we can check the entirety of it

Kent Fredric — 2012-05-24T21:43:48

Hmm,  http://devmanual.gentoo.org/ebuild-writing/file-format/index.html#indenting-and-whitespace

does sort of imply a "keep it under 80 chars where possible", but vims
wrap settings don't really see it like that, vim is more "80
characters, if you can't get it under that, WRAP" .

http://devmanual.gentoo.org/ebuild-writing/variables/index.html#required-variables

Indicates "less than 80", but 79 characters + 14 for 'DESCRIPTION=""'
= 93 so if you're pushing the description limitation, vim will try
wrapping each and every time you type a new letter.

And repoman doesn't even warn till you hit a 100 character description
(  col 114 )

So is there something that specifies this somewhere, or does that
required-variables and indenting-and-whitespace section need to be
made updated and made more clear, that

Description can be up to 100 ( not 80 ) and that the text-width is
merely a suggestion, not any sort of
defacto-but-hard-to-find-documented standard?

Re: [PATCH v2] repoman: unroll escaped lines so we can check the entirety of it

Zac Medico — 2012-05-24T20:08:24

-return unicode_escape_codec(s)[0]
+return unicode_escape_codec.decode(s)[0]

Re: [PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out

Zac Medico — 2012-05-24T20:04:30

Looks good to me.

Re: [PATCH v2] repoman: unroll escaped lines so we can check the entirety of it

Zac Medico — 2012-05-24T19:52:23

That decode('string_escape') method won't work in python3, because the
str object doesn't have a decode method. I think something like this
will work with both python3 and python2:

import codecs

unicode_escape_codec = codecs.lookup('unicode_escape')

def unicode_escape(s):
return unicode_escape_codec(s)[0]

line_escaped = unicode_escape(line.rstrip('\n') + '0')

[PATCH v2] repoman: unroll escaped lines so we can check the entirety of it

Mike Frysinger — 2012-05-24T19:20:24

Sometimes people wrap long lines in their ebuilds to make it easier to
read, but this causes us issues when doing line-by-line checking.  So
automatically unroll those lines before passing the full content down
to our checkers.

Signed-off-by: Mike Frysinger <vapier< at >gentoo.org>
---
v2
- re-order heredoc/multiline checking

 pym/repoman/checks.py |   60 +++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/pym/repoman/checks.py b/pym/repoman/checks.py
index c17a0bd..cd8d3d2 100644
--- a/pym/repoman/checks.py
+++ b/pym/repoman/checks.py
< at >< at > -759,6 +759,7 < at >< at > _ignore_comment_re = re.compile(r'^\s*#')
 def run_checks(contents, pkg):
 checks = _constant_checks
 here_doc_delim = None
+multiline = None
 
 for lc in checks:
 lc.new(pkg)
< at >< at > -772,19 +773,54 < at >< at > def run_checks(contents, pkg):
 here_doc = _here_doc_re.match(line)
 if here_doc is not None:
 here_doc_delim = re.compile(r'^\s*%s$' % here_doc.group(1))
+if here_doc_delim is not None:
+continue
+
+# Unroll multiline escaped strings so that we can check things:
+#inherit foo bar \
+#moo \
+#cow
+# This will merge these lines like so:
+#inherit foo bar moo cow
+try:
+# A normal line will end in the two bytes: <\> <\n>.  So decoding
+# that will result in python thinking the <\n> is being escaped
+# and eat the single <\> which makes it hard for us to detect.
+# Instead, strip the newline (which we know all lines have), and
+# append a <0>.  Then when python escapes it, if the line ended
+# in a <\>, we'll end up with a <\0> marker to key off of.  This
+# shouldn't be a problem with any valid ebuild ...
+line_escaped = (line.rstrip('\n') + '0').decode('string_escape')
+except:
+# Who knows what kind of crazy crap an ebuild will have
+# in it -- don't allow it to kill us.
+line_escaped = line
+if multiline:
+# Chop off the \ and \n bytes from the previous line.
+multiline = multiline[:-2] + line
+if not line_escaped.endswith('\0'):
+line = multiline
+num = multinum
+multiline = None
+else:
+continue
+else:
+if line_escaped.endswith('\0'):
+multinum = num
+multiline = line
+continue
 
-if here_doc_delim is None:
-# We're not in a here-document.
-is_comment = _ignore_comment_re.match(line) is not None
-for lc in checks:
-if is_comment and lc.ignore_comment:
-continue
-if lc.check_eapi(pkg.metadata['EAPI']):
-ignore = lc.ignore_line
-if not ignore or not ignore.match(line):
-e = lc.check(num, line)
-if e:
-yield lc.repoman_check_name, e % (num + 1)
+# Finally we have a full line to parse.
+is_comment = _ignore_comment_re.match(line) is not None
+for lc in checks:
+if is_comment and lc.ignore_comment:
+continue
+if lc.check_eapi(pkg.metadata['EAPI']):
+ignore = lc.ignore_line
+if not ignore or not ignore.match(line):
+e = lc.check(num, line)
+if e:
+yield lc.repoman_check_name, e % (num + 1)
 
 for lc in checks:
 i = lc.end()

[PATCH v2] repoman: add a mini framework for checking eclasses, and fill it out

Mike Frysinger — 2012-05-24T19:20:13

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< at >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
< at >< at > -315,8 +315,9 < at >< at > 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",
< at >< at > -382,7 +383,6 < at >< at > 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",
< at >< at > -425,7 +425,7 < at >< at > 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
< at >< at > -331,24 +331,6 < at >< at > 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
< at >< at > -464,43 +446,124 < at >< at > 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):
 """
< at >< at > -679,8 +742,10 < at >< at > _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
< at >< at > -19,7 +19,6 < at >< at > 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'

Re: [PATCH] repoman: add a mini framework for checking eclasses, and fill it out

Mike Frysinger — 2012-05-24T19:11:27

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


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

Re: [RFC/PATCH] repoman: unroll escaped lines so we can check the entirety of it

Mike Frysinger — 2012-05-24T16:33:09

i was thinking this would handle wrapped lines and heredocs together better, 
but i'll ignore that until i can come up with a concrete case.


the reason i didn't go the regex route is because this fails with:
echo foo \\\\\
cow
whereas letting python take care of all the escaping works much better
-mike

Re: [RFC/PATCH] repoman: unroll escaped lines so we can check the entirety of it

Mike Frysinger — 2012-05-24T16:18:15

there is no policy dictating line length.  but certainly lines that are 200+ 
chars should get wrapped ...

as for changing the vim settings, you'll have to file a request with the vim 
maintainer.
-mike

Re: [RFC/PATCH] repoman: unroll escaped lines so we can check the entirety of it

Kent Fredric — 2012-05-24T10:27:36


This may be a result of the default vim settings that get enabled if
you're editing an ebuild.

If you don't manually wrap the lines, vim will do it for you, because
the ebuild profile for vim ( app-vim/gentoo-syntax ,
/usr/share/vim/vimfiles/ftplugin/ebuild.vim ) sets textwidth=80

This in my personal experience is unhelpful, and every time the text
wrap fires it risks producing broken code, and I've resorted to
manually wrapping things with the proper escapes just to avoid being
attacked by the wrap.

( I had assumed that the forced 80 char text width was something that
was enforced by policy and wasn't game to try bucking the curve )

If there is no such policy, and a forced text-wrap at 80 characters is
not needed, I would love for that setting to be removed.

Re: [RFC/PATCH] repoman: unroll escaped lines so we can check the entirety of it

Zac Medico — 2012-05-24T04:19:45

This code should come right after the line that says "We're not in a
here-document", because we only need it to trigger when we're not in a
here-document.

I think it's going to be cleaner to detect an escaped newline with a
regular expression, like r'(^|[^\\])\\$'.

[RFC/PATCH] repoman: unroll escaped lines so we can check the entirety of it

Mike Frysinger — 2012-05-24T04:06:39

Sometimes people wrap long lines in their ebuilds to make it easier to
read, but this causes us issues when doing line-by-line checking.  So
automatically unroll those lines before passing the full content down
to our checkers.

This seems to work, but maybe someone can suggest something simpler.

Signed-off-by: Mike Frysinger <vapier< at >gentoo.org>
---
 pym/repoman/checks.py |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/pym/repoman/checks.py b/pym/repoman/checks.py
index 65e7136..67f2b0a 100644
--- a/pym/repoman/checks.py
+++ b/pym/repoman/checks.py
< at >< at > -750,11 +750,46 < at >< at > _ignore_comment_re = re.compile(r'^\s*#')
 def run_checks(contents, pkg):
 checks = _constant_checks
 here_doc_delim = None
+multiline = None
 
 for lc in checks:
 lc.new(pkg)
 for num, line in enumerate(contents):
 
+# Unroll multiline escaped strings so that we can check things:
+#inherit foo bar \
+#moo \
+#cow
+# This will merge these lines like so:
+#inherit foo bar moo cow
+try:
+# A normal line will end in the two bytes: <\> <\n>.  So decoding
+# that will result in python thinking the <\n> is being escaped
+# and eat the single <\> which makes it hard for us to detect.
+# Instead, strip the newline (which we know all lines have), and
+# append a <0>.  Then when python escapes it, if the line ended
+# in a <\>, we'll end up with a <\0> marker to key off of.  This
+# shouldn't be a problem with any valid ebuild ...
+line_escaped = (line.rstrip('\n') + '0').decode('string_escape')
+except:
+# Who knows what kind of crazy crap an ebuild will have
+# in it -- don't allow it to kill us.
+line_escaped = line
+if multiline:
+# Chop off the \ and \n bytes from the previous line.
+multiline = multiline[:-2] + line
+if not line_escaped.endswith('\0'):
+line = multiline
+num = multinum
+multiline = None
+else:
+continue
+else:
+if line_escaped.endswith('\0'):
+multinum = num
+multiline = line
+continue
+
 # Check if we're inside a here-document.
 if here_doc_delim is not None:
 if here_doc_delim.match(line):

Re: [PATCH] repoman: add a mini framework for checking eclasses, and fill it out

Zac Medico — 2012-05-24T04:01:06

Seems reasonable.


Yeah, we could do that for all escaped newlines (not just the ones
involving 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.

Re: [PATCH] repoman: add a mini framework for checking eclasses, and fill it out

Mike Frysinger — 2012-05-24T03:15:05

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

Re: [PATCH] repoman: add a mini framework for checking eclasses, and fill it out

Zac Medico — 2012-05-23T19:52:11

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.



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.


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

[PATCH] repoman: add a mini framework for checking eclasses, and fill it out

Mike Frysinger — 2012-05-23T19:21:51

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< at >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
< at >< at > -315,8 +315,9 < at >< at > 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",
< at >< at > -382,7 +383,6 < at >< at > 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",
< at >< at > -425,7 +425,7 < at >< at > 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
< at >< at > -331,24 +331,6 < at >< at > 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
< at >< at > -464,43 +446,104 < at >< at > 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):
 """
< at >< at > -679,8 +722,10 < at >< at > _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
< at >< at > -19,7 +19,6 < at >< at > 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'

Re: About some settings to auto-replace with dispatch-conf

Pacho Ramos — 2012-05-15T19:46:49

El mar, 15-05-2012 a las 21:43 +0200, Pacho Ramos escribió:

Just noticed the problem of current behavior after updating my chroot to
generate emul packages and needing to review 40 files with
dispatch-conf, all of them never touched and all of them needing to be
updated to new version

Re: About some settings to auto-replace with dispatch-conf

Pacho Ramos — 2012-05-15T19:43:58

El mar, 15-05-2012 a las 08:31 -0700, Zac Medico escribió:

Didn't know about that option, thanks for pointing it :D

About trying to enable it by default, I think would make sense per
previous exposed reasons (and I am sure there are more examples that
could show that behavior is better than keeping obsolete config files by
default)

Re: About some settings to auto-replace with dispatch-conf

Zac Medico — 2012-05-15T15:31:38

FEATURES=config-protect-if-modified is what you really want. We could
probably enable it by default, but we should ask for comment on the
gentoo-dev mailing list before doing that.