Re: [PATCH] bitbake: parse: allow vars_from_file to consider inc files as recipes

[Joe MacDonald <joe@deserted.net>]

[-- Attachment #1: Type: text/plain, Size: 3236 bytes --]

[Re: [bitbake-devel] [PATCH] bitbake: parse: allow vars_from_file to consider inc files as recipes] On 14.04.01 (Tue 09:58) Richard Purdie wrote:

> On Mon, 2014-03-31 at 16:44 -0400, joe@deserted.net wrote:
> > From: Joe MacDonald <joe@deserted.net>
> > 
> > A side-effect of making vars_from_file return None for non-recipes is that
> > PV gets 'None' if you have an included file named <recipe>.inc.  If there
> > is a recipe with a version number in the name for a .inc file, it's
> > probably reasonable to calculate PV based on that file, rather than giving
> > it 'None' (which becomes 1.0 in most cases).
> > 
> > Signed-off-by: Joe MacDonald <joe@deserted.net>
> > ---
> >  bitbake/lib/bb/parse/__init__.py |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > I ran into this when building meta-selinux where there's a chain of includes:
> > 
> >    refpolicy-mls_2.20130424.bb
> >     -> refpolicy_2.20130424.inc
> >       -> refpolicy_common.inc
> 
> The question to ask here is why is PV being expanded during the .inc
> file?
> 
> When this code was changed, we did find some issues like this but it
> usually means something unexpected was happening and "bad" PV values
> were actually being used in some cases. It was therefore a conscious
> decision not to do this.

Oh, okay.  I scanned back through the mailing list traffic from the time
when it went in but didn't see a discussion there, but likely it was a
more interactive thing.

> If I take this change, you could get PV of "common" being used in the
> second .inc file which is nasty.

I don't want to keep using meta-selinux as an example here, (since it
mostly ends up looking bad for meta-selinux, I suppose) but I can
actually see a case where -common would be organizationally desirable.
Or at least not ugly.  recipies-security/selinux has a collection of
core selinux recipes, each has their own patches directory which
potentially co-mingles patches for versioned and git-based recipes or
both.  Right now none of the recipes in there are doing the
<specific> -> <versioned> -> <common> include thing, but it's not a huge
stretch to imagine it being useful.

In other cases that'd be done with a versioned directory name and a
files/ directory, but that would make things less clear, not more in
this scenario.  So having a <recipie>-common directory would be kind of
good.

> I'd therefore much rather figure out what is happening to cause the
> expansion of PV in the .inc file and see if we can't avoid that.
> 
> Looking at the layer, the issue is:
> 
> FILESEXTRAPATHS_prepend := "${THISDIR}/refpolicy-${PV}:"
> 
> and I'd suggest that line move to the .bb files in this case.

That's what I'll do if you're not convinced on this patch, but when I
started down that path yesterday afternoon I got thinking about what I'd
laid out above and that I would be duplicating the exact same line in
three recipes right now (-mcs, -mls and -standard) and adding it to any
other new policies that get submitted.  That really is a common piece
shared among all the variants and it seemed like the right thing to do
was have it only in one place.

-- 
-Joe MacDonald.
:wq

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]