Skip to contentSkip to conversation

Bug fixing and code clarity

PATCH In review Joshua Lock 2 participants 8 messages Aug 11, 2011 – Aug 12, 2011 Arm STM32

lore
Share & tools
lore

Joshua Lock

author
#0 lore raw

cover Bug fixing and code clarity

The following series addresses some thinko/typo's I'd introduced to hob, by using variable names which are too similar, as well as adding some changes to increase clarity in various places.

Regards,

Joshua

The following changes since commit 705d14d1e1108e0544c7eab827f1242f0839add9:

bb/cooker: only emit ConfigFilePathFound for files which were parsed (2011-08-10 13:38:11 +0100)

are available in the git repository at:
git://github.com/incandescant/bitbake hob
https://github.com/incandescant/bitbake/tree/hob

Joshua Lock (4):
bb/ui/crumbs/tasklistmodel: fix some typos and add comments to mark() bb/ui/crumbs/tasklistmodel: correctly uniquify dependency list bb/ui/crumbs/tasklistmodel: don't include an item in its own depends bb/cache: rename confusing variable

 lib/bb/cache.py                   |    6 +++---
 lib/bb/ui/crumbs/tasklistmodel.py |   28 ++++++++++++++++++++--------
 2 files changed, 23 insertions(+), 11 deletions(-)
Signature · 1 line
1.7.6
author
#1 lore raw

PATCH 1/4 bb/ui/crumbs/tasklistmodel: fix some typos and add comments to mark()

Two similarly named variables in the mark() method resulted in the wrong variable being used in a couple of places. This patch adresses this in several ways:
1) Renames the variables to be less similar
2) Uses the correct variables
3) Adds some coments to document the methods intent

