[PATCH] data_smart: Add _remove operator

[PATCH] data_smart: Add _remove operator

[Richard Purdie]

There are long standing complaints about the fact its very difficult
to remove a portion of a variable. The immediate request is for a -=
and =- operator. The trouble is that += and =+ are "immediate"
operators and are applied straight away. Most people would expect
-= and =- to be deferred to have the effect most people desire and
therefore implementing -= and =- would just make the situation more
confusing.

This deferred operation is much more similar to the override syntax
which happens at data store finalisation. The _remove operator is
therefore in keeping with the _append and _prepend operations.

This code is loosely based on a patch from Peter Seebach although it
has been rewritten to be simpler, more efficient and avoid some
potential bugs.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 bitbake/lib/bb/data_smart.py |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py
index 5bf11e5..3806fdc 100644
--- a/bitbake/lib/bb/data_smart.py
+++ b/bitbake/lib/bb/data_smart.py
@@ -38,8 +38,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"\${@.+?}")
 
@@ -357,7 +357,8 @@ 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.
         #
 
         # We only want to report finalization once per variable overridden.
@@ -392,7 +393,7 @@ class DataSmart(MutableMapping):
                 except Exception:
                     logger.info("Untracked delVar")
 
-        # now on to the appends and prepends
+        # now on to the appends and prepends, and stashing the removes
         for op in __setvar_keyword__:
             if op in self._special_values:
                 appends = self._special_values[op] or []
@@ -415,6 +416,10 @@ class DataSmart(MutableMapping):
                         elif op == "_prepend":
                             sval = a + (self.getVar(append, False) or "")
                             self.setVar(append, sval)
+                        elif op == "_remove":
+                            removes = self.getVarFlag(append, "_removeactive", False) or []
+                            removes.append(a)
+                            self.setVarFlag(append, "_removeactive", removes, ignore=True)
 
                     # We save overrides that may be applied at some later stage
                     if keep:
@@ -515,7 +520,7 @@ class DataSmart(MutableMapping):
             self.varhistory.record(**loginfo)
             self.setVar(newkey, val, ignore=True)
 
-        for i in ('_append', '_prepend'):
+        for i in (__setvar_keyword__):
             src = self.getVarFlag(key, i)
             if src is None:
                 continue
@@ -576,6 +581,9 @@ class DataSmart(MutableMapping):
                 value = copy.copy(local_var["defaultval"])
         if expand and value:
             value = self.expand(value, None)
+        if value and flag == "_content" and local_var and "_removeactive" in local_var:
+            for i in local_var["_removeactive"]:
+                value = value.replace(i, "")
         return value
 
     def delVarFlag(self, var, flag, **loginfo):

Re: [PATCH] data_smart: Add _remove operator

[Chris Larson]

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

On Sat, Mar 9, 2013 at 5:47 AM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> There are long standing complaints about the fact its very difficult
> to remove a portion of a variable. The immediate request is for a -=
> and =- operator. The trouble is that += and =+ are "immediate"
> operators and are applied straight away. Most people would expect
> -= and =- to be deferred to have the effect most people desire and
> therefore implementing -= and =- would just make the situation more
> confusing.
>
> This deferred operation is much more similar to the override syntax
> which happens at data store finalisation. The _remove operator is
> therefore in keeping with the _append and _prepend operations.
>
> This code is loosely based on a patch from Peter Seebach although it
> has been rewritten to be simpler, more efficient and avoid some
> potential bugs.
>

This is a nice idea, though I'm slightly concerned about the naive
implementation (use of str.replace). Without it being either word-based
(limits flexibility to optimize for the common case) or regex-based (more
complex), we may be greatly limiting the usefulness. I wouldn't want to try
to remove a word from DISTRO_FEATURES and end up removing part of a word,
for example.
-- 
Christopher Larson

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

Re: [PATCH] data_smart: Add _remove operator

[Paul Eggleton]

