From: Brian Harring <ferringb@gmail.com>
To: Ulrich Mueller <ulm@gentoo.org>
Cc: gentoo-pms@lists.gentoo.org
Subject: Re: [gentoo-pms] [PATCH] EAPI must be at least a single char.
Date: Sun, 30 Sep 2012 12:44:55 -0700 [thread overview]
Message-ID: <20120930194455.GB2180@localhost> (raw)
In-Reply-To: <20584.3387.660954.8184@a1i15.kph.uni-mainz.de>
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
next prev parent reply other threads:[~2012-09-30 19:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-30 8:28 [gentoo-pms] [PATCH] EAPI must be at least a single char Brian Harring
2012-09-30 9:13 ` Ulrich Mueller
2012-09-30 19:44 ` Brian Harring [this message]
2012-09-30 19:52 ` Zac Medico
2012-09-30 22:10 ` Ulrich Mueller
2012-10-01 0:48 ` Zac Medico
2012-09-30 22:22 ` Brian Harring
2012-10-01 0:25 ` Zac Medico
2012-10-01 7:15 ` Ulrich Mueller
2012-10-01 16:33 ` Zac Medico
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120930194455.GB2180@localhost \
--to=ferringb@gmail.com \
--cc=gentoo-pms@lists.gentoo.org \
--cc=ulm@gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox