Openembedded Bitbake Development
 help / color / mirror / Atom feed
* [PATCH 0/2] variable/include tracking and _remove
@ 2012-08-16  1:14 Peter Seebach
  2012-08-16  1:14 ` [PATCH 1/2] data_smart.py: track file inclusion and variable modifications Peter Seebach
  2012-08-16  1:14 ` [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append Peter Seebach
  0 siblings, 2 replies; 8+ messages in thread
From: Peter Seebach @ 2012-08-16  1:14 UTC (permalink / raw)
  To: bitbake-devel

These aren't entirely related, but I don't think I could get
the _remove feature separated out cleanly, and there was no way
I could implement it on a tree without the variable tracking. :)

Patch #1 is a rebase/merge/cleanup of the variable and include tracking
feature I've previously sent in; this version is much cleaned up,
and produces a lot less output (<1MB instead of >10MB for a typical
project in our build system) which is substantially more informative.

Patch #2 is a proof-of-concept demonstration of how to make a _remove
keyword. Usage:

        DUMMY = "foo"
        DUMMY += "bar"
        DUMMY_remove = "foo"
        # DUMMY="bar"

This only works for space-separated lists, but we have a LOT of those.
The only obvious things I see that really are lists, and aren't
space-separated, are OVERRIDES and PATH, and I'm pretty sure that
we don't *need* to remove things from those.

The following changes since commit 23bd5300b4a99218a15f4f6b0ab4091d63a602a5:
  Richard Purdie (1):
        data_smart: Fix unanchored regexp causing strange parsing issue

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib seebs/remove
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=seebs/remove

Peter Seebach (2):
  data_smart.py: track file inclusion and variable modifications
  data_smart.py: implement _remove as a keyword like _append.

 lib/bb/cooker.py                     |    2 +
 lib/bb/data.py                       |   65 ++++++++--
 lib/bb/data_smart.py                 |  220 +++++++++++++++++++++++++++-------
 lib/bb/parse/__init__.py             |    6 +-
 lib/bb/parse/ast.py                  |   76 +++++++-----
 lib/bb/parse/parse_py/ConfHandler.py |    6 +-
 6 files changed, 286 insertions(+), 89 deletions(-)




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] data_smart.py: track file inclusion and variable modifications
  2012-08-16  1:14 [PATCH 0/2] variable/include tracking and _remove Peter Seebach
