Re: [gentoo-pms] [PATCH] EAPI must be at least a single char.

[Brian Harring]

On Sun, Sep 30, 2012 at 11:13:31AM +0200, Ulrich Mueller wrote:
> >>>>> On Sun, 30 Sep 2012, Brian Harring wrote:
>
> > The example regex allowed for
> > EAPI=
>
> > Which isn't used, alloewd, nor desired; at best, that's short hand
> > for EAPI=0.
>
> The regexp was never meant to match exactly what is allowed.
> It matches a superset, and if the matched substring turns out to be an
> illegal or unknown EAPI, then the PM can catch it after parsing.
>
> Otherwise, an empty EAPI is already forbidden by section 3.1
> "Restrictions upon Names".

Going by the fact this sort of scenario has come up before for me, I 
guess I just do things differently.

The rules are such that "either an EAPI is matched, or EAPI=0 if the 
first non blank/non comment isn't a matching EAPI assignment".

You want the rules to be left as:

"Here's this regex you use to find the EAPI.  The actual rules of the 
EAPI value are off in another part of the doc that we're not going to 
refer to, but you need to make sure it still is valid despite this 
regex allowing invalid EAPI's in."

Respectfully, that is daft since it's going to result in anyone who 
tries to write a PM, likely fucking that up.  If what you were saying 
was the actual intention behind it, that assignment would've just been 
along the lines of EAPI=("[^"]*"|'[^']*'|[^\t ]); aka "here's how you 
grab what looks like an EAPI assignment".

Either way, I strongly disagree with your counterargument of leaving 
the spec as it is.

The following is saner:

^[ \t]*EAPI=(['"]?)([A-Za-z0-9_][A-Za-z0-9+_.-]*)\1[ \t]*([ \t]#.*)?$

And the following multiline match covers the rules in full (ie, ready 
to go meaning people aren't likely to fuck it up):

r"(?:^[ \t]*(?:#[^\n]*)?(?:$|\n))*^[ \t]*EAPI=(['\"]?)([A-Za-z0-9][A-Za-z0-9+_.-]*)\1"

Roughly, if the spec is left the way you intend it,

EAPI=-1 will get picked up, then it's reliant on the PM to spot 
it/explode; which in a quick check of portages source, it doesn't 
do.

Now, if a QA tool wants to use a looser version to spot EAPI 
assignments of an invalid EAPI, have at it; but the core spec's "find 
an eapi via this" should block invalid EAPI's from being picked up, 
period.

~harring