On Saturday 09 March 2013 16:20:10 Chris Larson wrote:
> On Sat, Mar 9, 2013 at 5:47 AM, Richard Purdie <
> 
> richard.purdie@linuxfoundation.org> wrote:
> > There are long standing complaints about the fact its very difficult
> > to remove a portion of a variable. The immediate request is for a -=
> > and =- operator. The trouble is that += and =+ are "immediate"
> > operators and are applied straight away. Most people would expect
> > -= and =- to be deferred to have the effect most people desire and
> > therefore implementing -= and =- would just make the situation more
> > confusing.
> > 
> > This deferred operation is much more similar to the override syntax
> > which happens at data store finalisation. The _remove operator is
> > therefore in keeping with the _append and _prepend operations.
> > 
> > This code is loosely based on a patch from Peter Seebach although it
> > has been rewritten to be simpler, more efficient and avoid some
> > potential bugs.
> 
> This is a nice idea, though I'm slightly concerned about the naive
> implementation (use of str.replace). Without it being either word-based
> (limits flexibility to optimize for the common case) or regex-based (more
> complex), we may be greatly limiting the usefulness. I wouldn't want to try
> to remove a word from DISTRO_FEATURES and end up removing part of a word,
> for example.

I can't recall if we've discussed this before, but shouldn't we be looking at 
introducing some understanding of list type variables into BitBake itself? If 
BitBake knew the variable was a list it would be possible to have language 
elements that understood how to behave in this situation.

(I've been thinking about this for a while. It could also help to implement 
better dependencies on values of list variables, to only rebuild what's really 
necessary when DISTRO_FEATURES changes for example).

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] data_smart: Add _remove operator

[Peter Seebach]

On Sun, 10 Mar 2013 20:39:07 +0000
Paul Eggleton <paul.eggleton@linux.intel.com> wrote:

> I can't recall if we've discussed this before, but shouldn't we be looking at 
> introducing some understanding of list type variables into BitBake itself? If 
> BitBake knew the variable was a list it would be possible to have language 
> elements that understood how to behave in this situation.

I rather like this. I would like to have _remove, and it may even make sense
to adopt _remove in the short term, but really I think most of the things
where we're using append/remove should probably be lists, which would also
eliminate the entire category of bugs in which it is unclear whether or not
to add a space.

Rough thoughts about implementation: Listness could be a flag.

LIST[list] = " "
LIST = "blah blah blah"
# Oh, hey, what if...
PATH[list] = ":"

or as shorthand:

LIST = [blah blah blah]

If this is done, the behavior of append/prepend is slightly altered (to
always ensure separators are added when needed), and then we could more
easily handle the remove case, too.

That said: I'd rather see a plain _remove go in than wait on a larger and
fancier design.

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

Re: [PATCH] data_smart: Add _remove operator

[Chris Larson]

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

On Wed, Mar 13, 2013 at 11:28 AM, Peter Seebach <peter.seebach@windriver.com
> wrote:

> On Sun, 10 Mar 2013 20:39:07 +0000
> Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
>
> > I can't recall if we've discussed this before, but shouldn't we be
> looking at
> > introducing some understanding of list type variables into BitBake
> itself? If
> > BitBake knew the variable was a list it would be possible to have
> language
> > elements that understood how to behave in this situation.
>

Yeah, this gets brought up from time to time, it's something we've wanted
for a while, but nobody has taken the time to make it happen. I would love
to see some form of typing go in, and I know Richard has expressed a desire
to see this as well in the past.

I rather like this. I would like to have _remove, and it may even make sense
> to adopt _remove in the short term, but really I think most of the things
> where we're using append/remove should probably be lists, which would also
> eliminate the entire category of bugs in which it is unclear whether or not
> to add a space.
>
> Rough thoughts about implementation: Listness could be a flag.
>
> LIST[list] = " "
> LIST = "blah blah blah"
> # Oh, hey, what if...
> PATH[list] = ":"
>

We do have a flag-based typing implementation in the metadata, just not in
bitbake itself, see
http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?h=e4921fd for the
original implementation. I particularly like the bits that automatically
convert the function signature of the type callback (or class) into a
request for additional information via flags. An added required argument to
the function = a required flag of that name, same for optional. See
http://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/meta/lib/oe/types.py#n27
 or
http://git.yoctoproject.org/cgit/cgit.cgi/poky/tree/meta/lib/oe/types.py#n13
for
examples of that particular implementation.

or as shorthand:
>
> LIST = [blah blah blah]
>

I played briefly around with a *really* preliminary prototype of something
similar to this. See https://gist.github.com/kergoth/2788410 - any unquoted
values are interpreted as json types, and the append/prepend operations are
switched to better work with non-string values. It works surprisingly well
considering how naive the implementation is, see the functional local.conf
snippet in the gist.
-- 
Christopher Larson

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