@ 2012-08-16  1:14 ` Peter Seebach
  2012-08-16 14:32   ` Richard Purdie
  2012-08-16  1:14 ` [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append Peter Seebach
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Seebach @ 2012-08-16  1:14 UTC (permalink / raw)
  To: bitbake-devel

It is often the case that bitbake variables have values which
strike the user as wrong, but finding out how they got that way
can be gratuitously complicated.

This patch adds two levels of tracking. First, file inclusions
are recorded, so the system can offer a tree structure showing
which files were processed, and which other files they included.

Second, variable assignments, flag settings, and such are recorded
with source information showing which file and line, or which block
of Python code, produced each change.

This information is then used in the bitbake -e output to provide
a usable history.

For variables:
1.  We create a history dict in the data_smart object, the
members of which are lists of tuples.  It's three, count them,
THREE unrelated data types in a single object!
2.  There is an eventLog(...) function which allows recording
things which happen to variables.
3.  setVar and friends are updated to take optional arguments
indicating where a change occurred, and optional information
about how to describe the change.
4.  We log pretty much everything if logging is enabled.
5.  You can query the history of a thing with data.getHistory(var)

In addition to file and line (which will be inferred to be
"the calling Python code" if file is None), you can specify
the operation to report and the value to report. This is used
because append operations would otherwise report the entire
new value after the append, which is almost never what you
want.

The special file name 'Ignore' indicates that an event need
not be logged.

For file inclusion:
The file inclusion code maintains a stack of files in process,
and creates a tree structure reflecting those changes. This is
also attached to bitbake -e output.

Tracking must be specifically turned on.  If it's not, the
history object is a single empty table (cheap) and the calls
to eventLog() and extra argument passing are also probably
quite cheap.  As a proof of concept, the showEnvironment function
used by bitbake -e now uses this if available.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 lib/bb/cooker.py                     |    2 +
 lib/bb/data.py                       |   65 ++++++++++---
 lib/bb/data_smart.py                 |  173 +++++++++++++++++++++++++++-------
 lib/bb/parse/__init__.py             |    6 +-
 lib/bb/parse/ast.py                  |   70 ++++++++------
 lib/bb/parse/parse_py/ConfHandler.py |    4 +-
 6 files changed, 239 insertions(+), 81 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 23fffc9..8d59774 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -830,6 +830,8 @@ class BBCooker:
 
     def parseConfigurationFiles(self, prefiles, postfiles):
         data = self.configuration.data
+        if self.configuration.show_environment:
+            data.enableTracking()
         bb.parse.init_parser(data)
 
         # Parse files for loading *before* bitbake.conf and any includes
diff --git a/lib/bb/data.py b/lib/bb/data.py
index 5b7a092..a762a40 100644
--- a/lib/bb/data.py
+++ b/lib/bb/data.py
@@ -74,14 +74,22 @@ def createCopy(source):
     """
     return source.createCopy()
 
+# These are used in dataSmart, here as protection against KeyErrors.
+def enableTracking():
+    pass
+
+def disableTracking():
+    pass
+
 def initVar(var, d):
     """Non-destructive var init for data structure"""
     d.initVar(var)
 
 
-def setVar(var, value, d):
+def setVar(var, value, d, filename = None, lineno = None):
     """Set a variable to a given value"""
-    d.setVar(var, value)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.setVar(var, value, filename, lineno)
 
 
 def getVar(var, d, exp = 0):
@@ -89,27 +97,31 @@ def getVar(var, d, exp = 0):
     return d.getVar(var, exp)
 
 
-def renameVar(key, newkey, d):
+def renameVar(key, newkey, d, filename = None, lineno = None):
     """Renames a variable from key to newkey"""
-    d.renameVar(key, newkey)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.renameVar(key, newkey, filename, lineno)
 
-def delVar(var, d):
+def delVar(var, d, filename = None, lineno = None):
     """Removes a variable from the data set"""
-    d.delVar(var)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.delVar(var, filename, lineno)
 
-def setVarFlag(var, flag, flagvalue, d):
+def setVarFlag(var, flag, flagvalue, d, filename = None, lineno = None):
     """Set a flag for a given variable to a given value"""
-    d.setVarFlag(var, flag, flagvalue)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.setVarFlag(var, flag, flagvalue, filename, lineno)
 
 def getVarFlag(var, flag, d):
     """Gets given flag from given var"""
     return d.getVarFlag(var, flag)
 
-def delVarFlag(var, flag, d):
+def delVarFlag(var, flag, d, filename = None, lineno = None):
     """Removes a given flag from the variable's flags"""
-    d.delVarFlag(var, flag)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.delVarFlag(var, flag, filename, lineno)
 
-def setVarFlags(var, flags, d):
+def setVarFlags(var, flags, d, filename = None, lineno = None):
     """Set the flags for a given variable
 
     Note:
@@ -117,15 +129,17 @@ def setVarFlags(var, flags, d):
         flags. Think of this method as
         addVarFlags
     """
-    d.setVarFlags(var, flags)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.setVarFlags(var, flags, filename, lineno)
 
 def getVarFlags(var, d):
     """Gets a variable's flags"""
     return d.getVarFlags(var)
 
-def delVarFlags(var, d):
+def delVarFlags(var, d, filename = None, lineno = None):
     """Removes a variable's flags"""
-    d.delVarFlags(var)
+    filename, lineno = d.infer_file_and_line(filename, lineno)
+    d.delVarFlags(var, filename, lineno)
 
 def keys(d):
     """Return a list of keys in d"""
@@ -195,6 +209,13 @@ def emit_var(var, o=sys.__stdout__, d = init(), all=False):
 
     if all:
         commentVal = re.sub('\n', '\n#', str(oval))
+        history = d.getHistory(var)
+        if history:
+            o.write('#\n# %s [%d]\n' % (var, len(history)))
+            for events in history:
+                events = (events[0], events[1], events[2], re.sub('\n', '\n#     ', str(events[3])))
+                o.write('#   %s %s:%s:\n#     <%s>\n' % events)
+            o.write('#\n')
         o.write('# %s=%s\n' % (var, commentVal))
 
     if (var.find("-") != -1 or var.find(".") != -1 or var.find('{') != -1 or var.find('}') != -1 or var.find('+') != -1) and not all:
@@ -232,10 +253,26 @@ def emit_env(o=sys.__stdout__, d = init(), all=False):
     isfunc = lambda key: bool(d.getVarFlag(key, "func"))
     keys = sorted((key for key in d.keys() if not key.startswith("__")), key=isfunc)
     grouped = groupby(keys, isfunc)
+    # Include history!
+    if d.tracking():
+        o.write('#\n# INCLUDE HISTORY:\n#\n')
+        emit_history(o, d.getIncludeHistory())
+        
     for isfunc, keys in grouped:
         for key in keys:
             emit_var(key, o, d, all and not isfunc) and o.write('\n')
 
+def emit_history(o, h, depth = 0):
+    if not h:
+        return
+    for event in h:
+        o.write("# %*s%s" % (depth * 2, "", event[0]))
+        if event[1]:
+            o.write(" includes:\n")
+            emit_history(o, event[1], depth + 1)
+        else:
+            o.write("\n")
+
 def exported_keys(d):
     return (key for key in d.keys() if not key.startswith('__') and
                                       d.getVarFlag(key, 'export') and
diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
index 31216e0..a128914 100644
--- a/lib/bb/data_smart.py
+++ b/lib/bb/data_smart.py
@@ -28,7 +28,9 @@ BitBake build tools.
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 # Based on functions from the base bb module, Copyright 2003 Holger Schurig
 
+import traceback
 import copy, re
+import sys
 from collections import MutableMapping
 import logging
 import hashlib
@@ -114,13 +116,51 @@ class ExpansionError(Exception):
 class DataSmart(MutableMapping):
     def __init__(self, special = COWDictBase.copy(), seen = COWDictBase.copy() ):
         self.dict = {}
+        self.history = {}
+        self.include_history = []
+        self.include_stack = [(-1, self.include_history)]
 
         # cookie monster tribute
         self._special_values = special
         self._seen_overrides = seen
+        self._tracking_enabled = False
 
         self.expand_cache = {}
 
+    def includeLog(self, filename):
+        """includeLog(included_file) shows that the file was included
+        by the currently-processed file or context."""
+        if self._tracking_enabled:
+            event = (filename, [])
+            position = (len(self.include_stack[-1][1]), event[1])
+            self.include_stack[-1][1].append(event)
+            self.include_stack.append(position)
+
+    def includeLogDone(self, filename):
+        if self._tracking_enabled:
+            if len(self.include_stack) > 1:
+                self.include_stack.pop()
+            else:
+                bb.warn("Uh-oh:  includeLogDone(%s) tried to empty the stack." % filename)
+
+    def getIncludeHistory(self):
+        return self.include_history
+
+    def tracking(self):
+        return self._tracking_enabled
+
+    def enableTracking(self):
+        self._tracking_enabled = True
+
+    def disableTracking(self):
+        self._tracking_enabled = False
+
+    def eventLog(self, var, event, value, filename = None, lineno = None):
+        if self._tracking_enabled and filename != 'Ignore':
+            if var not in self.history:
+                self.history[var] = []
+            self.history[var].append((event, filename, lineno, value))
+
     def expandWithRefs(self, s, varname):
 
         if not isinstance(s, basestring): # sanity check
@@ -153,6 +193,14 @@ class DataSmart(MutableMapping):
     def expand(self, s, varname = None):
         return self.expandWithRefs(s, varname).value
 
+    # Figure out how to describe the caller when file/line weren't
+    # specified.
+    def infer_file_and_line(self, filename, lineno):
+        details = lineno
+        if self._tracking_enabled and not filename and filename != 'Ignore':
+            filename, lineno, func, line = traceback.extract_stack(limit=3)[0]
+            details = "%d [%s]" % (lineno, func)
+        return filename, details
 
     def finalize(self):
         """Performs final steps upon the datastore, including application of overrides"""
@@ -186,10 +234,14 @@ class DataSmart(MutableMapping):
             for var in vars:
                 name = var[:-l]
                 try:
-                    self.setVar(name, self.getVar(var, False))
-                    self.delVar(var)
-                except Exception:
-                    logger.info("Untracked delVar")
+                    # Move the history of the override into the history of
+                    # the overridden:
+                    for event in self.getHistory(var):
+                        self.eventLog(name, 'override:%s' % o, event[3], event[1], event[2])
+                    self.setVar(name, self.getVar(var, False), 'Ignore')
+                    self.delVar(var, 'Ignore')
+                except Exception, e:
+                    logger.info("Untracked delVar %s: %s" % (var, e))
 
         # now on to the appends and prepends
         for op in __setvar_keyword__:
@@ -210,16 +262,17 @@ class DataSmart(MutableMapping):
                         if op == "_append":
                             sval = self.getVar(append, False) or ""
                             sval += a
-                            self.setVar(append, sval)
+                            self.setVar(append, sval, 'Ignore')
                         elif op == "_prepend":
                             sval = a + (self.getVar(append, False) or "")
-                            self.setVar(append, sval)
+                            self.setVar(append, sval, 'Ignore')
 
                     # We save overrides that may be applied at some later stage
+                    # ... but we don't need to report on this.
                     if keep:
-                        self.setVarFlag(append, op, keep)
+                        self.setVarFlag(append, op, keep, 'Ignore')
                     else:
-                        self.delVarFlag(append, op)
+                        self.delVarFlag(append, op, 'Ignore')
 
     def initVar(self, var):
         self.expand_cache = {}
@@ -247,7 +300,15 @@ class DataSmart(MutableMapping):
         else:
             self.initVar(var)
 
-    def setVar(self, var, value):
+    # In some cases, we want to set a value, but only record part of it;
+    # for instance, when appending something, we want to record what we
+    # appended, not what the complete value of the now-appended value.
+    # This becomes especially obvious when looking at the output for
+    # BBCLASSEXTEND. "details", if provided, replace the variable value
+    # in the log.
+
+    def setVar(self, var, value, filename = None, lineno = None, op = 'set', details = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
         self.expand_cache = {}
         match  = __setvar_regexp__.match(var)
         if match and match.group("keyword") in __setvar_keyword__:
@@ -255,8 +316,13 @@ class DataSmart(MutableMapping):
             keyword = match.group("keyword")
             override = match.group('add')
             l = self.getVarFlag(base, keyword) or []
-            l.append([value, override])
-            self.setVarFlag(base, keyword, l)
+            # Compute new details: This is what we're actually appending.
+            details = [value, override]
+            l.append(details)
+            # Log the details using the keyword as the op name, instead
+            # of logging the entire new value as a flag change.
+            self.setVarFlag(base, keyword, l, 'Ignore')
+            self.eventLog(base, keyword, details, filename, lineno)
 
             # todo make sure keyword is not __doc__ or __module__
             # pay the cookie monster
@@ -281,6 +347,12 @@ class DataSmart(MutableMapping):
 
         # setting var
         self.dict[var]["content"] = value
+        self.eventLog(var, op, details or value, filename, lineno)
+
+    def getHistory(self, var):
+        if var in self.history:
+            return self.history[var]
+        return []
 
     def getVar(self, var, expand=False, noweakdefault=False):
         value = self.getVarFlag(var, "content", False, noweakdefault)
@@ -290,13 +362,14 @@ class DataSmart(MutableMapping):
             return self.expand(value, var)
         return value
 
-    def renameVar(self, key, newkey):
+    def renameVar(self, key, newkey, filename = None, lineno = None):
         """
         Rename the variable key to newkey
         """
+        filename, lineno = self.infer_file_and_line(filename, lineno)
         val = self.getVar(key, 0)
         if val is not None:
-            self.setVar(newkey, val)
+            self.setVar(newkey, val, filename, lineno, 'rename-create')
 
         for i in ('_append', '_prepend'):
             src = self.getVarFlag(key, i)
@@ -305,34 +378,40 @@ class DataSmart(MutableMapping):
 
             dest = self.getVarFlag(newkey, i) or []
             dest.extend(src)
-            self.setVarFlag(newkey, i, dest)
+            self.setVarFlag(newkey, i, dest, filename, lineno, 'rename')
 
             if i in self._special_values and key in self._special_values[i]:
                 self._special_values[i].remove(key)
                 self._special_values[i].add(newkey)
 
-        self.delVar(key)
+        self.delVar(key, filename, lineno, 'rename-delete')
 
-    def appendVar(self, key, value):
-        value = (self.getVar(key, False) or "") + value
-        self.setVar(key, value)
+    def appendVar(self, key, newValue, filename = None, lineno = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
+        value = (self.getVar(key, False) or "") + newValue
+        self.setVar(key, value, filename, lineno, 'append', newValue)
 
-    def prependVar(self, key, value):
-        value = value + (self.getVar(key, False) or "")
-        self.setVar(key, value)
+    def prependVar(self, key, newValue, filename = None, lineno = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
+        value = newValue + (self.getVar(key, False) or "")
+        self.setVar(key, value, filename, lineno, 'prepend', newValue)
 
-    def delVar(self, var):
+    def delVar(self, var, filename = None, lineno = None, op = 'del'):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
         self.expand_cache = {}
         self.dict[var] = {}
+        self.eventLog(var, op, '', filename, lineno)
         if '_' in var:
             override = var[var.rfind('_')+1:]
             if override and override in self._seen_overrides and var in self._seen_overrides[override]:
                 self._seen_overrides[override].remove(var)
 
-    def setVarFlag(self, var, flag, flagvalue):
+    def setVarFlag(self, var, flag, flagvalue, filename = None, lineno = None, op = 'set', details = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
         if not var in self.dict:
             self._makeShadowCopy(var)
         self.dict[var][flag] = flagvalue
+        self.eventLog(var, '[flag %s] %s' % (flag, op), details or flagvalue, filename, lineno)
 
     def getVarFlag(self, var, flag, expand=False, noweakdefault=False):
         local_var = self._findVar(var)
@@ -346,31 +425,37 @@ class DataSmart(MutableMapping):
             value = self.expand(value, None)
         return value
 
-    def delVarFlag(self, var, flag):
+    def delVarFlag(self, var, flag, filename = None, lineno = None):
         local_var = self._findVar(var)
         if not local_var:
             return
         if not var in self.dict:
             self._makeShadowCopy(var)
 
+        filename, lineno = self.infer_file_and_line(filename, lineno)
+        self.eventLog(var, 'del flag %s' % flag, '', filename, lineno)
         if var in self.dict and flag in self.dict[var]:
             del self.dict[var][flag]
 
-    def appendVarFlag(self, key, flag, value):
-        value = (self.getVarFlag(key, flag, False) or "") + value
-        self.setVarFlag(key, flag, value)
+    def appendVarFlag(self, key, flag, newValue, filename = None, lineno = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
+        value = (self.getVarFlag(key, flag, False) or "") + newValue
+        self.setVarFlag(key, flag, value, filename, lineno, 'append', newValue)
 
-    def prependVarFlag(self, key, flag, value):
-        value = value + (self.getVarFlag(key, flag, False) or "")
-        self.setVarFlag(key, flag, value)
+    def prependVarFlag(self, key, flag, newValue, filename = None, lineno = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
+        value = newValue + (self.getVarFlag(key, flag, False) or "")
+        self.setVarFlag(key, flag, value, filename, lineno, 'prepend', newValue)
 
-    def setVarFlags(self, var, flags):
+    def setVarFlags(self, var, flags, filename = None, lineno = None, details = None):
+        filename, lineno = self.infer_file_and_line(filename, lineno)
         if not var in self.dict:
             self._makeShadowCopy(var)
 
         for i in flags:
             if i == "content":
                 continue
+            self.eventLog(var, 'set flag %s' % i, details or flags[i], filename, lineno)
             self.dict[var][i] = flags[i]
 
     def getVarFlags(self, var):
@@ -388,13 +473,15 @@ class DataSmart(MutableMapping):
         return flags
 
 
-    def delVarFlags(self, var):
+    def delVarFlags(self, var, filename = None, lineno = None):
         if not var in self.dict:
             self._makeShadowCopy(var)
 
         if var in self.dict:
             content = None
 
+            filename, lineno = self.infer_file_and_line(filename, lineno)
+            self.eventLog(var, 'clear all flags', '', filename, lineno)
             # try to save the content
             if "content" in self.dict[var]:
                 content  = self.dict[var]["content"]
@@ -411,10 +498,25 @@ class DataSmart(MutableMapping):
         # we really want this to be a DataSmart...
         data = DataSmart(seen=self._seen_overrides.copy(), special=self._special_values.copy())
         data.dict["_data"] = self.dict
-
+        if self._tracking_enabled:
+            data._tracking_enabled = self._tracking_enabled
+            data.history = copy.deepcopy(self.history)
+            data.include_history = copy.deepcopy(self.include_history)
+            data.include_stack = []
+            oldref = self.include_history
+            newref = data.include_history
+            # Create corresponding references, if we can
+            try:
+                for item in self.include_stack:
+                    if item[0] >= 0:
+                        newref = newref[item[0]][1]
+                    newevent = (item[0], newref)
+                    data.include_stack.append(newevent)
+            except Exception:
+                sys.exc_clear()
         return data
 
-    def expandVarref(self, variable, parents=False):
+    def expandVarref(self, variable, parents=False, filename = None, lineno = None):
         """Find all references to variable in the data and expand it
            in place, optionally descending to parent datastores."""
 
@@ -423,12 +525,13 @@ class DataSmart(MutableMapping):
         else:
             keys = self.localkeys()
 
+        filename, lineno = self.infer_file_and_line(filename, lineno)
         ref = '${%s}' % variable
         value = self.getVar(variable, False)
         for key in keys:
             referrervalue = self.getVar(key, False)
             if referrervalue and ref in referrervalue:
-                self.setVar(key, referrervalue.replace(ref, value))
+                self.setVar(key, referrervalue.replace(ref, value), filename, lineno, 'expandVarref')
 
     def localkeys(self):
         for key in self.dict:
diff --git a/lib/bb/parse/__init__.py b/lib/bb/parse/__init__.py
index 7b9c47e..0ea6a55 100644
--- a/lib/bb/parse/__init__.py
+++ b/lib/bb/parse/__init__.py
@@ -88,7 +88,11 @@ def handle(fn, data, include = 0):
     """Call the handler that is appropriate for this file"""
     for h in handlers:
         if h['supports'](fn, data):
-            return h['handle'](fn, data, include)
+            data.includeLog(fn)
+            try:
+                return h['handle'](fn, data, include)
+            finally:
+                data.includeLogDone(fn)
     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 86f9463..dbc2237 100644
--- a/lib/bb/parse/ast.py
+++ b/lib/bb/parse/ast.py
@@ -69,7 +69,7 @@ class ExportNode(AstNode):
         self.var = var
 
     def eval(self, data):
-        data.setVarFlag(self.var, "export", 1)
+        data.setVarFlag(self.var, "export", 1, self.filename, self.lineno)
 
 class DataNode(AstNode):
     """
@@ -91,33 +91,45 @@ class DataNode(AstNode):
     def eval(self, data):
         groupd = self.groupd
         key = groupd["var"]
+        op = 'set'
+        details = None
         if "exp" in groupd and groupd["exp"] != None:
-            data.setVarFlag(key, "export", 1)
+            data.setVarFlag(key, "export", 1, self.filename, self.lineno)
         if "ques" in groupd and groupd["ques"] != None:
             val = self.getFunc(key, data)
             if val == None:
                 val = groupd["value"]
+                op = 'set?'
         elif "colon" in groupd and groupd["colon"] != None:
             e = data.createCopy()
             bb.data.update_data(e)
             val = e.expand(groupd["value"], key + "[:=]")
+            op = 'immediate'
         elif "append" in groupd and groupd["append"] != None:
             val = "%s %s" % ((self.getFunc(key, data) or ""), groupd["value"])
+            op = 'append'
+            details = groupd["value"]
         elif "prepend" in groupd and groupd["prepend"] != None:
             val = "%s %s" % (groupd["value"], (self.getFunc(key, data) or ""))
+            op = 'prepend'
+            details = groupd["value"]
         elif "postdot" in groupd and groupd["postdot"] != None:
             val = "%s%s" % ((self.getFunc(key, data) or ""), groupd["value"])
+            op = 'postdot'
+            details = groupd["value"]
         elif "predot" in groupd and groupd["predot"] != None:
             val = "%s%s" % (groupd["value"], (self.getFunc(key, data) or ""))
+            op = 'predot'
+            details = groupd["value"]
         else:
             val = groupd["value"]
 
         if 'flag' in groupd and groupd['flag'] != None:
-            data.setVarFlag(key, groupd['flag'], val)
+            data.setVarFlag(key, groupd['flag'], val, self.filename, self.lineno, op)
         elif groupd["lazyques"]:
-            data.setVarFlag(key, "defaultval", val)
+            data.setVarFlag(key, "defaultval", val, self.filename, self.lineno, op)
         else:
-            data.setVar(key, val)
+            data.setVar(key, val, self.filename, self.lineno, op, details)
 
 class MethodNode(AstNode):
     def __init__(self, filename, lineno, func_name, body):
@@ -133,10 +145,10 @@ class MethodNode(AstNode):
                 bb.methodpool.insert_method(funcname, text, self.filename)
             anonfuncs = data.getVar('__BBANONFUNCS') or []
             anonfuncs.append(funcname)
-            data.setVar('__BBANONFUNCS', anonfuncs)
+            data.setVar('__BBANONFUNCS', anonfuncs, self.filename, self.lineno)
         else:
-            data.setVarFlag(self.func_name, "func", 1)
-            data.setVar(self.func_name, '\n'.join(self.body))
+            data.setVarFlag(self.func_name, "func", 1, self.filename, self.lineno)
+            data.setVar(self.func_name, '\n'.join(self.body), self.filename, self.lineno)
 
 class PythonMethodNode(AstNode):
     def __init__(self, filename, lineno, function, define, body):
@@ -152,9 +164,9 @@ class PythonMethodNode(AstNode):
         text = '\n'.join(self.body)
         if not bb.methodpool.parsed_module(self.define):
             bb.methodpool.insert_method(self.define, text, self.filename)
-        data.setVarFlag(self.function, "func", 1)
-        data.setVarFlag(self.function, "python", 1)
-        data.setVar(self.function, text)
+        data.setVarFlag(self.function, "func", 1, self.filename, self.lineno)
+        data.setVarFlag(self.function, "python", 1, self.filename, self.lineno)
+        data.setVar(self.function, text, self.filename, self.lineno)
 
 class MethodFlagsNode(AstNode):
     def __init__(self, filename, lineno, key, m):
@@ -166,16 +178,16 @@ class MethodFlagsNode(AstNode):
         if data.getVar(self.key):
             # clean up old version of this piece of metadata, as its
             # flags could cause problems
-            data.setVarFlag(self.key, 'python', None)
-            data.setVarFlag(self.key, 'fakeroot', None)
+            data.setVarFlag(self.key, 'python', None, self.filename, self.lineno)
+            data.setVarFlag(self.key, 'fakeroot', None, self.filename, self.lineno)
         if self.m.group("py") is not None:
-            data.setVarFlag(self.key, "python", "1")
+            data.setVarFlag(self.key, "python", "1", self.filename, self.lineno)
         else:
-            data.delVarFlag(self.key, "python")
+            data.delVarFlag(self.key, "python", self.filename, self.lineno)
         if self.m.group("fr") is not None:
-            data.setVarFlag(self.key, "fakeroot", "1")
+            data.setVarFlag(self.key, "fakeroot", "1", self.filename, self.lineno)
         else:
-            data.delVarFlag(self.key, "fakeroot")
+            data.delVarFlag(self.key, "fakeroot", self.filename, self.lineno)
 
 class ExportFuncsNode(AstNode):
     def __init__(self, filename, lineno, fns, classes):
@@ -201,21 +213,21 @@ class ExportFuncsNode(AstNode):
                     continue
 
                 if data.getVar(var):
-                    data.setVarFlag(var, 'python', None)
-                    data.setVarFlag(var, 'func', None)
+                    data.setVarFlag(var, 'python', None, self.filename, self.lineno)
+                    data.setVarFlag(var, 'func', None, self.filename, self.lineno)
 
                 for flag in [ "func", "python" ]:
                     if data.getVarFlag(calledvar, flag):
-                        data.setVarFlag(var, flag, data.getVarFlag(calledvar, flag))
+                        data.setVarFlag(var, flag, data.getVarFlag(calledvar, flag), self.filename, self.lineno)
                 for flag in [ "dirs" ]:
                     if data.getVarFlag(var, flag):
-                        data.setVarFlag(calledvar, flag, data.getVarFlag(var, flag))
+                        data.setVarFlag(calledvar, flag, data.getVarFlag(var, flag), self.filename, self.lineno)
 
                 if data.getVarFlag(calledvar, "python"):
-                    data.setVar(var, "    bb.build.exec_func('" + calledvar + "', d)\n")
+                    data.setVar(var, "    bb.build.exec_func('" + calledvar + "', d)\n", self.filename, self.lineno)
                 else:
-                    data.setVar(var, "    " + calledvar + "\n")
-                data.setVarFlag(var, 'export_func', '1')
+                    data.setVar(var, "    " + calledvar + "\n", self.filename, self.lineno)
+                data.setVarFlag(var, 'export_func', '1', self.filename, self.lineno)
 
 class AddTaskNode(AstNode):
     def __init__(self, filename, lineno, func, before, after):
@@ -229,11 +241,11 @@ class AddTaskNode(AstNode):
         if self.func[:3] != "do_":
             var = "do_" + self.func
 
-        data.setVarFlag(var, "task", 1)
+        data.setVarFlag(var, "task", 1, self.filename, self.lineno)
         bbtasks = data.getVar('__BBTASKS') or []
         if not var in bbtasks:
             bbtasks.append(var)
-        data.setVar('__BBTASKS', bbtasks)
+        data.setVar('__BBTASKS', bbtasks, self.filename, self.lineno)
 
         existing = data.getVarFlag(var, "deps") or []
         if self.after is not None:
@@ -241,13 +253,13 @@ class AddTaskNode(AstNode):
             for entry in self.after.split():
                 if entry not in existing:
                     existing.append(entry)
-        data.setVarFlag(var, "deps", existing)
+        data.setVarFlag(var, "deps", existing, self.filename, self.lineno)
         if self.before is not None:
             # set up things that depend on this func
             for entry in self.before.split():
                 existing = data.getVarFlag(entry, "deps") or []
                 if var not in existing:
-                    data.setVarFlag(entry, "deps", [var] + existing)
+                    data.setVarFlag(entry, "deps", [var] + existing, self.filename, self.lineno)
 
 class BBHandlerNode(AstNode):
     def __init__(self, filename, lineno, fns):
@@ -258,7 +270,7 @@ class BBHandlerNode(AstNode):
         bbhands = data.getVar('__BBHANDLERS') or []
         for h in self.hs:
             bbhands.append(h)
-            data.setVarFlag(h, "handler", 1)
+            data.setVarFlag(h, "handler", 1, self.filename, self.lineno)
         data.setVar('__BBHANDLERS', bbhands)
 
 class InheritNode(AstNode):
diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py
index 6f77bd4..4a1012e 100644
--- a/lib/bb/parse/parse_py/ConfHandler.py
+++ b/lib/bb/parse/parse_py/ConfHandler.py
@@ -110,10 +110,10 @@ def handle(fn, data, include):
         feeder(lineno, s, fn, statements)
 
     # DONE WITH PARSING... time to evaluate
-    data.setVar('FILE', abs_fn)
+    data.setVar('FILE', abs_fn, 'Ignore')
     statements.eval(data)
     if oldfile:
-        data.setVar('FILE', oldfile)
+        data.setVar('FILE', oldfile, 'Ignore')
 
     for f in confFilters:
         f(fn, data)
-- 
1.7.0.4




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append.
  2012-08-16  1:14 [PATCH 0/2] variable/include tracking and _remove Peter Seebach
  2012-08-16  1:14 ` [PATCH 1/2] data_smart.py: track file inclusion and variable modifications Peter Seebach
