[PATCH 0/7][PULL] Hob: Bug fixes

[PATCH 0/7][PULL] Hob: Bug fixes

[Dongxiao Xu]

Hi Richard,

This pull request is some Hob related bug fixes, please help to review and pull.

Thanks,
Dongxiao

The following changes since commit 8fed4fdf4bbbc9ef036ff96755c0bfe15c3a9dd0:

  knotty: Add back getTasks() call to ensure we don't repeat messges (2012-03-29 21:38:25 +0100)

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 (7):
  Hob: Fix MACHINE setting
  Hob: Update the cache when setting changed
  Hob: Make layers define in bblayers.conf as default
  Revert "lib/bb/ui/crumbs/hig: fix layers_changed test"
  Hob: Remove some calling of initiate_new_build()
  Hob: Remove duplication for certain bitbake variables
  Hob: Set stop button sensitive after task started

 lib/bb/ui/crumbs/builddetailspage.py |    2 ++
 lib/bb/ui/crumbs/builder.py          |   10 +++++-----
 lib/bb/ui/crumbs/hig.py              |   28 ++++++++++------------------
 lib/bb/ui/crumbs/hobeventhandler.py  |   22 +++++++++++++++-------
 lib/bb/ui/crumbs/imagedetailspage.py |    2 --
 5 files changed, 32 insertions(+), 32 deletions(-)

-- 
1.7.4.1

[PATCH 1/7] Hob: Fix MACHINE setting

[Dongxiao Xu]

Define the empty curr_mach to be "" instead of None.
Fix the judgement for ' if self.curr_mach == "" ' to be
' if self.curr_mach '.
Also set machine to bitbake server when "MACHINE" is not empty.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 lib/bb/ui/crumbs/builder.py         |    4 ++--
 lib/bb/ui/crumbs/hobeventhandler.py |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/bb/ui/crumbs/builder.py b/lib/bb/ui/crumbs/builder.py
index c1a75bd..02c9a9c 100755
--- a/lib/bb/ui/crumbs/builder.py
+++ b/lib/bb/ui/crumbs/builder.py
@@ -519,7 +519,7 @@ class Builder(gtk.Window):
             response = dialog.run()
             dialog.destroy()
         self.handler.clear_busy()
-        self.configuration.curr_mach = None
+        self.configuration.curr_mach = ""
         self.image_configuration_page.switch_machine_combo()
         self.switch_page(self.MACHINE_SELECTION)
 
@@ -868,7 +868,7 @@ class Builder(gtk.Window):
 
     def reparse_post_adv_settings(self):
         # DO reparse recipes
-        if self.configuration.curr_mach == "":
+        if not self.configuration.curr_mach:
             self.switch_page(self.MACHINE_SELECTION)
         else:
             self.switch_page(self.RCPPKGINFO_POPULATING)
diff --git a/lib/bb/ui/crumbs/hobeventhandler.py b/lib/bb/ui/crumbs/hobeventhandler.py
index a329380..3d5df9e 100644
--- a/lib/bb/ui/crumbs/hobeventhandler.py
+++ b/lib/bb/ui/crumbs/hobeventhandler.py
@@ -256,7 +256,8 @@ class HobHandler(gobject.GObject):
         self.server.runCommand(["setVariable", "BBLAYERS", " ".join(bblayers)])
 
     def set_machine(self, machine):
-        self.server.runCommand(["setVariable", "MACHINE", machine])
+        if machine:
+            self.server.runCommand(["setVariable", "MACHINE", machine])
 
     def set_sdk_machine(self, sdk_machine):
         self.server.runCommand(["setVariable", "SDKMACHINE", sdk_machine])
-- 
1.7.4.1

[PATCH 2/7] Hob: Update the cache when setting changed

[Dongxiao Xu]

If values in advanced is changed, we also need to reparse the cache to
get the latest value.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 lib/bb/ui/crumbs/builder.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/bb/ui/crumbs/builder.py b/lib/bb/ui/crumbs/builder.py
index 02c9a9c..44b208a 100755
--- a/lib/bb/ui/crumbs/builder.py
+++ b/lib/bb/ui/crumbs/builder.py
@@ -869,7 +869,7 @@ class Builder(gtk.Window):
     def reparse_post_adv_settings(self):
         # DO reparse recipes
         if not self.configuration.curr_mach:
-            self.switch_page(self.MACHINE_SELECTION)
+            self.switch_page(self.CONFIG_UPDATED)
         else:
             self.switch_page(self.RCPPKGINFO_POPULATING)
 
-- 
1.7.4.1

[PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Dongxiao Xu]

For layers defined in bblayers.conf, we treat them as default layers
and users are not allowed to remove them.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 lib/bb/ui/crumbs/builder.py |    4 ++--
 lib/bb/ui/crumbs/hig.py     |   23 +++++++++--------------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/lib/bb/ui/crumbs/builder.py b/lib/bb/ui/crumbs/builder.py
index 44b208a..3be799c 100755
--- a/lib/bb/ui/crumbs/builder.py
+++ b/lib/bb/ui/crumbs/builder.py
@@ -178,7 +178,7 @@ class Parameters:
         self.all_distros = []
         self.all_sdk_machines = []
         self.max_threads = params["max_threads"]
-        self.all_layers = []
+        self.layers_default = params["layer"].split()
         self.core_base = params["core_base"]
         self.image_names = []
         self.image_addr = params["image_addr"]
@@ -764,7 +764,7 @@ class Builder(gtk.Window):
     def show_layer_selection_dialog(self):
         dialog = LayerSelectionDialog(title = "Layers",
                      layers = copy.deepcopy(self.configuration.layers),
-                     all_layers = self.parameters.all_layers,
+                     layers_default = self.parameters.layers_default,
                      parent = self,
                      flags = gtk.DIALOG_MODAL
                          | gtk.DIALOG_DESTROY_WITH_PARENT
diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
index 9adb281..9a38bc0 100644
--- a/lib/bb/ui/crumbs/hig.py
+++ b/lib/bb/ui/crumbs/hig.py
@@ -850,7 +850,7 @@ class LayerSelectionDialog (CrumbsDialog):
             layer_store.remove(iter)
 
 
-    def gen_layer_widget(self, layers, layers_avail, window, tooltip=""):
+    def gen_layer_widget(self, layers, layers_default, window, tooltip=""):
         hbox = gtk.HBox(False, 6)
 
         layer_tv = gtk.TreeView()
@@ -881,8 +881,8 @@ class LayerSelectionDialog (CrumbsDialog):
         for layer in layers:
             if layer.endswith("/meta"):
                 core_iter = layer_store.prepend([layer])
-            elif layer.endswith("/meta-hob") and core_iter:
-                layer_store.insert_after(core_iter, [layer])
+            elif layer in layers_default:
+                core_iter = layer_store.insert_after(core_iter, [layer])
             else:
                 layer_store.append([layer])
 
@@ -924,12 +924,12 @@ class LayerSelectionDialog (CrumbsDialog):
     def add_leave_cb(self, button, event):
         self.im.set_from_file(hic.ICON_INDI_ADD_FILE)
 
-    def __init__(self, title, layers, all_layers, parent, flags, buttons=None):
+    def __init__(self, title, layers, layers_default, parent, flags, buttons=None):
         super(LayerSelectionDialog, self).__init__(title, parent, flags, buttons)
 
         # class members from other objects
         self.layers = layers
-        self.all_layers = all_layers
+        self.layers_default = layers_default
         self.layers_changed = False
 
         # icon for remove button in TreeView
@@ -945,7 +945,7 @@ class LayerSelectionDialog (CrumbsDialog):
         self.connect("response", self.response_cb)
                 
     def create_visual_elements(self):
-        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.all_layers, self, None)
+        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.layers_default, self, None)
         layer_widget.set_size_request(450, 250)
         self.vbox.pack_start(layer_widget, expand=True, fill=True)
         self.show_all()
@@ -972,9 +972,7 @@ class LayerSelectionDialog (CrumbsDialog):
     def draw_delete_button_cb(self, col, cell, model, it, tv):
         path =  model.get_value(it, 0)
         # Trailing slashes are uncommon in bblayers.conf but confuse os.path.basename
-        path.rstrip('/')
-        name = os.path.basename(path)
-        if name == "meta" or name == "meta-hob":
+        if path in self.layers_default:
             cell.set_sensitive(False)
             cell.set_property('pixbuf', None)
             cell.set_property('mode', gtk.CELL_RENDERER_MODE_INERT)
@@ -992,11 +990,8 @@ class LayerSelectionDialog (CrumbsDialog):
     """
     def draw_layer_path_cb(self, col, cell, model, it):
         path = model.get_value(it, 0)
-        name = os.path.basename(path)
-        if name == "meta":
-            cell.set_property('markup', "<b>Core layer for images: it cannot be removed</b>\n%s" % path)
-        elif name == "meta-hob":
-            cell.set_property('markup', "<b>Core layer for Hob: it cannot be removed</b>\n%s" % path)
+        if path in self.layers_default:
+            cell.set_property('markup', "<b>Core layer: it cannot be removed</b>\n%s" % path)
         else:
             cell.set_property('text', path)
 
-- 
1.7.4.1

[PATCH 4/7] Revert "lib/bb/ui/crumbs/hig: fix layers_changed test"

[Dongxiao Xu]

We do not sort the layer list, therefore when response_cb, the ordering
is also not needed.

This reverts commit 71c1de347eef07a2059fc4544dc3cae090181f44.
---
 lib/bb/ui/crumbs/hig.py |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
index 9a38bc0..0ac4496 100644
--- a/lib/bb/ui/crumbs/hig.py
+++ b/lib/bb/ui/crumbs/hig.py
@@ -958,10 +958,7 @@ class LayerSelectionDialog (CrumbsDialog):
             layers.append(model.get_value(it, 0))
             it = model.iter_next(it)
 
-        orig_layers = sorted(self.layers)
-        layers.sort()
-
-        self.layers_changed = (orig_layers != layers)
+        self.layers_changed = (self.layers != layers)
         self.layers = layers
 
     """
-- 
1.7.4.1

[PATCH 5/7] Hob: Remove some calling of initiate_new_build()

[Dongxiao Xu]

initiate_new_build() function is in async mode and could not be called
before another async function.

Also we could not initialize the build if user simply change a setting,
therefore remove this function.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 lib/bb/ui/crumbs/imagedetailspage.py |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/lib/bb/ui/crumbs/imagedetailspage.py b/lib/bb/ui/crumbs/imagedetailspage.py
index b70440d..b12014a 100755
--- a/lib/bb/ui/crumbs/imagedetailspage.py
+++ b/lib/bb/ui/crumbs/imagedetailspage.py
@@ -403,7 +403,6 @@ class ImageDetailsPage (HobPage):
         response, path = self.builder.show_load_template_dialog()
         if not response:
             return
-        self.builder.initiate_new_build()
         if path:
             self.builder.load_template(path)
 
@@ -415,6 +414,5 @@ class ImageDetailsPage (HobPage):
         response, settings_changed = self.builder.show_adv_settings_dialog()
         if not response:
             return
-        self.builder.initiate_new_build()
         if settings_changed:
             self.builder.reparse_post_adv_settings()
-- 
1.7.4.1

[PATCH 6/7] Hob: Remove duplication for certain bitbake variables

[Dongxiao Xu]

Sometimes, certain variables have duplicated values inside, for example,
IMAGE_FSTYPES = "tar.bz2 ext3 tar.bz2 ext3"

We need to remove the redundancy for those values.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 lib/bb/ui/crumbs/hobeventhandler.py |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/lib/bb/ui/crumbs/hobeventhandler.py b/lib/bb/ui/crumbs/hobeventhandler.py
index 3d5df9e..74081dc 100644
--- a/lib/bb/ui/crumbs/hobeventhandler.py
+++ b/lib/bb/ui/crumbs/hobeventhandler.py
@@ -382,6 +382,13 @@ class HobHandler(gobject.GObject):
     def reset_build(self):
         self.build.reset()
 
+    def _remove_redundant(self, string):
+        ret = []
+        for i in string.split():
+            if i not in ret:
+                ret.append(i)
+        return " ".join(ret)
+
     def get_parameters(self):
         # retrieve the parameters from bitbake
         params = {}
@@ -445,19 +452,19 @@ class HobHandler(gobject.GObject):
             image_overhead_factor = float(image_overhead_factor)
         params['image_overhead_factor'] = image_overhead_factor
 
-        params["incompat_license"] = self.server.runCommand(["getVariable", "INCOMPATIBLE_LICENSE"]) or ""
+        params["incompat_license"] = self._remove_redundant(self.server.runCommand(["getVariable", "INCOMPATIBLE_LICENSE"]) or "")
         params["sdk_machine"] = self.server.runCommand(["getVariable", "SDKMACHINE"]) or self.server.runCommand(["getVariable", "SDK_ARCH"]) or ""
 
-        params["image_fstypes"] = self.server.runCommand(["getVariable", "IMAGE_FSTYPES"]) or ""
+        params["image_fstypes"] = self._remove_redundant(self.server.runCommand(["getVariable", "IMAGE_FSTYPES"]) or "")
 
-        params["image_types"] = self.server.runCommand(["getVariable", "IMAGE_TYPES"]) or ""
+        params["image_types"] = self._remove_redundant(self.server.runCommand(["getVariable", "IMAGE_TYPES"]) or "")
 
         params["conf_version"] = self.server.runCommand(["getVariable", "CONF_VERSION"]) or ""
         params["lconf_version"] = self.server.runCommand(["getVariable", "LCONF_VERSION"]) or ""
 
-        params["runnable_image_types"] = self.server.runCommand(["getVariable", "RUNNABLE_IMAGE_TYPES"]) or ""
-        params["runnable_machine_patterns"] = self.server.runCommand(["getVariable", "RUNNABLE_MACHINE_PATTERNS"]) or ""
-        params["deployable_image_types"] = self.server.runCommand(["getVariable", "DEPLOYABLE_IMAGE_TYPES"]) or ""
+        params["runnable_image_types"] = self._remove_redundant(self.server.runCommand(["getVariable", "RUNNABLE_IMAGE_TYPES"]) or "")
+        params["runnable_machine_patterns"] = self._remove_redundant(self.server.runCommand(["getVariable", "RUNNABLE_MACHINE_PATTERNS"]) or "")
+        params["deployable_image_types"] = self._remove_redundant(self.server.runCommand(["getVariable", "DEPLOYABLE_IMAGE_TYPES"]) or "")
         params["tmpdir"] = self.server.runCommand(["getVariable", "TMPDIR"]) or ""
         params["distro_version"] = self.server.runCommand(["getVariable", "DISTRO_VERSION"]) or ""
         params["target_os"] = self.server.runCommand(["getVariable", "TARGET_OS"]) or ""
-- 
1.7.4.1

[PATCH 7/7] Hob: Set stop button sensitive after task started

[Dongxiao Xu]

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 lib/bb/ui/crumbs/builddetailspage.py |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/bb/ui/crumbs/builddetailspage.py b/lib/bb/ui/crumbs/builddetailspage.py
index 1440039..84ab72a 100755
--- a/lib/bb/ui/crumbs/builddetailspage.py
+++ b/lib/bb/ui/crumbs/builddetailspage.py
@@ -133,6 +133,7 @@ class BuildDetailsPage (HobPage):
         self.progress_hbox.pack_start(self.progress_bar, expand=True, fill=True)
         self.stop_button = HobAltButton("Stop")
         self.stop_button.connect("clicked", self.stop_button_clicked_cb)
+        self.stop_button.set_sensitive(False)
         self.progress_hbox.pack_end(self.stop_button, expand=False, fill=False)
 
         self.notebook = HobNotebook()
@@ -169,6 +170,7 @@ class BuildDetailsPage (HobPage):
         recipe = os.path.basename(recipe_path).rstrip(".bb")
         tsk_msg = "<b>Running task %s of %s:</b> %s\n<b>Recipe:</b> %s" % (current, total, recipe_task, recipe)
         self.task_status.set_markup(tsk_msg)
+        self.stop_button.set_sensitive(True)
 
     def reset_build_status(self):
         self.task_status.set_markup("\n") # to ensure layout is correct
-- 
1.7.4.1

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Joshua Lock]

On 30/03/12 05:01, Dongxiao Xu wrote:
> For layers defined in bblayers.conf, we treat them as default layers
> and users are not allowed to remove them.

Can you explain the rationale behind this change? I see what you're 
doing but it's not entirely clear why.

I think this is a bad idea. Early on in the design of Hob we decided we 
didn't want configuration made for non-Hob builds to affect builds made 
with Hob, and vice versa.

Thanks,
Joshua

