| Message ID | 20231010014835.947330-1-bhstalel@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Ensure that BBFILE_COLLECTIONS and BBPATH exist at add-layers and parsing | expand |
I was coding this at 2 in the morning then I realized a simple check on the start of the line for a given variable is enough. But also it can be a pattern for BBFILE_COLLECTIONS or other ideas. This can be fixed, but I need a review on the idea itself first. Kind regards Talel On Tue, Oct 10, 2023 at 2:49 AM Talel BELHAJSALEM <bhstalel@gmail.com> wrote: > Empty layer.conf is added to bblayers.conf and not shown by show-layers. > > It will have no effect on BBFILES or BBPATH so no need to add it in the > first place. > > Other than this, when working with the layer.conf variables, I noticed the > following: > > 1) No BBFILE_COLLECTIONS will lead to no effect for BBFILE_PATTERN, so > BBFILES on its own work well. > 2) No BBFILE_COLLECTIONS and no BBPATH with BBFILES set to a directoy will > work as well but custom inherits will fail only if the recipe matches: > meta-*/*.bb. > So checking on BBPATH will also fix this. > > In conclusion any messing around with layer.conf's variables will lead to > unexpected behavior. > > Making sure BBFILE_COLLECTIONS exist will ensure: > - error if no LAYERSERIES_COMPAT > - error if no BBFILE_PATTERN > - warning if no BBFILES > - Layer shown in (bitbake-layers show-layers) > > Making sure BBPATH exist will ensure that classes and conf found > regardless of BBFILES content (it can be a directory that will trigger > find_bbfiles) > > Signed-off-by: Talel BELHAJSALEM <bhstalel@gmail.com> > --- > bitbake/lib/bb/cooker.py | 2 +- > bitbake/lib/bb/cookerdata.py | 7 ++++++- > bitbake/lib/bblayers/action.py | 26 ++++++++++++++++++++++++++ > 3 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py > index 2adf4d297d..f163d3a2a3 100644 > --- a/bitbake/lib/bb/cooker.py > +++ b/bitbake/lib/bb/cooker.py > @@ -1827,7 +1827,7 @@ class CookerCollectFiles(object): > for ignored in ('SCCS', 'CVS', '.svn'): > if ignored in dirs: > dirs.remove(ignored) > - found += [os.path.join(dir, f) for f in files if > (f.endswith(['.bb', '.bbappend']))] > + found += [os.path.join(dir, f) for f in files if > (f.endswith(('.bb', '.bbappend')))] > > return found > > diff --git a/bitbake/lib/bb/cookerdata.py b/bitbake/lib/bb/cookerdata.py > index ec3741cc1d..fd1e0cf06f 100644 > --- a/bitbake/lib/bb/cookerdata.py > +++ b/bitbake/lib/bb/cookerdata.py > @@ -18,6 +18,7 @@ from functools import wraps > import bb > from bb import data > import bb.parse > +from bblayers.action import ActionPlugin > > logger = logging.getLogger("BitBake") > parselog = logging.getLogger("BitBake.Parsing") > @@ -377,7 +378,11 @@ class CookerDataBuilder(object): > layer = layer.rstrip('/') > data.setVar('LAYERDIR', layer) > data.setVar('LAYERDIR_RE', re.escape(layer)) > - data = parse_config_file(os.path.join(layer, "conf", > "layer.conf"), data) > + layer_conf = os.path.join(layer, "conf", "layer.conf") > + res, msg = ActionPlugin.do_check_layer(layer_conf) > + if not res: > + bb.fatal(msg) > + data = parse_config_file(layer_conf, data) > data.expandVarref('LAYERDIR') > data.expandVarref('LAYERDIR_RE') > > diff --git a/bitbake/lib/bblayers/action.py > b/bitbake/lib/bblayers/action.py > index 454c251410..5a2fcf7f8f 100644 > --- a/bitbake/lib/bblayers/action.py > +++ b/bitbake/lib/bblayers/action.py > @@ -23,6 +23,27 @@ def plugin_init(plugins): > > > class ActionPlugin(LayerPlugin): > + > + def do_check_layer(path): > + """ > + make sure that BBFILE_COLLECTIONS and BBPATH are found in > layer.conf > + they should exist in non-commented lines > + """ > + def _is_found(var, line): > + return var in line and not line.startswith('#') > + > + with open(path) as layer_conf_fd: > + lines = layer_conf_fd.readlines() > + find_result = {} > + for to_find in ['BBFILE_COLLECTIONS', 'BBPATH']: > + for line in lines: > + find_result[to_find] = True if _is_found(to_find, > line) else False > + if find_result[to_find]: > + break > + if not find_result[to_find]: > + return False, "Specified layer configuration %s > doesn't contain %s\n" % (path, to_find) > + return True, "" > + > def do_add_layer(self, args): > """Add one or more layers to bblayers.conf.""" > layerdirs = [os.path.abspath(ldir) for ldir in args.layerdir] > @@ -37,6 +58,11 @@ class ActionPlugin(LayerPlugin): > sys.stderr.write("Specified layer directory %s doesn't > contain a conf/layer.conf file\n" % layerdir) > return 1 > > + res, msg = ActionPlugin.do_check_layer(layer_conf) > + if not res: > + sys.stderr.write(msg) > + return 1 > + > bblayers_conf = os.path.join('conf', 'bblayers.conf') > if not os.path.exists(bblayers_conf): > sys.stderr.write("Unable to find bblayers.conf\n") > -- > 2.25.1 > >
On Tue, 2023-10-10 at 02:48 +0100, BELHADJ SALEM Talel wrote: > Empty layer.conf is added to bblayers.conf and not shown by show-layers. > > It will have no effect on BBFILES or BBPATH so no need to add it in the first place. > > Other than this, when working with the layer.conf variables, I noticed the following: > > 1) No BBFILE_COLLECTIONS will lead to no effect for BBFILE_PATTERN, so BBFILES on its own work well. > 2) No BBFILE_COLLECTIONS and no BBPATH with BBFILES set to a directoy will work as well but custom inherits will fail only if the recipe matches: meta-*/*.bb. > So checking on BBPATH will also fix this. > > In conclusion any messing around with layer.conf's variables will lead to unexpected behavior. > > Making sure BBFILE_COLLECTIONS exist will ensure: > - error if no LAYERSERIES_COMPAT > - error if no BBFILE_PATTERN > - warning if no BBFILES > - Layer shown in (bitbake-layers show-layers) > > Making sure BBPATH exist will ensure that classes and conf found regardless of BBFILES content (it can be a directory that will trigger find_bbfiles) > > Signed-off-by: Talel BELHAJSALEM <bhstalel@gmail.com> > --- > bitbake/lib/bb/cooker.py | 2 +- > bitbake/lib/bb/cookerdata.py | 7 ++++++- > bitbake/lib/bblayers/action.py | 26 ++++++++++++++++++++++++++ > 3 files changed, 33 insertions(+), 2 deletions(-) > > diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py > index 2adf4d297d..f163d3a2a3 100644 > --- a/bitbake/lib/bb/cooker.py > +++ b/bitbake/lib/bb/cooker.py > @@ -1827,7 +1827,7 @@ class CookerCollectFiles(object): > for ignored in ('SCCS', 'CVS', '.svn'): > if ignored in dirs: > dirs.remove(ignored) > - found += [os.path.join(dir, f) for f in files if (f.endswith(['.bb', '.bbappend']))] > + found += [os.path.join(dir, f) for f in files if (f.endswith(('.bb', '.bbappend')))] > > return found I think this piece should become a separate commit. > > diff --git a/bitbake/lib/bb/cookerdata.py b/bitbake/lib/bb/cookerdata.py > index ec3741cc1d..fd1e0cf06f 100644 > --- a/bitbake/lib/bb/cookerdata.py > +++ b/bitbake/lib/bb/cookerdata.py > @@ -18,6 +18,7 @@ from functools import wraps > import bb > from bb import data > import bb.parse > +from bblayers.action import ActionPlugin > > logger = logging.getLogger("BitBake") > parselog = logging.getLogger("BitBake.Parsing") > @@ -377,7 +378,11 @@ class CookerDataBuilder(object): > layer = layer.rstrip('/') > data.setVar('LAYERDIR', layer) > data.setVar('LAYERDIR_RE', re.escape(layer)) > - data = parse_config_file(os.path.join(layer, "conf", "layer.conf"), data) > + layer_conf = os.path.join(layer, "conf", "layer.conf") > + res, msg = ActionPlugin.do_check_layer(layer_conf) > + if not res: > + bb.fatal(msg) > + data = parse_config_file(layer_conf, data) > data.expandVarref('LAYERDIR') > data.expandVarref('LAYERDIR_RE') > > diff --git a/bitbake/lib/bblayers/action.py b/bitbake/lib/bblayers/action.py > index 454c251410..5a2fcf7f8f 100644 > --- a/bitbake/lib/bblayers/action.py > +++ b/bitbake/lib/bblayers/action.py > @@ -23,6 +23,27 @@ def plugin_init(plugins): > > > class ActionPlugin(LayerPlugin): > + > + def do_check_layer(path): > + """ > + make sure that BBFILE_COLLECTIONS and BBPATH are found in layer.conf > + they should exist in non-commented lines > + """ > + def _is_found(var, line): > + return var in line and not line.startswith('#') > + > + with open(path) as layer_conf_fd: > + lines = layer_conf_fd.readlines() > + find_result = {} > + for to_find in ['BBFILE_COLLECTIONS', 'BBPATH']: > + for line in lines: > + find_result[to_find] = True if _is_found(to_find, line) else False > + if find_result[to_find]: > + break > + if not find_result[to_find]: > + return False, "Specified layer configuration %s doesn't contain %s\n" % (path, to_find) > + return True, "" > + I don't think writing a new parser is a great idea. There are other ways we could detect if those variables were set/changed. I'd also question whether this is correct. It would be perfectly fine for a layer just to add python library files using addpylib for example and not set BB_FILE_COLLECTIONS or BBPATH. Cheers, Richard
Hello Richard Thanks for the reply, Regarding the second comment: Do you mean using addpylib can be an alternative for not setting BBFILE_COLLECTIONS and BBPATH ? If yes, I can't see an example on how to do that. Kind regards, Talel On Tue, Oct 10, 2023 at 1:48 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Tue, 2023-10-10 at 02:48 +0100, BELHADJ SALEM Talel wrote: > > Empty layer.conf is added to bblayers.conf and not shown by show-layers. > > > > It will have no effect on BBFILES or BBPATH so no need to add it in the > first place. > > > > Other than this, when working with the layer.conf variables, I noticed > the following: > > > > 1) No BBFILE_COLLECTIONS will lead to no effect for BBFILE_PATTERN, so > BBFILES on its own work well. > > 2) No BBFILE_COLLECTIONS and no BBPATH with BBFILES set to a directoy > will work as well but custom inherits will fail only if the recipe matches: > meta-*/*.bb. > > So checking on BBPATH will also fix this. > > > > In conclusion any messing around with layer.conf's variables will lead > to unexpected behavior. > > > > Making sure BBFILE_COLLECTIONS exist will ensure: > > - error if no LAYERSERIES_COMPAT > > - error if no BBFILE_PATTERN > > - warning if no BBFILES > > - Layer shown in (bitbake-layers show-layers) > > > > Making sure BBPATH exist will ensure that classes and conf found > regardless of BBFILES content (it can be a directory that will trigger > find_bbfiles) > > > > Signed-off-by: Talel BELHAJSALEM <bhstalel@gmail.com> > > --- > > bitbake/lib/bb/cooker.py | 2 +- > > bitbake/lib/bb/cookerdata.py | 7 ++++++- > > bitbake/lib/bblayers/action.py | 26 ++++++++++++++++++++++++++ > > 3 files changed, 33 insertions(+), 2 deletions(-) > > > > diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py > > index 2adf4d297d..f163d3a2a3 100644 > > --- a/bitbake/lib/bb/cooker.py > > +++ b/bitbake/lib/bb/cooker.py > > @@ -1827,7 +1827,7 @@ class CookerCollectFiles(object): > > for ignored in ('SCCS', 'CVS', '.svn'): > > if ignored in dirs: > > dirs.remove(ignored) > > - found += [os.path.join(dir, f) for f in files if > (f.endswith(['.bb', '.bbappend']))] > > + found += [os.path.join(dir, f) for f in files if > (f.endswith(('.bb', '.bbappend')))] > > > > return found > > I think this piece should become a separate commit. > > > > > diff --git a/bitbake/lib/bb/cookerdata.py b/bitbake/lib/bb/cookerdata.py > > index ec3741cc1d..fd1e0cf06f 100644 > > --- a/bitbake/lib/bb/cookerdata.py > > +++ b/bitbake/lib/bb/cookerdata.py > > @@ -18,6 +18,7 @@ from functools import wraps > > import bb > > from bb import data > > import bb.parse > > +from bblayers.action import ActionPlugin > > > > logger = logging.getLogger("BitBake") > > parselog = logging.getLogger("BitBake.Parsing") > > @@ -377,7 +378,11 @@ class CookerDataBuilder(object): > > layer = layer.rstrip('/') > > data.setVar('LAYERDIR', layer) > > data.setVar('LAYERDIR_RE', re.escape(layer)) > > - data = parse_config_file(os.path.join(layer, "conf", > "layer.conf"), data) > > + layer_conf = os.path.join(layer, "conf", "layer.conf") > > + res, msg = ActionPlugin.do_check_layer(layer_conf) > > + if not res: > > + bb.fatal(msg) > > + data = parse_config_file(layer_conf, data) > > data.expandVarref('LAYERDIR') > > data.expandVarref('LAYERDIR_RE') > > > > diff --git a/bitbake/lib/bblayers/action.py > b/bitbake/lib/bblayers/action.py > > index 454c251410..5a2fcf7f8f 100644 > > --- a/bitbake/lib/bblayers/action.py > > +++ b/bitbake/lib/bblayers/action.py > > @@ -23,6 +23,27 @@ def plugin_init(plugins): > > > > > > class ActionPlugin(LayerPlugin): > > + > > + def do_check_layer(path): > > + """ > > + make sure that BBFILE_COLLECTIONS and BBPATH are found in > layer.conf > > + they should exist in non-commented lines > > + """ > > + def _is_found(var, line): > > + return var in line and not line.startswith('#') > > + > > + with open(path) as layer_conf_fd: > > + lines = layer_conf_fd.readlines() > > + find_result = {} > > + for to_find in ['BBFILE_COLLECTIONS', 'BBPATH']: > > + for line in lines: > > + find_result[to_find] = True if _is_found(to_find, > line) else False > > + if find_result[to_find]: > > + break > > + if not find_result[to_find]: > > + return False, "Specified layer configuration %s > doesn't contain %s\n" % (path, to_find) > > + return True, "" > > + > > I don't think writing a new parser is a great idea. There are other > ways we could detect if those variables were set/changed. > > I'd also question whether this is correct. It would be perfectly fine > for a layer just to add python library files using addpylib for example > and not set BB_FILE_COLLECTIONS or BBPATH. > > Cheers, > > Richard >
On Tue, 2023-10-10 at 15:35 +0100, Talel BELHADJ SALEM wrote: > Thanks for the reply, > > Regarding the second comment: > > Do you mean using addpylib can be an alternative for not setting > BBFILE_COLLECTIONS and BBPATH ? If yes, I can't see an example on how > to do that. Layers don't have to contain recipes. Whether or not there is an example, a layer with just library functions would be a valid layer and I'm not taking patches which show them as invalid. Cheers, Richard
"*Layers don't have to contain recipes*" is what I missed in the first place that led me to this analysis. I think it should stand out in the documentation as well. So, I think I will keep the first patch of find_bbfiles. Do you have any further ideas for BBFILE_COLLECTIONS and BBPATH ? Because still show-layers are based on BBFILE_COLLECTIONS, is it expected behavior or it should be fixed ? Kind regards Talel On Tue, Oct 10, 2023 at 3:59 PM Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Tue, 2023-10-10 at 15:35 +0100, Talel BELHADJ SALEM wrote: > > Thanks for the reply, > > > > Regarding the second comment: > > > > Do you mean using addpylib can be an alternative for not setting > > BBFILE_COLLECTIONS and BBPATH ? If yes, I can't see an example on how > > to do that. > > Layers don't have to contain recipes. Whether or not there is an > example, a layer with just library functions would be a valid layer and > I'm not taking patches which show them as invalid. > > Cheers, > > Richard > >
diff --git a/bitbake/lib/bb/cooker.py b/bitbake/lib/bb/cooker.py index 2adf4d297d..f163d3a2a3 100644 --- a/bitbake/lib/bb/cooker.py +++ b/bitbake/lib/bb/cooker.py @@ -1827,7 +1827,7 @@ class CookerCollectFiles(object): for ignored in ('SCCS', 'CVS', '.svn'): if ignored in dirs: dirs.remove(ignored) - found += [os.path.join(dir, f) for f in files if (f.endswith(['.bb', '.bbappend']))] + found += [os.path.join(dir, f) for f in files if (f.endswith(('.bb', '.bbappend')))] return found diff --git a/bitbake/lib/bb/cookerdata.py b/bitbake/lib/bb/cookerdata.py index ec3741cc1d..fd1e0cf06f 100644 --- a/bitbake/lib/bb/cookerdata.py +++ b/bitbake/lib/bb/cookerdata.py @@ -18,6 +18,7 @@ from functools import wraps import bb from bb import data import bb.parse +from bblayers.action import ActionPlugin logger = logging.getLogger("BitBake") parselog = logging.getLogger("BitBake.Parsing") @@ -377,7 +378,11 @@ class CookerDataBuilder(object): layer = layer.rstrip('/') data.setVar('LAYERDIR', layer) data.setVar('LAYERDIR_RE', re.escape(layer)) - data = parse_config_file(os.path.join(layer, "conf", "layer.conf"), data) + layer_conf = os.path.join(layer, "conf", "layer.conf") + res, msg = ActionPlugin.do_check_layer(layer_conf) + if not res: + bb.fatal(msg) + data = parse_config_file(layer_conf, data) data.expandVarref('LAYERDIR') data.expandVarref('LAYERDIR_RE') diff --git a/bitbake/lib/bblayers/action.py b/bitbake/lib/bblayers/action.py index 454c251410..5a2fcf7f8f 100644 --- a/bitbake/lib/bblayers/action.py +++ b/bitbake/lib/bblayers/action.py @@ -23,6 +23,27 @@ def plugin_init(plugins): class ActionPlugin(LayerPlugin): + + def do_check_layer(path): + """ + make sure that BBFILE_COLLECTIONS and BBPATH are found in layer.conf + they should exist in non-commented lines + """ + def _is_found(var, line): + return var in line and not line.startswith('#') + + with open(path) as layer_conf_fd: + lines = layer_conf_fd.readlines() + find_result = {} + for to_find in ['BBFILE_COLLECTIONS', 'BBPATH']: + for line in lines: + find_result[to_find] = True if _is_found(to_find, line) else False + if find_result[to_find]: + break + if not find_result[to_find]: + return False, "Specified layer configuration %s doesn't contain %s\n" % (path, to_find) + return True, "" + def do_add_layer(self, args): """Add one or more layers to bblayers.conf.""" layerdirs = [os.path.abspath(ldir) for ldir in args.layerdir] @@ -37,6 +58,11 @@ class ActionPlugin(LayerPlugin): sys.stderr.write("Specified layer directory %s doesn't contain a conf/layer.conf file\n" % layerdir) return 1 + res, msg = ActionPlugin.do_check_layer(layer_conf) + if not res: + sys.stderr.write(msg) + return 1 + bblayers_conf = os.path.join('conf', 'bblayers.conf') if not os.path.exists(bblayers_conf): sys.stderr.write("Unable to find bblayers.conf\n")
Empty layer.conf is added to bblayers.conf and not shown by show-layers. It will have no effect on BBFILES or BBPATH so no need to add it in the first place. Other than this, when working with the layer.conf variables, I noticed the following: 1) No BBFILE_COLLECTIONS will lead to no effect for BBFILE_PATTERN, so BBFILES on its own work well. 2) No BBFILE_COLLECTIONS and no BBPATH with BBFILES set to a directoy will work as well but custom inherits will fail only if the recipe matches: meta-*/*.bb. So checking on BBPATH will also fix this. In conclusion any messing around with layer.conf's variables will lead to unexpected behavior. Making sure BBFILE_COLLECTIONS exist will ensure: - error if no LAYERSERIES_COMPAT - error if no BBFILE_PATTERN - warning if no BBFILES - Layer shown in (bitbake-layers show-layers) Making sure BBPATH exist will ensure that classes and conf found regardless of BBFILES content (it can be a directory that will trigger find_bbfiles) Signed-off-by: Talel BELHAJSALEM <bhstalel@gmail.com> --- bitbake/lib/bb/cooker.py | 2 +- bitbake/lib/bb/cookerdata.py | 7 ++++++- bitbake/lib/bblayers/action.py | 26 ++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-)