diff mbox series

parse: Add support for addpylib conf file directive and BB_GLOBAL_PYMODULES

Message ID 20221207141539.2129083-1-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit afb8478d3853f6edf3669b93588314627d617d6b
Headers show
Series parse: Add support for addpylib conf file directive and BB_GLOBAL_PYMODULES | expand

Commit Message

Richard Purdie Dec. 7, 2022, 2:15 p.m. UTC 
For many years OE-Core has injected it's own python modules into the
python namespace using an immediate expansion of a variable in base.bbclass.

We really need this to become a first class citizen of the langauge, this new
addpylib directive allows that. Usage is of the form:

addpylib <directory> <namespace>

The namespace is imported and if there is an attribute BBIMPORT, that
list of names is iterated and imported too.

This mirrors what OE-Core has done for a long time with one difference in
implmentation, sys.path is only appended to. This means later imported
namespaces can't overwrite an earlier one and can't overwrite the main
python module space. In practice we've never done that and it isn't
something we should encourage or support.

The new directive is only applied for .conf files and not other filetypes
as it only makes sense in that context. It is also only allowed in the
"base configuration" context of cookerdata since adding it at the recipe
level wouldn't work properly due to the way it changes the global namespace.

At the same time, move the list of modules to place in the global namespace
into a BB_GLOBAL_PYMODULES variable. It is intended that only the core layer
should touch this and it is meant to be a very small list, usually os and sys.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cookerdata.py                 |  2 +-
 lib/bb/parse/__init__.py             |  4 ++--
 lib/bb/parse/ast.py                  | 27 +++++++++++++++++++++++++++
 lib/bb/parse/parse_py/BBHandler.py   |  4 ++--
 lib/bb/parse/parse_py/ConfHandler.py | 14 +++++++++++---
 5 files changed, 43 insertions(+), 8 deletions(-)

Comments

Christopher Larson Dec. 7, 2022, 3:31 p.m. UTC | #1 
On Wed, Dec 7, 2022 at 7:15 AM Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> For many years OE-Core has injected it's own python modules into the
> python namespace using an immediate expansion of a variable in
> base.bbclass.
>
> We really need this to become a first class citizen of the langauge, this
> new
> addpylib directive allows that. Usage is of the form:
>
> addpylib <directory> <namespace>
>
> The namespace is imported and if there is an attribute BBIMPORT, that
> list of names is iterated and imported too.
>
> This mirrors what OE-Core has done for a long time with one difference in
> implmentation, sys.path is only appended to. This means later imported
> namespaces can't overwrite an earlier one and can't overwrite the main
> python module space. In practice we've never done that and it isn't
> something we should encourage or support.
>
> The new directive is only applied for .conf files and not other filetypes
> as it only makes sense in that context. It is also only allowed in the
> "base configuration" context of cookerdata since adding it at the recipe
> level wouldn't work properly due to the way it changes the global
> namespace.
>
> At the same time, move the list of modules to place in the global namespace
> into a BB_GLOBAL_PYMODULES variable. It is intended that only the core
> layer
> should touch this and it is meant to be a very small list, usually os and
> sys.
>

This looks great in general, thanks for working on this. Anything is better
than the current hack, and using a proper directive seems clean. It seems
like BB_GLOBAL_PYMODULES and BBIMPORT will be checked in every addpylib
directive, though, which seems to indicate that metadata which doesn't have
any addpylib directives won't get those variables processed, is that
correct? The semantics there don't seem entirely clear. It also seems like
the import happens right there at the addpylib line, so it's an immediate
operation, right? To be expected, but we might want to mention that
specifically in the docs and/or commit message.
Richard Purdie Dec. 7, 2022, 5:25 p.m. UTC | #2 
On Wed, 2022-12-07 at 08:31 -0700, Christopher Larson wrote:
> This looks great in general, thanks for working on this. Anything is
> better than the current hack, and using a proper directive seems
> clean. It seems like BB_GLOBAL_PYMODULES and BBIMPORT will be checked
> in every addpylib directive, though, which seems to indicate that
> metadata which doesn't have any addpylib directives won't get those
> variables processed, is that correct?

That is correct, yes. We're assuming BB_GLOBAL_PYMODULES is set before
the first addpylib. I was ultimately thinking that BBIMPORT could then
limit scope to just the current module. I should perhaps just make that
explict now?

>  The semantics there don't seem entirely clear. It also seems like
> the import happens right there at the addpylib line, so it's an
> immediate operation, right?

I think so, although the statement cache makes this a little unclear
whether it is immediate or end of file.

>  To be expected, but we might want to mention that specifically in
> the docs and/or commit message.

Agreed, I'll make that clear. Thanks for the feedback.

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/cookerdata.py b/lib/bb/cookerdata.py
index c322ab2ffb..28cb59d83a 100644
--- a/lib/bb/cookerdata.py
+++ b/lib/bb/cookerdata.py
@@ -184,7 +184,7 @@  def catch_parse_error(func):
 
 @catch_parse_error
 def parse_config_file(fn, data, include=True):
-    return bb.parse.handle(fn, data, include)
+    return bb.parse.handle(fn, data, include, baseconfig=True)
 
 @catch_parse_error
 def _inherit(bbclass, data):
diff --git a/lib/bb/parse/__init__.py b/lib/bb/parse/__init__.py
index 347609513b..4cd82f115b 100644
--- a/lib/bb/parse/__init__.py
+++ b/lib/bb/parse/__init__.py
@@ -99,12 +99,12 @@  def supports(fn, data):
             return 1
     return 0
 
-def handle(fn, data, include = 0):
+def handle(fn, data, include=0, baseconfig=False):
     """Call the handler that is appropriate for this file"""
     for h in handlers:
         if h['supports'](fn, data):
             with data.inchistory.include(fn):