>
> Signed-off-by: Dongxiao Xu<dongxiao.xu@intel.com>
> ---
>   lib/bb/ui/crumbs/builder.py |    4 ++--
>   lib/bb/ui/crumbs/hig.py     |   23 +++++++++--------------
>   2 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/lib/bb/ui/crumbs/builder.py b/lib/bb/ui/crumbs/builder.py
> index 44b208a..3be799c 100755
> --- a/lib/bb/ui/crumbs/builder.py
> +++ b/lib/bb/ui/crumbs/builder.py
> @@ -178,7 +178,7 @@ class Parameters:
>           self.all_distros = []
>           self.all_sdk_machines = []
>           self.max_threads = params["max_threads"]
> -        self.all_layers = []
> +        self.layers_default = params["layer"].split()
>           self.core_base = params["core_base"]
>           self.image_names = []
>           self.image_addr = params["image_addr"]
> @@ -764,7 +764,7 @@ class Builder(gtk.Window):
>       def show_layer_selection_dialog(self):
>           dialog = LayerSelectionDialog(title = "Layers",
>                        layers = copy.deepcopy(self.configuration.layers),
> -                     all_layers = self.parameters.all_layers,
> +                     layers_default = self.parameters.layers_default,
>                        parent = self,
>                        flags = gtk.DIALOG_MODAL
>                            | gtk.DIALOG_DESTROY_WITH_PARENT
> diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
> index 9adb281..9a38bc0 100644
> --- a/lib/bb/ui/crumbs/hig.py
> +++ b/lib/bb/ui/crumbs/hig.py
> @@ -850,7 +850,7 @@ class LayerSelectionDialog (CrumbsDialog):
>               layer_store.remove(iter)
>
>
> -    def gen_layer_widget(self, layers, layers_avail, window, tooltip=""):
> +    def gen_layer_widget(self, layers, layers_default, window, tooltip=""):
>           hbox = gtk.HBox(False, 6)
>
>           layer_tv = gtk.TreeView()
> @@ -881,8 +881,8 @@ class LayerSelectionDialog (CrumbsDialog):
>           for layer in layers:
>               if layer.endswith("/meta"):
>                   core_iter = layer_store.prepend([layer])
> -            elif layer.endswith("/meta-hob") and core_iter:
> -                layer_store.insert_after(core_iter, [layer])
> +            elif layer in layers_default:
> +                core_iter = layer_store.insert_after(core_iter, [layer])
>               else:
>                   layer_store.append([layer])
>
> @@ -924,12 +924,12 @@ class LayerSelectionDialog (CrumbsDialog):
>       def add_leave_cb(self, button, event):
>           self.im.set_from_file(hic.ICON_INDI_ADD_FILE)
>
> -    def __init__(self, title, layers, all_layers, parent, flags, buttons=None):
> +    def __init__(self, title, layers, layers_default, parent, flags, buttons=None):
>           super(LayerSelectionDialog, self).__init__(title, parent, flags, buttons)
>
>           # class members from other objects
>           self.layers = layers
> -        self.all_layers = all_layers
> +        self.layers_default = layers_default
>           self.layers_changed = False
>
>           # icon for remove button in TreeView
> @@ -945,7 +945,7 @@ class LayerSelectionDialog (CrumbsDialog):
>           self.connect("response", self.response_cb)
>
>       def create_visual_elements(self):
> -        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.all_layers, self, None)
> +        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.layers_default, self, None)
>           layer_widget.set_size_request(450, 250)
>           self.vbox.pack_start(layer_widget, expand=True, fill=True)
>           self.show_all()
> @@ -972,9 +972,7 @@ class LayerSelectionDialog (CrumbsDialog):
>       def draw_delete_button_cb(self, col, cell, model, it, tv):
>           path =  model.get_value(it, 0)
>           # Trailing slashes are uncommon in bblayers.conf but confuse os.path.basename
> -        path.rstrip('/')
> -        name = os.path.basename(path)
> -        if name == "meta" or name == "meta-hob":
> +        if path in self.layers_default:
>               cell.set_sensitive(False)
>               cell.set_property('pixbuf', None)
>               cell.set_property('mode', gtk.CELL_RENDERER_MODE_INERT)
> @@ -992,11 +990,8 @@ class LayerSelectionDialog (CrumbsDialog):
>       """
>       def draw_layer_path_cb(self, col, cell, model, it):
>           path = model.get_value(it, 0)
> -        name = os.path.basename(path)
> -        if name == "meta":
> -            cell.set_property('markup', "<b>Core layer for images: it cannot be removed</b>\n%s" % path)
> -        elif name == "meta-hob":
> -            cell.set_property('markup', "<b>Core layer for Hob: it cannot be removed</b>\n%s" % path)
> +        if path in self.layers_default:
> +            cell.set_property('markup', "<b>Core layer: it cannot be removed</b>\n%s" % path)
>           else:
>               cell.set_property('text', path)
>

-- 
Joshua '贾詡' Lock
         Yocto Project "Johannes factotum"
         Intel Open Source Technology Centre

Re: [PATCH 4/7] Revert "lib/bb/ui/crumbs/hig: fix layers_changed test"

[Joshua Lock]

On 30/03/12 05:01, Dongxiao Xu wrote:
> We do not sort the layer list, therefore when response_cb, the ordering
> is also not needed.
>
> This reverts commit 71c1de347eef07a2059fc4544dc3cae090181f44.

This only makes sense if 3/7 is merged, correct?

Cheers,
Joshua

> ---
>   lib/bb/ui/crumbs/hig.py |    5 +----
>   1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
> index 9a38bc0..0ac4496 100644
> --- a/lib/bb/ui/crumbs/hig.py
> +++ b/lib/bb/ui/crumbs/hig.py
> @@ -958,10 +958,7 @@ class LayerSelectionDialog (CrumbsDialog):
>               layers.append(model.get_value(it, 0))
>               it = model.iter_next(it)
>
> -        orig_layers = sorted(self.layers)
> -        layers.sort()
> -
> -        self.layers_changed = (orig_layers != layers)
> +        self.layers_changed = (self.layers != layers)
>           self.layers = layers
>
>       """

-- 
Joshua '贾詡' Lock
         Yocto Project "Johannes factotum"
         Intel Open Source Technology Centre

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Xu, Dongxiao]

On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
> 
> On 30/03/12 05:01, Dongxiao Xu wrote:
> > For layers defined in bblayers.conf, we treat them as default layers
> > and users are not allowed to remove them.
> 
> Can you explain the rationale behind this change? I see what you're 
> doing but it's not entirely clear why.
> 
> I think this is a bad idea. Early on in the design of Hob we decided we 
> didn't want configuration made for non-Hob builds to affect builds made 
> with Hob, and vice versa.

