[OE-core] [PATCH v2 01/25] bitbake.conf: support for merged usr with DISTRO_FEATURE usrmerge

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces at lists.openembedded.org
> [mailto:openembedded-core-bounces at lists.openembedded.org] On Behalf Of
> Patrick Ohly
> Sent: den 7 juni 2017 16:32
> To: Richard Purdie <richard.purdie at linuxfoundation.org>
> Cc: Joshua Lock <joshua.g.lock at intel.com>; openembedded-
> core at lists.openembedded.org
> Subject: Re: [OE-core] [PATCH v2 01/25] bitbake.conf: support for
> merged usr with DISTRO_FEATURE usrmerge
> 
> On Wed, 2017-06-07 at 12:52 +0100, Richard Purdie wrote:
> > On Wed, 2017-02-22 at 10:26 +0200, Amarnath Valluri wrote:
> > > From: Joshua Lock <joshua.g.lock at intel.com>
> > >
> > > Modify bindir, libdir and sbindir to be exec_prefix/$d, rather than
> > > base_prefix/$d, when the usrmerge DISTRO_FEATURE is enabled.
> > >
> > > Signed-off-by: Joshua Lock <joshua.g.lock at intel.com>
> > > ---
> > >  meta/conf/bitbake.conf | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > I was asked to clarify the status of this series. We've taken many of
> > the changes but the core changes to bitbake.conf haven't been merged.
> >
> > Ross/I discussed it and we both feel that the bitbake.conf make the
> > file pretty unreadable. We'd therefore like to see this implemented
> > as a .inc file which gets included when the relevant DISTRO_FEATURE 
> > is set (or could just be included by the appropriate distro configs). 
> > This .inc file would then simply override the paths. I believe this 
> > would be a lot more readable than the current approach which is a 
> > key concern in the core config.

I actually suggested an alternative change to bitbake.conf in the GitHub 
review that changes bitbake.conf to take usrmerge into account:

https://github.com/avalluri/openembedded-core/commit/1655a93238902a883052a55d88ae14dd02243530

I will include my suggested solution here since I will refer to it 
below:

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 0f27e92e96..305eaff583 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -17,11 +17,13 @@ export base_prefix = ""
 export prefix = "/usr"
 export exec_prefix = "${prefix}"
 
+root_prefix = "${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '${exec_prefix}', '${base_prefix}', d)}"
+
 # Base paths
-export base_bindir = "${base_prefix}/bin"
-export base_sbindir = "${base_prefix}/sbin"
-export base_libdir = "${base_prefix}/${baselib}"
-export nonarch_base_libdir = "${base_prefix}/lib"
+export base_bindir = "${root_prefix}/bin"
+export base_sbindir = "${root_prefix}/sbin"
+export base_libdir = "${root_prefix}/${baselib}"
+export nonarch_base_libdir = "${root_prefix}/lib"
 
 # Architecture independent paths
 export sysconfdir = "${base_prefix}/etc"

I think that better maintains the readability of the bitbake.conf file, 
while making the $root_prefix variable available as it can be useful in 
its own (e.g., in the systemd recipe).

> > The merge of the rest of this feature is therefore pending on the
> > development of such a patch, or a discussion convincing Ross/I why we
> > shouldn't take that approach for some reason.
> 
> Such a .inc file would have to be included after the "Include the rest
> of the config files." section, because DISTRO_FEATURES only has its
> final value (?) at that point. One also needs the patch that I
> coincidentally just sent today to the bitbake list which allows
> including such a file only when "usrmerge" is in DISTRO_FEATURES.
> 
> Would that be okay?
> 
> One downside is that code using the variables earlier will just see the
> non-usrmerge version, with no indication in "bitbake -e" that the value
> might have been different in the middle of parsing. The if check in the
> proposed patch has the same effect on the actual values, but at least
> it shows up in "bitbake -e". This is not a particularly strong argument
> either way, I just wanted to mention it.

Based on your concerns about when the path variables affected by 
usrmerge would have what values, maybe my suggested change above 
should be changed to define root_prefix like this instead:

root_prefix ?= "${base_prefix}"

and then we create a usrmerge.inc file with the following contents:

DISTRO_FEATURES_append = " usrmerge"

root_prefix = "${exec_prefix}"

This file should then be required by a layer.conf file so that 
root_prefix is set before bitbake.conf is read. That should make sure 
that there never is any risk of the paths being defined differently 
based on whether usrmerge has been added to DISTRO_FEATURES yet or not.
And it should be clear in bitbake -e how such a variable got its value.

The only minor drawback I see is that the usrmerge.inc file needs to 
be required from a layer.conf file rather than just adding usrmerge 
to DISTRO_FEATURES in ${DISTRO}.conf, but at least I could live with 
that. One could of course add an appropriate comment before the 
definition of the root_prefix variable, to clarify its use and maybe 
even point at the usrmerge.inc file.

> --
> Best Regards, Patrick Ohly
> 
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.

//Peter