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

[Patrick Ohly <patrick.ohly@intel.com>]

On Wed, 2017-06-07 at 17:04 +0000, Peter Kjellerstedt wrote:

> 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).

Looks good to me.

> > 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's already a big no-no to me. It would get us back to where merely
adding a layer makes fundamental build configuration changes. Including
it in a distro config would not solve the problem and still be less
flexible than the current approach (user cannot enable or disable the
feature merely via changing DISTRO_FEATURES in local.conf).

So I think we have to and can live with the paths changing during base
configuration parsing.

Perhaps we can move the definitions below the include section? That
would emphasize that they are only final at that point and might flush
out any unwanted evaluation of them during the include phase.

-- 
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.