@ 2012-08-16  1:14 ` Peter Seebach
  2012-08-16 10:13   ` Richard Purdie
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Seebach @ 2012-08-16  1:14 UTC (permalink / raw)
  To: bitbake-devel

There has historically been no way to remove a single word
from a list, but many lists (such as DISTRO_FEATURES) are
assembled from many sources all over the place. The _remove
keyword offers a clean(er) way to do this.

Implementation: _remove is added to the keyword list next
to _append and _prepend. During finalize(), the list of
removes for a given variable is subjected to the usual
filtering for overrides. However, the winning entries are
not removed; they are added to a list stored in the new
variable flag _remove_later.

At expansion time, after a variable has been expanded,
removes for it (if any) are processed. We use the same
_special_values magic here that we use elsewhere. Currently
there's no caching; perhaps there should be.

Additionally, "-=" and "=-" are accepted as synonyms for
_remove.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 lib/bb/data_smart.py                 |   45 +++++++++++++++++++++++++++++----
 lib/bb/parse/ast.py                  |    6 ++++
 lib/bb/parse/parse_py/ConfHandler.py |    2 +-
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
index a128914..d59f8ab 100644
--- a/lib/bb/data_smart.py
+++ b/lib/bb/data_smart.py
@@ -40,8 +40,8 @@ from bb.COW  import COWDictBase
 
 logger = logging.getLogger("BitBake.Data")
 
-__setvar_keyword__ = ["_append", "_prepend"]
-__setvar_regexp__ = re.compile('(?P<base>.*?)(?P<keyword>_append|_prepend)(_(?P<add>.*))?$')
+__setvar_keyword__ = ["_append", "_prepend", "_remove"]
+__setvar_regexp__ = re.compile('(?P<base>.*?)(?P<keyword>_append|_prepend|_remove)(_(?P<add>.*))?$')
 __expand_var_regexp__ = re.compile(r"\${[^{}]+}")
 __expand_python_regexp__ = re.compile(r"\${@.+?}")
 