The issue I am going to solve is that, with current local.conf
(DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we are
not able to delete the meta-yocto layer in Hob since it will meet
local.conf parsing error since it could not find where "poky" is
defined.

This patch is to set those layers define in bblayers.conf as default
layers and they could not be removed. For example, in Yocto Project,
"meta", "meta-yocto", and "meta-hob" are not removable. For pure OE-Core
environment, "meta" and "meta-hob" are not removable.

Thanks,
Dongxiao

> 
> Thanks,
> Joshua
> 
> >
> > Signed-off-by: Dongxiao Xu<dongxiao.xu@intel.com>
> > ---
> >   lib/bb/ui/crumbs/builder.py |    4 ++--
> >   lib/bb/ui/crumbs/hig.py     |   23 +++++++++--------------
> >   2 files changed, 11 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/bb/ui/crumbs/builder.py b/lib/bb/ui/crumbs/builder.py
> > index 44b208a..3be799c 100755
> > --- a/lib/bb/ui/crumbs/builder.py
> > +++ b/lib/bb/ui/crumbs/builder.py
> > @@ -178,7 +178,7 @@ class Parameters:
> >           self.all_distros = []
> >           self.all_sdk_machines = []
> >           self.max_threads = params["max_threads"]
> > -        self.all_layers = []
> > +        self.layers_default = params["layer"].split()
> >           self.core_base = params["core_base"]
> >           self.image_names = []
> >           self.image_addr = params["image_addr"]
> > @@ -764,7 +764,7 @@ class Builder(gtk.Window):
> >       def show_layer_selection_dialog(self):
> >           dialog = LayerSelectionDialog(title = "Layers",
> >                        layers = copy.deepcopy(self.configuration.layers),
> > -                     all_layers = self.parameters.all_layers,
> > +                     layers_default = self.parameters.layers_default,
> >                        parent = self,
> >                        flags = gtk.DIALOG_MODAL
> >                            | gtk.DIALOG_DESTROY_WITH_PARENT
> > diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
> > index 9adb281..9a38bc0 100644
> > --- a/lib/bb/ui/crumbs/hig.py
> > +++ b/lib/bb/ui/crumbs/hig.py
> > @@ -850,7 +850,7 @@ class LayerSelectionDialog (CrumbsDialog):
> >               layer_store.remove(iter)
> >
> >
> > -    def gen_layer_widget(self, layers, layers_avail, window, tooltip=""):
> > +    def gen_layer_widget(self, layers, layers_default, window, tooltip=""):
> >           hbox = gtk.HBox(False, 6)
> >
> >           layer_tv = gtk.TreeView()
> > @@ -881,8 +881,8 @@ class LayerSelectionDialog (CrumbsDialog):
> >           for layer in layers:
> >               if layer.endswith("/meta"):
> >                   core_iter = layer_store.prepend([layer])
> > -            elif layer.endswith("/meta-hob") and core_iter:
> > -                layer_store.insert_after(core_iter, [layer])
> > +            elif layer in layers_default:
> > +                core_iter = layer_store.insert_after(core_iter, [layer])
> >               else:
> >                   layer_store.append([layer])
> >
> > @@ -924,12 +924,12 @@ class LayerSelectionDialog (CrumbsDialog):
> >       def add_leave_cb(self, button, event):
> >           self.im.set_from_file(hic.ICON_INDI_ADD_FILE)
> >
> > -    def __init__(self, title, layers, all_layers, parent, flags, buttons=None):
> > +    def __init__(self, title, layers, layers_default, parent, flags, buttons=None):
> >           super(LayerSelectionDialog, self).__init__(title, parent, flags, buttons)
> >
> >           # class members from other objects
> >           self.layers = layers
> > -        self.all_layers = all_layers
> > +        self.layers_default = layers_default
> >           self.layers_changed = False
> >
> >           # icon for remove button in TreeView
> > @@ -945,7 +945,7 @@ class LayerSelectionDialog (CrumbsDialog):
> >           self.connect("response", self.response_cb)
> >
> >       def create_visual_elements(self):
> > -        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.all_layers, self, None)
> > +        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.layers_default, self, None)
> >           layer_widget.set_size_request(450, 250)
> >           self.vbox.pack_start(layer_widget, expand=True, fill=True)
> >           self.show_all()
> > @@ -972,9 +972,7 @@ class LayerSelectionDialog (CrumbsDialog):
> >       def draw_delete_button_cb(self, col, cell, model, it, tv):
> >           path =  model.get_value(it, 0)
> >           # Trailing slashes are uncommon in bblayers.conf but confuse os.path.basename
> > -        path.rstrip('/')
> > -        name = os.path.basename(path)
> > -        if name == "meta" or name == "meta-hob":
> > +        if path in self.layers_default:
> >               cell.set_sensitive(False)
> >               cell.set_property('pixbuf', None)
> >               cell.set_property('mode', gtk.CELL_RENDERER_MODE_INERT)
> > @@ -992,11 +990,8 @@ class LayerSelectionDialog (CrumbsDialog):
> >       """
> >       def draw_layer_path_cb(self, col, cell, model, it):
> >           path = model.get_value(it, 0)
> > -        name = os.path.basename(path)
> > -        if name == "meta":
> > -            cell.set_property('markup', "<b>Core layer for images: it cannot be removed</b>\n%s" % path)
> > -        elif name == "meta-hob":
> > -            cell.set_property('markup', "<b>Core layer for Hob: it cannot be removed</b>\n%s" % path)
> > +        if path in self.layers_default:
> > +            cell.set_property('markup', "<b>Core layer: it cannot be removed</b>\n%s" % path)
> >           else:
> >               cell.set_property('text', path)
> >
> 

Re: [PATCH 4/7] Revert "lib/bb/ui/crumbs/hig: fix layers_changed test"

[Xu, Dongxiao]

On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
> 
> On 30/03/12 05:01, Dongxiao Xu wrote:
> > We do not sort the layer list, therefore when response_cb, the ordering
> > is also not needed.
> >
> > This reverts commit 71c1de347eef07a2059fc4544dc3cae090181f44.
> 
> This only makes sense if 3/7 is merged, correct?

Yes, I should have made the comment more clear.

Thanks,
Dongxiao

> 
> Cheers,
> Joshua
> 
> > ---
> >   lib/bb/ui/crumbs/hig.py |    5 +----
> >   1 files changed, 1 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
> > index 9a38bc0..0ac4496 100644
> > --- a/lib/bb/ui/crumbs/hig.py
> > +++ b/lib/bb/ui/crumbs/hig.py
> > @@ -958,10 +958,7 @@ class LayerSelectionDialog (CrumbsDialog):
> >               layers.append(model.get_value(it, 0))
> >               it = model.iter_next(it)
> >
> > -        orig_layers = sorted(self.layers)
> > -        layers.sort()
> > -
> > -        self.layers_changed = (orig_layers != layers)
> > +        self.layers_changed = (self.layers != layers)
> >           self.layers = layers
> >
> >       """
> 

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Xu, Dongxiao]

On Sat, 2012-03-31 at 08:32 +0800, Xu, Dongxiao wrote:
> On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
> > 
> > On 30/03/12 05:01, Dongxiao Xu wrote:
> > > For layers defined in bblayers.conf, we treat them as default layers
> > > and users are not allowed to remove them.
> > 
> > Can you explain the rationale behind this change? I see what you're 
> > doing but it's not entirely clear why.
> > 
> > I think this is a bad idea. Early on in the design of Hob we decided we 
> > didn't want configuration made for non-Hob builds to affect builds made 
> > with Hob, and vice versa.
> 
> The issue I am going to solve is that, with current local.conf
> (DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we are
> not able to delete the meta-yocto layer in Hob since it will meet
> local.conf parsing error since it could not find where "poky" is
> defined.
> 
> This patch is to set those layers define in bblayers.conf as default
> layers and they could not be removed. For example, in Yocto Project,
> "meta", "meta-yocto", and "meta-hob" are not removable. For pure OE-Core
> environment, "meta" and "meta-hob" are not removable.
> 

Just discussed with Josh on this problem.

He suggested that meta-yocto should be still removable. If user met the
error after deleting the meta-yocto layer, he/she should change the
DISTRO setting in "Setting" dialog to "defaultsetup".

Current code to handle the distro setting is:

    def set_distro(self, distro):  
        if distro != "defaultsetup":   
            self.server.runCommand(["setVariable", "DISTRO", distro])

Therefore even if user has selected defaultsetup, it will not set any
value to bitbake server, and then DISTRO ?= "poky" will take effect
while parsing local.conf.

To make DISTRO ?= "poky" doesn't take effect, we need to change the code
to be:

    def set_distro(self, distro):  
        if distro == "defaultsetup":   
            distro = ""
        self.server.runCommand(["setVariable", "DISTRO", distro])

However Richard ever worried about this approach.
See:
http://lists.linuxtogo.org/pipermail/bitbake-devel/2012-March/002438.html

In summary, there are two solutions now:

1) Set those layers defined in bblayers.conf as default, and don't allow
users to delete them, like this [PATCH 3/7] does.
2) Use the empty string "" as the value of DISTRO variable for
"defaultsetup", and set this value before parsing configuration files,
making the DISTRO ?= "poky" in local.conf doesn't take effect.

Welcome for comments on this issue.

Thanks,
Dongxiao

> Thanks,
> Dongxiao
> 
> > 
> > Thanks,
> > Joshua
> > 
> > >
> > > Signed-off-by: Dongxiao Xu<dongxiao.xu@intel.com>
> > > ---
> > >   lib/bb/ui/crumbs/builder.py |    4 ++--
> > >   lib/bb/ui/crumbs/hig.py     |   23 +++++++++--------------
> > >   2 files changed, 11 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/lib/bb/ui/crumbs/builder.py b/lib/bb/ui/crumbs/builder.py
> > > index 44b208a..3be799c 100755
> > > --- a/lib/bb/ui/crumbs/builder.py
> > > +++ b/lib/bb/ui/crumbs/builder.py
> > > @@ -178,7 +178,7 @@ class Parameters:
> > >           self.all_distros = []
> > >           self.all_sdk_machines = []
> > >           self.max_threads = params["max_threads"]
> > > -        self.all_layers = []
> > > +        self.layers_default = params["layer"].split()
> > >           self.core_base = params["core_base"]
> > >           self.image_names = []
> > >           self.image_addr = params["image_addr"]
> > > @@ -764,7 +764,7 @@ class Builder(gtk.Window):
> > >       def show_layer_selection_dialog(self):
> > >           dialog = LayerSelectionDialog(title = "Layers",
> > >                        layers = copy.deepcopy(self.configuration.layers),
> > > -                     all_layers = self.parameters.all_layers,
> > > +                     layers_default = self.parameters.layers_default,
> > >                        parent = self,
> > >                        flags = gtk.DIALOG_MODAL
> > >                            | gtk.DIALOG_DESTROY_WITH_PARENT
> > > diff --git a/lib/bb/ui/crumbs/hig.py b/lib/bb/ui/crumbs/hig.py
> > > index 9adb281..9a38bc0 100644
> > > --- a/lib/bb/ui/crumbs/hig.py
> > > +++ b/lib/bb/ui/crumbs/hig.py
> > > @@ -850,7 +850,7 @@ class LayerSelectionDialog (CrumbsDialog):
> > >               layer_store.remove(iter)
> > >
> > >
> > > -    def gen_layer_widget(self, layers, layers_avail, window, tooltip=""):
> > > +    def gen_layer_widget(self, layers, layers_default, window, tooltip=""):
> > >           hbox = gtk.HBox(False, 6)
> > >
> > >           layer_tv = gtk.TreeView()
> > > @@ -881,8 +881,8 @@ class LayerSelectionDialog (CrumbsDialog):
> > >           for layer in layers:
> > >               if layer.endswith("/meta"):
> > >                   core_iter = layer_store.prepend([layer])
> > > -            elif layer.endswith("/meta-hob") and core_iter:
> > > -                layer_store.insert_after(core_iter, [layer])
> > > +            elif layer in layers_default:
> > > +                core_iter = layer_store.insert_after(core_iter, [layer])
> > >               else:
> > >                   layer_store.append([layer])
> > >
> > > @@ -924,12 +924,12 @@ class LayerSelectionDialog (CrumbsDialog):
> > >       def add_leave_cb(self, button, event):
> > >           self.im.set_from_file(hic.ICON_INDI_ADD_FILE)
> > >
> > > -    def __init__(self, title, layers, all_layers, parent, flags, buttons=None):
> > > +    def __init__(self, title, layers, layers_default, parent, flags, buttons=None):
> > >           super(LayerSelectionDialog, self).__init__(title, parent, flags, buttons)
> > >
> > >           # class members from other objects
> > >           self.layers = layers
> > > -        self.all_layers = all_layers
> > > +        self.layers_default = layers_default
> > >           self.layers_changed = False
> > >
> > >           # icon for remove button in TreeView
> > > @@ -945,7 +945,7 @@ class LayerSelectionDialog (CrumbsDialog):
> > >           self.connect("response", self.response_cb)
> > >
> > >       def create_visual_elements(self):
> > > -        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.all_layers, self, None)
> > > +        layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.layers_default, self, None)
> > >           layer_widget.set_size_request(450, 250)
> > >           self.vbox.pack_start(layer_widget, expand=True, fill=True)
> > >           self.show_all()
> > > @@ -972,9 +972,7 @@ class LayerSelectionDialog (CrumbsDialog):
> > >       def draw_delete_button_cb(self, col, cell, model, it, tv):
> > >           path =  model.get_value(it, 0)
> > >           # Trailing slashes are uncommon in bblayers.conf but confuse os.path.basename
> > > -        path.rstrip('/')
> > > -        name = os.path.basename(path)
> > > -        if name == "meta" or name == "meta-hob":
> > > +        if path in self.layers_default:
> > >               cell.set_sensitive(False)
> > >               cell.set_property('pixbuf', None)
> > >               cell.set_property('mode', gtk.CELL_RENDERER_MODE_INERT)
> > > @@ -992,11 +990,8 @@ class LayerSelectionDialog (CrumbsDialog):
> > >       """
> > >       def draw_layer_path_cb(self, col, cell, model, it):
> > >           path = model.get_value(it, 0)
> > > -        name = os.path.basename(path)
> > > -        if name == "meta":
> > > -            cell.set_property('markup', "<b>Core layer for images: it cannot be removed</b>\n%s" % path)
> > > -        elif name == "meta-hob":
> > > -            cell.set_property('markup', "<b>Core layer for Hob: it cannot be removed</b>\n%s" % path)
> > > +        if path in self.layers_default:
> > > +            cell.set_property('markup', "<b>Core layer: it cannot be removed</b>\n%s" % path)
> > >           else:
> > >               cell.set_property('text', path)
> > >
> > 
> 
> 
> 
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/bitbake-devel

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Richard Purdie]

On Sat, 2012-03-31 at 13:29 +0800, Xu, Dongxiao wrote:
> On Sat, 2012-03-31 at 08:32 +0800, Xu, Dongxiao wrote:
> > On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
> > > 
> > > On 30/03/12 05:01, Dongxiao Xu wrote:
> > > > For layers defined in bblayers.conf, we treat them as default layers
> > > > and users are not allowed to remove them.
> > > 
> > > Can you explain the rationale behind this change? I see what you're 
> > > doing but it's not entirely clear why.
> > > 
> > > I think this is a bad idea. Early on in the design of Hob we decided we 
> > > didn't want configuration made for non-Hob builds to affect builds made 
> > > with Hob, and vice versa.
> > 
> > The issue I am going to solve is that, with current local.conf
> > (DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we are
> > not able to delete the meta-yocto layer in Hob since it will meet
> > local.conf parsing error since it could not find where "poky" is
> > defined.
> > 
> > This patch is to set those layers define in bblayers.conf as default
> > layers and they could not be removed. For example, in Yocto Project,
> > "meta", "meta-yocto", and "meta-hob" are not removable. For pure OE-Core
> > environment, "meta" and "meta-hob" are not removable.
> > 
> 
> Just discussed with Josh on this problem.
> 
> He suggested that meta-yocto should be still removable. If user met the
> error after deleting the meta-yocto layer, he/she should change the
> DISTRO setting in "Setting" dialog to "defaultsetup".
> 
> Current code to handle the distro setting is:
> 
>     def set_distro(self, distro):  
>         if distro != "defaultsetup":   
>             self.server.runCommand(["setVariable", "DISTRO", distro])
> 
> Therefore even if user has selected defaultsetup, it will not set any
> value to bitbake server, and then DISTRO ?= "poky" will take effect
> while parsing local.conf.
> 
> To make DISTRO ?= "poky" doesn't take effect, we need to change the code
> to be:
> 
>     def set_distro(self, distro):  
>         if distro == "defaultsetup":   
>             distro = ""
>         self.server.runCommand(["setVariable", "DISTRO", distro])
> 
> However Richard ever worried about this approach.
> See:
> http://lists.linuxtogo.org/pipermail/bitbake-devel/2012-March/002438.html
> 
> In summary, there are two solutions now:
> 
> 1) Set those layers defined in bblayers.conf as default, and don't allow
> users to delete them, like this [PATCH 3/7] does.
> 2) Use the empty string "" as the value of DISTRO variable for
> "defaultsetup", and set this value before parsing configuration files,
> making the DISTRO ?= "poky" in local.conf doesn't take effect.
> 
> Welcome for comments on this issue.

I'd be happy if you change the above code to do:

        self.server.runCommand(["deleteVariable", "DISTRO"])

which is subtly different but consistent with what we really want. I
have no idea if we have a deleteVariable command but if we don't, we
should add one.

Cheers,

Richard

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Xu, Dongxiao]

On Sat, 2012-03-31 at 16:51 +0100, Richard Purdie wrote:
> On Sat, 2012-03-31 at 13:29 +0800, Xu, Dongxiao wrote:
> > On Sat, 2012-03-31 at 08:32 +0800, Xu, Dongxiao wrote:
> > > On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
> > > > 
> > > > On 30/03/12 05:01, Dongxiao Xu wrote:
> > > > > For layers defined in bblayers.conf, we treat them as default layers
> > > > > and users are not allowed to remove them.
> > > > 
> > > > Can you explain the rationale behind this change? I see what you're 
> > > > doing but it's not entirely clear why.
> > > > 
> > > > I think this is a bad idea. Early on in the design of Hob we decided we 
> > > > didn't want configuration made for non-Hob builds to affect builds made 
> > > > with Hob, and vice versa.
> > > 
> > > The issue I am going to solve is that, with current local.conf
> > > (DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we are
> > > not able to delete the meta-yocto layer in Hob since it will meet
> > > local.conf parsing error since it could not find where "poky" is
> > > defined.
> > > 
> > > This patch is to set those layers define in bblayers.conf as default
> > > layers and they could not be removed. For example, in Yocto Project,
> > > "meta", "meta-yocto", and "meta-hob" are not removable. For pure OE-Core
> > > environment, "meta" and "meta-hob" are not removable.
> > > 
> > 
> > Just discussed with Josh on this problem.
> > 
> > He suggested that meta-yocto should be still removable. If user met the
> > error after deleting the meta-yocto layer, he/she should change the
> > DISTRO setting in "Setting" dialog to "defaultsetup".
> > 
> > Current code to handle the distro setting is:
> > 
> >     def set_distro(self, distro):  
> >         if distro != "defaultsetup":   
> >             self.server.runCommand(["setVariable", "DISTRO", distro])
> > 
> > Therefore even if user has selected defaultsetup, it will not set any
> > value to bitbake server, and then DISTRO ?= "poky" will take effect
> > while parsing local.conf.
> > 
> > To make DISTRO ?= "poky" doesn't take effect, we need to change the code
> > to be:
> > 
> >     def set_distro(self, distro):  
> >         if distro == "defaultsetup":   
> >             distro = ""
> >         self.server.runCommand(["setVariable", "DISTRO", distro])
> > 
> > However Richard ever worried about this approach.
> > See:
> > http://lists.linuxtogo.org/pipermail/bitbake-devel/2012-March/002438.html
> > 
> > In summary, there are two solutions now:
> > 
> > 1) Set those layers defined in bblayers.conf as default, and don't allow
> > users to delete them, like this [PATCH 3/7] does.
> > 2) Use the empty string "" as the value of DISTRO variable for
> > "defaultsetup", and set this value before parsing configuration files,
> > making the DISTRO ?= "poky" in local.conf doesn't take effect.
> > 
> > Welcome for comments on this issue.
> 
> I'd be happy if you change the above code to do:
> 
>         self.server.runCommand(["deleteVariable", "DISTRO"])
> 
> which is subtly different but consistent with what we really want. I
> have no idea if we have a deleteVariable command but if we don't, we
> should add one.

I ever tried this approach with the following patch, however it didn't
work. The setting of DISTRO ?= "poky" will still take effect.

It seems that delVar(DISTRO) is different from setting DISTRO="".


diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
index 73aaca0..a7b41ce 100644
--- a/bitbake/lib/bb/command.py
+++ b/bitbake/lib/bb/command.py
@@ -160,6 +160,13 @@ class CommandsSync:
         value = params[1]
         command.cooker.configuration.data.setVar(varname, value)

+    def delVariable(self, command, params):
+        """
+        Delete the variable in configuration.data
+        """
+        varname = params[0]
+        command.cooker.configuration.data.delVar(varname)
+
     def initCooker(self, command, params):
         """
         Init the cooker to initial state with nothing parsed
