[bitbake-devel] bitbake: command: add getOverrideData command

Submitted by Nicolas Cornu via bitbake-devel on Oct. 31, 2019, 5:35 p.m. | Patch ID: 166458

Details

Message ID 1572543354-4229-1-git-send-email-chris.laplante@agilent.com
State New
Headers show

Commit Message

Nicolas Cornu via bitbake-devel Oct. 31, 2019, 5:35 p.m.
While working on a script using tinfoil, I needed a way to access the
overridedata for a given variable. Unfortunately, I found that
data_smart does not have remotedata proxying for it.

By adding the getOverrideData command, I am able to use a method like
this to access override data (either for a particular var, or all data):

def get_override_data(d, var=None):
    if "_remote_data" in d:
        connector = d["_remote_data"]  # type: bb.tinfoil.TinfoilDataStoreConnector
        return connector.getOverrideData(var)

    if var:
        return d.overridedata[var]
    return d.overridedata

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 bitbake/lib/bb/command.py | 10 ++++++++++
 bitbake/lib/bb/tinfoil.py |  4 ++++
 2 files changed, 14 insertions(+)

Patch hide | download patch | download mbox

diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
index 378f389..19ee8d8 100644
--- a/bitbake/lib/bb/command.py
+++ b/bitbake/lib/bb/command.py
@@ -485,6 +485,16 @@  class CommandsSync:
         return datastore.varhistory.variable(name)
     dataStoreConnectorGetVarHistory.readonly = True
 
+    def dataStoreConnectorGetOverrideData(self, command, params):
+        dsindex = params[0]
+        datastore = command.remotedatastores[dsindex]
+        ret = datastore.overridedata
+        if len(params) > 1:
+            name = params[1]
+            return ret[name]
+        return ret
+    dataStoreConnectorGetOverrideData.readonly = True
+
     def dataStoreConnectorExpandPythonRef(self, command, params):
         config_data_dict = params[0]
         varname = params[1]
diff --git a/bitbake/lib/bb/tinfoil.py b/bitbake/lib/bb/tinfoil.py
index 0a1b913..7c8c493 100644
--- a/bitbake/lib/bb/tinfoil.py
+++ b/bitbake/lib/bb/tinfoil.py
@@ -65,6 +65,10 @@  class TinfoilDataStoreConnector:
         return set(self.tinfoil.run_command('dataStoreConnectorGetKeys', self.dsindex))
     def getVarHistory(self, name):
         return self.tinfoil.run_command('dataStoreConnectorGetVarHistory', self.dsindex, name)
+    def getOverrideData(self, name=None):
+        if name:
+            return self.tinfoil.run_command('dataStoreConnectorGetOverrideData', self.dsindex, name)
+        return self.tinfoil.run_command('dataStoreConnectorGetOverrideData', self.dsindex)
     def expandPythonRef(self, varname, expr, d):
         ds = bb.remotedata.RemoteDatastores.transmit_datastore(d)
         ret = self.tinfoil.run_command('dataStoreConnectorExpandPythonRef', ds, varname, expr)

Comments

Nicolas Cornu via bitbake-devel Oct. 31, 2019, 6:16 p.m.
> Sent: Thursday, October 31, 2019 1:36 PM
> Subject: [bitbake-devel] [PATCH] bitbake: command: add getOverrideData command
> 
> While working on a script using tinfoil, I needed a way to access the
> overridedata for a given variable. Unfortunately, I found that
> data_smart does not have remotedata proxying for it.
> 
> By adding the getOverrideData command, I am able to use a method like
> this to access override data (either for a particular var, or all data):

On a side note, can anyone explain why my patches are being identified by Patchwork as coming from Nicolas Cornu? See: https://patchwork.openembedded.org/project/bitbake/patches/?submitter=12924
Richard Purdie Nov. 2, 2019, 8:43 p.m.
On Thu, 2019-10-31 at 13:35 -0400, Chris Laplante via bitbake-devel
wrote:
> While working on a script using tinfoil, I needed a way to access the
> overridedata for a given variable. Unfortunately, I found that
> data_smart does not have remotedata proxying for it.
> 
> By adding the getOverrideData command, I am able to use a method like
> this to access override data (either for a particular var, or all
> data):
> 
> def get_override_data(d, var=None):
>     if "_remote_data" in d:
>         connector = d["_remote_data"]  # type:
> bb.tinfoil.TinfoilDataStoreConnector
>         return connector.getOverrideData(var)
> 
>     if var:
>         return d.overridedata[var]
>     return d.overridedata
> 
> Signed-off-by: Chris Laplante <chris.laplante@agilent.com>

The question here is what is the usecase?

The override data in this form is really a datastore internal and
shouldn't be something we expose an external API for. It happens to
exist in this form for the current implementation of the datastore but
it didn't used to and may not again, depending on how we change that
implementation, particularly as people look at parsing performance.

Cheers,

Richard
Nicolas Cornu via bitbake-devel Nov. 2, 2019, 11:11 p.m.
Hi Richard,

> The question here is what is the usecase?
> 
> The override data in this form is really a datastore internal and
> shouldn't be something we expose an external API for. It happens to
> exist in this form for the current implementation of the datastore but
> it didn't used to and may not again, depending on how we change that
> implementation, particularly as people look at parsing performance.

Would it make more sense to come up with an external representation of the data that we could expose as a stable API?

My particular use case is that I needed to identify if and where SRCREV is getting overridden for particular recipes. For instance, someone might have added a SRCREV_pn-recipe to their local.conf. I am working on a tool that produces "buildhistory-collect-srcrevs"-style output from the SRCREV cache itself, without actually requiring that buildhistory is enabled. It's actually a general tool to inspect and manipulate the SRCREV cache. My team uses AUTOREV pretty extensively, so this kind of tool will be very useful to us.

Chris
Richard Purdie Nov. 3, 2019, 9:44 a.m.
On Sat, 2019-11-02 at 23:11 +0000, chris.laplante@agilent.com wrote:
> Hi Richard,
> 
> > The question here is what is the usecase?
> > 
> > The override data in this form is really a datastore internal and
> > shouldn't be something we expose an external API for. It happens to
> > exist in this form for the current implementation of the datastore
> > but
> > it didn't used to and may not again, depending on how we change
> > that
> > implementation, particularly as people look at parsing performance.
> 
> Would it make more sense to come up with an external representation
> of the data that we could expose as a stable API?
> 
> My particular use case is that I needed to identify if and where
> SRCREV is getting overridden for particular recipes. For instance,
> someone might have added a SRCREV_pn-recipe to their local.conf. I am
> working on a tool that produces "buildhistory-collect-srcrevs"-style
> output from the SRCREV cache itself, without actually requiring that
> buildhistory is enabled. It's actually a general tool to inspect and
> manipulate the SRCREV cache. My team uses AUTOREV pretty extensively,
> so this kind of tool will be very useful to us.

I think it depends what kind of guarantees we're expecting around this.
For example:

python () {
    if d.getVar("MACHINE") == "bar":
        d.setVar("SRCREV", "problem")
}

Really the only way to be sure is to parse for each MACHINE and see
what the values are. I did wonder if you could just switch between
MACHINE values but that doesn't even cover the python above without the
datastore finalisation.

Basically my worry is what the expectations of the API are. Whilst you
may be fine with the limitations, its the makings of an unsolvable bug
report tomorrow :(.

Cheers,

Richard
Nicolas Cornu via bitbake-devel Nov. 3, 2019, 1:20 p.m.
> > My particular use case is that I needed to identify if and where
> > SRCREV is getting overridden for particular recipes. For instance,
> > someone might have added a SRCREV_pn-recipe to their local.conf. I am
> > working on a tool that produces "buildhistory-collect-srcrevs"-style
> > output from the SRCREV cache itself, without actually requiring that
> > buildhistory is enabled. It's actually a general tool to inspect and
> > manipulate the SRCREV cache. My team uses AUTOREV pretty extensively,
> > so this kind of tool will be very useful to us.
> 
> I think it depends what kind of guarantees we're expecting around this.
> For example:
> 
> python () {
>     if d.getVar("MACHINE") == "bar":
>         d.setVar("SRCREV", "problem")
> }
> 
> Really the only way to be sure is to parse for each MACHINE and see
> what the values are. I did wonder if you could just switch between
> MACHINE values but that doesn't even cover the python above without the
> datastore finalisation.
> 
> Basically my worry is what the expectations of the API are. Whilst you
> may be fine with the limitations, its the makings of an unsolvable bug
> report tomorrow :(.

Yep, I definitely get your point. I personally am not worried about the more "pathological" cases like what you have above. My tool basically only cares about overrides that originate in local.conf. But I'm sure most hypothetical consumers of this API would not accept that limitation, so I agree that this doesn't constitute a good long-term plan. 

I will therefore keep this patch as a local modification to bitbake.

Thanks,
Chris