diff mbox series

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

Message ID 20220824124234.3549247-2-alex@linutronix.de
State Accepted, archived
Commit c6f2b57be8893ee58f20cc29d8ec3a5a6edf7c07
Headers show
Series [1/2] meta/conf: move default configuration templates into meta/conf/templates/default | expand

Commit Message

Alexander Kanavin Aug. 24, 2022, 12:42 p.m. UTC
specifically that ../../layer.conf exists, and that second-from-last
component in the path is 'templates'.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 scripts/oe-setup-builddir | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Peter Kjellerstedt Aug. 24, 2022, 1:58 p.m. UTC | #1
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Alexander Kanavin
> Sent: den 24 augusti 2022 14:43
> To: openembedded-core@lists.openembedded.org
> Cc: Alexander Kanavin <alex@linutronix.de>
> Subject: [OE-core] [PATCH 2/2] scripts/oe-setup-builddir: add a check that TEMPLATECONF is valid
> 
> specifically that ../../layer.conf exists, and that second-from-last
> component in the path is 'templates'.

Please no. This happens to match your expectations, but doesn't at 
all match our use of TEMPLATECONF. In our .templateconf we set
TEMPLATECONF=${TEMPLATECONF:-templateconf} and create the templateconf 
directory in runtime where we generate the sample files that match the 
layers that repo has fetched for this particular build. It obviously 
has no layer.conf file as it is in no layer, and the path doesn't 
contain "templates/<something>". I can of course change our tools to 
create a couple of extra directories to match your expectations, but 
at the same time it seems very unnecessary.

> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>  scripts/oe-setup-builddir | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
> index 975619789a..db26e3b138 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 must point to meta-some-layer/conf/templates/template-name"
> +            exit 1
> +        fi
>      fi
>      OECORELAYERCONF="$TEMPLATECONF/bblayers.conf.sample"
>      OECORELOCALCONF="$TEMPLATECONF/local.conf.sample"
> --
> 2.30.2

//Peter
Alexander Kanavin Aug. 24, 2022, 2:08 p.m. UTC | #2
On Wed, 24 Aug 2022 at 15:58, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> Please no. This happens to match your expectations, but doesn't at
> all match our use of TEMPLATECONF. In our .templateconf we set
> TEMPLATECONF=${TEMPLATECONF:-templateconf} and create the templateconf
> directory in runtime where we generate the sample files that match the
> layers that repo has fetched for this particular build. It obviously
> has no layer.conf file as it is in no layer, and the path doesn't
> contain "templates/<something>". I can of course change our tools to
> create a couple of extra directories to match your expectations, but
> at the same time it seems very unnecessary.

If you create the template files on the fly, then you might as well
create the actual configs directly into build/conf and skip the
template step. No?

We do need to standardize the location for the templates for reasons
of discoverability and consistency with machine and distro
definitions, this has been discussed.

Alex
Peter Kjellerstedt Aug. 24, 2022, 2:43 p.m. UTC | #3
> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: den 24 augusti 2022 16:09
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: openembedded-core@lists.openembedded.org; Alexander Kanavin
> <alex@linutronix.de>
> Subject: Re: [OE-core] [PATCH 2/2] scripts/oe-setup-builddir: add a check
> that TEMPLATECONF is valid
> 
> On Wed, 24 Aug 2022 at 15:58, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> > Please no. This happens to match your expectations, but doesn't at
> > all match our use of TEMPLATECONF. In our .templateconf we set
> > TEMPLATECONF=${TEMPLATECONF:-templateconf} and create the templateconf
> > directory in runtime where we generate the sample files that match the
> > layers that repo has fetched for this particular build. It obviously
> > has no layer.conf file as it is in no layer, and the path doesn't
> > contain "templates/<something>". I can of course change our tools to
> > create a couple of extra directories to match your expectations, but
> > at the same time it seems very unnecessary.
> 
> If you create the template files on the fly, then you might as well
> create the actual configs directly into build/conf and skip the
> template step. No?

We create the sample files in a small wrapper for oe-init-build-env 
so that to the users, everything works just as described in the Yocto 
documentation. And since the OE scripts expect TEMPLATECONF to be set 
and contain sample files, that is what we create.

> We do need to standardize the location for the templates for reasons
> of discoverability and consistency with machine and distro
> definitions, this has been discussed.

Sure, that is fine. But the expectation I have is that TEMPLATECONF 
refers to the directory that has the sample files needed to set up 
the build environment. And that is it. As long as the sample files 
exist where it says they should be, why do you need the scripts to 
fail because the environment around the directory does not look 
like what you happen to expect? It is only the contents of the 
directory that matters. Or am I missing something?

> 
> Alex

//Peter
Alexander Kanavin Aug. 24, 2022, 2:52 p.m. UTC | #4
On Wed, 24 Aug 2022 at 16:43, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> We create the sample files in a small wrapper for oe-init-build-env
> so that to the users, everything works just as described in the Yocto
> documentation. And since the OE scripts expect TEMPLATECONF to be set
> and contain sample files, that is what we create.

Actually they don't expect that. You can simply create the configs
directly into the target build directory in your wrapper, and drop
TEMPLATECONF altogether. Then the OE scripts will detect that the
configs are already there and will skip the step of copying the
templates.

> Sure, that is fine. But the expectation I have is that TEMPLATECONF
> refers to the directory that has the sample files needed to set up
> the build environment. And that is it. As long as the sample files
> exist where it says they should be, why do you need the scripts to
> fail because the environment around the directory does not look
> like what you happen to expect? It is only the contents of the
> directory that matters. Or am I missing something?

We are trying to move towards standardizing build configuration
management. One step towards that goal is that config templates must
live in meta-some-layer/conf/templates, and aren't scattered around,
or generated on the fly. This rule only makes sense if there is some
way to enforce it, which is what this change does.

Alex
Peter Kjellerstedt Aug. 25, 2022, 7:19 a.m. UTC | #5
> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: den 24 augusti 2022 16:53
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: openembedded-core@lists.openembedded.org; Alexander Kanavin
> <alex@linutronix.de>
> Subject: Re: [OE-core] [PATCH 2/2] scripts/oe-setup-builddir: add a check
> that TEMPLATECONF is valid
> 
> On Wed, 24 Aug 2022 at 16:43, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> > We create the sample files in a small wrapper for oe-init-build-env
> > so that to the users, everything works just as described in the Yocto
> > documentation. And since the OE scripts expect TEMPLATECONF to be set
> > and contain sample files, that is what we create.
> 
> Actually they don't expect that. You can simply create the configs
> directly into the target build directory in your wrapper, and drop
> TEMPLATECONF altogether. Then the OE scripts will detect that the
> configs are already there and will skip the step of copying the
> templates.

Hmm, ok. I will have to look into that...

> > Sure, that is fine. But the expectation I have is that TEMPLATECONF
> > refers to the directory that has the sample files needed to set up
> > the build environment. And that is it. As long as the sample files
> > exist where it says they should be, why do you need the scripts to
> > fail because the environment around the directory does not look
> > like what you happen to expect? It is only the contents of the
> > directory that matters. Or am I missing something?
> 
> We are trying to move towards standardizing build configuration
> management. One step towards that goal is that config templates must
> live in meta-some-layer/conf/templates, and aren't scattered around,
> or generated on the fly. This rule only makes sense if there is some
> way to enforce it, which is what this change does.

Well, we gave up on the static templates almost immediately after we 
started using Yocto when we realized that they did not fit into our 
idea of being able to mix layers in different combinations in different 
builds. We rely on repo fetching the layers that are supposed to be part 
of the build, and then we generate a bblayers.conf.sample that matches 
the fetched layers.

> Alex

//Peter
Alexander Kanavin Aug. 25, 2022, 9:19 a.m. UTC | #6
On Thu, 25 Aug 2022 at 09:19, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
 >
> > We are trying to move towards standardizing build configuration
> > management. One step towards that goal is that config templates must
> > live in meta-some-layer/conf/templates, and aren't scattered around,
> > or generated on the fly. This rule only makes sense if there is some
> > way to enforce it, which is what this change does.
>
> Well, we gave up on the static templates almost immediately after we
> started using Yocto when we realized that they did not fit into our
> idea of being able to mix layers in different combinations in different
> builds. We rely on repo fetching the layers that are supposed to be part
> of the build, and then we generate a bblayers.conf.sample that matches
> the fetched layers.

The static templates is only a starting point. The next step would be
to support prefabricated 'configuration fragments' stored inside
layers that can be dynamically added/removed into local.conf and/or a
distro definition, so ideas for the design and UI would be welcome.

Alex
Peter Kjellerstedt Aug. 25, 2022, 7:41 p.m. UTC | #7
> -----Original Message-----
> From: Alexander Kanavin <alex.kanavin@gmail.com>
> Sent: den 25 augusti 2022 11:19
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: openembedded-core@lists.openembedded.org; Alexander Kanavin
> <alex@linutronix.de>; Richard Purdie <richard.purdie@linuxfoundation.org>
> Subject: Re: [OE-core] [PATCH 2/2] scripts/oe-setup-builddir: add a check
> that TEMPLATECONF is valid
> 
> On Thu, 25 Aug 2022 at 09:19, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
>  >
> > > We are trying to move towards standardizing build configuration
> > > management. One step towards that goal is that config templates must
> > > live in meta-some-layer/conf/templates, and aren't scattered around,
> > > or generated on the fly. This rule only makes sense if there is some
> > > way to enforce it, which is what this change does.
> >
> > Well, we gave up on the static templates almost immediately after we
> > started using Yocto when we realized that they did not fit into our
> > idea of being able to mix layers in different combinations in different
> > builds. We rely on repo fetching the layers that are supposed to be part
> > of the build, and then we generate a bblayers.conf.sample that matches
> > the fetched layers.
> 
> The static templates is only a starting point. The next step would be
> to support prefabricated 'configuration fragments' stored inside
> layers that can be dynamically added/removed into local.conf and/or a
> distro definition, so ideas for the design and UI would be welcome.

Well, the way we have solved that is to allow a layer to contain 
conf/local.conf.sample.XX files (and also conf/conf-notes.txt.XX files) 
where XX is any number between 00 and 99. We also assume the sample 
files from meta-poky have XX == 50. Then when we generate our 
templateconf directory, we concatenate all these files ordered by XX 
(regardless of which layer they come from).

> 
> Alex

//Peter
Luca Ceresoli Aug. 27, 2022, 7 a.m. UTC | #8
Hi Alex,

On Wed, 24 Aug 2022 14:42:34 +0200
"Alexander Kanavin" <alex.kanavin@gmail.com> wrote:

> specifically that ../../layer.conf exists, and that second-from-last
> component in the path is 'templates'.
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>  scripts/oe-setup-builddir | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
> index 975619789a..db26e3b138 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 must point to meta-some-layer/conf/templates/template-name"
> +            exit 1
> +        fi

This is making the AB fail:

https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/4140/steps/7/logs/stdio
Alexander Kanavin Aug. 27, 2022, 5 p.m. UTC | #9
On Sat, 27 Aug 2022 at 09:00, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:

> This is making the AB fail:
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/4140/steps/7/logs/stdio

I believe this needs the patch for poky that moves the template to the
expected location, was it included? I just started a test with both
patches:
https://git.yoctoproject.org/poky-contrib/log/?h=akanavin/force-templateconf

and it is able to proceed fine:
https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/4144

Alex
Alexander Kanavin Aug. 27, 2022, 7:13 p.m. UTC | #10
On Sat, 27 Aug 2022 at 19:00, Alexander Kanavin via
lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
wrote:
> > This is making the AB fail:
> >
> > https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/4140/steps/7/logs/stdio
>
> I believe this needs the patch for poky that moves the template to the
> expected location, was it included? I just started a test with both
> patches:
> https://git.yoctoproject.org/poky-contrib/log/?h=akanavin/force-templateconf
>
> and it is able to proceed fine:
> https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/4144

...except the change does break eSDKs, so that needs to be fixed first.

Alex
diff mbox series

Patch

diff --git a/scripts/oe-setup-builddir b/scripts/oe-setup-builddir
index 975619789a..db26e3b138 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 must point to meta-some-layer/conf/templates/template-name"
+            exit 1
+        fi
     fi
     OECORELAYERCONF="$TEMPLATECONF/bblayers.conf.sample"
     OECORELOCALCONF="$TEMPLATECONF/local.conf.sample"