diff --git a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
index 8909e01..d236a72 100644
--- a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
+++ b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
@@ -265,7 +265,9 @@ class HobHandler(gobject.GObject):
         self.server.runCommand(["setVariable", "IMAGE_FSTYPES",
image_fstypes])

     def set_distro(self, distro):
-        if distro != "defaultsetup":
+        if distro == "defaultsetup":
+            self.server.runCommand(["delVariable", "DISTRO"])
+        else:
             self.server.runCommand(["setVariable", "DISTRO", distro])

     def set_package_format(self, format):

Thanks,
Dongxiao
> 
> Cheers,
> 
> Richard
> 

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Richard Purdie]

On Sun, 2012-04-01 at 13:35 +0800, Xu, Dongxiao wrote:
> On Sat, 2012-03-31 at 16:51 +0100, Richard Purdie wrote:
> > I'd be happy if you change the above code to do:
> > 
> >         self.server.runCommand(["deleteVariable", "DISTRO"])
> > 
> > which is subtly different but consistent with what we really want. I
> > have no idea if we have a deleteVariable command but if we don't, we
> > should add one.
> 
> I ever tried this approach with the following patch, however it didn't
> work. The setting of DISTRO ?= "poky" will still take effect.
>
> It seems that delVar(DISTRO) is different from setting DISTRO="".

It is different but it is what setting up OE-Core without DISTRO set
does and that is known to work.

We need to understand why this isn't working as it should...

Cheers,

Richard


> 
> diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
> index 73aaca0..a7b41ce 100644
> --- a/bitbake/lib/bb/command.py
> +++ b/bitbake/lib/bb/command.py
> @@ -160,6 +160,13 @@ class CommandsSync:
>          value = params[1]
>          command.cooker.configuration.data.setVar(varname, value)
> 
> +    def delVariable(self, command, params):
> +        """
> +        Delete the variable in configuration.data
> +        """
> +        varname = params[0]
> +        command.cooker.configuration.data.delVar(varname)
> +
>      def initCooker(self, command, params):
>          """
>          Init the cooker to initial state with nothing parsed
> diff --git a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> index 8909e01..d236a72 100644
> --- a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> +++ b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> @@ -265,7 +265,9 @@ class HobHandler(gobject.GObject):
>          self.server.runCommand(["setVariable", "IMAGE_FSTYPES",
> image_fstypes])
> 
>      def set_distro(self, distro):
> -        if distro != "defaultsetup":
> +        if distro == "defaultsetup":
> +            self.server.runCommand(["delVariable", "DISTRO"])
> +        else:
>              self.server.runCommand(["setVariable", "DISTRO", distro])
> 
>      def set_package_format(self, format):

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Richard Purdie]

On Sun, 2012-04-01 at 13:35 +0800, Xu, Dongxiao wrote:
> On Sat, 2012-03-31 at 16:51 +0100, Richard Purdie wrote:
> > On Sat, 2012-03-31 at 13:29 +0800, Xu, Dongxiao wrote:
> > > On Sat, 2012-03-31 at 08:32 +0800, Xu, Dongxiao wrote:
> > > > On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
> > > > > 
> > > > > On 30/03/12 05:01, Dongxiao Xu wrote:
> > > > > > For layers defined in bblayers.conf, we treat them as default layers
> > > > > > and users are not allowed to remove them.
> > > > > 
> > > > > Can you explain the rationale behind this change? I see what you're 
> > > > > doing but it's not entirely clear why.
> > > > > 
> > > > > I think this is a bad idea. Early on in the design of Hob we decided we 
> > > > > didn't want configuration made for non-Hob builds to affect builds made 
> > > > > with Hob, and vice versa.
> > > > 
> > > > The issue I am going to solve is that, with current local.conf
> > > > (DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we are
> > > > not able to delete the meta-yocto layer in Hob since it will meet
> > > > local.conf parsing error since it could not find where "poky" is
> > > > defined.
> > > > 
> > > > This patch is to set those layers define in bblayers.conf as default
> > > > layers and they could not be removed. For example, in Yocto Project,
> > > > "meta", "meta-yocto", and "meta-hob" are not removable. For pure OE-Core
> > > > environment, "meta" and "meta-hob" are not removable.
> > > > 
> > > 
> > > Just discussed with Josh on this problem.
> > > 
> > > He suggested that meta-yocto should be still removable. If user met the
> > > error after deleting the meta-yocto layer, he/she should change the
> > > DISTRO setting in "Setting" dialog to "defaultsetup".
> > > 
> > > Current code to handle the distro setting is:
> > > 
> > >     def set_distro(self, distro):  
> > >         if distro != "defaultsetup":   
> > >             self.server.runCommand(["setVariable", "DISTRO", distro])
> > > 
> > > Therefore even if user has selected defaultsetup, it will not set any
> > > value to bitbake server, and then DISTRO ?= "poky" will take effect
> > > while parsing local.conf.
> > > 
> > > To make DISTRO ?= "poky" doesn't take effect, we need to change the code
> > > to be:
> > > 
> > >     def set_distro(self, distro):  
> > >         if distro == "defaultsetup":   
> > >             distro = ""
> > >         self.server.runCommand(["setVariable", "DISTRO", distro])
> > > 
> > > However Richard ever worried about this approach.
> > > See:
> > > http://lists.linuxtogo.org/pipermail/bitbake-devel/2012-March/002438.html
> > > 
> > > In summary, there are two solutions now:
> > > 
> > > 1) Set those layers defined in bblayers.conf as default, and don't allow
> > > users to delete them, like this [PATCH 3/7] does.
> > > 2) Use the empty string "" as the value of DISTRO variable for
> > > "defaultsetup", and set this value before parsing configuration files,
> > > making the DISTRO ?= "poky" in local.conf doesn't take effect.
> > > 
> > > Welcome for comments on this issue.
> > 
> > I'd be happy if you change the above code to do:
> > 
> >         self.server.runCommand(["deleteVariable", "DISTRO"])
> > 
> > which is subtly different but consistent with what we really want. I
> > have no idea if we have a deleteVariable command but if we don't, we
> > should add one.
> 
> I ever tried this approach with the following patch, however it didn't
> work. The setting of DISTRO ?= "poky" will still take effect.
> 
> It seems that delVar(DISTRO) is different from setting DISTRO="".
> 
> 
> diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
> index 73aaca0..a7b41ce 100644
> --- a/bitbake/lib/bb/command.py
> +++ b/bitbake/lib/bb/command.py
> @@ -160,6 +160,13 @@ class CommandsSync:
>          value = params[1]
>          command.cooker.configuration.data.setVar(varname, value)
> 
> +    def delVariable(self, command, params):
> +        """
> +        Delete the variable in configuration.data
> +        """
> +        varname = params[0]
> +        command.cooker.configuration.data.delVar(varname)
> +
>      def initCooker(self, command, params):
>          """
>          Init the cooker to initial state with nothing parsed
> diff --git a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> index 8909e01..d236a72 100644
> --- a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> +++ b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
> @@ -265,7 +265,9 @@ class HobHandler(gobject.GObject):
>          self.server.runCommand(["setVariable", "IMAGE_FSTYPES",
> image_fstypes])
> 
>      def set_distro(self, distro):
> -        if distro != "defaultsetup":
> +        if distro == "defaultsetup":
> +            self.server.runCommand(["delVariable", "DISTRO"])
> +        else:
>              self.server.runCommand(["setVariable", "DISTRO", distro])
> 
>      def set_package_format(self, format):

Thinking more about this, I think I can see a pattern in the issues I'm
seeing patches for from you and Shane.

I think you have things setup such that you do:

a) Set variables
b) Parse configuration

And the problem is that if anywhere in the configuration, something like
A = "B" happens, the value of A is overridden if you set it in A. This
also would apply if you run delVar, then the code does a ?= since it
will get reset.

The trouble is you have to do this since you need to set MACHINE and
DISTRO in advance.

I'm starting to think you may have to have hob have its own empty
"local.conf" which overrides the user provided one. You would parse the
user's local.conf separately and extract any settings into hob as needed
though (pass through all values except the ones you know need specific
values from the UI).

Its ugly but I can't see any better way to handle this situation that
will make sense to the user and give the correct user experience. As I
said to Shane elsewhere, we cannot force the local.conf file to be
written in a certain way, just to suit the requirements of hob.

Cheers,

Richard

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Joshua Lock]

On 02/04/12 05:55, Richard Purdie wrote:
> On Sun, 2012-04-01 at 13:35 +0800, Xu, Dongxiao wrote:
>> On Sat, 2012-03-31 at 16:51 +0100, Richard Purdie wrote:
>>> On Sat, 2012-03-31 at 13:29 +0800, Xu, Dongxiao wrote:
>>>> On Sat, 2012-03-31 at 08:32 +0800, Xu, Dongxiao wrote:
>>>>> On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
>>>>>>
>>>>>> On 30/03/12 05:01, Dongxiao Xu wrote:
>>>>>>> For layers defined in bblayers.conf, we treat them as default layers
>>>>>>> and users are not allowed to remove them.
>>>>>>
>>>>>> Can you explain the rationale behind this change? I see what you're
>>>>>> doing but it's not entirely clear why.
>>>>>>
>>>>>> I think this is a bad idea. Early on in the design of Hob we decided we
>>>>>> didn't want configuration made for non-Hob builds to affect builds made
>>>>>> with Hob, and vice versa.
>>>>>
>>>>> The issue I am going to solve is that, with current local.conf
>>>>> (DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we are
>>>>> not able to delete the meta-yocto layer in Hob since it will meet
>>>>> local.conf parsing error since it could not find where "poky" is
>>>>> defined.
>>>>>
>>>>> This patch is to set those layers define in bblayers.conf as default
>>>>> layers and they could not be removed. For example, in Yocto Project,
>>>>> "meta", "meta-yocto", and "meta-hob" are not removable. For pure OE-Core
>>>>> environment, "meta" and "meta-hob" are not removable.
>>>>>
>>>>
>>>> Just discussed with Josh on this problem.
>>>>
>>>> He suggested that meta-yocto should be still removable. If user met the
>>>> error after deleting the meta-yocto layer, he/she should change the
>>>> DISTRO setting in "Setting" dialog to "defaultsetup".
>>>>
>>>> Current code to handle the distro setting is:
>>>>
>>>>      def set_distro(self, distro):
>>>>          if distro != "defaultsetup":
>>>>              self.server.runCommand(["setVariable", "DISTRO", distro])
>>>>
>>>> Therefore even if user has selected defaultsetup, it will not set any
>>>> value to bitbake server, and then DISTRO ?= "poky" will take effect
>>>> while parsing local.conf.
>>>>
>>>> To make DISTRO ?= "poky" doesn't take effect, we need to change the code
>>>> to be:
>>>>
>>>>      def set_distro(self, distro):
>>>>          if distro == "defaultsetup":
>>>>              distro = ""
>>>>          self.server.runCommand(["setVariable", "DISTRO", distro])
>>>>
>>>> However Richard ever worried about this approach.
>>>> See:
>>>> http://lists.linuxtogo.org/pipermail/bitbake-devel/2012-March/002438.html
>>>>
>>>> In summary, there are two solutions now:
>>>>
>>>> 1) Set those layers defined in bblayers.conf as default, and don't allow
>>>> users to delete them, like this [PATCH 3/7] does.
>>>> 2) Use the empty string "" as the value of DISTRO variable for
>>>> "defaultsetup", and set this value before parsing configuration files,
>>>> making the DISTRO ?= "poky" in local.conf doesn't take effect.
>>>>
>>>> Welcome for comments on this issue.
>>>
>>> I'd be happy if you change the above code to do:
>>>
>>>          self.server.runCommand(["deleteVariable", "DISTRO"])
>>>
>>> which is subtly different but consistent with what we really want. I
>>> have no idea if we have a deleteVariable command but if we don't, we
>>> should add one.
>>
>> I ever tried this approach with the following patch, however it didn't
>> work. The setting of DISTRO ?= "poky" will still take effect.
>>
>> It seems that delVar(DISTRO) is different from setting DISTRO="".
>>
>>
>> diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
>> index 73aaca0..a7b41ce 100644
>> --- a/bitbake/lib/bb/command.py
>> +++ b/bitbake/lib/bb/command.py
>> @@ -160,6 +160,13 @@ class CommandsSync:
>>           value = params[1]
>>           command.cooker.configuration.data.setVar(varname, value)
>>
>> +    def delVariable(self, command, params):
>> +        """
>> +        Delete the variable in configuration.data
>> +        """
>> +        varname = params[0]
>> +        command.cooker.configuration.data.delVar(varname)
>> +
>>       def initCooker(self, command, params):
>>           """
>>           Init the cooker to initial state with nothing parsed
>> diff --git a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>> b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>> index 8909e01..d236a72 100644
>> --- a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>> +++ b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>> @@ -265,7 +265,9 @@ class HobHandler(gobject.GObject):
>>           self.server.runCommand(["setVariable", "IMAGE_FSTYPES",
>> image_fstypes])
>>
>>       def set_distro(self, distro):
>> -        if distro != "defaultsetup":
>> +        if distro == "defaultsetup":
>> +            self.server.runCommand(["delVariable", "DISTRO"])
>> +        else:
>>               self.server.runCommand(["setVariable", "DISTRO", distro])
>>
>>       def set_package_format(self, format):
>
> Thinking more about this, I think I can see a pattern in the issues I'm
> seeing patches for from you and Shane.
>
> I think you have things setup such that you do:
>
> a) Set variables
> b) Parse configuration
>
> And the problem is that if anywhere in the configuration, something like
> A = "B" happens, the value of A is overridden if you set it in A. This
> also would apply if you run delVar, then the code does a ?= since it
> will get reset.
>
> The trouble is you have to do this since you need to set MACHINE and
> DISTRO in advance.
>
> I'm starting to think you may have to have hob have its own empty
> "local.conf" which overrides the user provided one. You would parse the
> user's local.conf separately and extract any settings into hob as needed
> though (pass through all values except the ones you know need specific
> values from the UI).
>
> Its ugly but I can't see any better way to handle this situation that
> will make sense to the user and give the correct user experience. As I
> said to Shane elsewhere, we cannot force the local.conf file to be
> written in a certain way, just to suit the requirements of hob.

I wrote a quick hack this afternoon to do "settings import" from a users 
local.conf and whilst it is not a great solution it did spark some 
real-time conversation between Richard and I.

In an attempt at summary, it should be possible to have Hob:
* call bb.parse.handle() to load the user's settings from local.conf
* add appropriate values to the data store based on the parsed 
local.conf values and any appropriate hob settings
* parse as normal to fully populate the data store

Cheers,
Joshua
-- 
Joshua '贾詡' Lock
         Yocto Project "Johannes factotum"
         Intel Open Source Technology Centre

Re: [PATCH 3/7] Hob: Make layers define in bblayers.conf as default

[Wang, Shane]

Is that in Josh's hand or Dongxiao's hand?
There is a bug https://bugzilla.yoctoproject.org/show_bug.cgi?id=2254 reported by QA, which is related to this issue.
I assigned to Josh temporarily. Please assign to the right owner if I am wrong.

--
Shane

Joshua Lock wrote on 2012-04-04:

> 
> 
> On 02/04/12 05:55, Richard Purdie wrote:
>> On Sun, 2012-04-01 at 13:35 +0800, Xu, Dongxiao wrote:
>>> On Sat, 2012-03-31 at 16:51 +0100, Richard Purdie wrote:
>>>> On Sat, 2012-03-31 at 13:29 +0800, Xu, Dongxiao wrote:
>>>>> On Sat, 2012-03-31 at 08:32 +0800, Xu, Dongxiao wrote:
>>>>>> On Fri, 2012-03-30 at 09:10 -0700, Joshua Lock wrote:
>>>>>>> 
>>>>>>> On 30/03/12 05:01, Dongxiao Xu wrote:
>>>>>>>> For layers defined in bblayers.conf, we treat them as default layers
>>>>>>>> and users are not allowed to remove them.
>>>>>>> 
>>>>>>> Can you explain the rationale behind this change? I see what you're
>>>>>>> doing but it's not entirely clear why.
>>>>>>> 
>>>>>>> I think this is a bad idea. Early on in the design of Hob we
>>>>>>> decided we didn't want configuration made for non-Hob builds to
>>>>>>> affect builds made with Hob, and vice versa.
>>>>>> 
>>>>>> The issue I am going to solve is that, with current local.conf
>>>>>> (DISTRO="poky") and bblayers.conf (bblayers="meta meta-yocto"), we
>>>>>> are not able to delete the meta-yocto layer in Hob since it will
>>>>>> meet local.conf parsing error since it could not find where "poky"
>>>>>> is defined.
>>>>>> 
>>>>>> This patch is to set those layers define in bblayers.conf as
>>>>>> default layers and they could not be removed. For example, in Yocto
>>>>>> Project, "meta", "meta-yocto", and "meta-hob" are not removable.
>>>>>> For pure OE-Core environment, "meta" and "meta-hob" are not
>>>>>> removable.
>>>>>> 
>>>>> 
>>>>> Just discussed with Josh on this problem.
>>>>> 
>>>>> He suggested that meta-yocto should be still removable. If user met the
>>>>> error after deleting the meta-yocto layer, he/she should change the
>>>>> DISTRO setting in "Setting" dialog to "defaultsetup".
>>>>> 
>>>>> Current code to handle the distro setting is:
>>>>> 
>>>>>      def set_distro(self, distro):
>>>>>          if distro != "defaultsetup":
>>>>>              self.server.runCommand(["setVariable", "DISTRO",
> distro])
>>>>> 
>>>>> Therefore even if user has selected defaultsetup, it will not set any
>>>>> value to bitbake server, and then DISTRO ?= "poky" will take effect
>>>>> while parsing local.conf.
>>>>> 
>>>>> To make DISTRO ?= "poky" doesn't take effect, we need to change the
>>>>> code to be:
>>>>> 
>>>>>      def set_distro(self, distro):
>>>>>          if distro == "defaultsetup":
>>>>>              distro = ""
>>>>>          self.server.runCommand(["setVariable", "DISTRO", distro])
>>>>> However Richard ever worried about this approach.
>>>>> See:
>>>>> 
>>>>> http://lists.linuxtogo.org/pipermail/bitbake-devel/2012-March/002438.
>>>>> html
>>>>> 
>>>>> In summary, there are two solutions now:
>>>>> 
>>>>> 1) Set those layers defined in bblayers.conf as default, and don't allow
>>>>> users to delete them, like this [PATCH 3/7] does.
>>>>> 2) Use the empty string "" as the value of DISTRO variable for
>>>>> "defaultsetup", and set this value before parsing configuration files,
>>>>> making the DISTRO ?= "poky" in local.conf doesn't take effect.
>>>>> 
>>>>> Welcome for comments on this issue.
>>>> 
>>>> I'd be happy if you change the above code to do:
>>>> 
>>>>          self.server.runCommand(["deleteVariable", "DISTRO"])
>>>> which is subtly different but consistent with what we really want. I
>>>> have no idea if we have a deleteVariable command but if we don't, we
>>>> should add one.
>>> 
>>> I ever tried this approach with the following patch, however it didn't
>>> work. The setting of DISTRO ?= "poky" will still take effect.
>>> 
>>> It seems that delVar(DISTRO) is different from setting DISTRO="".
>>> 
>>> 
>>> diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
>>> index 73aaca0..a7b41ce 100644
>>> --- a/bitbake/lib/bb/command.py
>>> +++ b/bitbake/lib/bb/command.py
>>> @@ -160,6 +160,13 @@ class CommandsSync:
>>>           value = params[1]
>>>           command.cooker.configuration.data.setVar(varname, value)
>>> +    def delVariable(self, command, params):
>>> +        """
>>> +        Delete the variable in configuration.data
>>> +        """
>>> +        varname = params[0]
>>> +        command.cooker.configuration.data.delVar(varname)
>>> +
>>>       def initCooker(self, command, params):
>>>           """
>>>           Init the cooker to initial state with nothing parsed
>>> diff --git a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>>> b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>>> index 8909e01..d236a72 100644
>>> --- a/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>>> +++ b/bitbake/lib/bb/ui/crumbs/hobeventhandler.py
>>> @@ -265,7 +265,9 @@ class HobHandler(gobject.GObject):
>>>           self.server.runCommand(["setVariable", "IMAGE_FSTYPES",
>>> image_fstypes])
>>> 
>>>       def set_distro(self, distro):
>>> -        if distro != "defaultsetup":
>>> +        if distro == "defaultsetup":
>>> +            self.server.runCommand(["delVariable", "DISTRO"])
>>> +        else:
>>>               self.server.runCommand(["setVariable", "DISTRO",
> distro])
>>> 
>>>       def set_package_format(self, format):
>> 
>> Thinking more about this, I think I can see a pattern in the issues I'm
>> seeing patches for from you and Shane.
>> 
>> I think you have things setup such that you do:
>> 
>> a) Set variables
>> b) Parse configuration
>> 
>> And the problem is that if anywhere in the configuration, something like
>> A = "B" happens, the value of A is overridden if you set it in A. This
>> also would apply if you run delVar, then the code does a ?= since it
>> will get reset.
>> 
>> The trouble is you have to do this since you need to set MACHINE and
>> DISTRO in advance.
>> 
>> I'm starting to think you may have to have hob have its own empty
>> "local.conf" which overrides the user provided one. You would parse the
>> user's local.conf separately and extract any settings into hob as needed
>> though (pass through all values except the ones you know need specific
>> values from the UI).
>> 
>> Its ugly but I can't see any better way to handle this situation that
>> will make sense to the user and give the correct user experience. As I
>> said to Shane elsewhere, we cannot force the local.conf file to be
>> written in a certain way, just to suit the requirements of hob.
> 
> I wrote a quick hack this afternoon to do "settings import" from a users
> local.conf and whilst it is not a great solution it did spark some
> real-time conversation between Richard and I.
> 
> In an attempt at summary, it should be possible to have Hob:
> * call bb.parse.handle() to load the user's settings from local.conf
> * add appropriate values to the data store based on the parsed
> local.conf values and any appropriate hob settings
> * parse as normal to fully populate the data store
> 
> Cheers,
> Joshua