Patchwork [bitbake-devel,00/17,PULL] Hob: Bunch of bug fixes

login
register
mail settings
Submitter Dongxiao Xu
Date March 21, 2012, 12:55 p.m.
Message ID <cover.1332334068.git.dongxiao.xu@intel.com>
Download mbox
Permalink /patch/23983/
State New
Headers show

Pull-request

git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix

Comments

Dongxiao Xu - March 21, 2012, 12:55 p.m.
Hi Richard,

This pull request is Hob related bug fixes. Please help to review and pull.

Thanks,
Dongxiao

The following changes since commit d595960fea0988df9004d927bc2ec3439540dd9c:

  Hob: save CONF_VERSION and LCONF_VERSION into template (2012-03-20 14:39:45 +0000)

are available in the git repository at:
  git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dxu4/hob-bugfix

Dongxiao Xu (14):
  Hob: Still use initcmd != None as the if judgement
  Hob: Remove split model in GTK Hob
  cooker: terminate each process when quitting recipe parsing
  Hob: change package classes selection GUI
  Hob: Change template button name from "Template" to "Templates"
  command.py: Change parseConfigurationFiles API from async to sync
  process.py: Increase the timeout value for polling commands
  Hob: Change parseConfigurationFiles API usage
  Hob: Fix the workaround to get image types
  Hob: Change recipe selection tab order
  Hob: Change package selection tab order
  Hob: Move "Create your own image" to the bottom of image list
  cooker: add inherit information when generating package tree
  Hob: Use inherit information to judge if a recipe is image type

Shane Wang (3):
  Hob: Change window title from HOB to Hob
  Hob: Change "View Packages" description
  Hob: Change the titles for recipe view and package view

 lib/bb/command.py                          |   17 +-
 lib/bb/cooker.py                           |    3 +
 lib/bb/server/process.py                   |    2 +-
 lib/bb/ui/crumbs/builder.py                |   15 +--
 lib/bb/ui/crumbs/hig.py                    |  284 ++++++++--------------------
 lib/bb/ui/crumbs/hobeventhandler.py        |   48 ++---
 lib/bb/ui/crumbs/hoblistmodel.py           |   11 +-
 lib/bb/ui/crumbs/imageconfigurationpage.py |   13 +-
 lib/bb/ui/crumbs/packageselectionpage.py   |   42 ++--
 lib/bb/ui/crumbs/recipeselectionpage.py    |   50 +++---
 lib/bb/ui/hob.py                           |   14 +-
 11 files changed, 177 insertions(+), 322 deletions(-)
Joshua Lock - March 22, 2012, 12:59 a.m.
On 21/03/12 05:55, Dongxiao Xu wrote:
> Hi Richard,
>
> This pull request is Hob related bug fixes. Please help to review and pull.
>
> Thanks,
> Dongxiao
>
> The following changes since commit d595960fea0988df9004d927bc2ec3439540dd9c:
>
>    Hob: save CONF_VERSION and LCONF_VERSION into template (2012-03-20 14:39:45 +0000)
>
> are available in the git repository at:
>    git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix
>    http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dxu4/hob-bugfix
>
> Dongxiao Xu (14):
>    Hob: Still use initcmd != None as the if judgement
>    Hob: Remove split model in GTK Hob
>    cooker: terminate each process when quitting recipe parsing
>    Hob: change package classes selection GUI
>    Hob: Change template button name from "Template" to "Templates"

These all look fine.

>    command.py: Change parseConfigurationFiles API from async to sync
>    process.py: Increase the timeout value for polling commands
>    Hob: Change parseConfigurationFiles API usage

Can you explain some of the reasoning behind this set of changes? If the 
command truly belongs as an asynchronous one I'm not sure why we need to 
increase the timeout duration specifically for it.

>    Hob: Fix the workaround to get image types
>    Hob: Change recipe selection tab order
>    Hob: Change package selection tab order
>    Hob: Move "Create your own image" to the bottom of image list
>    cooker: add inherit information when generating package tree
>    Hob: Use inherit information to judge if a recipe is image type
>
> Shane Wang (3):
>    Hob: Change window title from HOB to Hob
>    Hob: Change "View Packages" description
>    Hob: Change the titles for recipe view and package view

These also look OK to me.

Cheers,
Joshua
Dongxiao Xu - March 22, 2012, 1:21 a.m.
On Wed, 2012-03-21 at 17:59 -0700, Joshua Lock wrote:
> 
> On 21/03/12 05:55, Dongxiao Xu wrote:
> > Hi Richard,
> >
> > This pull request is Hob related bug fixes. Please help to review and pull.
> >
> > Thanks,
> > Dongxiao
> >
> > The following changes since commit d595960fea0988df9004d927bc2ec3439540dd9c:
> >
> >    Hob: save CONF_VERSION and LCONF_VERSION into template (2012-03-20 14:39:45 +0000)
> >
> > are available in the git repository at:
> >    git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix
> >    http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dxu4/hob-bugfix
> >
> > Dongxiao Xu (14):
> >    Hob: Still use initcmd != None as the if judgement
> >    Hob: Remove split model in GTK Hob
> >    cooker: terminate each process when quitting recipe parsing
> >    Hob: change package classes selection GUI
> >    Hob: Change template button name from "Template" to "Templates"
> 
> These all look fine.
> 
> >    command.py: Change parseConfigurationFiles API from async to sync
> >    process.py: Increase the timeout value for polling commands
> >    Hob: Change parseConfigurationFiles API usage
> 
> Can you explain some of the reasoning behind this set of changes? If the 
> command truly belongs as an asynchronous one I'm not sure why we need to 
> increase the timeout duration specifically for it.
> 
> >    Hob: Fix the workaround to get image types

Hi Josh,

Actually the above three patches are preparations for this commit: "Hob:
Fix the workaround to get image types".

The background is that, we need to add extra inherits of
image_types.bbclass before parsing configuration files and getting
variables values. Therefore the modified code piece is something like:

def get_parameters(self):
    self.init_cooker()
    self.set_extra_inherits("image_types")
    self.parse_config()

    self.server.runCommand(["getVariable", "MACHINE"])
    ...

We can see from above code is that, the parse_config locates between two
SYNC commands (set_extra_inherits and getVariable). Therefore we also
need the parse_config to be a SYNC command.

As far as I know the parseConfigurationFiles API doesn't cost too much
time (should less than 1s), so that's the reason why I change it to SYNC
mode.

The increase of time duration for parseConfigurationFiles is that, the
process server use pipe mode to pass object (function) from one to
another. The original 0.5 second timeout isn't enough for transferring
"parseConfigurationFiles" object, the phenomenon is that on certain
machines, the runCommand result is messed up with wrong values. This
issue doesn't exist with xmlrpc server.

Thanks,
Dongxiao


> >    Hob: Change recipe selection tab order
> >    Hob: Change package selection tab order
> >    Hob: Move "Create your own image" to the bottom of image list
> >    cooker: add inherit information when generating package tree
> >    Hob: Use inherit information to judge if a recipe is image type
> >
> > Shane Wang (3):
> >    Hob: Change window title from HOB to Hob
> >    Hob: Change "View Packages" description
> >    Hob: Change the titles for recipe view and package view
> 
> These also look OK to me.
> 
> Cheers,
> Joshua
Joshua Lock - March 22, 2012, 10:32 p.m.
On 21/03/12 18:21, Xu, Dongxiao wrote:
> On Wed, 2012-03-21 at 17:59 -0700, Joshua Lock wrote:
>>
>> On 21/03/12 05:55, Dongxiao Xu wrote:
>>> Hi Richard,
>>>
>>> This pull request is Hob related bug fixes. Please help to review and pull.
>>>
>>> Thanks,
>>> Dongxiao
>>>
>>> The following changes since commit d595960fea0988df9004d927bc2ec3439540dd9c:
>>>
>>>     Hob: save CONF_VERSION and LCONF_VERSION into template (2012-03-20 14:39:45 +0000)
>>>
>>> are available in the git repository at:
>>>     git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix
>>>     http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dxu4/hob-bugfix
>>>
>>> Dongxiao Xu (14):
>>>     Hob: Still use initcmd != None as the if judgement
>>>     Hob: Remove split model in GTK Hob
>>>     cooker: terminate each process when quitting recipe parsing
>>>     Hob: change package classes selection GUI
>>>     Hob: Change template button name from "Template" to "Templates"
>>
>> These all look fine.
>>
>>>     command.py: Change parseConfigurationFiles API from async to sync
>>>     process.py: Increase the timeout value for polling commands
>>>     Hob: Change parseConfigurationFiles API usage
>>
>> Can you explain some of the reasoning behind this set of changes? If the
>> command truly belongs as an asynchronous one I'm not sure why we need to
>> increase the timeout duration specifically for it.
>>
>>>     Hob: Fix the workaround to get image types
>
> Hi Josh,
>
> Actually the above three patches are preparations for this commit: "Hob:
> Fix the workaround to get image types".
>
> The background is that, we need to add extra inherits of
> image_types.bbclass before parsing configuration files and getting
> variables values. Therefore the modified code piece is something like:
>
> def get_parameters(self):
>      self.init_cooker()
>      self.set_extra_inherits("image_types")
>      self.parse_config()
>
>      self.server.runCommand(["getVariable", "MACHINE"])
>      ...
>
> We can see from above code is that, the parse_config locates between two
> SYNC commands (set_extra_inherits and getVariable). Therefore we also
> need the parse_config to be a SYNC command.
>
> As far as I know the parseConfigurationFiles API doesn't cost too much
> time (should less than 1s), so that's the reason why I change it to SYNC
> mode.
>
> The increase of time duration for parseConfigurationFiles is that, the
> process server use pipe mode to pass object (function) from one to
> another. The original 0.5 second timeout isn't enough for transferring
> "parseConfigurationFiles" object, the phenomenon is that on certain
> machines, the runCommand result is messed up with wrong values. This
> issue doesn't exist with xmlrpc server.