Partially addresses [YOCTO #1355]

Signed-off-by Joshua Lock <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +15 -7
index 3e09757..96814c2 100644
@@ -316,9 +316,13 @@ class TaskListModel(gtk.ListStore):
     def mark(self, opath):
         usersel = {}
         removed = []
+
         it = self.get_iter_first()
-        name = self[opath][self.COL_NAME]
+        # The name of the item we're removing, so that we can use it to find
+        # other items which either depend on it, or were brought in by it
+        marked_name = self[opath][self.COL_NAME]
 
+        # Remove the passed item
         self.remove_item_path(opath)
 
         # Remove all dependent packages, update binb

@@ -330,7 +334,7 @@ class TaskListModel(gtk.ListStore):
             deps = self[path][self.COL_DEPS]
             binb = self[path][self.COL_BINB]
             itype = self[path][self.COL_TYPE]
-            iname = self[path][self.COL_NAME]
+            itname = self[path][self.COL_NAME]
 
             # We ignore anything that isn't a package
             if not itype == "package":

@@ -341,16 +345,20 @@ class TaskListModel(gtk.ListStore):
             # is to save its name and re-mark it for inclusion once dependency
             # processing is complete
             if binb == "User Selected":
-                usersel[iname] = self[path][self.COL_IMG]
+                usersel[itname] = self[path][self.COL_IMG]
 
+            # If the iterated item is included and depends on the removed
+            # item it should also be removed.
             # FIXME: need to ensure partial name matching doesn't happen
-            if inc and deps.count(name) and name not in removed:
+            if inc and deps.count(marked_name) and itname not in removed:
                 # found a dependency, remove it
-                removed.append(name)
+                removed.append(itname)
                 self.mark(path)
 
-            if inc and binb.count(name):
-                bib = self.find_alt_dependency(name)
+            # If the iterated item was brought in by the removed (passed) item
+            # try and find an alternative dependee and update the binb column
+            if inc and binb.count(marked_name):
+                bib = self.find_alt_dependency(itname)
                 self[path][self.COL_BINB] = bib
 
         # Re-add any removed user selected items
Signature · 1 line
1.7.6
author
#2 lore raw

PATCH 2/4 bb/ui/crumbs/tasklistmodel: correctly uniquify dependency list

In reply to Joshua Lock

Fix thinko - the squish method returns a uniquified list, it doesn't modify the list in place. Therefore the call to squish() was useless as its return value was never assigned.

Signed-off-by Joshua Lock <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +2 -1
index 96814c2..aec80e2 100644
@@ -235,7 +235,8 @@ class TaskListModel(gtk.ListStore):
                         if pn:
                             depends.append(pn)
 
-            self.squish(depends)
+            # uniquify the list of depends
+            depends = self.squish(depends)
             deps = " ".join(depends)
 
             if name.count('task-') > 0:
Signature · 1 line
1.7.6
author
#3 lore raw

PATCH 3/4 bb/ui/crumbs/tasklistmodel: don't include an item in its own depends

In reply to Joshua Lock

This causes the simple removal algorithm to perform needless circular logic

Signed-off-by Joshua Lock <[email protected]>
---
 lib/bb/ui/crumbs/tasklistmodel.py |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
lib/bb/ui/crumbs/tasklistmodel.py +3 -0
index aec80e2..7a463a6 100644
@@ -237,6 +237,9 @@ class TaskListModel(gtk.ListStore):
 
             # uniquify the list of depends
             depends = self.squish(depends)
+            # remove circular dependencies
+            if name in depends:
+                depends.remove(name)
             deps = " ".join(depends)
 
             if name.count('task-') > 0:
Signature · 1 line
1.7.6
author
#4 lore raw

PATCH 4/4 bb/cache: rename confusing variable

In reply to Joshua Lock

The bNeedUpdate variable doesn't reflect its purpose, and doesn't match coding style (type encoded in variable name, camel case) - rename to cache_ok.

Signed-off-by Joshua Lock <[email protected]>
---
 lib/bb/cache.py |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
lib/bb/cache.py +3 -3
index d805d46..d495f9e 100644
@@ -286,14 +286,14 @@ class Cache(object):
         old_mtimes.append(newest_mtime)
         newest_mtime = max(old_mtimes)
 
-        bNeedUpdate = True
+        cache_ok = True
         if self.caches_array:
             for cache_class in self.caches_array:
                 if type(cache_class) is type and issubclass(cache_class, RecipeInfoCommon):
                     cachefile = getCacheFile(self.cachedir, cache_class.cachefile)
-                    bNeedUpdate = bNeedUpdate and (bb.parse.cached_mtime_noerror(cachefile) >= newest_mtime)
+                    cache_ok = cache_ok and (bb.parse.cached_mtime_noerror(cachefile) >= newest_mtime)
                     cache_class.init_cacheData(self)
-        if bNeedUpdate:
+        if cache_ok:
             self.load_cachefile()
         elif os.path.isfile(self.cachefile):
             logger.info("Out of date cache found, rebuilding...")
Signature · 1 line
1.7.6
author
#5 lore raw
In reply to Joshua Lock
Quoted 3 lines · Joshua Lock

The following series addresses some thinko/typo's I'd introduced to hob, by using variable names which are too similar, as well as adding some changes to increase clarity in various places.

This series rolled into a new series submitted just now, please ignore this series.

Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project "Johannes Factotum"
        Intel Open Source Technology Centre
#6 lore raw
Quoted 7 lines · Joshua Lock
Quoted 3 lines · Joshua Lock

The following series addresses some thinko/typo's I'd introduced to hob, by using variable names which are too similar, as well as adding some changes to increase clarity in various places.

This series rolled into a new series submitted just now, please ignore this series.

I've actually already merged this series. Was there any change in the newer version I should be looking for?

Cheers,

Richard

author
#7 lore raw
Quoted 11 lines · Richard Purdie
Quoted 7 lines · Joshua Lock
Quoted 3 lines · Joshua Lock

The following series addresses some thinko/typo's I'd introduced to hob, by using variable names which are too similar, as well as adding some changes to increase clarity in various places.

This series rolled into a new series submitted just now, please ignore this series.

I've actually already merged this series. Was there any change in the newer version I should be looking for?

None at all, I was just using the same branch to push the new changes so included them in the new pull request.

Apologies for any confusion.

Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project "Johannes factotum"
        Intel Open Source Technology Centre