-                return h['handle'](fn, data, include)
+                return h['handle'](fn, data, include, baseconfig)
     raise ParseError("not a BitBake file", fn)
 
 def init(fn, data):
diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
index 9e0a0f5c98..85a9d760b0 100644
--- a/lib/bb/parse/ast.py
+++ b/lib/bb/parse/ast.py
@@ -9,6 +9,7 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 #
 
+import sys
 import bb
 from bb import methodpool
 from bb.parse import logger
@@ -269,6 +270,29 @@  class BBHandlerNode(AstNode):
             data.setVarFlag(h, "handler", 1)
         data.setVar('__BBHANDLERS', bbhands)
 
+class PyLibNode(AstNode):
+    def __init__(self, filename, lineno, libdir, namespace):
+        AstNode.__init__(self, filename, lineno)
+        self.libdir = libdir
+        self.namespace = namespace
+
+    def eval(self, data):
+        global_mods = (data.getVar("BB_GLOBAL_PYMODULES") or "").split()
+        for m in global_mods:
+            if m not in bb.utils._context:
+                bb.utils._context[m] = __import__(m)
+
+        libdir = data.expand(self.libdir)
+        if libdir not in sys.path:
+            sys.path.append(libdir)
+        try:
+            bb.utils._context[self.namespace] = __import__(self.namespace)
+            toimport = getattr(bb.utils._context[self.namespace], "BBIMPORTS", [])
+            for i in toimport:
+                bb.utils._context[i.split(".", 1)[0]] = __import__(i)
+        except AttributeError as e:
+            bb.error("Error importing OE modules: %s" % str(e))
+
 class InheritNode(AstNode):
     def __init__(self, filename, lineno, classes):
         AstNode.__init__(self, filename, lineno)
@@ -320,6 +344,9 @@  def handleDelTask(statements, filename, lineno, m):
 def handleBBHandlers(statements, filename, lineno, m):
     statements.append(BBHandlerNode(filename, lineno, m.group(1)))
 
+def handlePyLib(statements, filename, lineno, m):
+    statements.append(PyLibNode(filename, lineno, m.group(1), m.group(2)))
+
 def handleInherit(statements, filename, lineno, m):
     classes = m.group(1)
     statements.append(InheritNode(filename, lineno, classes))
diff --git a/lib/bb/parse/parse_py/BBHandler.py b/lib/bb/parse/parse_py/BBHandler.py
index 584966fea1..4d5b45e1ef 100644
--- a/lib/bb/parse/parse_py/BBHandler.py
+++ b/lib/bb/parse/parse_py/BBHandler.py
@@ -101,7 +101,7 @@  def get_statements(filename, absolute_filename, base_name):
             cached_statements[absolute_filename] = statements
         return statements
 
-def handle(fn, d, include):
+def handle(fn, d, include, baseconfig=False):
     global __infunc__, __body__, __residue__, __classname__
     __body__ = []
     __infunc__ = []
@@ -265,7 +265,7 @@  def feeder(lineno, s, fn, root, statements, eof=False):
         ast.handleInherit(statements, fn, lineno, m)
         return
 
-    return ConfHandler.feeder(lineno, s, fn, statements)
+    return ConfHandler.feeder(lineno, s, fn, statements, conffile=False)
 
 # Add us to the handlers list
 from .. import handlers
diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py
index 451e68dd66..3076067287 100644
--- a/lib/bb/parse/parse_py/ConfHandler.py
+++ b/lib/bb/parse/parse_py/ConfHandler.py
@@ -46,6 +46,7 @@  __require_regexp__ = re.compile( r"require\s+(.+)" )
 __export_regexp__ = re.compile( r"export\s+([a-zA-Z0-9\-_+.${}/~]+)$" )
 __unset_regexp__ = re.compile( r"unset\s+([a-zA-Z0-9\-_+.${}/~]+)$" )
 __unset_flag_regexp__ = re.compile( r"unset\s+([a-zA-Z0-9\-_+.${}/~]+)\[([a-zA-Z0-9\-_+.]+)\]$" )
+__addpylib_regexp__      = re.compile(r"addpylib\s+(.+)\s+(.+)" )
 
 def init(data):
     return
@@ -107,7 +108,7 @@  def include_single_file(parentfn, fn, lineno, data, error_out):
 # parsing. This turns out to be a hard problem to solve any other way.
 confFilters = []
 
-def handle(fn, data, include):
+def handle(fn, data, include, baseconfig=False):
     init(data)
 
     if include == 0:
@@ -144,7 +145,7 @@  def handle(fn, data, include):
             # skip comments
             if s[0] == '#':
                 continue
-            feeder(lineno, s, abs_fn, statements)
+            feeder(lineno, s, abs_fn, statements, baseconfig=baseconfig)
 
     # DONE WITH PARSING... time to evaluate
     data.setVar('FILE', abs_fn)
@@ -157,7 +158,9 @@  def handle(fn, data, include):
 
     return data
 
-def feeder(lineno, s, fn, statements):
+# baseconfig is set for the bblayers/layer.conf cookerdata config parsing
+# The function is also used by BBHandler, conffile would be False
+def feeder(lineno, s, fn, statements, baseconfig=False, conffile=True):
     m = __config_regexp__.match(s)
     if m:
         groupd = m.groupdict()
@@ -189,6 +192,11 @@  def feeder(lineno, s, fn, statements):
         ast.handleUnsetFlag(statements, fn, lineno, m)
         return
 
+    m = __addpylib_regexp__.match(s)
+    if baseconfig and conffile and m:
+        ast.handlePyLib(statements, fn, lineno, m)
+        return
+
     raise ParseError("unparsed line: '%s'" % s, fn, lineno);
 
 # Add us to the handlers list