Patchwork [bitbake-devel,v2] data_smart: Add _remove operator

login
register
mail settings
Submitter Richard Purdie
Date Aug. 24, 2013, 2:53 p.m.
Message ID <1377356035.6762.193.camel@ted>
Download mbox | patch
Permalink /patch/56523/
State New
Headers show

Comments

Richard Purdie - Aug. 24, 2013, 2:53 p.m.
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.

The code currently only works on space delimited variables, which
are by far the most commom type. If bitbake is ehanced to support
types natively in future, we can adjust this code to adapt to that.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Chris Larson - Aug. 26, 2013, 2:54 p.m.
On Sat, Aug 24, 2013 at 7:53 AM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> @@ -583,6 +588,14 @@ 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"]:
> +                if " " + i + " " in value:
> +                    value = value.replace(" " + i + " ", " ")
> +                if value.startswith(i + " "):
> +                    value = value[len(i + " "):]
> +                if value.endswith(" " + i):
> +                    value = value[:-len(" " + i)]
>

I'm curious, did you profile this implementation vs a split-filter-rejoin?
Richard Purdie - Aug. 26, 2013, 3:12 p.m.
On Mon, 2013-08-26 at 07:54 -0700, Chris Larson wrote:

> On Sat, Aug 24, 2013 at 7:53 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>         @@ -583,6 +588,14 @@ 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"]:
>         +                if " " + i + " " in value:
>         +                    value = value.replace(" " + i + " ", " ")
>         +                if value.startswith(i + " "):
>         +                    value = value[len(i + " "):]
>         +                if value.endswith(" " + i):
>         +                    value = value[:-len(" " + i)]
>
> I'm curious, did you profile this implementation vs a
> split-filter-rejoin?

No, in retrospect, that might have been better. I'll take a patch to
update it :)

Cheers,

Richard
Otavio Salvador - Aug. 31, 2013, 9:46 p.m.
On Mon, Aug 26, 2013 at 11:54 AM, Chris Larson <clarson@kergoth.com> wrote:
>
> On Sat, Aug 24, 2013 at 7:53 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>>
>> @@ -583,6 +588,14 @@ 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"]:
>> +                if " " + i + " " in value:
>> +                    value = value.replace(" " + i + " ", " ")
>> +                if value.startswith(i + " "):
>> +                    value = value[len(i + " "):]
>> +                if value.endswith(" " + i):
>> +                    value = value[:-len(" " + i)]
>
>
> I'm curious, did you profile this implementation vs a split-filter-rejoin?

I noticed it has already been merged but I couldn't comment on this
before as I was in a work trip.

I'd like to thank you both for doing it :)

Patch

diff --git a/bitbake/lib/bb/data_smart.py b/bitbake/lib/bb/data_smart.py
index dfa9afe..3fb88a9 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:
@@ -519,7 +524,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
@@ -583,6 +588,14 @@  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"]:
+                if " " + i + " " in value:
+                    value = value.replace(" " + i + " ", " ")
+                if value.startswith(i + " "):
+                    value = value[len(i + " "):]
+                if value.endswith(" " + i):
+                    value = value[:-len(" " + i)]
         return value
 
     def delVarFlag(self, var, flag, **loginfo):