yocto-check-layer: scan additional layers for tested layer dependencies

In case a LAYERDEPENDS of a tested layer contains a dependency which is
present in additional layers, such dependency does not get pulled into
the layer test as a dependency be default and instead of YCL stops and
reports it cannot find dependent layers.

An example of this is a layer with LAYERDEPENDS = " core openembedded-core "
and then by running YCL on such layer using the following invocation:
$ yocto-check-layer -d --additional-layers ../meta-openembedded/ -- ../meta-board-distro
...
ERROR: Layer meta-board-distro depends on openembedded-layer and isn't found.
...

The fix here adds all layers in the --additional-layers list into layers
which are searched for dependencies of the tested layer. That way, even
layers present in additional-layers list are searched and possibly used
as dependencies for the layer check.

Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Nicolas Dechesne <nicolas.dechesne@linaro.org>
Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 scripts/yocto-check-layer | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On Tue, Apr 4, 2023 at 11:36 PM Marek Vasut <marex@denx.de> wrote:

> In case a LAYERDEPENDS of a tested layer contains a dependency which is
> present in additional layers, such dependency does not get pulled into
> the layer test as a dependency be default and instead of YCL stops and
> reports it cannot find dependent layers.
>

Hmm. I know this script is slightly (!) convoluted.
But I think this is done 'by design'. Since you are expected to list all
your dependencies with --dependency when you are testing a layer.


>
> An example of this is a layer with LAYERDEPENDS = " core openembedded-core
> "
> and then by running YCL on such layer using the following invocation:
> $ yocto-check-layer -d --additional-layers ../meta-openembedded/ --
> ../meta-board-distro
>

In this specific example, you are supposed to use --dependency
../meta-openembedded instead of --additional-layers, no?
--additional-layers (see 31e53430f1bb6f73b82698b20ef7f1047313214a) is
supposed to bring extra layers when parsing/testing to mimic a full distro
or to manually force optional layers to be present. But --dependency is
still required to contain all potential dependencies.


> ...
> ERROR: Layer meta-board-distro depends on openembedded-layer and isn't
> found.
> ...
>
> The fix here adds all layers in the --additional-layers list into layers
> which are searched for dependencies of the tested layer. That way, even
> layers present in additional-layers list are searched and possibly used
> as dependencies for the layer check.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Nicolas Dechesne <nicolas.dechesne@linaro.org>
> Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  scripts/yocto-check-layer | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/yocto-check-layer b/scripts/yocto-check-layer
> index 67cc71950f..fbb715c0af 100755
> --- a/scripts/yocto-check-layer
> +++ b/scripts/yocto-check-layer
> @@ -144,7 +144,7 @@ def main():
>      if not args.no_auto_dependency:
>          depends = []
>          for layer in layers:
> -            layer_depends = get_layer_dependencies(layer, dep_layers,
> logger)
> +            layer_depends = get_layer_dependencies(layer, dep_layers +
> additional_layers, logger)
>              if layer_depends:
>                  for d in layer_depends:
>                      if d not in depends:
> @@ -188,7 +188,7 @@ def main():
>          missing_dependencies = not add_layer_dependencies(bblayersconf,
> layer, dep_layers, logger)
>          if not missing_dependencies:
>              for additional_layer in additional_layers:
> -                if not add_layer_dependencies(bblayersconf,
> additional_layer, dep_layers, logger):
> +                if not add_layer_dependencies(bblayersconf,
> additional_layer, dep_layers + additional_layers, logger):
>                      missing_dependencies = True
>                      break
>          if missing_dependencies:
> --
> 2.39.2
>
>

On 4/5/23 08:48, Nicolas Dechesne wrote:
> On Tue, Apr 4, 2023 at 11:36 PM Marek Vasut <marex@denx.de> wrote:
> 
>> In case a LAYERDEPENDS of a tested layer contains a dependency which is
>> present in additional layers, such dependency does not get pulled into
>> the layer test as a dependency be default and instead of YCL stops and
>> reports it cannot find dependent layers.
>>
> 
> Hmm. I know this script is slightly (!) convoluted.

Well ... yes.

> But I think this is done 'by design'. Since you are expected to list all
> your dependencies with --dependency when you are testing a layer.

I am not sure that is correct. I believe the script is designed to 
attempt to look up the dependencies of a layer automatically. That is 
what the --additional-layers parameter is for, to specify the search 
paths for the automatic layer dependency lookup. The --dependency is 
there to hard-include a layer in the test, e.g. in case it cannot be 
automatically detected because that dependency (layer) is somehow not 
resolvable.

At least that is my understanding.

And the result then is, you can have a directory with various layers, 
but the YCL with --additional-layers pointed into that directory will 
only pick the layers which are really required for validation of that 
tested layer, not the rest of them layers in that directory, which makes 
YCL runtime shorter . And that way, YCL can be used to test multiple 
arbitrary layers, which is useful in CI.

>> An example of this is a layer with LAYERDEPENDS = " core openembedded-core
>> "
>> and then by running YCL on such layer using the following invocation:
>> $ yocto-check-layer -d --additional-layers ../meta-openembedded/ --
>> ../meta-board-distro
>>
> 
> In this specific example, you are supposed to use --dependency
> ../meta-openembedded instead of --additional-layers, no?