OK, I understand what you're trying to achieve. I'm afraid I'm not 
convinced that we need to make such an invasive change though. Any 
reason we can't re-order the initialisation so that everything is done 
after the async parseConfigurationFiles?

I'm nervous of us pushing the 0.5s->1s change this late in the cycle. 
Have you any indication of whether it affects builds using knotty?

Just to be clear I think Richard needs to ack/nack this change to core 
BitBake. The Hob changes related to it are fine by me.

Cheers,
Joshua
Richard Purdie - March 22, 2012, 11:33 p.m.
On Thu, 2012-03-22 at 15:32 -0700, Joshua Lock wrote:
> On 21/03/12 18:21, Xu, Dongxiao wrote:
> > On Wed, 2012-03-21 at 17:59 -0700, Joshua Lock wrote:
> >>
> >> On 21/03/12 05:55, Dongxiao Xu wrote:
> >>> Hi Richard,
> >>>
> >>> This pull request is Hob related bug fixes. Please help to review and pull.
> >>>
> >>> Thanks,
> >>> Dongxiao
> >>>
> >>> The following changes since commit d595960fea0988df9004d927bc2ec3439540dd9c:
> >>>
> >>>     Hob: save CONF_VERSION and LCONF_VERSION into template (2012-03-20 14:39:45 +0000)
> >>>
> >>> are available in the git repository at:
> >>>     git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix
> >>>     http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dxu4/hob-bugfix
> >>>
> >>> Dongxiao Xu (14):
> >>>     Hob: Still use initcmd != None as the if judgement
> >>>     Hob: Remove split model in GTK Hob
> >>>     cooker: terminate each process when quitting recipe parsing
> >>>     Hob: change package classes selection GUI
> >>>     Hob: Change template button name from "Template" to "Templates"
> >>
> >> These all look fine.
> >>
> >>>     command.py: Change parseConfigurationFiles API from async to sync
> >>>     process.py: Increase the timeout value for polling commands
> >>>     Hob: Change parseConfigurationFiles API usage
> >>
> >> Can you explain some of the reasoning behind this set of changes? If the
> >> command truly belongs as an asynchronous one I'm not sure why we need to
> >> increase the timeout duration specifically for it.
> >>
> >>>     Hob: Fix the workaround to get image types
> >
> > Hi Josh,
> >
> > Actually the above three patches are preparations for this commit: "Hob:
> > Fix the workaround to get image types".
> >
> > The background is that, we need to add extra inherits of
> > image_types.bbclass before parsing configuration files and getting
> > variables values. Therefore the modified code piece is something like:
> >
> > def get_parameters(self):
> >      self.init_cooker()
> >      self.set_extra_inherits("image_types")
> >      self.parse_config()
> >
> >      self.server.runCommand(["getVariable", "MACHINE"])
> >      ...
> >
> > We can see from above code is that, the parse_config locates between two
> > SYNC commands (set_extra_inherits and getVariable). Therefore we also
> > need the parse_config to be a SYNC command.
> >
> > As far as I know the parseConfigurationFiles API doesn't cost too much
> > time (should less than 1s), so that's the reason why I change it to SYNC
> > mode.
> >
> > The increase of time duration for parseConfigurationFiles is that, the
> > process server use pipe mode to pass object (function) from one to
> > another. The original 0.5 second timeout isn't enough for transferring
> > "parseConfigurationFiles" object, the phenomenon is that on certain
> > machines, the runCommand result is messed up with wrong values. This
> > issue doesn't exist with xmlrpc server.
> 
> OK, I understand what you're trying to achieve. I'm afraid I'm not 
> convinced that we need to make such an invasive change though. Any 
> reason we can't re-order the initialisation so that everything is done 
> after the async parseConfigurationFiles?
> 
> I'm nervous of us pushing the 0.5s->1s change this late in the cycle. 
> Have you any indication of whether it affects builds using knotty?
> 
> Just to be clear I think Richard needs to ack/nack this change to core 
> BitBake. The Hob changes related to it are fine by me.

Having a sync command blocking the system for over 1 second isn't right.
Sync commands are meant to be fast (say 0.01s max) and this obviously
isn't.

I'm rather concerned about this and it suggests something is wrong with
hob's init/state code to be honest.

My suggestion would be to add an extra parameter to self.parse_config()
so you can set other classes to inherit (and variables to set). You will
still have to then read MACHINE later but I think that is reasonable.

Cheers,

Richard
Dongxiao Xu - March 23, 2012, 3:41 a.m.
On Thu, 2012-03-22 at 23:33 +0000, Richard Purdie wrote:
> On Thu, 2012-03-22 at 15:32 -0700, Joshua Lock wrote:
> > On 21/03/12 18:21, Xu, Dongxiao wrote:
> > > On Wed, 2012-03-21 at 17:59 -0700, Joshua Lock wrote:
> > >>
> > >> On 21/03/12 05:55, Dongxiao Xu wrote:
> > >>> Hi Richard,
> > >>>
> > >>> This pull request is Hob related bug fixes. Please help to review and pull.
> > >>>
> > >>> Thanks,
> > >>> Dongxiao
> > >>>
> > >>> The following changes since commit d595960fea0988df9004d927bc2ec3439540dd9c:
> > >>>
> > >>>     Hob: save CONF_VERSION and LCONF_VERSION into template (2012-03-20 14:39:45 +0000)
> > >>>
> > >>> are available in the git repository at:
> > >>>     git://git.pokylinux.org/poky-contrib dxu4/hob-bugfix
> > >>>     http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dxu4/hob-bugfix
> > >>>
> > >>> Dongxiao Xu (14):
> > >>>     Hob: Still use initcmd != None as the if judgement
> > >>>     Hob: Remove split model in GTK Hob
> > >>>     cooker: terminate each process when quitting recipe parsing
> > >>>     Hob: change package classes selection GUI
> > >>>     Hob: Change template button name from "Template" to "Templates"
> > >>
> > >> These all look fine.
> > >>
> > >>>     command.py: Change parseConfigurationFiles API from async to sync
> > >>>     process.py: Increase the timeout value for polling commands
> > >>>     Hob: Change parseConfigurationFiles API usage
> > >>
> > >> Can you explain some of the reasoning behind this set of changes? If the
> > >> command truly belongs as an asynchronous one I'm not sure why we need to
> > >> increase the timeout duration specifically for it.
> > >>
> > >>>     Hob: Fix the workaround to get image types
> > >
> > > Hi Josh,
> > >
> > > Actually the above three patches are preparations for this commit: "Hob:
> > > Fix the workaround to get image types".
> > >
> > > The background is that, we need to add extra inherits of
> > > image_types.bbclass before parsing configuration files and getting
> > > variables values. Therefore the modified code piece is something like:
> > >
> > > def get_parameters(self):
> > >      self.init_cooker()
> > >      self.set_extra_inherits("image_types")
> > >      self.parse_config()
> > >
> > >      self.server.runCommand(["getVariable", "MACHINE"])
> > >      ...
> > >
> > > We can see from above code is that, the parse_config locates between two
> > > SYNC commands (set_extra_inherits and getVariable). Therefore we also
> > > need the parse_config to be a SYNC command.
> > >
> > > As far as I know the parseConfigurationFiles API doesn't cost too much
> > > time (should less than 1s), so that's the reason why I change it to SYNC
> > > mode.
> > >
> > > The increase of time duration for parseConfigurationFiles is that, the
> > > process server use pipe mode to pass object (function) from one to
> > > another. The original 0.5 second timeout isn't enough for transferring
> > > "parseConfigurationFiles" object, the phenomenon is that on certain
> > > machines, the runCommand result is messed up with wrong values. This
> > > issue doesn't exist with xmlrpc server.
> > 
> > OK, I understand what you're trying to achieve. I'm afraid I'm not 
> > convinced that we need to make such an invasive change though. Any 
> > reason we can't re-order the initialisation so that everything is done 
> > after the async parseConfigurationFiles?
> > 
> > I'm nervous of us pushing the 0.5s->1s change this late in the cycle. 
> > Have you any indication of whether it affects builds using knotty?
> > 
> > Just to be clear I think Richard needs to ack/nack this change to core 
> > BitBake. The Hob changes related to it are fine by me.
> 
> Having a sync command blocking the system for over 1 second isn't right.
> Sync commands are meant to be fast (say 0.01s max) and this obviously
> isn't.
> 
> I'm rather concerned about this and it suggests something is wrong with
> hob's init/state code to be honest.

The above case is not in current Hob code, I added it only after I
change parseConfigurationFiles to SYNC mode. There is no problem with
current Hob's init code.

> 
> My suggestion would be to add an extra parameter to self.parse_config()
> so you can set other classes to inherit (and variables to set). You will
> still have to then read MACHINE later but I think that is reasonable.

I will try to implement it in ASYNC mode. 

Thanks,
Dongxiao

> 
> Cheers,
> 
> Richard
> 
> 
>