| Submitter | Richard Purdie |
|---|---|
| Date | March 9, 2013, 12:47 p.m. |
| Message ID | <1362833241.9443.208.camel@ted> |
| Download | mbox | patch |
| Permalink | /patch/45827/ |
| State | New |
| Headers | show |
Comments
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.
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
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
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.
Patch
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):
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(-)