I don't think so, the YCL should auto-lookup and auto-add the layers 
into dependencies from --additional-layers. That is my understanding of 
what --additional-layers is for, to do the auto-lookup/auto-add without 
having to explicitly list the dependencies on command line.

> --additional-layers (see 31e53430f1bb6f73b82698b20ef7f1047313214a) is
> supposed to bring extra layers when parsing/testing to mimic a full distro
> or to manually force optional layers to be present. But --dependency is
> still required to contain all potential dependencies.

Errr, uhhh.

That is really weird, the first question which comes to my mind 
regarding that commit message is -- why not use --dependency to cover 
those use cases listed in the commit message.

The second question which comes to my mind is, why does the 
additional-layers do automatic layer look up (based on LAYERDEPENDS) in 
the specified additional-layer directories, but then fails to actually 
add those layers into the build during the test (I think maybe that is 
what I am solving with this patch, without describing it well).

Well that turned out to be a wall of text too, sigh ...

On Wed, Apr 5, 2023 at 2:29 PM Marek Vasut <marex@denx.de> wrote:

> On 4/5/23 08:48, Nicolas Dechesne wrote:
> > On Tue, Apr 4, 2023 at 11:36 PM Marek Vasut <marex@denx.de> wrote:
> >
> >> In case a LAYERDEPENDS of a tested layer contains a dependency which is
> >> present in additional layers, such dependency does not get pulled into
> >> the layer test as a dependency be default and instead of YCL stops and
> >> reports it cannot find dependent layers.
> >>
> >
> > Hmm. I know this script is slightly (!) convoluted.
>
> Well ... yes.
>
> > But I think this is done 'by design'. Since you are expected to list all
> > your dependencies with --dependency when you are testing a layer.
>
> I am not sure that is correct. I believe the script is designed to
> attempt to look up the dependencies of a layer automatically. That is
> what the --additional-layers parameter is for, to specify the search
> paths for the automatic layer dependency lookup. The --dependency is
> there to hard-include a layer in the test, e.g. in case it cannot be
> automatically detected because that dependency (layer) is somehow not
> resolvable.
>
> At least that is my understanding.
>

I don't think it's right. --dependency holds the list of all layers that
can potentially be a dependency. So yes, YCL is designed to search all
dependencies automatically, but it is looking for them in the list of
layers from --dependency (the list of layers to test is also added in the
list of 'potential' dependencies).
So up to now, the script is designed to search for dependencies only there.
With your change you are trying to search for dependencies in layers
provided in --additional-layers too.

Layers in --dependencies are not 'hard included' in the test. only if they
are effectively a dependency of a tested layer.


>
> And the result then is, you can have a directory with various layers,
> but the YCL with --additional-layers pointed into that directory will
> only pick the layers which are really required for validation of that
> tested layer, not the rest of them layers in that directory, which makes
> YCL runtime shorter . And that way, YCL can be used to test multiple
> arbitrary layers, which is useful in CI.
>

On the opposite, layers from --additional-layers are 'hard included' in the
build, as per the commit message mentioned in my previous response.


> >> An example of this is a layer with LAYERDEPENDS = " core
> openembedded-core
> >> "
> >> and then by running YCL on such layer using the following invocation:
> >> $ yocto-check-layer -d --additional-layers ../meta-openembedded/ --
> >> ../meta-board-distro
> >>
> >
> > In this specific example, you are supposed to use --dependency
> > ../meta-openembedded instead of --additional-layers, no?
>
> I don't think so, the YCL should auto-lookup and auto-add the layers
> into dependencies from --additional-layers. That is my understanding of
> what --additional-layers is for, to do the auto-lookup/auto-add without
> having to explicitly list the dependencies on command line.
>

No, this code :

        missing_dependencies = not add_layer_dependencies(bblayersconf,
layer, dep_layers, logger)
        if not missing_dependencies:
            for additional_layer in additional_layers:
                if not add_layer_dependencies(bblayersconf,
additional_layer, dep_layers, logger):
                    missing_dependencies = True
                    break

is adding layers from --additional-layers, unconditionally, assuming all
dependencies from the 'layer' to test and from the 'additional-layer' are
found in the layers listed in --dependency.

So effectively, dependencies are only searched in --dependencies. I am not
saying it is the right thing and should stay like that, but it's the
current semantic of the 'api'.


>
> > --additional-layers (see 31e53430f1bb6f73b82698b20ef7f1047313214a) is
> > supposed to bring extra layers when parsing/testing to mimic a full
> distro
> > or to manually force optional layers to be present. But --dependency is
> > still required to contain all potential dependencies.
>
> Errr, uhhh.
>
> That is really weird, the first question which comes to my mind
> regarding that commit message is -- why not use --dependency to cover
> those use cases listed in the commit message.
>

layers in --dependency are *not* added into the build, unless they are
found to be an actual dependency of a layer which is added to the build.


>
> The second question which comes to my mind is, why does the
> additional-layers do automatic layer look up (based on LAYERDEPENDS) in
> the specified additional-layer directories, but then fails to actually
> add those layers into the build during the test (I think maybe that is
> what I am solving with this patch, without describing it well).
>

I am assuming you are refering to line 191 here, e.g.

                if not add_layer_dependencies(bblayersconf,
additional_layer, dep_layers, logger):
                    missing_dependencies = True
                    break

Here we are searching for dependencies of the additional layer in order to
pull them into the build, which makes sense.


>
> Well that turned out to be a wall of text too, sigh ...
>