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

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

[Mike Frysinger]

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@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
@@ -750,11 +750,46 @@ _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):
-- 
1.7.8.6

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

[Zac Medico]

On 05/23/2012 09:06 PM, Mike Frysinger wrote:
> 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.

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'(^|[^\\])\\$'.
-- 
Thanks,
Zac

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

[Kent Fredric]

On 24 May 2012 16:06, Mike Frysinger <vapier@gentoo.org> wrote:
> 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 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.



-- 
Kent

perl -e  "print substr( \"edrgmaM  SPA NOcomil.ic\\@tfrken\", \$_ * 3,
3 ) for ( 9,8,0,7,1,6,5,4,3,2 );"

http://kent-fredric.fox.geek.nz

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

[Mike Frysinger]

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

On Thursday 24 May 2012 06:27:36 Kent Fredric wrote:
> ( 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.

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

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

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

[Mike Frysinger]

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

On Thursday 24 May 2012 00:19:45 Zac Medico wrote:
> On 05/23/2012 09:06 PM, Mike Frysinger wrote:
> > 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.
> 
> 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 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.

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

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

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

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

[Mike Frysinger]

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@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
@@ -759,6 +759,7 @@ _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)
@@ -772,19 +773,54 @@ 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()
-- 
1.7.8.6

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

[Zac Medico]

On 05/24/2012 12:20 PM, Mike Frysinger wrote:
> +			# 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')

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')
-- 
Thanks,
Zac

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

[Zac Medico]

On 05/24/2012 12:52 PM, Zac Medico wrote:
> On 05/24/2012 12:20 PM, Mike Frysinger wrote:
>> +			# 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')
> 
> 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]

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

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


-- 
Thanks,
Zac

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

[Kent Fredric]

On 25 May 2012 04:18, Mike Frysinger <vapier@gentoo.org> wrote:
>> 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.

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?

-- 
Kent

perl -e  "print substr( \"edrgmaM  SPA NOcomil.ic\\@tfrken\", \$_ * 3,
3 ) for ( 9,8,0,7,1,6,5,4,3,2 );"

http://kent-fredric.fox.geek.nz

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

[Mike Frysinger]

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@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
@@ -5,6 +5,7 @@
 """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
@@ -757,8 +758,11 @@ _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)
@@ -772,19 +776,54 @@ 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()
-- 
1.7.8.6

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

[Zac Medico]

On 05/24/2012 10:22 PM, Mike Frysinger wrote:
> 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@gentoo.org>
> ---
> v3
> 	- use import codecs for escaping strings

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

> +		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 SystemExit:
+			raise

> +		except:


-- 
Thanks,
Zac

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

[Mike Frysinger]

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

On Friday 25 May 2012 04:47:24 Zac Medico wrote:
> On 05/24/2012 10:22 PM, Mike Frysinger wrote:
> > 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.
> 
> It looks good to me, except for this one part, where we should let
> SystemExit raise:

OK, i pushed with that fix
-mike

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