diff mbox series

Enable to use some bitbake server commands

Message ID 20221217084454.24346-1-yosuke.nky@gmail.com
State New
Headers show
Series Enable to use some bitbake server commands | expand

Commit Message

yosuke.nky@gmail.com Dec. 17, 2022, 8:44 a.m. UTC 
From: "yosuke.nakayama" <yosuke.nky@gmail.com>

Bitake server provides getRecipePackages, getRecipePackagesDynamic, getRProviders and dataStoreConnectorCmd.
getRecipePackages, getRecipePackagesDynamic and getRProviders always return data that can't be marshalled.
And dataStoreConnectorCmd also may return data that can't be marshalled, but this depends on sub command.
On the other hands, bitbake server uses xmlrpc module, but this supports only marshallizable data.
Therefore, when bitbake server returns data that can't be marshalled, xmlrpc raises an exception.
To fix this issue, this commit converts return-data to marshallizable data or None(if return-data can't be converted into marshallizable data)

Signed-off-by: yosuke.nakayama <yosuke.nky@gmail.com>
---
 bitbake/lib/bb/command.py | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

Comments

Richard Purdie Dec. 17, 2022, 11:34 a.m. UTC | #1 
On Sat, 2022-12-17 at 17:44 +0900, yosuke.nky@gmail.com wrote:
> From: "yosuke.nakayama" <yosuke.nky@gmail.com>
> 
> Bitake server provides getRecipePackages, getRecipePackagesDynamic, getRProviders and dataStoreConnectorCmd.
> getRecipePackages, getRecipePackagesDynamic and getRProviders always return data that can't be marshalled.
> And dataStoreConnectorCmd also may return data that can't be marshalled, but this depends on sub command.
> On the other hands, bitbake server uses xmlrpc module, but this supports only marshallizable data.
> Therefore, when bitbake server returns data that can't be marshalled, xmlrpc raises an exception.
> To fix this issue, this commit converts return-data to marshallizable data or None(if return-data can't be converted into marshallizable data)
> 
> Signed-off-by: yosuke.nakayama <yosuke.nky@gmail.com>
> ---
>  bitbake/lib/bb/command.py | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)

Thanks for the patch. I'd like to understand which objects are being
passed which can't be marshalled and think about fixing those issues
more specifically. I'm worried that two different behaviours between
the transports where one silently drops data isn't a good idea.

I'd also be interested to learn more about how you're using bitbake if
you can share anything?

Cheers,

Richard
yosuke.nky@gmail.com Dec. 17, 2022, 3:52 p.m. UTC | #2 
> I'd like to understand which objects are being passed which can't be marshalled
AFAIK, the type for the objects which can't be marshelled is `collections.defaultdict`.

> I'm worried that two different behaviours between the transports where one silently drops data isn't a good idea.
I agree. need to see if there is a way to convert to marshallizable object.

> I'd also be interested to learn more about how you're using bitbake if you can share anything?
I'm developing bbclient ( https://pypi.org/project/bbclient/ ) that provides command interface for bitbake server. I'm working this as an part of visual studio language server for bitbake.
I'm sorry. Not very common use, I know
Richard Purdie Dec. 18, 2022, 9:41 p.m. UTC | #3 
On Sat, 2022-12-17 at 07:52 -0800, yosuke.nky@gmail.com wrote:
> > I'd like to understand which objects are being passed which can't
> > be marshalled 
> AFAIK, the type for the objects which can't be marshelled is
> `collections.defaultdict`. 
> 
> >  I'm worried that two different behaviours between the transports
> > where one silently drops data isn't a good idea.
> I agree. need to see if there is a way to convert to marshallizable
> object. 

It looks like we may just be able to add a dict() wrapper around the
return values if these are all defaultdict objects?

We also need to go through the code and make sure the code using these
commands is not using functionality of the defaultdict.

> > I'd also be interested to learn more about how you're using bitbake
> > if you can share anything?
> I'm developing bbclient that provides command interface for bitbake
> server. I'm working this as an part of visual studio language server
> for bitbake.
> I'm sorry. Not very common use, I know
yosuke.nky@gmail.com Dec. 19, 2022, 1:48 p.m. UTC | #4 
> It looks like we may just be able to add a dict() wrapper around the return values if these are all defaultdict objects?
Sorry, I forgot that some commands return " collections.abc.KeysView ". Therefore, enable_marshel function in my patch is trying to cast list and dict.
For instance, dataStoreConnectorCmd returns " collections.abc.KeysView " when subcommand is "keys".
However, dataStoreConnectorCmd has so many subcommands that we have not been able to verify the return types of all of them.
Perhaps we should consider removing dataStoreConnectorCmd from the scope of this patch, as we will look into how to fix it properly later.

To make matters easier to understand, I created gits file for reproducing the error with tinfoil, not with my private work.
https://gist.github.com/AngryMane/9e0f57dac862b209ae406a729c4fa4e5
You can reproduce the error by following below steps.
1. bitbake-error in my gist and copy it to bitbake/bin
1. source oe-init-build env
1. bitbake --server-only --bind localhost:8081
1.bitbake-error

Please note that this error occurs with only XMLRPCserver and ProcessServer doen't occur this error.
In other words, the problem does not occur except in use cases where the bitbake --server-only option is used.
Richard Purdie Dec. 20, 2022, 4:01 p.m. UTC | #5 
On Mon, 2022-12-19 at 05:48 -0800, yosuke.nky@gmail.com wrote:
> > It looks like we may just be able to add a dict() wrapper around
> > the return values if these are all defaultdict objects?
> Sorry, I forgot that some commands return "collections.abc.KeysView".
> Therefore, enable_marshel function in my patch is trying to cast list
> and dict.   
> For instance, dataStoreConnectorCmd returns
> "collections.abc.KeysView" when subcommand is "keys".   
> However, dataStoreConnectorCmd has so many subcommands that we have
> not been able to verify the return types of all of them.
> Perhaps we should consider removing dataStoreConnectorCmd from the
> scope of this patch, as we will look into how to fix it properly
> later.

I think that would make sense. We may end up having to document that
the datastore connector can only be used on local connections as that
functionality is much more complex that the others.

> To make matters easier to understand, I created gits file for
> reproducing the error with tinfoil, not with my private work.
> https://gist.github.com/AngryMane/9e0f57dac862b209ae406a729c4fa4e5
> You can reproduce the error by following below steps.
> 1. bitbake-error in my gist and copy it to bitbake/bin
> 1. source oe-init-build env
> 1. bitbake --server-only --bind localhost:8081
> 1.bitbake-error
> 
> Please note that this error occurs with only XMLRPCserver and
> ProcessServer doen't occur this error.
> In other words, the problem does not occur except in use cases where
> the bitbake --server-only option is used.

Sounds good, thanks for putting that together, I'll have to take a
look!

Cheers,

Richard
diff mbox series

Patch

diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
index ec86885220..6efb5dfa26 100644
--- a/bitbake/lib/bb/command.py
+++ b/bitbake/lib/bb/command.py
@@ -21,6 +21,7 @@  Commands are queued in a CommandQueue
 from collections import OrderedDict, defaultdict
 
 import io
+import marshal
 import bb.event
 import bb.cooker
 import bb.remotedata
@@ -340,7 +341,7 @@  class CommandsSync:
             mc = params[0]
         except IndexError:
             mc = ''
-        return command.cooker.recipecaches[mc].packages
+        return CommandsSync.enable_marshel(command.cooker.recipecaches[mc].packages)
     getRecipePackages.readonly = True
 
     def getRecipePackagesDynamic(self, command, params):
@@ -348,7 +349,7 @@  class CommandsSync:
             mc = params[0]
         except IndexError:
             mc = ''
-        return command.cooker.recipecaches[mc].packages_dynamic
+        return CommandsSync.enable_marshel(command.cooker.recipecaches[mc].packages_dynamic)
     getRecipePackagesDynamic.readonly = True
 
     def getRProviders(self, command, params):
@@ -356,7 +357,7 @@  class CommandsSync:
             mc = params[0]
         except IndexError:
             mc = ''
-        return command.cooker.recipecaches[mc].rproviders
+        return CommandsSync.enable_marshel(command.cooker.recipecaches[mc].rproviders)
     getRProviders.readonly = True
 
     def getRuntimeDepends(self, command, params):
@@ -498,7 +499,7 @@  class CommandsSync:
             idx = command.remotedatastores.store(ret)
             return DataStoreConnectionHandle(idx)
 
-        return ret
+        return CommandsSync.enable_marshel(ret)
 
     def dataStoreConnectorVarHistCmd(self, command, params):
         dsindex = params[0]
@@ -573,6 +574,31 @@  class CommandsSync:
         return DataStoreConnectionHandle(idx)
     parseRecipeFile.readonly = True
 
+    @staticmethod
+    def enable_marshel(obj):
+        """
+        This function converts the data to be able to marshal as much as possible
+        because XML RPC can hands off only the data that can be marshalled.
+        """
+        try:
+            marshal.dumps(obj)
+            return obj
+        except Exception:
+            pass
+
+        try:
+            return dict(obj)
+        except Exception:
+            pass
+
+        try:
+            return list(obj)
+        except Exception:
+            pass
+
+        return None
+
+
 class CommandsAsync:
     """
     A class of asynchronous commands