@@ -219,7 +219,10 @@ class DataSmart(MutableMapping):
 
         #
         # First we apply all overrides
-        # Then  we will handle _append and _prepend
+        # Then we will handle _append and _prepend and store the _remove
+        # information for later. (_remove has to be processed during
+        # expansion, but there's no reason to duplicate the override-checking
+        # logic.)
         #
 
         for o in overrides:
@@ -243,12 +246,16 @@ class DataSmart(MutableMapping):
                 except Exception, e:
                     logger.info("Untracked delVar %s: %s" % (var, e))
 
-        # now on to the appends and prepends
+        # now on to the appends and prepends, and stashing the removes
+        # for processing during expansion
         for op in __setvar_keyword__:
             if op in self._special_values:
                 appends = self._special_values[op] or []
                 for append in appends:
                     keep = []
+                    # Anything we already have tagged for removal should be
+                    # added to.
+                    remove_later = self.getVarFlag(append, '_remove_later') or []
                     for (a, o) in self.getVarFlag(append, op) or []:
                         match = True
                         if o:
@@ -266,6 +273,19 @@ class DataSmart(MutableMapping):
                         elif op == "_prepend":
                             sval = a + (self.getVar(append, False) or "")
                             self.setVar(append, sval, 'Ignore')
+                        elif op == "_remove":
+                            # stash for later processing
+                            remove_later.append(a)
+
+                    if remove_later:
+                        # We build a list of applicable _remove values,
+                        # which can then be used after expansion.
+                        self.setVarFlag(append, '_remove_later', remove_later, 'Ignore')
+                        try:
+                            self._special_values['_remove_later'].add(append)
+                        except KeyError:
+                            self._special_values['_remove_later'] = set()
+                            self._special_values['_remove_later'].add(append)
 
                     # We save overrides that may be applied at some later stage
                     # ... but we don't need to report on this.
@@ -356,10 +376,23 @@ class DataSmart(MutableMapping):
 
     def getVar(self, var, expand=False, noweakdefault=False):
         value = self.getVarFlag(var, "content", False, noweakdefault)
+        removes = None
+        try:
+            # See whether any _remove values made it through overrides
+            if var in self._special_values['_remove_later']:
+                removes = self.getVarFlag(var, '_remove_later', True)
+        except KeyError:
+            pass
 
         # Call expand() separately to make use of the expand cache
         if expand and value:
-            return self.expand(value, var)
+            value = self.expand(value, var)
+        if removes:
+            list = value.split(' ')
+            for r in removes:
+                if r in list:
+                    list.remove(r)
+            value = " ".join(list)
         return value
 
     def renameVar(self, key, newkey, filename = None, lineno = None):
@@ -371,7 +404,7 @@ class DataSmart(MutableMapping):
         if val is not None:
             self.setVar(newkey, val, filename, lineno, 'rename-create')
 
-        for i in ('_append', '_prepend'):
+        for i in (__setvar_keyword__):
             src = self.getVarFlag(key, i)
             if src is None:
                 continue
diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
index dbc2237..b2e038b 100644
--- a/lib/bb/parse/ast.py
+++ b/lib/bb/parse/ast.py
@@ -113,6 +113,12 @@ class DataNode(AstNode):
             val = "%s %s" % (groupd["value"], (self.getFunc(key, data) or ""))
             op = 'prepend'
             details = groupd["value"]
+        elif (("preminus" in groupd and groupd["preminus"] != None) or
+              ("postminus" in groupd and groupd["postminus"] != None)):
+            # There's no correct value to set in this case; it has to
+            # convert into a flag.
+            key = key + "_remove"
+            val = "%s" % groupd["value"]
         elif "postdot" in groupd and groupd["postdot"] != None:
             val = "%s%s" % ((self.getFunc(key, data) or ""), groupd["value"])
             op = 'postdot'
diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py
index 4a1012e..a5a25fc 100644
--- a/lib/bb/parse/parse_py/ConfHandler.py
+++ b/lib/bb/parse/parse_py/ConfHandler.py
@@ -29,7 +29,7 @@ import logging
 import bb.utils
 from bb.parse import ParseError, resolve_file, ast, logger
 
-__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"])(?P<value>.*)(?P=apo)$")
+__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<preminus>=-)|(?P<postminus>-=)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"])(?P<value>.*)(?P=apo)$")
 __include_regexp__ = re.compile( r"include\s+(.+)" )
 __require_regexp__ = re.compile( r"require\s+(.+)" )
 __export_regexp__ = re.compile( r"export\s+([a-zA-Z0-9\-_+.${}/]+)$" )
-- 
1.7.0.4




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append.
  2012-08-16  1:14 ` [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append Peter Seebach
@ 2012-08-16 10:13   ` Richard Purdie
  2012-08-16 13:32     ` Peter Seebach
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Purdie @ 2012-08-16 10:13 UTC (permalink / raw)
  To: Peter Seebach; +Cc: bitbake-devel

On Wed, 2012-08-15 at 20:14 -0500, Peter Seebach wrote:
> There has historically been no way to remove a single word
> from a list, but many lists (such as DISTRO_FEATURES) are
> assembled from many sources all over the place. The _remove
> keyword offers a clean(er) way to do this.
> 
> Implementation: _remove is added to the keyword list next
> to _append and _prepend. During finalize(), the list of
> removes for a given variable is subjected to the usual
> filtering for overrides. However, the winning entries are
> not removed; they are added to a list stored in the new
> variable flag _remove_later.
> 
> At expansion time, after a variable has been expanded,
> removes for it (if any) are processed. We use the same
> _special_values magic here that we use elsewhere. Currently
> there's no caching; perhaps there should be.
> 
> Additionally, "-=" and "=-" are accepted as synonyms for
> _remove.

I read this far.

This is not going to make any friends. += and =+ behave differently to
_append with immediate vs. delayed functionality. I think equating -=
and =- is just going to confuse users even more. So no, I don't think we
can do this.

Why does this depend on 1/2 ?

Cheers,

Richard

> Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
> ---
>  lib/bb/data_smart.py                 |   45 +++++++++++++++++++++++++++++----
>  lib/bb/parse/ast.py                  |    6 ++++
>  lib/bb/parse/parse_py/ConfHandler.py |    2 +-
>  3 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
> index a128914..d59f8ab 100644
> --- a/lib/bb/data_smart.py
> +++ b/lib/bb/data_smart.py
> @@ -40,8 +40,8 @@ from bb.COW  import COWDictBase
>  
>  logger = logging.getLogger("BitBake.Data")
>  
> -__setvar_keyword__ = ["_append", "_prepend"]
> -__setvar_regexp__ = re.compile('(?P<base>.*?)(?P<keyword>_append|_prepend)(_(?P<add>.*))?$')
> +__setvar_keyword__ = ["_append", "_prepend", "_remove"]
> +__setvar_regexp__ = re.compile('(?P<base>.*?)(?P<keyword>_append|_prepend|_remove)(_(?P<add>.*))?$')
>  __expand_var_regexp__ = re.compile(r"\${[^{}]+}")
>  __expand_python_regexp__ = re.compile(r"\${@.+?}")
>  
> @@ -219,7 +219,10 @@ class DataSmart(MutableMapping):
>  
>          #
>          # First we apply all overrides
> -        # Then  we will handle _append and _prepend
> +        # Then we will handle _append and _prepend and store the _remove
> +        # information for later. (_remove has to be processed during
> +        # expansion, but there's no reason to duplicate the override-checking
> +        # logic.)
>          #
>  
>          for o in overrides:
> @@ -243,12 +246,16 @@ class DataSmart(MutableMapping):
>                  except Exception, e:
>                      logger.info("Untracked delVar %s: %s" % (var, e))
>  
> -        # now on to the appends and prepends
> +        # now on to the appends and prepends, and stashing the removes
> +        # for processing during expansion
>          for op in __setvar_keyword__:
>              if op in self._special_values:
>                  appends = self._special_values[op] or []
>                  for append in appends:
>                      keep = []
> +                    # Anything we already have tagged for removal should be
> +                    # added to.
> +                    remove_later = self.getVarFlag(append, '_remove_later') or []
>                      for (a, o) in self.getVarFlag(append, op) or []:
>                          match = True
>                          if o:
> @@ -266,6 +273,19 @@ class DataSmart(MutableMapping):
>                          elif op == "_prepend":
>                              sval = a + (self.getVar(append, False) or "")
>                              self.setVar(append, sval, 'Ignore')
> +                        elif op == "_remove":
> +                            # stash for later processing
> +                            remove_later.append(a)
> +
> +                    if remove_later:
> +                        # We build a list of applicable _remove values,
> +                        # which can then be used after expansion.
> +                        self.setVarFlag(append, '_remove_later', remove_later, 'Ignore')
> +                        try:
> +                            self._special_values['_remove_later'].add(append)
> +                        except KeyError:
> +                            self._special_values['_remove_later'] = set()
> +                            self._special_values['_remove_later'].add(append)
>  
>                      # We save overrides that may be applied at some later stage
>                      # ... but we don't need to report on this.
> @@ -356,10 +376,23 @@ class DataSmart(MutableMapping):
>  
>      def getVar(self, var, expand=False, noweakdefault=False):
>          value = self.getVarFlag(var, "content", False, noweakdefault)
> +        removes = None
> +        try:
> +            # See whether any _remove values made it through overrides
> +            if var in self._special_values['_remove_later']:
> +                removes = self.getVarFlag(var, '_remove_later', True)
> +        except KeyError:
> +            pass
>  
>          # Call expand() separately to make use of the expand cache
>          if expand and value:
> -            return self.expand(value, var)
> +            value = self.expand(value, var)
> +        if removes:
> +            list = value.split(' ')
> +            for r in removes:
> +                if r in list:
> +                    list.remove(r)
> +            value = " ".join(list)
>          return value
>  
>      def renameVar(self, key, newkey, filename = None, lineno = None):
> @@ -371,7 +404,7 @@ class DataSmart(MutableMapping):
>          if val is not None:
>              self.setVar(newkey, val, filename, lineno, 'rename-create')
>  
> -        for i in ('_append', '_prepend'):
> +        for i in (__setvar_keyword__):
>              src = self.getVarFlag(key, i)
>              if src is None:
>                  continue
> diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
> index dbc2237..b2e038b 100644
> --- a/lib/bb/parse/ast.py
> +++ b/lib/bb/parse/ast.py
> @@ -113,6 +113,12 @@ class DataNode(AstNode):
>              val = "%s %s" % (groupd["value"], (self.getFunc(key, data) or ""))
>              op = 'prepend'
>              details = groupd["value"]
> +        elif (("preminus" in groupd and groupd["preminus"] != None) or
> +              ("postminus" in groupd and groupd["postminus"] != None)):
> +            # There's no correct value to set in this case; it has to
> +            # convert into a flag.
> +            key = key + "_remove"
> +            val = "%s" % groupd["value"]
>          elif "postdot" in groupd and groupd["postdot"] != None:
>              val = "%s%s" % ((self.getFunc(key, data) or ""), groupd["value"])
>              op = 'postdot'
> diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py
> index 4a1012e..a5a25fc 100644
> --- a/lib/bb/parse/parse_py/ConfHandler.py
> +++ b/lib/bb/parse/parse_py/ConfHandler.py
> @@ -29,7 +29,7 @@ import logging
>  import bb.utils
>  from bb.parse import ParseError, resolve_file, ast, logger
>  
> -__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"])(?P<value>.*)(?P=apo)$")
> +__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<preminus>=-)|(?P<postminus>-=)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"])(?P<value>.*)(?P=apo)$")
>  __include_regexp__ = re.compile( r"include\s+(.+)" )
>  __require_regexp__ = re.compile( r"require\s+(.+)" )
>  __export_regexp__ = re.compile( r"export\s+([a-zA-Z0-9\-_+.${}/]+)$" )





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append.
  2012-08-16 10:13   ` Richard Purdie
@ 2012-08-16 13:32     ` Peter Seebach
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Seebach @ 2012-08-16 13:32 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On Thu, 16 Aug 2012 11:13:46 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> I read this far.
> 
> This is not going to make any friends. += and =+ behave differently to
> _append with immediate vs. delayed functionality. I think equating -=
> and =- is just going to confuse users even more. So no, I don't think
> we can do this.

I'm fine with dropping that part, I just put it in because it was a
pretty close analogue to +=/=+. It would not be hard to remove that.

> Why does this depend on 1/2 ?

Because I didn't think I could write it without the tracking code to
see what was happening.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] data_smart.py: track file inclusion and variable modifications
  2012-08-16  1:14 ` [PATCH 1/2] data_smart.py: track file inclusion and variable modifications Peter Seebach
@ 2012-08-16 14:32   ` Richard Purdie
  2012-08-16 16:39     ` Peter Seebach
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Purdie @ 2012-08-16 14:32 UTC (permalink / raw)
  To: Peter Seebach; +Cc: bitbake-devel

On Wed, 2012-08-15 at 20:14 -0500, Peter Seebach wrote:
> It is often the case that bitbake variables have values which
> strike the user as wrong, but finding out how they got that way
> can be gratuitously complicated.
> 
> This patch adds two levels of tracking. First, file inclusions
> are recorded, so the system can offer a tree structure showing
> which files were processed, and which other files they included.
> 
> Second, variable assignments, flag settings, and such are recorded
> with source information showing which file and line, or which block
> of Python code, produced each change.
> 
> This information is then used in the bitbake -e output to provide
> a usable history.
> 
> For variables:
> 1.  We create a history dict in the data_smart object, the
> members of which are lists of tuples.  It's three, count them,
> THREE unrelated data types in a single object!

Do we really need comments like "count them" in the commit message?

> 2.  There is an eventLog(...) function which allows recording
> things which happen to variables.
> 3.  setVar and friends are updated to take optional arguments
> indicating where a change occurred, and optional information
> about how to describe the change.
> 4.  We log pretty much everything if logging is enabled.
> 5.  You can query the history of a thing with data.getHistory(var)
> 
> In addition to file and line (which will be inferred to be
> "the calling Python code" if file is None), you can specify
> the operation to report and the value to report. This is used
> because append operations would otherwise report the entire
> new value after the append, which is almost never what you
> want.
> 
> The special file name 'Ignore' indicates that an event need
> not be logged.

This isn't particularly pythonic but I'm not going to lose sleep over
it.

> 
> For file inclusion:
> The file inclusion code maintains a stack of files in process,
> and creates a tree structure reflecting those changes. This is
> also attached to bitbake -e output.
> 
> Tracking must be specifically turned on.  If it's not, the
> history object is a single empty table (cheap) and the calls
> to eventLog() and extra argument passing are also probably
> quite cheap.  As a proof of concept, the showEnvironment function
> used by bitbake -e now uses this if available.
> 
> Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
> ---
>  lib/bb/cooker.py                     |    2 +
>  lib/bb/data.py                       |   65 ++++++++++---
>  lib/bb/data_smart.py                 |  173 +++++++++++++++++++++++++++-------
>  lib/bb/parse/__init__.py             |    6 +-
>  lib/bb/parse/ast.py                  |   70 ++++++++------
>  lib/bb/parse/parse_py/ConfHandler.py |    4 +-
>  6 files changed, 239 insertions(+), 81 deletions(-)
> 
> diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
> index 23fffc9..8d59774 100644
> --- a/lib/bb/cooker.py
> +++ b/lib/bb/cooker.py
> @@ -830,6 +830,8 @@ class BBCooker:
>  
>      def parseConfigurationFiles(self, prefiles, postfiles):
>          data = self.configuration.data
> +        if self.configuration.show_environment:
> +            data.enableTracking()
>          bb.parse.init_parser(data)
>  
>          # Parse files for loading *before* bitbake.conf and any includes
> diff --git a/lib/bb/data.py b/lib/bb/data.py
> index 5b7a092..a762a40 100644
> --- a/lib/bb/data.py
> +++ b/lib/bb/data.py
> @@ -74,14 +74,22 @@ def createCopy(source):
>      """
>      return source.createCopy()
>  
> +# These are used in dataSmart, here as protection against KeyErrors.
> +def enableTracking():
> +    pass
> +
> +def disableTracking():
> +    pass
> +

Where would these get called from that would trigger a keyerror?


>  def initVar(var, d):
>      """Non-destructive var init for data structure"""
>      d.initVar(var)
>  
> 
> -def setVar(var, value, d):
> +def setVar(var, value, d, filename = None, lineno = None):
>      """Set a variable to a given value"""
> -    d.setVar(var, value)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.setVar(var, value, filename, lineno)
>  

Changing any of these in data.py looks like a pointless waste of time.
The parser should be calling the object (dataSmart) variants and that is
now the preferred mechanism. If some old user does call here, the
parameters will be unset and the dataSmart setVar will call
infer_file_and_line for us anyway.

So in summary, I don't think any of these changes to data.py are
particularly useful.

>  def getVar(var, d, exp = 0):
> @@ -89,27 +97,31 @@ def getVar(var, d, exp = 0):
>      return d.getVar(var, exp)
>  
> 
> -def renameVar(key, newkey, d):
> +def renameVar(key, newkey, d, filename = None, lineno = None):
>      """Renames a variable from key to newkey"""
> -    d.renameVar(key, newkey)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.renameVar(key, newkey, filename, lineno)
>  
> -def delVar(var, d):
> +def delVar(var, d, filename = None, lineno = None):
>      """Removes a variable from the data set"""
> -    d.delVar(var)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.delVar(var, filename, lineno)
>  
> -def setVarFlag(var, flag, flagvalue, d):
> +def setVarFlag(var, flag, flagvalue, d, filename = None, lineno = None):
>      """Set a flag for a given variable to a given value"""
> -    d.setVarFlag(var, flag, flagvalue)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.setVarFlag(var, flag, flagvalue, filename, lineno)
>  
>  def getVarFlag(var, flag, d):
>      """Gets given flag from given var"""
>      return d.getVarFlag(var, flag)
>  
> -def delVarFlag(var, flag, d):
> +def delVarFlag(var, flag, d, filename = None, lineno = None):
>      """Removes a given flag from the variable's flags"""
> -    d.delVarFlag(var, flag)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.delVarFlag(var, flag, filename, lineno)
>  
> -def setVarFlags(var, flags, d):
> +def setVarFlags(var, flags, d, filename = None, lineno = None):
>      """Set the flags for a given variable
>  
>      Note:
> @@ -117,15 +129,17 @@ def setVarFlags(var, flags, d):
>          flags. Think of this method as
>          addVarFlags
>      """
> -    d.setVarFlags(var, flags)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.setVarFlags(var, flags, filename, lineno)
>  
>  def getVarFlags(var, d):
>      """Gets a variable's flags"""
>      return d.getVarFlags(var)
>  
> -def delVarFlags(var, d):
> +def delVarFlags(var, d, filename = None, lineno = None):
>      """Removes a variable's flags"""
> -    d.delVarFlags(var)
> +    filename, lineno = d.infer_file_and_line(filename, lineno)
> +    d.delVarFlags(var, filename, lineno)

So all the above can be dropped.

>  def keys(d):
>      """Return a list of keys in d"""
> @@ -195,6 +209,13 @@ def emit_var(var, o=sys.__stdout__, d = init(), all=False):
>  
>      if all:
>          commentVal = re.sub('\n', '\n#', str(oval))
> +        history = d.getHistory(var)
> +        if history:
> +            o.write('#\n# %s [%d]\n' % (var, len(history)))
> +            for events in history:
> +                events = (events[0], events[1], events[2], re.sub('\n', '\n#     ', str(events[3])))
> +                o.write('#   %s %s:%s:\n#     <%s>\n' % events)
> +            o.write('#\n')
>          o.write('# %s=%s\n' % (var, commentVal))
>  
>      if (var.find("-") != -1 or var.find(".") != -1 or var.find('{') != -1 or var.find('}') != -1 or var.find('+') != -1) and not all:
> @@ -232,10 +253,26 @@ def emit_env(o=sys.__stdout__, d = init(), all=False):
>      isfunc = lambda key: bool(d.getVarFlag(key, "func"))
>      keys = sorted((key for key in d.keys() if not key.startswith("__")), key=isfunc)
>      grouped = groupby(keys, isfunc)
> +    # Include history!
> +    if d.tracking():
> +        o.write('#\n# INCLUDE HISTORY:\n#\n')
> +        emit_history(o, d.getIncludeHistory())
> +        
>      for isfunc, keys in grouped:
>          for key in keys:
>              emit_var(key, o, d, all and not isfunc) and o.write('\n')
>  
> +def emit_history(o, h, depth = 0):
> +    if not h:
> +        return
> +    for event in h:
> +        o.write("# %*s%s" % (depth * 2, "", event[0]))
> +        if event[1]:
> +            o.write(" includes:\n")
> +            emit_history(o, event[1], depth + 1)
> +        else:
> +            o.write("\n")
> +
>  def exported_keys(d):
>      return (key for key in d.keys() if not key.startswith('__') and
>                                        d.getVarFlag(key, 'export') and
> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
> index 31216e0..a128914 100644
> --- a/lib/bb/data_smart.py
> +++ b/lib/bb/data_smart.py
> @@ -28,7 +28,9 @@ BitBake build tools.
>  # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>  # Based on functions from the base bb module, Copyright 2003 Holger Schurig
>  
> +import traceback
>  import copy, re
> +import sys
>  from collections import MutableMapping
>  import logging
>  import hashlib
> @@ -114,13 +116,51 @@ class ExpansionError(Exception):
>  class DataSmart(MutableMapping):
>      def __init__(self, special = COWDictBase.copy(), seen = COWDictBase.copy() ):
>          self.dict = {}
> +        self.history = {}
> +        self.include_history = []
> +        self.include_stack = [(-1, self.include_history)]
>  
>          # cookie monster tribute
>          self._special_values = special
>          self._seen_overrides = seen
> +        self._tracking_enabled = False
>  
>          self.expand_cache = {}
>  
> +    def includeLog(self, filename):
> +        """includeLog(included_file) shows that the file was included
> +        by the currently-processed file or context."""
> +        if self._tracking_enabled:
> +            event = (filename, [])
> +            position = (len(self.include_stack[-1][1]), event[1])
> +            self.include_stack[-1][1].append(event)
> +            self.include_stack.append(position)
> +
> +    def includeLogDone(self, filename):
> +        if self._tracking_enabled:
> +            if len(self.include_stack) > 1:
> +                self.include_stack.pop()
> +            else:
> +                bb.warn("Uh-oh:  includeLogDone(%s) tried to empty the stack." % filename)

There has to be a better way to write code to do this. This looks
horrible.


> +    def getIncludeHistory(self):
> +        return self.include_history
> +
> +    def tracking(self):
> +        return self._tracking_enabled
> +
> +    def enableTracking(self):
> +        self._tracking_enabled = True
> +
> +    def disableTracking(self):
> +        self._tracking_enabled = False
> +
> +    def eventLog(self, var, event, value, filename = None, lineno = None):
> +        if self._tracking_enabled and filename != 'Ignore':
> +            if var not in self.history:
> +                self.history[var] = []
> +            self.history[var].append((event, filename, lineno, value))
> +
>      def expandWithRefs(self, s, varname):
>  
>          if not isinstance(s, basestring): # sanity check
> @@ -153,6 +193,14 @@ class DataSmart(MutableMapping):
>      def expand(self, s, varname = None):
>          return self.expandWithRefs(s, varname).value
>  
> +    # Figure out how to describe the caller when file/line weren't
> +    # specified.
> +    def infer_file_and_line(self, filename, lineno):
> +        details = lineno
> +        if self._tracking_enabled and not filename and filename != 'Ignore':
> +            filename, lineno, func, line = traceback.extract_stack(limit=3)[0]
> +            details = "%d [%s]" % (lineno, func)
> +        return filename, details
>  
>      def finalize(self):
>          """Performs final steps upon the datastore, including application of overrides"""
> @@ -186,10 +234,14 @@ class DataSmart(MutableMapping):
>              for var in vars:
>                  name = var[:-l]
>                  try:
> -                    self.setVar(name, self.getVar(var, False))
> -                    self.delVar(var)
> -                except Exception:
> -                    logger.info("Untracked delVar")
> +                    # Move the history of the override into the history of
> +                    # the overridden:
> +                    for event in self.getHistory(var):
> +                        self.eventLog(name, 'override:%s' % o, event[3], event[1], event[2])
> +                    self.setVar(name, self.getVar(var, False), 'Ignore')
> +                    self.delVar(var, 'Ignore')
> +                except Exception, e:
> +                    logger.info("Untracked delVar %s: %s" % (var, e))
>  
>          # now on to the appends and prepends
>          for op in __setvar_keyword__:
> @@ -210,16 +262,17 @@ class DataSmart(MutableMapping):
>                          if op == "_append":
>                              sval = self.getVar(append, False) or ""
>                              sval += a
> -                            self.setVar(append, sval)
> +                            self.setVar(append, sval, 'Ignore')
>                          elif op == "_prepend":
>                              sval = a + (self.getVar(append, False) or "")
> -                            self.setVar(append, sval)
> +                            self.setVar(append, sval, 'Ignore')
>  
>                      # We save overrides that may be applied at some later stage
> +                    # ... but we don't need to report on this.
>                      if keep:
> -                        self.setVarFlag(append, op, keep)
> +                        self.setVarFlag(append, op, keep, 'Ignore')
>                      else:
> -                        self.delVarFlag(append, op)
> +                        self.delVarFlag(append, op, 'Ignore')
>  
>      def initVar(self, var):
>          self.expand_cache = {}
> @@ -247,7 +300,15 @@ class DataSmart(MutableMapping):
>          else:
>              self.initVar(var)
>  
> -    def setVar(self, var, value):
> +    # In some cases, we want to set a value, but only record part of it;
> +    # for instance, when appending something, we want to record what we
> +    # appended, not what the complete value of the now-appended value.
> +    # This becomes especially obvious when looking at the output for
> +    # BBCLASSEXTEND. "details", if provided, replace the variable value
> +    # in the log.
> +
> +    def setVar(self, var, value, filename = None, lineno = None, op = 'set', details = None):


As a function prototype, this scares me and shouts that there is
something going wrong here. The conversion of lineno to details and back
under a variety of different circumstances doesn't seem particularly
elegant or easy to understand.

Also, I want to hightlight that these changes effectively set our data
API in stone, making it effectively impossible to ever add sensible
external facing API changes. We should really change all these calls to
be named parameters so it doesn't totally freeze the API. I hate having
to do that but I hate the alternatives more.


> +        filename, lineno = self.infer_file_and_line(filename, lineno)
>          self.expand_cache = {}
>          match  = __setvar_regexp__.match(var)
>          if match and match.group("keyword") in __setvar_keyword__:
> @@ -255,8 +316,13 @@ class DataSmart(MutableMapping):
>              keyword = match.group("keyword")
>              override = match.group('add')
>              l = self.getVarFlag(base, keyword) or []
> -            l.append([value, override])
> -            self.setVarFlag(base, keyword, l)
> +            # Compute new details: This is what we're actually appending.
> +            details = [value, override]
> +            l.append(details)
> +            # Log the details using the keyword as the op name, instead
> +            # of logging the entire new value as a flag change.
> +            self.setVarFlag(base, keyword, l, 'Ignore')
> +            self.eventLog(base, keyword, details, filename, lineno)
>  
>              # todo make sure keyword is not __doc__ or __module__
>              # pay the cookie monster
> @@ -281,6 +347,12 @@ class DataSmart(MutableMapping):
>  
>          # setting var
>          self.dict[var]["content"] = value
> +        self.eventLog(var, op, details or value, filename, lineno)
> +
> +    def getHistory(self, var):
> +        if var in self.history:
> +            return self.history[var]
> +        return []
>  
>      def getVar(self, var, expand=False, noweakdefault=False):
>          value = self.getVarFlag(var, "content", False, noweakdefault)
> @@ -290,13 +362,14 @@ class DataSmart(MutableMapping):
>              return self.expand(value, var)
>          return value
>  
> -    def renameVar(self, key, newkey):
> +    def renameVar(self, key, newkey, filename = None, lineno = None):
>          """
>          Rename the variable key to newkey
>          """
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
>          val = self.getVar(key, 0)
>          if val is not None:
> -            self.setVar(newkey, val)
> +            self.setVar(newkey, val, filename, lineno, 'rename-create')
>  
>          for i in ('_append', '_prepend'):
>              src = self.getVarFlag(key, i)
> @@ -305,34 +378,40 @@ class DataSmart(MutableMapping):
>  
>              dest = self.getVarFlag(newkey, i) or []
>              dest.extend(src)
> -            self.setVarFlag(newkey, i, dest)
> +            self.setVarFlag(newkey, i, dest, filename, lineno, 'rename')
>  
>              if i in self._special_values and key in self._special_values[i]:
>                  self._special_values[i].remove(key)
>                  self._special_values[i].add(newkey)
>  
> -        self.delVar(key)
> +        self.delVar(key, filename, lineno, 'rename-delete')
>  
> -    def appendVar(self, key, value):
> -        value = (self.getVar(key, False) or "") + value
> -        self.setVar(key, value)
> +    def appendVar(self, key, newValue, filename = None, lineno = None):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
> +        value = (self.getVar(key, False) or "") + newValue
> +        self.setVar(key, value, filename, lineno, 'append', newValue)
>  
> -    def prependVar(self, key, value):
> -        value = value + (self.getVar(key, False) or "")
> -        self.setVar(key, value)
> +    def prependVar(self, key, newValue, filename = None, lineno = None):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
> +        value = newValue + (self.getVar(key, False) or "")
> +        self.setVar(key, value, filename, lineno, 'prepend', newValue)
>  
> -    def delVar(self, var):
> +    def delVar(self, var, filename = None, lineno = None, op = 'del'):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
>          self.expand_cache = {}
>          self.dict[var] = {}
> +        self.eventLog(var, op, '', filename, lineno)
>          if '_' in var:
>              override = var[var.rfind('_')+1:]
>              if override and override in self._seen_overrides and var in self._seen_overrides[override]:
>                  self._seen_overrides[override].remove(var)
>  
> -    def setVarFlag(self, var, flag, flagvalue):
> +    def setVarFlag(self, var, flag, flagvalue, filename = None, lineno = None, op = 'set', details = None):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
>          if not var in self.dict:
>              self._makeShadowCopy(var)
>          self.dict[var][flag] = flagvalue
> +        self.eventLog(var, '[flag %s] %s' % (flag, op), details or flagvalue, filename, lineno)
>  
>      def getVarFlag(self, var, flag, expand=False, noweakdefault=False):
>          local_var = self._findVar(var)
> @@ -346,31 +425,37 @@ class DataSmart(MutableMapping):
>              value = self.expand(value, None)
>          return value
>  
> -    def delVarFlag(self, var, flag):
> +    def delVarFlag(self, var, flag, filename = None, lineno = None):
>          local_var = self._findVar(var)
>          if not local_var:
>              return
>          if not var in self.dict:
>              self._makeShadowCopy(var)
>  
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
> +        self.eventLog(var, 'del flag %s' % flag, '', filename, lineno)
>          if var in self.dict and flag in self.dict[var]:
>              del self.dict[var][flag]
>  
> -    def appendVarFlag(self, key, flag, value):
> -        value = (self.getVarFlag(key, flag, False) or "") + value
> -        self.setVarFlag(key, flag, value)
> +    def appendVarFlag(self, key, flag, newValue, filename = None, lineno = None):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
> +        value = (self.getVarFlag(key, flag, False) or "") + newValue
> +        self.setVarFlag(key, flag, value, filename, lineno, 'append', newValue)
>  
> -    def prependVarFlag(self, key, flag, value):
> -        value = value + (self.getVarFlag(key, flag, False) or "")
> -        self.setVarFlag(key, flag, value)
> +    def prependVarFlag(self, key, flag, newValue, filename = None, lineno = None):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
> +        value = newValue + (self.getVarFlag(key, flag, False) or "")
> +        self.setVarFlag(key, flag, value, filename, lineno, 'prepend', newValue)
>  
> -    def setVarFlags(self, var, flags):
> +    def setVarFlags(self, var, flags, filename = None, lineno = None, details = None):
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
>          if not var in self.dict:
>              self._makeShadowCopy(var)
>  
>          for i in flags:
>              if i == "content":
>                  continue
> +            self.eventLog(var, 'set flag %s' % i, details or flags[i], filename, lineno)
>              self.dict[var][i] = flags[i]
>  
>      def getVarFlags(self, var):
> @@ -388,13 +473,15 @@ class DataSmart(MutableMapping):
>          return flags
>  
> 
> -    def delVarFlags(self, var):
> +    def delVarFlags(self, var, filename = None, lineno = None):
>          if not var in self.dict:
>              self._makeShadowCopy(var)
>  
>          if var in self.dict:
>              content = None
>  
> +            filename, lineno = self.infer_file_and_line(filename, lineno)
> +            self.eventLog(var, 'clear all flags', '', filename, lineno)
>              # try to save the content
>              if "content" in self.dict[var]:
>                  content  = self.dict[var]["content"]
> @@ -411,10 +498,25 @@ class DataSmart(MutableMapping):
>          # we really want this to be a DataSmart...
>          data = DataSmart(seen=self._seen_overrides.copy(), special=self._special_values.copy())
>          data.dict["_data"] = self.dict
> -
> +        if self._tracking_enabled:
> +            data._tracking_enabled = self._tracking_enabled
> +            data.history = copy.deepcopy(self.history)
> +            data.include_history = copy.deepcopy(self.include_history)
> +            data.include_stack = []
> +            oldref = self.include_history
> +            newref = data.include_history
> +            # Create corresponding references, if we can
> +            try:
> +                for item in self.include_stack:
> +                    if item[0] >= 0:
> +                        newref = newref[item[0]][1]
> +                    newevent = (item[0], newref)
> +                    data.include_stack.append(newevent)
> +            except Exception:
> +                sys.exc_clear()
>          return data
>  
> -    def expandVarref(self, variable, parents=False):
> +    def expandVarref(self, variable, parents=False, filename = None, lineno = None):
>          """Find all references to variable in the data and expand it
>             in place, optionally descending to parent datastores."""
>  
> @@ -423,12 +525,13 @@ class DataSmart(MutableMapping):
>          else:
>              keys = self.localkeys()
>  
> +        filename, lineno = self.infer_file_and_line(filename, lineno)
>          ref = '${%s}' % variable
>          value = self.getVar(variable, False)
>          for key in keys:
>              referrervalue = self.getVar(key, False)
>              if referrervalue and ref in referrervalue:
> -                self.setVar(key, referrervalue.replace(ref, value))
> +                self.setVar(key, referrervalue.replace(ref, value), filename, lineno, 'expandVarref')
>  
>      def localkeys(self):
>          for key in self.dict:
> diff --git a/lib/bb/parse/__init__.py b/lib/bb/parse/__init__.py
> index 7b9c47e..0ea6a55 100644
> --- a/lib/bb/parse/__init__.py
> +++ b/lib/bb/parse/__init__.py
> @@ -88,7 +88,11 @@ def handle(fn, data, include = 0):
>      """Call the handler that is appropriate for this file"""
>      for h in handlers:
>          if h['supports'](fn, data):
> -            return h['handle'](fn, data, include)
> +            data.includeLog(fn)
> +            try:
> +                return h['handle'](fn, data, include)
> +            finally:
> +                data.includeLogDone(fn)
>      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 86f9463..dbc2237 100644
> --- a/lib/bb/parse/ast.py
> +++ b/lib/bb/parse/ast.py
> @@ -69,7 +69,7 @@ class ExportNode(AstNode):
>          self.var = var
>  
>      def eval(self, data):
> -        data.setVarFlag(self.var, "export", 1)
> +        data.setVarFlag(self.var, "export", 1, self.filename, self.lineno)
>  
>  class DataNode(AstNode):
>      """
> @@ -91,33 +91,45 @@ class DataNode(AstNode):
>      def eval(self, data):
>          groupd = self.groupd
>          key = groupd["var"]
> +        op = 'set'
> +        details = None
>          if "exp" in groupd and groupd["exp"] != None:
> -            data.setVarFlag(key, "export", 1)
> +            data.setVarFlag(key, "export", 1, self.filename, self.lineno)
>          if "ques" in groupd and groupd["ques"] != None:
>              val = self.getFunc(key, data)
>              if val == None:
>                  val = groupd["value"]
> +                op = 'set?'
>          elif "colon" in groupd and groupd["colon"] != None:
>              e = data.createCopy()
>              bb.data.update_data(e)
>              val = e.expand(groupd["value"], key + "[:=]")
> +            op = 'immediate'
>          elif "append" in groupd and groupd["append"] != None:
>              val = "%s %s" % ((self.getFunc(key, data) or ""), groupd["value"])
> +            op = 'append'
> +            details = groupd["value"]
>          elif "prepend" in groupd and groupd["prepend"] != None:
>              val = "%s %s" % (groupd["value"], (self.getFunc(key, data) or ""))
> +            op = 'prepend'
> +            details = groupd["value"]
>          elif "postdot" in groupd and groupd["postdot"] != None:
>              val = "%s%s" % ((self.getFunc(key, data) or ""), groupd["value"])
> +            op = 'postdot'
> +            details = groupd["value"]
>          elif "predot" in groupd and groupd["predot"] != None:
>              val = "%s%s" % (groupd["value"], (self.getFunc(key, data) or ""))
> +            op = 'predot'
> +            details = groupd["value"]
>          else:
>              val = groupd["value"]
>  
>          if 'flag' in groupd and groupd['flag'] != None:
> -            data.setVarFlag(key, groupd['flag'], val)
> +            data.setVarFlag(key, groupd['flag'], val, self.filename, self.lineno, op)
>          elif groupd["lazyques"]:
> -            data.setVarFlag(key, "defaultval", val)
> +            data.setVarFlag(key, "defaultval", val, self.filename, self.lineno, op)
>          else:
> -            data.setVar(key, val)
> +            data.setVar(key, val, self.filename, self.lineno, op, details)
>  
>  class MethodNode(AstNode):
>      def __init__(self, filename, lineno, func_name, body):
> @@ -133,10 +145,10 @@ class MethodNode(AstNode):
>                  bb.methodpool.insert_method(funcname, text, self.filename)
>              anonfuncs = data.getVar('__BBANONFUNCS') or []
>              anonfuncs.append(funcname)
> -            data.setVar('__BBANONFUNCS', anonfuncs)
> +            data.setVar('__BBANONFUNCS', anonfuncs, self.filename, self.lineno)

filename/lineno info is not useful here (should be Invalid)

>          else:
> -            data.setVarFlag(self.func_name, "func", 1)
> -            data.setVar(self.func_name, '\n'.join(self.body))
> +            data.setVarFlag(self.func_name, "func", 1, self.filename, self.lineno)
> +            data.setVar(self.func_name, '\n'.join(self.body), self.filename, self.lineno)
>  
>  class PythonMethodNode(AstNode):
>      def __init__(self, filename, lineno, function, define, body):
> @@ -152,9 +164,9 @@ class PythonMethodNode(AstNode):
>          text = '\n'.join(self.body)
>          if not bb.methodpool.parsed_module(self.define):
>              bb.methodpool.insert_method(self.define, text, self.filename)
> -        data.setVarFlag(self.function, "func", 1)
> -        data.setVarFlag(self.function, "python", 1)
> -        data.setVar(self.function, text)
> +        data.setVarFlag(self.function, "func", 1, self.filename, self.lineno)
> +        data.setVarFlag(self.function, "python", 1, self.filename, self.lineno)
> +        data.setVar(self.function, text, self.filename, self.lineno)
>  
>  class MethodFlagsNode(AstNode):
>      def __init__(self, filename, lineno, key, m):
> @@ -166,16 +178,16 @@ class MethodFlagsNode(AstNode):
>          if data.getVar(self.key):
>              # clean up old version of this piece of metadata, as its
>              # flags could cause problems
> -            data.setVarFlag(self.key, 'python', None)
> -            data.setVarFlag(self.key, 'fakeroot', None)
> +            data.setVarFlag(self.key, 'python', None, self.filename, self.lineno)
> +            data.setVarFlag(self.key, 'fakeroot', None, self.filename, self.lineno)
>          if self.m.group("py") is not None:
> -            data.setVarFlag(self.key, "python", "1")
> +            data.setVarFlag(self.key, "python", "1", self.filename, self.lineno)
>          else:
> -            data.delVarFlag(self.key, "python")
> +            data.delVarFlag(self.key, "python", self.filename, self.lineno)
>          if self.m.group("fr") is not None:
> -            data.setVarFlag(self.key, "fakeroot", "1")
> +            data.setVarFlag(self.key, "fakeroot", "1", self.filename, self.lineno)
>          else:
> -            data.delVarFlag(self.key, "fakeroot")
> +            data.delVarFlag(self.key, "fakeroot", self.filename, self.lineno)
>  
>  class ExportFuncsNode(AstNode):
>      def __init__(self, filename, lineno, fns, classes):
> @@ -201,21 +213,21 @@ class ExportFuncsNode(AstNode):
>                      continue
>  
>                  if data.getVar(var):
> -                    data.setVarFlag(var, 'python', None)
> -                    data.setVarFlag(var, 'func', None)
> +                    data.setVarFlag(var, 'python', None, self.filename, self.lineno)
> +                    data.setVarFlag(var, 'func', None, self.filename, self.lineno)
>  
>                  for flag in [ "func", "python" ]:
>                      if data.getVarFlag(calledvar, flag):
> -                        data.setVarFlag(var, flag, data.getVarFlag(calledvar, flag))
> +                        data.setVarFlag(var, flag, data.getVarFlag(calledvar, flag), self.filename, self.lineno)
>                  for flag in [ "dirs" ]:
>                      if data.getVarFlag(var, flag):
> -                        data.setVarFlag(calledvar, flag, data.getVarFlag(var, flag))
> +                        data.setVarFlag(calledvar, flag, data.getVarFlag(var, flag), self.filename, self.lineno)
>  
>                  if data.getVarFlag(calledvar, "python"):
> -                    data.setVar(var, "    bb.build.exec_func('" + calledvar + "', d)\n")
> +                    data.setVar(var, "    bb.build.exec_func('" + calledvar + "', d)\n", self.filename, self.lineno)
>                  else:
> -                    data.setVar(var, "    " + calledvar + "\n")
> -                data.setVarFlag(var, 'export_func', '1')
> +                    data.setVar(var, "    " + calledvar + "\n", self.filename, self.lineno)
> +                data.setVarFlag(var, 'export_func', '1', self.filename, self.lineno)
>  
>  class AddTaskNode(AstNode):
>      def __init__(self, filename, lineno, func, before, after):
> @@ -229,11 +241,11 @@ class AddTaskNode(AstNode):
>          if self.func[:3] != "do_":
>              var = "do_" + self.func
>  
> -        data.setVarFlag(var, "task", 1)
> +        data.setVarFlag(var, "task", 1, self.filename, self.lineno)
>          bbtasks = data.getVar('__BBTASKS') or []
>          if not var in bbtasks:
>              bbtasks.append(var)
> -        data.setVar('__BBTASKS', bbtasks)
> +        data.setVar('__BBTASKS', bbtasks, self.filename, self.lineno)

filename/lineno info is not useful here (should be Invalid)

>  
>          existing = data.getVarFlag(var, "deps") or []
>          if self.after is not None:
> @@ -241,13 +253,13 @@ class AddTaskNode(AstNode):
>              for entry in self.after.split():
>                  if entry not in existing:
>                      existing.append(entry)
> -        data.setVarFlag(var, "deps", existing)
> +        data.setVarFlag(var, "deps", existing, self.filename, self.lineno)
>          if self.before is not None:
>              # set up things that depend on this func
>              for entry in self.before.split():
>                  existing = data.getVarFlag(entry, "deps") or []
>                  if var not in existing:
> -                    data.setVarFlag(entry, "deps", [var] + existing)
> +                    data.setVarFlag(entry, "deps", [var] + existing, self.filename, self.lineno)

filename/lineno info is not useful here (should be Invalid) or change
this to an append preserving the info.

>  
>  class BBHandlerNode(AstNode):
>      def __init__(self, filename, lineno, fns):
> @@ -258,7 +270,7 @@ class BBHandlerNode(AstNode):
>          bbhands = data.getVar('__BBHANDLERS') or []
>          for h in self.hs:
>              bbhands.append(h)
> -            data.setVarFlag(h, "handler", 1)
> +            data.setVarFlag(h, "handler", 1, self.filename, self.lineno)
>          data.setVar('__BBHANDLERS', bbhands)
>  
>  class InheritNode(AstNode):
> diff --git a/lib/bb/parse/parse_py/ConfHandler.py b/lib/bb/parse/parse_py/ConfHandler.py
> index 6f77bd4..4a1012e 100644
> --- a/lib/bb/parse/parse_py/ConfHandler.py
> +++ b/lib/bb/parse/parse_py/ConfHandler.py
> @@ -110,10 +110,10 @@ def handle(fn, data, include):
>          feeder(lineno, s, fn, statements)
>  
>      # DONE WITH PARSING... time to evaluate
> -    data.setVar('FILE', abs_fn)
> +    data.setVar('FILE', abs_fn, 'Ignore')
>      statements.eval(data)
>      if oldfile:
> -        data.setVar('FILE', oldfile)
> +        data.setVar('FILE', oldfile, 'Ignore')

I wonder if a _setVar() variant which behaves like 'Ignore' would be a
better way of doing this?

So I do like the intent of this series but I don't like the impact on
the code base and think it needs some work to improve readability at the
very least.

Cheers,

Richard




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] data_smart.py: track file inclusion and variable modifications
  2012-08-16 14:32   ` Richard Purdie
@ 2012-08-16 16:39     ` Peter Seebach
  2012-08-17 10:09       ` Richard Purdie
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Seebach @ 2012-08-16 16:39 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel

On Thu, 16 Aug 2012 15:32:44 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> On Wed, 2012-08-15 at 20:14 -0500, Peter Seebach wrote:

> > 1.  We create a history dict in the data_smart object, the
> > members of which are lists of tuples.  It's three, count them,
> > THREE unrelated data types in a single object!
 
> Do we really need comments like "count them" in the commit message?

Probably not. That commit message dates to the end of a very long
day. It could be altered without much harm.

> > The special file name 'Ignore' indicates that an event need
> > not be logged.
 
> This isn't particularly pythonic but I'm not going to lose sleep over
> it.

I am very open to the idea of a better idiom, I just couldn't think of
a good way to make some suitable value visible. (Actually, I suspect
the right answer is some kind of named-arguments thing, but I don't
know off the top of my head whether that can be adapted here.)

> > +# These are used in dataSmart, here as protection against
> > KeyErrors. +def enableTracking():
> > +    pass
> > +
> > +def disableTracking():
> > +    pass
> > +

> Where would these get called from that would trigger a keyerror?

I honestly don't know. I believe that it happened to me once, but I
can't imagine why. This is from the first pass of the code, and could
well be not particularly useful or correct; I did not have a good
mental map of the relationship between data and data_smart. Come to
think of it, I probably still don't.
 
> > -def setVar(var, value, d):
> > +def setVar(var, value, d, filename = None, lineno = None):
> >      """Set a variable to a given value"""
> > -    d.setVar(var, value)
> > +    filename, lineno = d.infer_file_and_line(filename, lineno)
> > +    d.setVar(var, value, filename, lineno)
> >  

> Changing any of these in data.py looks like a pointless waste of time.
> The parser should be calling the object (dataSmart) variants and that
> is now the preferred mechanism. If some old user does call here, the
> parameters will be unset and the dataSmart setVar will call
> infer_file_and_line for us anyway.

Yes, but in that case it would just report the line inside
data.py/setVar, which is not as useful. Hmm. Well, that is an easy
thing to check on!

Answer: In a bitbake -e (no target), there are 14 instances of hits
on these functions. All of which come from:

    # DONE WITH PARSING... time to evaluate
    if ext != ".bbclass":
        data.setVar('FILE', abs_fn, d)

in BBHandler.py.

So I'd be totally happy with dropping all the data.py changes and
instead fixing BBHandler.

> > +    def includeLog(self, filename):
> > +        """includeLog(included_file) shows that the file was
> > included
> > +        by the currently-processed file or context."""
> > +        if self._tracking_enabled:
> > +            event = (filename, [])
> > +            position = (len(self.include_stack[-1][1]), event[1])
> > +            self.include_stack[-1][1].append(event)
> > +            self.include_stack.append(position)
> > +
> > +    def includeLogDone(self, filename):
> > +        if self._tracking_enabled:
> > +            if len(self.include_stack) > 1:
> > +                self.include_stack.pop()
> > +            else:
> > +                bb.warn("Uh-oh:  includeLogDone(%s) tried to empty
> > the stack." % filename)

> There has to be a better way to write code to do this. This looks
> horrible.

I agree, but I wasn't able to come up with one.

> > +    def setVar(self, var, value, filename = None, lineno = None,
> > op = 'set', details = None):

> As a function prototype, this scares me and shouts that there is
> something going wrong here. The conversion of lineno to details and
> back under a variety of different circumstances doesn't seem
> particularly elegant or easy to understand.

Yes. At a bare minimum, it should probably be done with, say, a dict or
something as a single additional/optional argument.

> Also, I want to hightlight that these changes effectively set our data
> API in stone, making it effectively impossible to ever add sensible
> external facing API changes. We should really change all these calls
> to be named parameters so it doesn't totally freeze the API. I hate
> having to do that but I hate the alternatives more.

Hmm. What if it were
   def setVar(self, var, value, logging = None)

And then logging could be, say, { filename = ..., lineno = ..., op
= ..., deteails = ... } where each component is optional with sane
defaults.

I think that solves a LOT of the problems. It's a single optional
parameter, with named components, and it avoids the addition of four
separate arguments to do a single thing.

> So I do like the intent of this series but I don't like the impact on
> the code base and think it needs some work to improve readability at
> the very least.

Okay. I will do another pass.

BTW, if anyone can think of a clearer way to express that includelog
history, I'd love to hear about it; I spent a while trying to think of
something and came up blank.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] data_smart.py: track file inclusion and variable modifications
  2012-08-16 16:39     ` Peter Seebach
@ 2012-08-17 10:09       ` Richard Purdie
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Purdie @ 2012-08-17 10:09 UTC (permalink / raw)
  To: Peter Seebach; +Cc: bitbake-devel

On Thu, 2012-08-16 at 11:39 -0500, Peter Seebach wrote:
> On Thu, 16 Aug 2012 15:32:44 +0100
> Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> 
> > On Wed, 2012-08-15 at 20:14 -0500, Peter Seebach wrote:
> 
> > > 1.  We create a history dict in the data_smart object, the
> > > members of which are lists of tuples.  It's three, count them,
> > > THREE unrelated data types in a single object!
>  
> > Do we really need comments like "count them" in the commit message?
> 
> Probably not. That commit message dates to the end of a very long
> day. It could be altered without much harm.

Fair enough, I think we do need the final commit message to avoid things
like that though :)

> 
> > > The special file name 'Ignore' indicates that an event need
> > > not be logged.
>  
> > This isn't particularly pythonic but I'm not going to lose sleep over
> > it.
> 
> I am very open to the idea of a better idiom, I just couldn't think of
> a good way to make some suitable value visible. (Actually, I suspect
> the right answer is some kind of named-arguments thing, but I don't
> know off the top of my head whether that can be adapted here.)

I think I'm leaning in favour of an internal _setVar() call for these...

> > > +# These are used in dataSmart, here as protection against
> > > KeyErrors. +def enableTracking():
> > > +    pass
> > > +
> > > +def disableTracking():
> > > +    pass
> > > +
> 
> > Where would these get called from that would trigger a keyerror?
> 
> I honestly don't know. I believe that it happened to me once, but I
> can't imagine why. This is from the first pass of the code, and could
> well be not particularly useful or correct; I did not have a good
> mental map of the relationship between data and data_smart. Come to
> think of it, I probably still don't.
>  
> > > -def setVar(var, value, d):
> > > +def setVar(var, value, d, filename = None, lineno = None):
> > >      """Set a variable to a given value"""
> > > -    d.setVar(var, value)
> > > +    filename, lineno = d.infer_file_and_line(filename, lineno)
> > > +    d.setVar(var, value, filename, lineno)
> > >  
> 
> > Changing any of these in data.py looks like a pointless waste of time.
> > The parser should be calling the object (dataSmart) variants and that
> > is now the preferred mechanism. If some old user does call here, the
> > parameters will be unset and the dataSmart setVar will call
> > infer_file_and_line for us anyway.
> 
> Yes, but in that case it would just report the line inside
> data.py/setVar, which is not as useful. Hmm. Well, that is an easy
> thing to check on!
> 
> Answer: In a bitbake -e (no target), there are 14 instances of hits
> on these functions. All of which come from:
> 
>     # DONE WITH PARSING... time to evaluate
>     if ext != ".bbclass":
>         data.setVar('FILE', abs_fn, d)
> 
> in BBHandler.py.
> 
> So I'd be totally happy with dropping all the data.py changes and
> instead fixing BBHandler.


Right, lets fix BBHandler.

> > > +    def includeLog(self, filename):
> > > +        """includeLog(included_file) shows that the file was
> > > included
> > > +        by the currently-processed file or context."""
> > > +        if self._tracking_enabled:
> > > +            event = (filename, [])
> > > +            position = (len(self.include_stack[-1][1]), event[1])
> > > +            self.include_stack[-1][1].append(event)
> > > +            self.include_stack.append(position)
> > > +
> > > +    def includeLogDone(self, filename):
> > > +        if self._tracking_enabled:
> > > +            if len(self.include_stack) > 1:
> > > +                self.include_stack.pop()
> > > +            else:
> > > +                bb.warn("Uh-oh:  includeLogDone(%s) tried to empty
> > > the stack." % filename)
> 
> > There has to be a better way to write code to do this. This looks
> > horrible.
> 
> I agree, but I wasn't able to come up with one.

I'll try and see if I can come up with something when I get a free
moment. I spent a short while yesterday thinking about this but I need
more time on it.

> > > +    def setVar(self, var, value, filename = None, lineno = None,
> > > op = 'set', details = None):
> 
> > As a function prototype, this scares me and shouts that there is
> > something going wrong here. The conversion of lineno to details and
> > back under a variety of different circumstances doesn't seem
> > particularly elegant or easy to understand.
> 
> Yes. At a bare minimum, it should probably be done with, say, a dict or
> something as a single additional/optional argument.
> 
> > Also, I want to hightlight that these changes effectively set our data
> > API in stone, making it effectively impossible to ever add sensible
> > external facing API changes. We should really change all these calls
> > to be named parameters so it doesn't totally freeze the API. I hate
> > having to do that but I hate the alternatives more.
> 
> Hmm. What if it were
>    def setVar(self, var, value, logging = None)
> 
> And then logging could be, say, { filename = ..., lineno = ..., op
> = ..., deteails = ... } where each component is optional with sane
> defaults.
> 
> I think that solves a LOT of the problems. It's a single optional
> parameter, with named components, and it avoids the addition of four
> separate arguments to do a single thing.

If you already know this please ignore me but what I meant by named
parameter was that you can make a call to a function like this:

d.setVar(x, y, filename=a, lineno=b)

which means when you can later extend the function without all users
needing to specify filename and lineno. This would avoid some of the API
lockin problems I worry about since we could make logging a named
parameter which is then effectively internal to data_smart, yet still
add external API without needing to specify it.

For the sake of readability, I might want to truncate it to "log" if
we're specifying it in many places. I do like the idea of collapsing the
info into one parameter rather than many.

> > So I do like the intent of this series but I don't like the impact on
> > the code base and think it needs some work to improve readability at
> > the very least.
> 
> Okay. I will do another pass.
> 
> BTW, if anyone can think of a clearer way to express that includelog
> history, I'd love to hear about it; I spent a while trying to think of
> something and came up blank.

I will see if I can find a few minutes to give that some thought...

Cheers,

Richard




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-08-17 10:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-16  1:14 [PATCH 0/2] variable/include tracking and _remove Peter Seebach
2012-08-16  1:14 ` [PATCH 1/2] data_smart.py: track file inclusion and variable modifications Peter Seebach
2012-08-16 14:32   ` Richard Purdie
2012-08-16 16:39     ` Peter Seebach
2012-08-17 10:09       ` Richard Purdie
2012-08-16  1:14 ` [PATCH 2/2] data_smart.py: implement _remove as a keyword like _append Peter Seebach
2012-08-16 10:13   ` Richard Purdie
2012-08-16 13:32     ` Peter Seebach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox