Re: [gentoo-portage-dev] [PATCH] Add repoman check to warn if src_prepare/src_configure are used in EAPI 0/1

[Tom Wijsman]

On Mon, 13 Jan 2014 19:50:44 -0500
Mike Frysinger <vapier@g.o> wrote:

> On Monday 13 January 2014 19:08:30 creffett@g.o wrote:
> > From: Chris Reffett <creffett@g.o>
> > Subject: [gentoo-portage-dev] [PATCH] Add repoman check to warn if
> > src_prepare/src_configure are used in EAPI 0/1

Well, I was planning to cover these bugs; seems we really need to
organize with regards to avoiding double work, as two others have got
the same happen to them. Let's synchronize it.

So, what you have done here is wrote a partial patch for this bug:

https://bugs.gentoo.org/show_bug.cgi?id=379491

Therefore I suggest you that you mention it in the commit subject.

Now that you're working on phases, I wonder whether you plan to deal
with the other bug as well:

https://bugs.gentoo.org/show_bug.cgi?id=365691

While vapier's case could be considered as valid; I think we should
consider that as a bad practice, as one could just as well put the if
inside a phase which is the much more common practice.

Can you let me know whether you would work on this one as well as to
avoid both of us doing the same work on these?

As a note, my WIP work is at the next url; there you can see the bug
numbers I've been working on (still need to revise them, properly
document the added check errors [eg. in `man repoman`] and so on).

https://github.com/TomWij/gentoo-portage-next/branches

If you do something similar, we could check whether we attempt to work
on the same bug; as to avoid clashing. (In this case I think I told
you in #gentoo-qa that I planned to do this; but well, then I can be
lucky that I've not started on it. ^^)

> you might want to set ~/.gitconfig to have your name/email so that
> git sendemail uses the right info in the actual e-mail From: field

Then what is the From: field that it puts separately into the e-mail
itself? Seems quite odd for these to differ; or, is the latter an
attempt to mark the author? Then why does the e-mail From: field need
to be correct? Afaik it would only use one of both, so only one of both
needs to be correct. But that would make the actual question: Which one?
 
> > +	undefined_phases_re =
> > re.compile(r'^\s*(src_configure|src_prepare)\s*\(\)')
>
> you use re.match, so technically the ^ is redundant

Indeed, they match on a line basis from the beginning of the string.

> i'd suggest merging the regex a little:
> 	r'^\ssrc_(configure|prepare)\s*\(\)'

Note that the * behind the first \s was lost; so, with the redundant ^
removed and the * put back it would be:

        r'\s*src_(configure|prepare)\s*\(\)'

You can then proceed further and move the re outside:

        r'\s*src_(configu|prepa)re\s*\(\)'

The closing parentheses can also be removed because phase( ) is valid:

        r'\s*src_(configu|prepa)re\s*\('

(We don't need to account for "phase( invalid )" here; as that results
in invalid Bash syntax, which would bail out harder than our checks)

But if we would do that, it would be like:

        r'\s*src_(configu|prepa)re\s*\(\s*\)'

> could check for pkg_pretend/pkg_info too (it's new to EAPI 4)

+1

> the regex is naive and can match valid ebuilds.  consider ones that
> handle $EAPI itself and will call the right funcs as necessary.

From a QA point of view it seems more preferable to move away from old
EAPIs, than to conditionally support them. The common case is that
ebuilds move from older to newer EAPI and thus would get these
functions as they get to a newer EAPI.

Is there an actual case for downgrading back to an older EAPI?

If not, conditional code that checks $EAPI has no purpose.

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : TomWij@g.o
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D