gmane.linux.gentoo.portage.devel

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

Mike Frysinger — 2012-05-25T16:20:00

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

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

Mike Frysinger — 2012-05-25T16:18:33

OK, i pushed with that fix
-mike

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_escap

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 Non

[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

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:
+#

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.auto

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