diff mbox series

[1/7] scripts/oe-setup-builddir: add a check that TEMPLATECONF is valid

Message ID 20220831111401.3330342-1-alex@linutronix.de
State Accepted, archived
Commit c6f2b57be8893ee58f20cc29d8ec3a5a6edf7c07
Headers show
Series [1/7] scripts/oe-setup-builddir: add a check that TEMPLATECONF is valid | expand

Commit Message

Alexander Kanavin Aug. 31, 2022, 11:13 a.m. UTC
specifically that ../../layer.conf exists, and that second-from-last
component in the path is 'templates'.

This requires tweaking template.conf creation in eSDK bbclass, as
we need to ensure that the path in it is valid, and exists
(which may not be the case if the SDK is poky-based).

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/classes-recipe/populate_sdk_ext.bbclass | 3 ++-
 scripts/oe-setup-builddir                    | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

ChenQi Sept. 14, 2022, 3:27 a.m. UTC | #1
Hi Alex,

Why must TEMPLATECONF be under a layer?

Regards,
Qi

On 8/31/22 19:13, Alexander Kanavin wrote:
> specifically that ../../layer.conf exists, and that second-from-last
> component in the path is 'templates'.
>
> This requires tweaking template.conf creation in eSDK bbclass, as
> we need to ensure that the path in it is valid, and exists
> (which may not be the case if the SDK is poky-based).
>
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>   meta/classes-recipe/populate_sdk_ext.bbclass | 3 ++-
>   scripts/oe-setup-builddir                    | 5 +++++
>   2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes-recipe/populate_sdk_ext.bbclass b/meta/classes-recipe/populate_sdk_ext.bbclass
> index 56e24c4eed..925cb313fc 100644
> --- a/meta/classes-recipe/populate_sdk_ext.bbclass
> +++ b/meta/classes-recipe/populate_sdk_ext.bbclass
> @@ -438,7 +438,8 @@ python copy_buildsystem () {
>       else:
>           # Write a templateconf.cfg
>           with open(baseoutpath + '/conf/templateconf.cfg', 'w') as f:
> -            f.write('meta/conf\n')
> +            f.write('meta/conf/templates/default\n')
> +        os.makedirs(os.path.join(baseoutpath, core_meta_subdir, 'conf/templates/default'), exist_ok=True)
>   
>       # Ensure any variables set from the external environment (by way of
>       # BB_ENV_PASSTHROUGH_ADDITIONS) are set in the SDK's configuration
> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
> index 5d644168cb..bf832ee0ca 100755
> --- a/scripts/oe-setup-builddir
> +++ b/scripts/oe-setup-builddir
> @@ -61,6 +61,11 @@ if [ -n "$TEMPLATECONF" ]; then
>               echo >&2 "Error: TEMPLATECONF value points to nonexistent directory '$TEMPLATECONF'"
>               exit 1
>           fi
> +        templatesdir=$(python3 -c "import sys; print(sys.argv[1].strip('/').split('/')[-2])" $TEMPLATECONF)
> +        if [ ! -f "$TEMPLATECONF/../../layer.conf" -o $templatesdir != "templates" ]; then
> +            echo >&2 "Error: TEMPLATECONF value (which is $TEMPLATECONF) must point to meta-some-layer/conf/templates/template-name"
> +            exit 1
> +        fi
>       fi
>       OECORELAYERCONF="$TEMPLATECONF/bblayers.conf.sample"
>       OECORELOCALCONF="$TEMPLATECONF/local.conf.sample"
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#170101): https://lists.openembedded.org/g/openembedded-core/message/170101
> Mute This Topic: https://lists.openembedded.org/mt/93368468/3618072
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Alexander Kanavin Sept. 14, 2022, 5 a.m. UTC | #2
Hello,

this is done for reasons of discoverability and consistency with
machine and distro definitions: they all should be under
meta-somelayer/conf/. This way both tools and humans can easily find
the config templates, and be certain that this is the only location..

Alex

On Wed, 14 Sept 2022 at 05:27, ChenQi <Qi.Chen@windriver.com> wrote:
>
> Hi Alex,
>
> Why must TEMPLATECONF be under a layer?
>
> Regards,
> Qi
>
> On 8/31/22 19:13, Alexander Kanavin wrote:
> > specifically that ../../layer.conf exists, and that second-from-last
> > component in the path is 'templates'.
> >
> > This requires tweaking template.conf creation in eSDK bbclass, as
> > we need to ensure that the path in it is valid, and exists
> > (which may not be the case if the SDK is poky-based).
> >
> > Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > ---
> >   meta/classes-recipe/populate_sdk_ext.bbclass | 3 ++-
> >   scripts/oe-setup-builddir                    | 5 +++++
> >   2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/meta/classes-recipe/populate_sdk_ext.bbclass b/meta/classes-recipe/populate_sdk_ext.bbclass
> > index 56e24c4eed..925cb313fc 100644
> > --- a/meta/classes-recipe/populate_sdk_ext.bbclass
> > +++ b/meta/classes-recipe/populate_sdk_ext.bbclass
> > @@ -438,7 +438,8 @@ python copy_buildsystem () {
> >       else:
> >           # Write a templateconf.cfg
> >           with open(baseoutpath + '/conf/templateconf.cfg', 'w') as f:
> > -            f.write('meta/conf\n')
> > +            f.write('meta/conf/templates/default\n')
> > +        os.makedirs(os.path.join(baseoutpath, core_meta_subdir, 'conf/templates/default'), exist_ok=True)
> >
> >       # Ensure any variables set from the external environment (by way of
> >       # BB_ENV_PASSTHROUGH_ADDITIONS) are set in the SDK's configuration
> > diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
> > index 5d644168cb..bf832ee0ca 100755
> > --- a/scripts/oe-setup-builddir
> > +++ b/scripts/oe-setup-builddir
> > @@ -61,6 +61,11 @@ if [ -n "$TEMPLATECONF" ]; then
> >               echo >&2 "Error: TEMPLATECONF value points to nonexistent directory '$TEMPLATECONF'"
> >               exit 1
> >           fi
> > +        templatesdir=$(python3 -c "import sys; print(sys.argv[1].strip('/').split('/')[-2])" $TEMPLATECONF)
> > +        if [ ! -f "$TEMPLATECONF/../../layer.conf" -o $templatesdir != "templates" ]; then
> > +            echo >&2 "Error: TEMPLATECONF value (which is $TEMPLATECONF) must point to meta-some-layer/conf/templates/template-name"
> > +            exit 1
> > +        fi
> >       fi
> >       OECORELAYERCONF="$TEMPLATECONF/bblayers.conf.sample"
> >       OECORELOCALCONF="$TEMPLATECONF/local.conf.sample"
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#170101): https://lists.openembedded.org/g/openembedded-core/message/170101
> > Mute This Topic: https://lists.openembedded.org/mt/93368468/3618072
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@windriver.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>
ChenQi Sept. 14, 2022, 5:49 a.m. UTC | #3
On 9/14/22 13:00, Alexander Kanavin wrote:
> Hello,
>
> this is done for reasons of discoverability and consistency with
> machine and distro definitions: they all should be under
> meta-somelayer/conf/. This way both tools and humans can easily find
> the config templates, and be certain that this is the only location..
>
> Alex

Hi Alex,

I'm reluctant to agree that this is like machine and distro, because 
it's a hard requirement that machine and distro definitions be under 
some layer, otherwise how can bitbake get info about it? But 
TEMPLATECONF seems to be a different case, it could be everywhere 
because it's used by the project setup script.

In our case, the TEMPLATECONF is <project_dir>/config/, and layers are 
<project_dir>/<layer>/. This directory hierarchy has been working for 
years until recent changes.

Do you think such directory hierarchy makes sense? How about we deleting 
such check if there's no strong technical reason to do so? By 'strong 
technical reason', I mean that some codes in oe-core are written based 
on this assumption (this is the part I'm sure about).

Regards,

Qi


> On Wed, 14 Sept 2022 at 05:27, ChenQi <Qi.Chen@windriver.com> wrote:
>> Hi Alex,
>>
>> Why must TEMPLATECONF be under a layer?
>>
>> Regards,
>> Qi
>>
>> On 8/31/22 19:13, Alexander Kanavin wrote:
>>> specifically that ../../layer.conf exists, and that second-from-last
>>> component in the path is 'templates'.
>>>
>>> This requires tweaking template.conf creation in eSDK bbclass, as
>>> we need to ensure that the path in it is valid, and exists
>>> (which may not be the case if the SDK is poky-based).
>>>
>>> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
>>> ---
>>>    meta/classes-recipe/populate_sdk_ext.bbclass | 3 ++-
>>>    scripts/oe-setup-builddir                    | 5 +++++
>>>    2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/meta/classes-recipe/populate_sdk_ext.bbclass b/meta/classes-recipe/populate_sdk_ext.bbclass
>>> index 56e24c4eed..925cb313fc 100644
>>> --- a/meta/classes-recipe/populate_sdk_ext.bbclass
>>> +++ b/meta/classes-recipe/populate_sdk_ext.bbclass
>>> @@ -438,7 +438,8 @@ python copy_buildsystem () {
>>>        else:
>>>            # Write a templateconf.cfg
>>>            with open(baseoutpath + '/conf/templateconf.cfg', 'w') as f:
>>> -            f.write('meta/conf\n')
>>> +            f.write('meta/conf/templates/default\n')
>>> +        os.makedirs(os.path.join(baseoutpath, core_meta_subdir, 'conf/templates/default'), exist_ok=True)
>>>
>>>        # Ensure any variables set from the external environment (by way of
>>>        # BB_ENV_PASSTHROUGH_ADDITIONS) are set in the SDK's configuration
>>> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
>>> index 5d644168cb..bf832ee0ca 100755
>>> --- a/scripts/oe-setup-builddir
>>> +++ b/scripts/oe-setup-builddir
>>> @@ -61,6 +61,11 @@ if [ -n "$TEMPLATECONF" ]; then
>>>                echo >&2 "Error: TEMPLATECONF value points to nonexistent directory '$TEMPLATECONF'"
>>>                exit 1
>>>            fi
>>> +        templatesdir=$(python3 -c "import sys; print(sys.argv[1].strip('/').split('/')[-2])" $TEMPLATECONF)
>>> +        if [ ! -f "$TEMPLATECONF/../../layer.conf" -o $templatesdir != "templates" ]; then
>>> +            echo >&2 "Error: TEMPLATECONF value (which is $TEMPLATECONF) must point to meta-some-layer/conf/templates/template-name"
>>> +            exit 1
>>> +        fi
>>>        fi
>>>        OECORELAYERCONF="$TEMPLATECONF/bblayers.conf.sample"
>>>        OECORELOCALCONF="$TEMPLATECONF/local.conf.sample"
>>>
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>> Links: You receive all messages sent to this group.
>>> View/Reply Online (#170101): https://lists.openembedded.org/g/openembedded-core/message/170101
>>> Mute This Topic: https://lists.openembedded.org/mt/93368468/3618072
>>> Group Owner: openembedded-core+owner@lists.openembedded.org
>>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@windriver.com]
>>> -=-=-=-=-=-=-=-=-=-=-=-
>>>
Alexander Kanavin Sept. 14, 2022, 6:39 a.m. UTC | #4
On Wed, 14 Sept 2022 at 07:49, ChenQi <Qi.Chen@windriver.com> wrote:
> I'm reluctant to agree that this is like machine and distro, because
> it's a hard requirement that machine and distro definitions be under
> some layer, otherwise how can bitbake get info about it? But
> TEMPLATECONF seems to be a different case, it could be everywhere
> because it's used by the project setup script.
>
> In our case, the TEMPLATECONF is <project_dir>/config/, and layers are
> <project_dir>/<layer>/. This directory hierarchy has been working for
> years until recent changes.
>
> Do you think such directory hierarchy makes sense? How about we deleting
> such check if there's no strong technical reason to do so? By 'strong
> technical reason', I mean that some codes in oe-core are written based
> on this assumption (this is the part I'm sure about).

Again, it's not about only code. It's about humans too: we benefit
from having things where we expect them to be. If your templates are
in meta-layer/conf/templates/ you do not have to document, explain or
support this; anyone new to the project will simply pick this up from
prior experience or official documentation.

That said, there is already code that makes this assumption too: both
'bitbake-layers save-template-conf' and upcoming 'oe-setup-build' (the
patch was sent for review here) consider only
meta-layer/conf/templates.

I have to ask the same question to you: is there a strong technical
reason that you cannot move the templates to the new standard
location?

Alex
ChenQi Sept. 14, 2022, 7:42 a.m. UTC | #5
On 9/14/22 14:39, Alexander Kanavin wrote:
> On Wed, 14 Sept 2022 at 07:49, ChenQi <Qi.Chen@windriver.com> wrote:
>> I'm reluctant to agree that this is like machine and distro, because
>> it's a hard requirement that machine and distro definitions be under
>> some layer, otherwise how can bitbake get info about it? But
>> TEMPLATECONF seems to be a different case, it could be everywhere
>> because it's used by the project setup script.
>>
>> In our case, the TEMPLATECONF is <project_dir>/config/, and layers are
>> <project_dir>/<layer>/. This directory hierarchy has been working for
>> years until recent changes.
>>
>> Do you think such directory hierarchy makes sense? How about we deleting
>> such check if there's no strong technical reason to do so? By 'strong
>> technical reason', I mean that some codes in oe-core are written based
>> on this assumption (this is the part I'm sure about).
> Again, it's not about only code. It's about humans too: we benefit
> from having things where we expect them to be.

$TEMPLATECONF/bblayers.conf.sample has a list of layers. These layers 
may have dependencies on each other or they may not. Which layer do you 
think should this TEMPLATECONF locate?

TEMPLATECONF, by its nature, is a project setup variable. It logically 
does not belong to any layer.

Why would people expect some project level variable to point to some 
directory under a layer?


> If your templates are
> in meta-layer/conf/templates/ you do not have to document, explain or
> support this;

Again, why this meta-layer should have knowledge about the whole 
project? It should be the project that has knowledge about layer, not 
the other way around.


> anyone new to the project will simply pick this up from
> prior experience or official documentation.

When users see a file in a layer that refers to other layers which this 
layer does not depend on and not been dependent upon, they may ask why.


>
> That said, there is already code that makes this assumption too: both
> 'bitbake-layers save-template-conf' and upcoming 'oe-setup-build' (the
> patch was sent for review here) consider only
> meta-layer/conf/templates.

Give it a second thought.

> I have to ask the same question to you: is there a strong technical
> reason that you cannot move the templates to the new standard
> location?

Yes. The bblayers.conf.sample is generated dynamically according to 
project setup.

That said, I could of course create a useless layer that does nothing 
but only holds these sample files to satisfy this sanity check. But I do 
think this sanity check is logically wrong.

Again, project contains layers, project setup could choose to use layers 
the project wants. Forcing a project level variable to point to a 
location under a layer is not reasonable.

Regards,

Qi


> Alex
Alexander Kanavin Sept. 14, 2022, 8:03 a.m. UTC | #6
On Wed, 14 Sept 2022 at 09:42, ChenQi <Qi.Chen@windriver.com> wrote:

> Yes. The bblayers.conf.sample is generated dynamically according to
> project setup.

Can we step back for a moment, I'd like to know more about how you do
this. Is the code that does dynamic generation of the list in bblayers
published somewhere? How a decision to include (or not) a layer is
made?

Alex
ChenQi Sept. 14, 2022, 8:17 a.m. UTC | #7
On 9/14/22 16:03, Alexander Kanavin wrote:
> On Wed, 14 Sept 2022 at 09:42, ChenQi <Qi.Chen@windriver.com> wrote:
>
>> Yes. The bblayers.conf.sample is generated dynamically according to
>> project setup.
> Can we step back for a moment, I'd like to know more about how you do
> this. Is the code that does dynamic generation of the list in bblayers
> published somewhere? How a decision to include (or not) a layer is
> made?
>
> Alex

The codes are in: https://github.com/WindRiver-Labs/wrlinux-x

e.g.

./wrlinux-x/setup.sh --all-layers (this pulls down all supported layers)

./wrlinux-x/setup.sh --all-layers --dl-layers (this pulls down all 
supported layers, together with their needed tarballs/git repos)

./wrlinux-x/setup.sh (this pulls down the default list of layers)

./wrlinux-x/setup.sh --layers meta-xxx (this pulls down the default list 
of layers + meta-xxx and its dependencies)

And the final list of layers are in 
<project_dir>/config/bblayers.conf.sample, which will be updated if 
needed when re-running the setup.sh script.

Regards,

Qi
Alexander Kanavin Sept. 14, 2022, 8:42 a.m. UTC | #8
On Wed, 14 Sept 2022 at 10:17, ChenQi <Qi.Chen@windriver.com> wrote:
> The codes are in: https://github.com/WindRiver-Labs/wrlinux-x
>
> e.g.
>
> ./wrlinux-x/setup.sh --all-layers (this pulls down all supported layers)
>
> ./wrlinux-x/setup.sh --all-layers --dl-layers (this pulls down all
> supported layers, together with their needed tarballs/git repos)
>
> ./wrlinux-x/setup.sh (this pulls down the default list of layers)
>
> ./wrlinux-x/setup.sh --layers meta-xxx (this pulls down the default list
> of layers + meta-xxx and its dependencies)
>
> And the final list of layers are in
> <project_dir>/config/bblayers.conf.sample, which will be updated if
> needed when re-running the setup.sh script.

May I suggest that the tool skips the bblayers.conf.sample step
altogether, and simply writes the desired layers directly into
bblayers.conf? It can do this with 'bitbake-layers add-layer'. Is
there a need for doing the intermediate step of generating the
template?

Just to be clear where I am coming from: anything in TEMPLATECONF must
not be auto-generated during build setup, and must be under version
control. We are trying to standardize layer setup and configuration
handling, and this would be a necessary step towards that.

Alex
Peter Kjellerstedt Sept. 15, 2022, 1:07 a.m. UTC | #9
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Alexander Kanavin
> Sent: den 14 september 2022 09:42
> To: ChenQi <Qi.Chen@windriver.com>
> Cc: OE-core <openembedded-core@lists.openembedded.org>; Alexander Kanavin
> <alex@linutronix.de>
> Subject: Re: [OE-core] [PATCH 1/7] scripts/oe-setup-builddir: add a check
> that TEMPLATECONF is valid
> 
> On Wed, 14 Sept 2022 at 10:17, ChenQi <Qi.Chen@windriver.com> wrote:
> > The codes are in: https://github.com/WindRiver-Labs/wrlinux-x
> >
> > e.g.
> >
> > ./wrlinux-x/setup.sh --all-layers (this pulls down all supported layers)
> >
> > ./wrlinux-x/setup.sh --all-layers --dl-layers (this pulls down all
> > supported layers, together with their needed tarballs/git repos)
> >
> > ./wrlinux-x/setup.sh (this pulls down the default list of layers)
> >
> > ./wrlinux-x/setup.sh --layers meta-xxx (this pulls down the default list
> > of layers + meta-xxx and its dependencies)
> >
> > And the final list of layers are in
> > <project_dir>/config/bblayers.conf.sample, which will be updated if
> > needed when re-running the setup.sh script.
> 
> May I suggest that the tool skips the bblayers.conf.sample step
> altogether, and simply writes the desired layers directly into
> bblayers.conf? It can do this with 'bitbake-layers add-layer'. Is
> there a need for doing the intermediate step of generating the
> template?

I know you have suggested this to me too as we use a similar setup to 
generate the bblayers.conf.sample file based on the layers that repo 
has checked out in our environment, and now I realized why this is not 
as simple as it seems. The problem (at least in our case) is that we 
run as a wrapper around oe-init-build-env. This means that before the 
wrapper sources oe-init-build-env, the build directory and thus the 
conf directory inside it (where the bblayers.conf file is supposed to 
go) may not exist. And after it has sourced oe-init-build-env it is 
too late as the bblayers.conf file needs to exists before. So to create 
the bblayers.conf file, the wrapper would have to duplicate all the 
non-trivial code in oe-setup-builddir that determines what the build 
directory should be called. And while this of course is possible to 
do, it means duplicating code that is internal to bitbake, and risking 
missing important changes to the upstream code in the future. Thus it 
is much easier and safer to generate the template files and let 
oe-init-build-env do its job.

> 
> Just to be clear where I am coming from: anything in TEMPLATECONF must
> not be auto-generated during build setup, and must be under version
> control. We are trying to standardize layer setup and configuration
> handling, and this would be a necessary step towards that.

Where I (and apparently WindRiver) are coming from, the decision on what 
layers are part of the configuration is not something a specific layer 
is responsible for. I.e., in our setup we fetch the layers we want for 
a build, write the list of layers to bblayers.conf.sample and source 
oe-init-build-env. With your setup, if I want a project with the two 
layers meta-a and meta-b, I would also have to create a layer meta-x 
where I create a static bblayers.conf.sample file that lists these 
three layers. Then, if another project wants to use meta-a and meta-c, 
they would have to add a second static bblayers.conf.sample file in 
meta-x that lists those three layers. Basically every time there is a 
new combination of layers fetched, there would also have to be a 
duplicate of the information in a static bblayers.conf.sample file 
somewhere. I am probably missing something here, but to me this seems 
suboptimal. If the list of layers in the static file had been used to 
fetch the layers, I would better understand it, but then the problem 
becomes that the list is now in one of the arbitrary layers that are 
supposed to be fetched.

For reference, today when one of our teams start a new development 
project, what they do is they create a specific layer (e.g., meta-foo) 
for that project where they can do all their development work and 
experimentation until it is time for them to integrate to our main 
platform (referred to as cfp/cfp.xml below). Then they create a repo 
manifest that looks like: 

<?xml version="1.0" encoding="UTF-8"?>
<manifest>
  <include name="cfp/cfp.xml"/>

  <project path="meta-foo" name="layers/projects/meta-foo" revision="master"/>
</manifest>

in our manifest repository. This simple manifest is all the project 
team needs to create to build with their project specific layer. It 
also means that the only input to our Jenkins jobs to build for this 
project (instead of the main platform) is the name of the project's 
repo manifest. It also means that if we make any changes to the main 
platform, e.g., add a new layer (which is a one line change to the 
cfp.xml manifest), all projects will automatically get and use that 
new layer without having to make any changes to all the project 
manifests.

> 
> Alex

//Peter
Alexander Kanavin Sept. 15, 2022, 9:20 a.m. UTC | #10
On Thu, 15 Sept 2022 at 03:07, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> I know you have suggested this to me too as we use a similar setup to
> generate the bblayers.conf.sample file based on the layers that repo
> has checked out in our environment, and now I realized why this is not
> as simple as it seems. The problem (at least in our case) is that we
> run as a wrapper around oe-init-build-env. This means that before the
> wrapper sources oe-init-build-env, the build directory and thus the
> conf directory inside it (where the bblayers.conf file is supposed to
> go) may not exist. And after it has sourced oe-init-build-env it is
> too late as the bblayers.conf file needs to exists before. So to create
> the bblayers.conf file, the wrapper would have to duplicate all the
> non-trivial code in oe-setup-builddir that determines what the build
> directory should be called. And while this of course is possible to
> do, it means duplicating code that is internal to bitbake, and risking
> missing important changes to the upstream code in the future. Thus it
> is much easier and safer to generate the template files and let
> oe-init-build-env do its job.

Wait, I have to ask for details here. What is the non-trivial code
specifically? Isn't it simply

mkdir -p builddir/conf
write_out bblayers.conf

where builddir is the same argument that is passed to
oe-init-build-env? Something doesn't compute here for me.

> Where I (and apparently WindRiver) are coming from, the decision on what
> layers are part of the configuration is not something a specific layer
> is responsible for. I.e., in our setup we fetch the layers we want for
> a build, write the list of layers to bblayers.conf.sample and source
> oe-init-build-env. With your setup, if I want a project with the two
> layers meta-a and meta-b, I would also have to create a layer meta-x
> where I create a static bblayers.conf.sample file that lists these
> three layers. Then, if another project wants to use meta-a and meta-c,
> they would have to add a second static bblayers.conf.sample file in
> meta-x that lists those three layers. Basically every time there is a
> new combination of layers fetched, there would also have to be a
> duplicate of the information in a static bblayers.conf.sample file
> somewhere. I am probably missing something here, but to me this seems
> suboptimal. If the list of layers in the static file had been used to
> fetch the layers, I would better understand it, but then the problem
> becomes that the list is now in one of the arbitrary layers that are
> supposed to be fetched.

The idea is that yes, every possible combination of layers needs its
own template, and it goes under version control into a layer.
Typically this would be a layer that is common to all of your build
configurations (which is meta-a in the above example - *not* meta-x)
and that is where the templates go. Do you have such a layer?

I also have to ask, what prevents you from simply listing all of the
layers in a single unified bblayers.conf.sample. Do they step on each
other? How?

I also need to remind you that bblayers.conf(.sample) does support
includes, so you can implement a structured hierarchy of layer config
fragments, where the common bits are only written once.

Basically, there are alternatives, and I think all of them are better
than writing out the list of layers just before setting up a build
with a custom script.

> in our manifest repository. This simple manifest is all the project
> team needs to create to build with their project specific layer. It
> also means that the only input to our Jenkins jobs to build for this
> project (instead of the main platform) is the name of the project's
> repo manifest. It also means that if we make any changes to the main
> platform, e.g., add a new layer (which is a one line change to the
> cfp.xml manifest), all projects will automatically get and use that
> new layer without having to make any changes to all the project
> manifests.

This is somewhat tangential, as it is about extending the default
layer setup config, not the combinations of layers in the build
configuration. So far, the json is not extensible, but we can think
about how to extend and overlay it too. As far as development in
meta-foo goes, the templates (including layer lists) for it can be
hosted there, until the layer 'graduates' into the common structure.

Alex
Peter Kjellerstedt Sept. 15, 2022, 10:59 p.m. UTC | #11
> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: den 15 september 2022 10:20
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: ChenQi <Qi.Chen@windriver.com>; OE-core <openembedded-
> core@lists.openembedded.org>; Alexander Kanavin <alex@linutronix.de>
> Subject: Re: [OE-core] [PATCH 1/7] scripts/oe-setup-builddir: add a check
> that TEMPLATECONF is valid
> 
> On Thu, 15 Sept 2022 at 03:07, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> > I know you have suggested this to me too as we use a similar setup to
> > generate the bblayers.conf.sample file based on the layers that repo
> > has checked out in our environment, and now I realized why this is not
> > as simple as it seems. The problem (at least in our case) is that we
> > run as a wrapper around oe-init-build-env. This means that before the
> > wrapper sources oe-init-build-env, the build directory and thus the
> > conf directory inside it (where the bblayers.conf file is supposed to
> > go) may not exist. And after it has sourced oe-init-build-env it is
> > too late as the bblayers.conf file needs to exists before. So to create
> > the bblayers.conf file, the wrapper would have to duplicate all the
> > non-trivial code in oe-setup-builddir that determines what the build
> > directory should be called. And while this of course is possible to
> > do, it means duplicating code that is internal to bitbake, and risking
> > missing important changes to the upstream code in the future. Thus it
> > is much easier and safer to generate the template files and let
> > oe-init-build-env do its job.
> 
> Wait, I have to ask for details here. What is the non-trivial code
> specifically? Isn't it simply
> 
> mkdir -p builddir/conf
> write_out bblayers.conf
> 
> where builddir is the same argument that is passed to
> oe-init-build-env? Something doesn't compute here for me.

Since our code is a wrapper around oe-init-build-env, it would have to do 
the exact same calculations as are done in oe-setup-builddir, and if you 
look at that script, you will see that there are actually quite a lot of 
options to determine the name of the build directory.

> 
> > Where I (and apparently WindRiver) are coming from, the decision on what
> > layers are part of the configuration is not something a specific layer
> > is responsible for. I.e., in our setup we fetch the layers we want for
> > a build, write the list of layers to bblayers.conf.sample and source
> > oe-init-build-env. With your setup, if I want a project with the two
> > layers meta-a and meta-b, I would also have to create a layer meta-x
> > where I create a static bblayers.conf.sample file that lists these
> > three layers. Then, if another project wants to use meta-a and meta-c,
> > they would have to add a second static bblayers.conf.sample file in
> > meta-x that lists those three layers. Basically every time there is a
> > new combination of layers fetched, there would also have to be a
> > duplicate of the information in a static bblayers.conf.sample file
> > somewhere. I am probably missing something here, but to me this seems
> > suboptimal. If the list of layers in the static file had been used to
> > fetch the layers, I would better understand it, but then the problem
> > becomes that the list is now in one of the arbitrary layers that are
> > supposed to be fetched.
> 
> The idea is that yes, every possible combination of layers needs its
> own template, and it goes under version control into a layer.

This seems incredibly inflexible if you have many optional layers. 
We use repo's --groups option to handle optional layers. This means 
we can, e.g., use one manifest to build the main firmware, but also 
to build special firmware for production testing by adding an option 
to repo, telling it to fetch an extra layer. This also means that all 
the project manifests don't need to exist in two versions (one for 
the main firmware and one for the production testing) as this is 
handled by the base manifest. 

With your solution, every project would have to create two templates 
to cover this. And if we add another optional layer (we currently don't 
have one, but we used to do), then we would have to add one or two 
more templates and so on.

Also, if we decide to add a layer to the base platform, then all the 
project templates are broken until someone updates them all. But since 
they would be in the project layers, that is not something that I as 
the maintainer of the base platform can do.

> Typically this would be a layer that is common to all of your build
> configurations (which is meta-a in the above example - *not* meta-x)
> and that is where the templates go. Do you have such a layer?

Well, we do for the base platform, but then the project templates would 
have to be in the project layers.

I know you are currently not solving the problem of actually fetching 
the layers, but I do not understand how you are planning to that when 
the definition of what layers to use is inside some random layer. And 
a solution that would fetch all layers would definitely not be usable 
by us at least. Which means that to setup a random product I would 
actually need to have inside knowledge of which layer actually happens 
to have the list of what layers are needed for that product.

Or are you planning on having some definition of what layers to fetch 
outside the layers (where I think it belongs)? Then why would one want 
to have duplicate information in one of the fetched layers to setup 
the BBLAYERS variable?

> 
> I also have to ask, what prevents you from simply listing all of the
> layers in a single unified bblayers.conf.sample. Do they step on each
> other? How?

The project layers can definitely step on each other. We also have some 
layers that modify the build completely by being included (they are 
obviously not Yocto Compatible as there is no reason for them to be, 
given that we only fetch and include the layers in our builds that are 
actually used).

> 
> I also need to remind you that bblayers.conf(.sample) does support
> includes, so you can implement a structured hierarchy of layer config
> fragments, where the common bits are only written once.

AFAIK, oe-init-build-env does not support that. It will only replace 
##OEROOT## in the bblayers.conf.sample file.

> 
> Basically, there are alternatives, and I think all of them are better
> than writing out the list of layers just before setting up a build
> with a custom script.

I cannot for the life of me see how your solution is better in any way 
than the one we have. In our solution we have simple, hierarchical 
manifests that specify what layers are to be used. Then the init scripts 
automatically set up the bitbake environment to use those layers. All 
the manifests are in one repository. Adding a layer to the platform means 
adding one line to one manifest. Adding a new project means adding one 
manifest (with basically one line specifying the project layer).

> 
> > in our manifest repository. This simple manifest is all the project
> > team needs to create to build with their project specific layer. It
> > also means that the only input to our Jenkins jobs to build for this
> > project (instead of the main platform) is the name of the project's
> > repo manifest. It also means that if we make any changes to the main
> > platform, e.g., add a new layer (which is a one line change to the
> > cfp.xml manifest), all projects will automatically get and use that
> > new layer without having to make any changes to all the project
> > manifests.
> 
> This is somewhat tangential, as it is about extending the default
> layer setup config, not the combinations of layers in the build
> configuration. So far, the json is not extensible, but we can think
> about how to extend and overlay it too. As far as development in
> meta-foo goes, the templates (including layer lists) for it can be
> hosted there, until the layer 'graduates' into the common structure.
> 
> Alex

//Peter
Alexander Kanavin Sept. 16, 2022, 10:57 a.m. UTC | #12
On Fri, 16 Sept 2022 at 00:59, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
>
> Since our code is a wrapper around oe-init-build-env, it would have to do
> the exact same calculations as are done in oe-setup-builddir, and if you
> look at that script, you will see that there are actually quite a lot of
> options to determine the name of the build directory.

Line numbers as of current master HEAD please. I am looking at the
script right now and I am not seeing any options: it simply takes
$BUILDDIR var passed in from oe-init-build-env. And oe-init-build-env
sets that to either what was given on the command line, or defaults to
'build'.

Again, to the best of my knowledge this will work in the wrapper, and
you need to show otherwise:

mkdir -p $builddir/conf
write_out $builddir/conf/bblayers.conf
. oe-init-build-env $builddir

Something still doesn't compute here and we need to get to the bottom
of it. Let's focus on this for now, please.

Alex
Peter Kjellerstedt Sept. 17, 2022, 12:06 a.m. UTC | #13
> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: den 16 september 2022 11:57
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: ChenQi <Qi.Chen@windriver.com>; OE-core <openembedded-
> core@lists.openembedded.org>; Alexander Kanavin <alex@linutronix.de>
> Subject: Re: [OE-core] [PATCH 1/7] scripts/oe-setup-builddir: add a check
> that TEMPLATECONF is valid
> 
> On Fri, 16 Sept 2022 at 00:59, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> >
> > Since our code is a wrapper around oe-init-build-env, it would have to do
> > the exact same calculations as are done in oe-setup-builddir, and if you
> > look at that script, you will see that there are actually quite a lot of
> > options to determine the name of the build directory.
> 
> Line numbers as of current master HEAD please. I am looking at the
> script right now and I am not seeing any options: it simply takes
> $BUILDDIR var passed in from oe-init-build-env. And oe-init-build-env
> sets that to either what was given on the command line, or defaults to
> 'build'.

Sorry, my fault. I forgot that while the build directory is created 
in oe-setup-builddir, the path that is used is actually determined in 
oe-buildenv-internal, line 42-81. And this is the part that I would 
have to duplicate. Now, I do not need all of it (it can be reduced to
19 lines), but it still means I am duplicating the logic from OE-Core 
and if that changes, we may or may not notice...

That said, unless I can convince you that the right thing to do is to 
remove templateconf.cfg for all builds if it does not contain a valid 
path according to the new rules (and not only for OE-Core and Poky), 
then I still need to duplicate the code to determine the build 
directory to be able to remove the templateconf.cfg file to avoid 
having to make all our developers do it manually...

> 
> Again, to the best of my knowledge this will work in the wrapper, and
> you need to show otherwise:
> 
> mkdir -p $builddir/conf
> write_out $builddir/conf/bblayers.conf
> . oe-init-build-env $builddir
> 
> Something still doesn't compute here and we need to get to the bottom
> of it. Let's focus on this for now, please.
> 
> Alex

//Peter
Alexander Kanavin Sept. 17, 2022, 8:17 a.m. UTC | #14
On Sat, 17 Sept 2022 at 02:06, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> Sorry, my fault. I forgot that while the build directory is created
> in oe-setup-builddir, the path that is used is actually determined in
> oe-buildenv-internal, line 42-81. And this is the part that I would
> have to duplicate. Now, I do not need all of it (it can be reduced to
> 19 lines), but it still means I am duplicating the logic from OE-Core
> and if that changes, we may or may not notice...

Then you can do it in the opposite order through the official tooling:

. oe-init-build-env (against a template with a lowest common
denominator set of layers, perhaps even one from poky)
bitbake-layers add-layer /path/to/meta-b
bitbake-layers add-layer /path/to/meta-c
...

This is better, as it avoids having to write bblayers.conf[.template]
directly. You say you don't want to duplicate the script logic; you
should try to avoid duplicating the content of config files too then.

Alex
diff mbox series

Patch

diff --git a/meta/classes-recipe/populate_sdk_ext.bbclass b/meta/classes-recipe/populate_sdk_ext.bbclass
index 56e24c4eed..925cb313fc 100644
--- a/meta/classes-recipe/populate_sdk_ext.bbclass
+++ b/meta/classes-recipe/populate_sdk_ext.bbclass
@@ -438,7 +438,8 @@  python copy_buildsystem () {
     else:
         # Write a templateconf.cfg
         with open(baseoutpath + '/conf/templateconf.cfg', 'w') as f:
-            f.write('meta/conf\n')
+            f.write('meta/conf/templates/default\n')
+        os.makedirs(os.path.join(baseoutpath, core_meta_subdir, 'conf/templates/default'), exist_ok=True)
 
     # Ensure any variables set from the external environment (by way of
     # BB_ENV_PASSTHROUGH_ADDITIONS) are set in the SDK's configuration
diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
index 5d644168cb..bf832ee0ca 100755
--- a/scripts/oe-setup-builddir
+++ b/scripts/oe-setup-builddir
@@ -61,6 +61,11 @@  if [ -n "$TEMPLATECONF" ]; then
             echo >&2 "Error: TEMPLATECONF value points to nonexistent directory '$TEMPLATECONF'"
             exit 1
         fi
+        templatesdir=$(python3 -c "import sys; print(sys.argv[1].strip('/').split('/')[-2])" $TEMPLATECONF)
+        if [ ! -f "$TEMPLATECONF/../../layer.conf" -o $templatesdir != "templates" ]; then
+            echo >&2 "Error: TEMPLATECONF value (which is $TEMPLATECONF) must point to meta-some-layer/conf/templates/template-name"
+            exit 1
+        fi
     fi
     OECORELAYERCONF="$TEMPLATECONF/bblayers.conf.sample"
     OECORELOCALCONF="$TEMPLATECONF/local.conf.sample"