[PATCH 0/1] devtool: warn user about multiple layer having the same base name

[PATCH 0/1] devtool: warn user about multiple layer having the same base name

[Chen Qi]

*** BLURB HERE ***
The following changes since commit 67266331b0f557c01cde1cc1b1a1de7197443a6c:

  local.conf.sample.extended: remove redundant RUNTIMETARGET assignment (2019-06-24 17:34:25 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib ChenQi/devtool-finish
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/devtool-finish

Chen Qi (1):
  devtool: warn user about multiple layer having the same base name

 scripts/lib/devtool/standard.py | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

-- 
1.9.1

[PATCH 1/1] devtool: warn user about multiple layer having the same base name

[Chen Qi]

Currently `devtool finish RECIPE meta' will silently succeed even
if there are multiple layers having the same base name of 'meta'.
e.g. meta layer from oe-core and meta layer from meta-secure-core.

We should at least give user a warning in such case. With the patch,
we will get warning like below.

WARNING: Multiple layers have the same base name 'meta', use the first one '<PROJ_DIR>/oe-core/meta'.
WARNING: Consider using path instead of base name to specify layer:
	 	  <PROJ_DIR>/oe-core/meta
		  <PROJ_DIR>/meta-secure-core/meta

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 scripts/lib/devtool/standard.py | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index aca74b1..47ed531 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -1866,13 +1866,27 @@ def reset(args, config, basepath, workspace):
 def _get_layer(layername, d):
     """Determine the base layer path for the specified layer name/path"""
     layerdirs = d.getVar('BBLAYERS').split()
-    layers = {os.path.basename(p): p for p in layerdirs}
+    layers = {}    # {basename: layer_paths}
+    for p in layerdirs:
+        bn = os.path.basename(p)
+        if bn not in layers:
+            layers[bn] = [p]
+        else:
+            layers[bn].append(p)
     # Provide some shortcuts
     if layername.lower() in ['oe-core', 'openembedded-core']:
-        layerdir = layers.get('meta', None)
+        layername = 'meta'
+    layer_paths = layers.get(layername, None)
+    if not layer_paths:
+        return os.path.abspath(layername)
+    elif len(layer_paths) == 1:
+        return os.path.abspath(layer_paths[0])
     else:
-        layerdir = layers.get(layername, None)
-    return os.path.abspath(layerdir or layername)
+        # multiple layers having the same base name
+        logger.warning("Multiple layers have the same base name '%s', use the first one '%s'." % (layername, layer_paths[0]))
+        logger.warning("Consider using path instead of base name to specify layer:\n\t\t%s" % '\n\t\t'.join(layer_paths))
+        return os.path.abspath(layer_paths[0])
+
 
 def finish(args, config, basepath, workspace):
     """Entry point for the devtool 'finish' subcommand"""
-- 
1.9.1

Re: [PATCH 1/1] devtool: warn user about multiple layer having the same base name

[Paul Eggleton]

On Thursday, 27 June 2019 3:14:55 PM NZST Chen Qi wrote:
> Currently `devtool finish RECIPE meta' will silently succeed even
> if there are multiple layers having the same base name of 'meta'.
> e.g. meta layer from oe-core and meta layer from meta-secure-core.

Good catch!

> We should at least give user a warning in such case. With the patch,
> we will get warning like below.
> 
> WARNING: Multiple layers have the same base name 'meta', use the first one '<PROJ_DIR>/oe-core/meta'.
> WARNING: Consider using path instead of base name to specify layer:
> 	 	  <PROJ_DIR>/oe-core/meta
> 		  <PROJ_DIR>/meta-secure-core/meta
> 

"use the first one" -> "using the first one"

However I'm wondering if this is the right behaviour. What if the user realises they actually wanted the second one? Given it's a warning the finish operation would have proceeded and the user will have to clean things up manually. Rather than a warning, should we in fact be erroring out and requiring the user to be more specific in this case?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 1/1] devtool: warn user about multiple layer having the same base name

[ChenQi]

On 06/28/2019 07:25 AM, Paul Eggleton wrote:
> On Thursday, 27 June 2019 3:14:55 PM NZST Chen Qi wrote:
>> Currently `devtool finish RECIPE meta' will silently succeed even
>> if there are multiple layers having the same base name of 'meta'.
>> e.g. meta layer from oe-core and meta layer from meta-secure-core.
> Good catch!
>
>> We should at least give user a warning in such case. With the patch,
>> we will get warning like below.
>>
>> WARNING: Multiple layers have the same base name 'meta', use the first one '<PROJ_DIR>/oe-core/meta'.
>> WARNING: Consider using path instead of base name to specify layer:
>> 	 	  <PROJ_DIR>/oe-core/meta
>> 		  <PROJ_DIR>/meta-secure-core/meta
>>
> "use the first one" -> "using the first one"
>
> However I'm wondering if this is the right behaviour. What if the user realises they actually wanted the second one? Given it's a warning the finish operation would have proceeded and the user will have to clean things up manually. Rather than a warning, should we in fact be erroring out and requiring the user to be more specific in this case?
>
> Cheers,
> Paul
>
More reasonable. I'll send out V2.

Best Regards,

Chen Qi

Re: [PATCH 1/1] devtool: warn user about multiple layer having the same base name

[Paul Eggleton]

Hi Peter,

On Friday, 28 June 2019 10:09:22 PM NZST Peter Kjellerstedt wrote:
> I had not realized the layer name in the devtool finish command supports
> abbreviations/patterns (whichever it is). My expectation from running
> `devtool finish foo meta` would be to update the foo recipe in the meta
> layer,  not that it should go looking for every layer called meta-something.

That's not how it works. The logic is here:

  https://git.openembedded.org/openembedded-core/tree/scripts/lib/devtool/standard.py#n1866

We only have the subdirectory the layer is in to use to determine what the name of the layer is, if you only specify the name. The only shortcuts in this function are "oe-core" and "openembedded-core" map to "meta". The case this patch is attempting to handle is where you have more than one layer in your configuration where the subdirectory in which the layer appears is called "meta". (People should generally avoid this and put the layer in a subdirectory whose name matches the name of the layer, but of course people can do whatever they want.)

Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 1/1] devtool: warn user about multiple layer having the same base name

[Peter Kjellerstedt]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

I had not realized the layer name in the devtool finish command supports abbreviations/patterns (whichever it is). My expectation from running `devtool finish foo meta` would be to update the foo recipe in the meta layer,  not that it should go looking for every layer called meta-something.

How useful is that support compared to be able to specify the exact layer name? Normally in this situation, I would typically expect to use an extra option,  e.g., --abbrev or --regexp, to enable support for using loose matching.

//Peter

[-- Attachment #2: Type: text/html, Size: 737 bytes --]