Skip to contentSkip to conversation

Several fixes for layerindex

PATCH Superseded Robert Yang 2 participants 26 messages Jun 13, 2017 – Jun 23, 2017

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover Several fixes for layerindex

* V4
- Add an option -i to update_layer.py to get initial layer value (dependencies, versions and so on), update.py doesn't use tinfoil any more since tinfoil before morty can't be shutdown totally if the process is not exited, this was why splitted update_layer.py from update.py.
- Squash 11 patches (v3) into 10, since two of them have overlap.

* V3:
- Better error messages when required collections not found
- Delete layerbranch for non-existed branch
- update_layer.py: remove --update-dependencies

* V2:
- Remove "import bb" from update.py to avoid causing confusions when switch branches.

* V1:
- The "-a ACTUAL_BRANCH" is a litle different from what we had talked, now it respects "-l" and "-b", we can use -l to specify layers or default to all, and it requires "-b" to work, only one branch is supported in a run, for example:

$ update.py -b master -a branch_20170526
All the layers which have branch master and actual_branch branch_20170526 will be updated to branch_20170526.

$ update.py -b master -l meta-oe -a branch_20170526 Only meta-oe layer will be updated.

$ update.py -b master -l bitbake -a branch_20170526 The bitbake's bitbake_branch will be updated.

// Robert

The following changes since commit 8dfe44ed5e618d517d4acb4a3107cb3907e773c8:

TODO: drop implemented items (2017-03-22 14:21:22 +1300)

are available in the git repository at:

git://git.pokylinux.org/poky-contrib rbt/layerindex-10fixes
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=rbt/layerindex-10fixes

Robert Yang (10):
update.py: update actual branch for layer and bitbake layerconfparse.py: remove unused layerbranch from parse_layer() utils.py: remove obsolete dependencies
update_layer.py: set layerbranch's collection before add_dependencies recipeparse.py: restore cwd when the parsing is done update.py: update layers orderly
update_layer.py: remove --update-dependencies update_layer.py: move the location of transaction.atomic() update.py: add -p to git fetch
update_layer.py: delete layerbranch for non-existed branch

 layerindex/layerconfparse.py     |   2 +-
 layerindex/recipeparse.py        |   3 +
 layerindex/tools/import_layer.py |   2 +-
 layerindex/update.py             | 186 ++++++++++++++++++++++++++++++---------
 layerindex/update_layer.py       | 105 ++++++++++------------
 layerindex/utils.py              |  62 ++++++++++++-
 6 files changed, 255 insertions(+), 105 deletions(-)
Signature · 1 line
2.10.2
#1 lore raw

PATCH v4 1/10 update.py: update actual branch for layer and bitbake

Add an option "-a" to update actual branch for layer and bitbake, it is useful when there are many layers and need update actual branches frequantly. We only can update them via website without this patch, which is not funny and easy to make mistakes.

* It works with "-l", and "-l bitbake" means update bitbake branch.
* It requires "-b" to work, and only one branch is supported in a run.

For example:

$ update.py -b master -a branch_20170526
All the layers which have branch master and actual_branch branch_20170526
will be updated to branch_20170526.
$ update.py -b master -l meta-oe -a branch_20170526
Only meta-oe layer will be updated.
$ update.py -b master -l bitbake -a branch_20170526
The bitbake's bitbake_branch will be updated.
---
 layerindex/update.py | 81 ++++++++++++++++++++++++++++++++++++++++++++++------
 layerindex/utils.py  | 11 +++++++
 2 files changed, 83 insertions(+), 9 deletions(-)
layerindex/update.py +72 -9
index d5c56cd..7d2e367 100755
@@ -85,6 +85,43 @@ def prepare_update_layer_command(options, branch, layer, updatedeps=False):
         cmd += ' -q'
     return cmd
 
+def update_actual_branch(layerquery, fetchdir, branch, options, update_bitbake, bitbakepath):
+    """Update actual branch for layers and bitbake in database"""
+    to_save = set()
+    actual_branch = options.actual_branch
+    if update_bitbake:
+        branchobj = utils.get_branch(branch)
+        if actual_branch != branchobj.bitbake_branch:
+            if utils.is_branch_valid(bitbakepath, actual_branch):
+                logger.info("bitbake: %s.bitbake_branch: %s -> %s" % (branch, branchobj.bitbake_branch, actual_branch))
+                branchobj.bitbake_branch = actual_branch
+                to_save.add(branchobj)
+            else:
+                logger.info("Skipping update bitbake_branch for bitbake - branch %s doens't exist" % actual_branch)
+        else:
+            logger.info("bitbake: %s.bitbake_branch is already %s, so no change" % (branch, actual_branch))
+
+    for layer in layerquery:
+        urldir = layer.get_fetch_dir()
+        repodir = os.path.join(fetchdir, urldir)
+        if not utils.is_branch_valid(repodir, actual_branch):
+            logger.info("Skipping update actual_branch for %s - branch %s doens't exist" % (layer.name, actual_branch))
+            continue
+        layerbranch = layer.get_layerbranch(branch)
+        if not layerbranch:
+            logger.info("Skipping update actual_branch for %s - layerbranch %s doens't exist" % (layer.name, branch))
+            continue
+        if actual_branch != layerbranch.actual_branch:
+            logger.info("%s: %s.actual_branch: %s -> %s" % (layer.name, branch, layerbranch.actual_branch, actual_branch))
+            layerbranch.actual_branch = actual_branch
+            to_save.add(layerbranch)
+        else:
+            logger.info("%s: %s.actual_branch is already %s, so no change" % (layer.name, branch, actual_branch))
+
+    # At last, do the save
+    if not options.dryrun:
+        for s in to_save:
+            s.save()
 
 def main():
     if LooseVersion(git.__version__) < '0.3.1':
@@ -102,6 +139,9 @@ def main():
     parser.add_option("-l", "--layer",
             help = "Specify layers to update (use commas to separate multiple). Default is all published layers.",
             action="store", dest="layers")
+    parser.add_option("-a", "--actual-branch",
+            help = "Update actual branch for layer and bitbake",
+            action="store", dest="actual_branch", default='')
     parser.add_option("-r", "--reload",
             help = "Reload recipe data instead of updating since last update",
             action="store_true", dest="reload")
@@ -151,16 +191,35 @@ def main():
         logger.error("Please set LAYER_FETCH_DIR in settings.py")
         sys.exit(1)
 
+    # We deliberately exclude status == 'X' ("no update") here
+    layerquery_all = LayerItem.objects.filter(classic=False).filter(status='P')
+    if layerquery_all.count() == 0:
+        logger.info("No published layers to update")
+        sys.exit(1)
+
+    # For -a option to update bitbake branch
+    update_bitbake = False
     if options.layers:
-        layerquery = LayerItem.objects.filter(classic=False).filter(name__in=options.layers.split(','))
-        if layerquery.count() == 0:
-            logger.error('No layers matching specified query "%s"' % options.layers)
-            sys.exit(1)
+        layers = options.layers.split(',')
+        if 'bitbake' in layers:
+            update_bitbake = True
+            layers.remove('bitbake')
+        for layer in layers:
+            layerquery = LayerItem.objects.filter(classic=False).filter(name=layer)
+            if layerquery.count() == 0:
+                logger.error('No layers matching specified query "%s"' % layer)
+                sys.exit(1)
+        layerquery = LayerItem.objects.filter(classic=False).filter(name__in=layers)
     else:
-        # We deliberately exclude status == 'X' ("no update") here
-        layerquery = LayerItem.objects.filter(classic=False).filter(status='P')
-        if layerquery.count() == 0:
-            logger.info("No published layers to update")
+        update_bitbake = True
+        layerquery = layerquery_all
+
+    if options.actual_branch:
+        if not options.branch:
+            logger.error("-a option requires -b")
+            sys.exit(1)
+        elif len(branches) != 1:
+            logger.error("Only one branch should be used with -a")
             sys.exit(1)
 
     if not os.path.exists(fetchdir):
@@ -209,7 +268,7 @@ def main():
                             continue
                         fetchedrepos.append(layer.vcs_url)
 
-                if not fetchedrepos:
+                if not (fetchedrepos or update_bitbake):
                     logger.error("No repositories could be fetched, exiting")
                     sys.exit(1)
 
@@ -219,6 +278,10 @@ def main():
                 else:
                     out = utils.runcmd("git fetch", bitbakepath, logger=logger)
 
+            if options.actual_branch:
+                update_actual_branch(layerquery, fetchdir, branches[0], options, update_bitbake, bitbakepath)
+                return
+
             # Process and extract data from each layer
             # We now do this by calling out to a separate script; doing otherwise turned out to be
             # unreliable due to leaking memory (we're using bitbake internals in a manner in which
layerindex/utils.py +11 -0
index 3b45b07..442b121 100644
@@ -160,6 +160,17 @@ def is_layer_valid(layerdir):
         return False
     return True
 
+def is_branch_valid(layerdir, branch):
+    import git
+
+    g = git.cmd.Git(layerdir)
+    assert g.rev_parse('--is-bare-repository') == 'false'
+    try:
+        g.rev_parse('--verify', 'origin/%s' % branch)
+    except Exception:
+        return False
+    return True
+
 def parse_conf(conf_file, d):
     if hasattr(bb.parse, "handle"):
         # Newer BitBake
Signature · 1 line
2.10.2
#2 lore raw

Re: [PATCH V4 01/10] update.py: update actual branch for layer and bitbake

Hi Robert,

Apologies for the delay on reviewing these patches. Some comments below.

Quoted 4 lines · CEST Robert Yang

Add an option "-a" to update actual branch for layer and bitbake, it is useful when there are many layers and need update actual branches frequantly. We only can update them via website without this patch, which is not funny and easy to make mistakes.

frequantly -> frequently
funny -> fun

Quoted 31 lines

* It works with "-l", and "-l bitbake" means update bitbake branch.
* It requires "-b" to work, and only one branch is supported in a run.

For example:

$ update.py -b master -a branch_20170526
All the layers which have branch master and actual_branch branch_20170526
will be updated to branch_20170526.
$ update.py -b master -l meta-oe -a branch_20170526
Only meta-oe layer will be updated.
$ update.py -b master -l bitbake -a branch_20170526
The bitbake's bitbake_branch will be updated.
---
 layerindex/update.py | 81 ++++++++++++++++++++++++++++++++++++++++++++++------
 layerindex/utils.py  | 11 +++++++
 2 files changed, 83 insertions(+), 9 deletions(-)
layerindex/update.py +3 -0
index d5c56cd..7d2e367 100755
@@ -85,6 +85,43 @@ def prepare_update_layer_command(options, branch, layer, updatedeps=False):
         cmd += ' -q'
     return cmd
 
+def update_actual_branch(layerquery, fetchdir, branch, options, update_bitbake, bitbakepath):
+    """Update actual branch for layers and bitbake in database"""
+    to_save = set()

layerindex/update.py def prepare_update_layer_command(options, branch, layer, updatedeps=False): · line 100

Quoted 10 lines
layerindex/update.py +10 -0
+    actual_branch = options.actual_branch
+    if update_bitbake:
+        branchobj = utils.get_branch(branch)
+        if actual_branch != branchobj.bitbake_branch:
+            if utils.is_branch_valid(bitbakepath, actual_branch):
+                logger.info("bitbake: %s.bitbake_branch: %s -> %s" % (branch, branchobj.bitbake_branch, actual_branch))
+                branchobj.bitbake_branch = actual_branch
+                to_save.add(branchobj)
+            else:
+                logger.info("Skipping update bitbake_branch for bitbake - branch %s doens't exist" % actual_branch)

doens't -> doesn't (there are several of these)

Quoted 28 lines
layerindex/update.py +24 -0
+        else:
+            logger.info("bitbake: %s.bitbake_branch is already %s, so no change" % (branch, actual_branch))
+
+    for layer in layerquery:
+        urldir = layer.get_fetch_dir()
+        repodir = os.path.join(fetchdir, urldir)
+        if not utils.is_branch_valid(repodir, actual_branch):
+            logger.info("Skipping update actual_branch for %s - branch %s doens't exist" % (layer.name, actual_branch))
+            continue
+        layerbranch = layer.get_layerbranch(branch)
+        if not layerbranch:
+            logger.info("Skipping update actual_branch for %s - layerbranch %s doens't exist" % (layer.name, branch))
+            continue
+        if actual_branch != layerbranch.actual_branch:
+            logger.info("%s: %s.actual_branch: %s -> %s" % (layer.name, branch, layerbranch.actual_branch, actual_branch))
+            layerbranch.actual_branch = actual_branch
+            to_save.add(layerbranch)
+        else:
+            logger.info("%s: %s.actual_branch is already %s, so no change" % (layer.name, branch, actual_branch))
+
+    # At last, do the save
+    if not options.dryrun:
+        for s in to_save:
+            s.save()
 
 def main():
     if LooseVersion(git.__version__) < '0.3.1':

layerindex/update.py def main(): · line 143

Quoted 5 lines
layerindex/update.py +2 -0
@@ -102,6 +139,9 @@ def main():
     parser.add_option("-l", "--layer",
             help = "Specify layers to update (use commas to separate multiple). Default is all published layers.",
             action="store", dest="layers")
+    parser.add_option("-a", "--actual-branch",
+            help = "Update actual branch for layer and bitbake",

We should add "only" to the end of this since this option will suppress the normal layer update since it returns afterwards, correct?

layerindex/update.py def main(): · line 198

Quoted 13 lines
layerindex/update.py +6 -0
+            action="store", dest="actual_branch", default='')
     parser.add_option("-r", "--reload",
             help = "Reload recipe data instead of updating since last update",
             action="store_true", dest="reload")
@@ -151,16 +191,35 @@ def main():
         logger.error("Please set LAYER_FETCH_DIR in settings.py")
         sys.exit(1)
 
+    # We deliberately exclude status == 'X' ("no update") here
+    layerquery_all = LayerItem.objects.filter(classic=False).filter(status='P')
+    if layerquery_all.count() == 0:
+        logger.info("No published layers to update")
+        sys.exit(1)

So near as I can tell this change isn't required as part of the other changes here, it's just to tell you up front that no layers are published if you've specified layers instead of seeing an error. I don't have an issue with that but this change should be mentioned in the commit message ("additionally ..."), or split out as a separate patch.

layerindex/update.py def main(): · line 206

Quoted 11 lines
layerindex/update.py +6 -4
+    # For -a option to update bitbake branch
+    update_bitbake = False
     if options.layers:
-        layerquery = LayerItem.objects.filter(classic=False).filter(name__in=options.layers.split(','))
-        if layerquery.count() == 0:
-            logger.error('No layers matching specified query "%s"' % options.layers)
-            sys.exit(1)
+        layers = options.layers.split(',')
+        if 'bitbake' in layers:
+            update_bitbake = True
+            layers.remove('bitbake')

So since this means 'bitbake' is a reserved name for layers we should add validation upon layer submission to ensure you can't create a layer with this name. I can take care of that if you like.

Quoted 50 lines
layerindex/update.py +20 -5
+        for layer in layers:
+            layerquery = LayerItem.objects.filter(classic=False).filter(name=layer)
+            if layerquery.count() == 0:
+                logger.error('No layers matching specified query "%s"' % layer)
+                sys.exit(1)
+        layerquery = LayerItem.objects.filter(classic=False).filter(name__in=layers)
     else:
-        # We deliberately exclude status == 'X' ("no update") here
-        layerquery = LayerItem.objects.filter(classic=False).filter(status='P')
-        if layerquery.count() == 0:
-            logger.info("No published layers to update")
+        update_bitbake = True
+        layerquery = layerquery_all
+
+    if options.actual_branch:
+        if not options.branch:
+            logger.error("-a option requires -b")
+            sys.exit(1)
+        elif len(branches) != 1:
+            logger.error("Only one branch should be used with -a")
             sys.exit(1)
 
     if not os.path.exists(fetchdir):
@@ -209,7 +268,7 @@ def main():
                             continue
                         fetchedrepos.append(layer.vcs_url)
 
-                if not fetchedrepos:
+                if not (fetchedrepos or update_bitbake):
                     logger.error("No repositories could be fetched, exiting")
                     sys.exit(1)
 
@@ -219,6 +278,10 @@ def main():
                 else:
                     out = utils.runcmd("git fetch", bitbakepath, logger=logger)
 
+            if options.actual_branch:
+                update_actual_branch(layerquery, fetchdir, branches[0], options, update_bitbake, bitbakepath)
+                return
+
             # Process and extract data from each layer
             # We now do this by calling out to a separate script; doing otherwise turned out to be
             # unreliable due to leaking memory (we're using bitbake internals in a manner in which
layerindex/utils.py +0 -0
index 3b45b07..442b121 100644
@@ -160,6 +160,17 @@ def is_layer_valid(layerdir):
         return False
     return True

layerindex/utils.py def is_layer_valid(layerdir): · line 171

Quoted 10 lines
layerindex/utils.py +9 -0
 
+def is_branch_valid(layerdir, branch):
+    import git
+
+    g = git.cmd.Git(layerdir)
+    assert g.rev_parse('--is-bare-repository') == 'false'
+    try:
+        g.rev_parse('--verify', 'origin/%s' % branch)
+    except Exception:
+        return False

Is there a more specific exception class we can respond to here instead of Exception in case something unexpected happens?

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#3 lore raw

Re: [PATCH V4 01/10] update.py: update actual branch for layer and bitbake

Hi Paul,

Thanks for the review, I've fixed all the issues you commented except the following ones.

Quoted 9 lines · Paul Eggleton

Hi Robert,

layerindex/update.py def main(): · line 198

Quoted 6 lines
layerindex/update.py +5 -0
+    # We deliberately exclude status == 'X' ("no update") here
+    layerquery_all = LayerItem.objects.filter(classic=False).filter(status='P')
+    if layerquery_all.count() == 0:
+        logger.info("No published layers to update")
+        sys.exit(1)
Quoted 5 lines

So near as I can tell this change isn't required as part of the other changes here, it's just to tell you up front that no layers are published if you've specified layers instead of seeing an error. I don't have an issue with that but this change should be mentioned in the commit message ("additionally ..."), or split out as a separate patch.

Sorry, it should be in another patch, I added it to wrong place when rebase. I add this patch into "update.py: update layers orderly" now. I need layerquery_all to know all the provided collections in database, so that we can know whether the dependencies match or not.

Quoted 14 lines

layerindex/update.py def main(): · line 206

Quoted 13 lines
layerindex/update.py +6 -4
+    # For -a option to update bitbake branch
+    update_bitbake = False
     if options.layers:
-        layerquery = LayerItem.objects.filter(classic=False).filter(name__in=options.layers.split(','))
-        if layerquery.count() == 0:
-            logger.error('No layers matching specified query "%s"' % options.layers)
-            sys.exit(1)
+        layers = options.layers.split(',')
+        if 'bitbake' in layers:
+            update_bitbake = True
+            layers.remove('bitbake')
Quoted 3 lines

So since this means 'bitbake' is a reserved name for layers we should add validation upon layer submission to ensure you can't create a layer with this name. I can take care of that if you like.

Good idea, I will do it later, or you can do it if you have time.

// Robert

#4 lore raw

PATCH v4 2/10 layerconfparse.py: remove unused layerbranch from parse_layer()

In reply to Robert Yang

The layerbranch is not used in parse_layer(), so remove it.

---
 layerindex/layerconfparse.py     | 2 +-
 layerindex/tools/import_layer.py | 2 +-
 layerindex/update_layer.py       | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)
layerindex/layerconfparse.py +1 -1
index d599ed5..526d2c2 100644
@@ -32,7 +32,7 @@ class LayerConfParse:
 
         self.config_data_copy = bb.data.createCopy(self.tinfoil.config_data)
 
-    def parse_layer(self, layerbranch, layerdir):
+    def parse_layer(self, layerdir):
 
         # This is not a valid layer, parsing will cause exception.
         if not utils.is_layer_valid(layerdir):
layerindex/tools/import_layer.py +1 -1
index ad16e40..a806dd1 100755
@@ -377,7 +377,7 @@ def main():
                         layerdep.save()
                     layerconfparser = LayerConfParse(logger=logger)
                     try:
-                        config_data = layerconfparser.parse_layer(layerbranch, layerdir)
+                        config_data = layerconfparser.parse_layer(layerdir)
                         if config_data:
                             utils.add_dependencies(layerbranch, config_data, logger=logger)
                             utils.add_recommends(layerbranch, config_data, logger=logger)
layerindex/update_layer.py +2 -2
index 9fe96fb..05ab3e7 100644
@@ -267,7 +267,7 @@ def main():
                     sys.exit(0)
 
                 layerconfparser = layerconfparse.LayerConfParse(logger=logger, bitbakepath=bitbakepath, tinfoil=tinfoil)
-                config_data = layerconfparser.parse_layer(layerbranch, layerdir)
+                config_data = layerconfparser.parse_layer(layerdir)
                 if not config_data:
                     logger.debug("Layer %s does not appear to be valid for branch %s" % (layer.name, branchdesc))
                     sys.exit(0)
@@ -355,7 +355,7 @@ def main():
                 logger.info("Collecting data for layer %s on branch %s" % (layer.name, branchdesc))
 
                 layerconfparser = layerconfparse.LayerConfParse(logger=logger, tinfoil=tinfoil)
-                layer_config_data = layerconfparser.parse_layer(layerbranch, layerdir)
+                layer_config_data = layerconfparser.parse_layer(layerdir)
                 if not layer_config_data:
                     logger.info("Skipping update of layer %s for branch %s - conf/layer.conf may have parse issues" % (layer.name, branchdesc))
                     layerconfparser.shutdown()
Signature · 1 line
2.10.2
#5 lore raw

PATCH v4 3/10 utils.py: remove obsolete dependencies

In reply to Robert Yang

Fixed:
- set LAYERDEPENDS_openembedded-layer = "core"
- $ "update.py -l meta-oe -b master"
Check from web, its dependency is "openembedded-core"
- Change LAYERDEPENDS_openembedded-layer = "foo"
- Run "update.py -l meta-oe -b master"
Check from web, its dependency is "openembedded-core and foo", this is wrong, it should be "foo" only, this patch can fix the problem.

And also the existing checking should filter(required=required), otherwise it can't work well when a layer is in both depends and recommends, this can't happen in a normal case, but it would surprise the user when this happens.

---
 layerindex/utils.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
layerindex/utils.py +12 -1
index 442b121..b7165d0 100644
@@ -88,6 +88,7 @@ def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T
         logger.debug('Error parsing %s_%s for %s\n%s' % (var, var_name, layer_name, str(vse)))
         return
 
+    need_delete = None
     for dep, ver_list in list(dep_dict.items()):
         ver_str = None
         if ver_list:
@@ -106,8 +107,14 @@ def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T
                 logger.error('Cannot resolve %s %s (version %s) for %s' % (name, dep, ver_str, layer_name))
                 continue
 
+        # Preparing to remove obsolete ones
+        if not need_delete:
+            need_delete = LayerDependency.objects.filter(layerbranch=layerbranch).filter(required=required).exclude(dependency=dep_layer)
+        else:
+            need_delete = need_delete.exclude(dependency=dep_layer)
+
         # Skip existing entries.
-        existing = list(LayerDependency.objects.filter(layerbranch=layerbranch).filter(dependency=dep_layer))
+        existing = list(LayerDependency.objects.filter(layerbranch=layerbranch).filter(required=required).filter(dependency=dep_layer))
         if existing:
             logger.debug('Skipping %s - already a dependency for %s' % (dep, layer_name))
             continue
@@ -121,6 +128,10 @@ def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T
         layerdep.required = required
         layerdep.save()
 
+    if need_delete:
+        logger.debug("Removing obsolete dependencies: %s" % need_delete)
+        need_delete.delete()
+
 def set_layerbranch_collection_version(layerbranch, config_data, logger=None):
     layerbranch.collection = config_data.getVar('BBFILE_COLLECTIONS', True)
     ver_str = "LAYERVERSION_"
Signature · 1 line
2.10.2
#6 lore raw

Re: [PATCH V4 03/10] utils.py: remove obsolete dependencies

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data,

Quoted 24 lines · CEST Robert Yang

Fixed:
- set LAYERDEPENDS_openembedded-layer = "core"
- $ "update.py -l meta-oe -b master"
Check from web, its dependency is "openembedded-core"
- Change LAYERDEPENDS_openembedded-layer = "foo"
- Run "update.py -l meta-oe -b master"
Check from web, its dependency is "openembedded-core and foo", this is wrong, it should be "foo" only, this patch can fix the problem.

And also the existing checking should filter(required=required), otherwise it can't work well when a layer is in both depends and recommends, this can't happen in a normal case, but it would surprise the user when this happens.

---
 layerindex/utils.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
layerindex/utils.py +0 -0
index 442b121..b7165d0 100644
@@ -88,6 +88,7 @@ def _add_dependency(var, name, layerbranch, config_data, 

logger=None, required=T

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, · line 88

Quoted 1 line
layerindex/utils.py +0 -0
         logger.debug('Error parsing %s_%s for %s\n%s' % (var, var_name, 

layer_name, str(vse)))

layerindex/utils.py def _add_dependency(var, name, layerbranch,

Quoted 7 lines
layerindex/utils.py +1 -0
         return
 
+    need_delete = None
     for dep, ver_list in list(dep_dict.items()):
         ver_str = None
         if ver_list:
@@ -106,8 +107,14 @@ def _add_dependency(var, name, layerbranch, 

config_data, logger=None, required=T

layerindex/utils.py def _add_dependency(var, name, layerbranch, · line 107

Quoted 1 line
layerindex/utils.py +0 -0
                 logger.error('Cannot resolve %s %s (version %s) for %s' % 

(name, dep, ver_str, layer_name))

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T

Quoted 5 lines
layerindex/utils.py +3 -0
                 continue
 
+        # Preparing to remove obsolete ones
+        if not need_delete:
+            need_delete = 

LayerDependency.objects.filter(layerbranch=layerbranch).filter(required=required).exclude(dependency=dep_layer)

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T

Quoted 5 lines
layerindex/utils.py +3 -1
+        else:
+            need_delete = need_delete.exclude(dependency=dep_layer)
+
         # Skip existing entries.
-        existing = 

list(LayerDependency.objects.filter(layerbranch=layerbranch).filter(dependency=dep_layer))

layerindex/utils.py def _add_dependency(var, name, layerbranch, · line 117

Quoted 1 line
layerindex/utils.py +1 -0
+        existing = 

list(LayerDependency.objects.filter(layerbranch=layerbranch).filter(required=required).filter(dependency=dep_layer))

layerindex/utils.py def _add_dependency(var, name, layerbranch, · line 119

Quoted 2 lines
layerindex/utils.py +0 -0
         if existing:
             logger.debug('Skipping %s - already a dependency for %s' % 

(dep, layer_name))

layerindex/utils.py def _add_dependency(var, name, layerbranch,

Quoted 2 lines
layerindex/utils.py +0 -0
             continue
@@ -121,6 +128,10 @@ def _add_dependency(var, name, layerbranch, 

config_data, logger=None, required=T

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T

Quoted 8 lines
layerindex/utils.py +4 -0
         layerdep.required = required
         layerdep.save()
 
+    if need_delete:
+        logger.debug("Removing obsolete dependencies: %s" % need_delete)
+        need_delete.delete()
+
 def set_layerbranch_collection_version(layerbranch, config_data, 

logger=None):

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T · line 137

Quoted 2 lines
layerindex/utils.py +0 -0
     layerbranch.collection = config_data.getVar('BBFILE_COLLECTIONS', True)
     ver_str = "LAYERVERSION_"

So I do see the need to clear out old dependency values when your list of dependencies is coming entirely from LAYERDEPENDS, however if you have only manually added dependencies and LAYERDEPENDS is not complete (as is often the case in the OE layer index), will this automatically remove those? If so I think we're going to have to come up with some mechanism to avoid that. Probably the easiest one would be to avoid touching the dependencies unless LAYERDEPENDS actually changes. Of course if that means we're going to store the previous LAYERDEPENDS value somewhere to compare to, we'd need to avoid it triggering initially when none of those values are set.

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#7 lore raw

Re: [PATCH V4 03/10] utils.py: remove obsolete dependencies

Quoted 66 lines · Paul Eggleton

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data,

Quoted 24 lines · CEST Robert Yang

Fixed:
- set LAYERDEPENDS_openembedded-layer = "core"
- $ "update.py -l meta-oe -b master"
Check from web, its dependency is "openembedded-core"
- Change LAYERDEPENDS_openembedded-layer = "foo"
- Run "update.py -l meta-oe -b master"
Check from web, its dependency is "openembedded-core and foo", this is wrong, it should be "foo" only, this patch can fix the problem.

And also the existing checking should filter(required=required), otherwise it can't work well when a layer is in both depends and recommends, this can't happen in a normal case, but it would surprise the user when this happens.

---
 layerindex/utils.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
layerindex/utils.py +0 -0
index 442b121..b7165d0 100644
@@ -88,6 +88,7 @@ def _add_dependency(var, name, layerbranch, config_data,

logger=None, required=T

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, · line 88

Quoted 1 line
layerindex/utils.py +0 -0
         logger.debug('Error parsing %s_%s for %s\n%s' % (var, var_name,

layer_name, str(vse)))

layerindex/utils.py def _add_dependency(var, name, layerbranch,

Quoted 7 lines
layerindex/utils.py +1 -0
         return
 
+    need_delete = None
     for dep, ver_list in list(dep_dict.items()):
         ver_str = None
         if ver_list:
@@ -106,8 +107,14 @@ def _add_dependency(var, name, layerbranch,

config_data, logger=None, required=T

layerindex/utils.py def _add_dependency(var, name, layerbranch, · line 107

Quoted 1 line
layerindex/utils.py +0 -0
                 logger.error('Cannot resolve %s %s (version %s) for %s' %

(name, dep, ver_str, layer_name))

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T

Quoted 5 lines
layerindex/utils.py +3 -0
                 continue
 
+        # Preparing to remove obsolete ones
+        if not need_delete:
+            need_delete =

LayerDependency.objects.filter(layerbranch=layerbranch).filter(required=required).exclude(dependency=dep_layer)

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T

Quoted 5 lines
layerindex/utils.py +3 -1
+        else:
+            need_delete = need_delete.exclude(dependency=dep_layer)
+
         # Skip existing entries.
-        existing =

list(LayerDependency.objects.filter(layerbranch=layerbranch).filter(dependency=dep_layer))

layerindex/utils.py def _add_dependency(var, name, layerbranch, · line 117

Quoted 1 line
layerindex/utils.py +1 -0
+        existing =

list(LayerDependency.objects.filter(layerbranch=layerbranch).filter(required=required).filter(dependency=dep_layer))

layerindex/utils.py def _add_dependency(var, name, layerbranch, · line 119

Quoted 2 lines
layerindex/utils.py +0 -0
         if existing:
             logger.debug('Skipping %s - already a dependency for %s' %

(dep, layer_name))

layerindex/utils.py def _add_dependency(var, name, layerbranch,

Quoted 2 lines
layerindex/utils.py +0 -0
             continue
@@ -121,6 +128,10 @@ def _add_dependency(var, name, layerbranch,

config_data, logger=None, required=T

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T

Quoted 8 lines
layerindex/utils.py +4 -0
         layerdep.required = required
         layerdep.save()
 
+    if need_delete:
+        logger.debug("Removing obsolete dependencies: %s" % need_delete)
+        need_delete.delete()
+
 def set_layerbranch_collection_version(layerbranch, config_data,
Quoted 8 lines

logger=None):

layerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T · line 137

Quoted 2 lines
layerindex/utils.py +0 -0
     layerbranch.collection = config_data.getVar('BBFILE_COLLECTIONS', True)
     ver_str = "LAYERVERSION_"

So I do see the need to clear out old dependency values when your list of dependencies is coming entirely from LAYERDEPENDS, however if you have only manually added dependencies and LAYERDEPENDS is not complete (as is often the case in the OE layer index), will this automatically remove those? If so I

The dependencies will be removed if it is not in conf/layer.conf.

Quoted 5 lines

think we're going to have to come up with some mechanism to avoid that. Probably the easiest one would be to avoid touching the dependencies unless LAYERDEPENDS actually changes. Of course if that means we're going to store the previous LAYERDEPENDS value somewhere to compare to, we'd need to avoid it triggering initially when none of those values are set.

Why not fix conf/layer.conf rather than fix it from database ? If conf/layer.conf is wrong, we would meet problems elsewhere. The database should obey local.conf when mismatches, otherwise, this would confuse users a lot when they want to update LAYERDEPENDS.

I'm glad to fix oe-classic's LAYERDEPENDS issues if they are wrong.

// Robert

Quoted 4 lines

Cheers,
Paul

#8 lore raw

Re: [PATCH V4 03/10] utils.py: remove obsolete dependencies

Quoted 8 lines · CEST Robert Yang
Quoted 4 lines · Paul Eggleton

So I do see the need to clear out old dependency values when your list of dependencies is coming entirely from LAYERDEPENDS, however if you have only manually added dependencies and LAYERDEPENDS is not complete (as is often the case in the OE layer index), will this automatically remove those? If so I

The dependencies will be removed if it is not in conf/layer.conf.

Quoted 8 lines
Quoted 5 lines

think we're going to have to come up with some mechanism to avoid that. Probably the easiest one would be to avoid touching the dependencies unless LAYERDEPENDS actually changes. Of course if that means we're going to store the previous LAYERDEPENDS value somewhere to compare to, we'd need to avoid it triggering initially when none of those values are set.

Why not fix conf/layer.conf rather than fix it from database ? If conf/layer.conf is wrong, we would meet problems elsewhere.

Up until now it hasn't caused any issues other than not getting an error up front from bitbake when a dependencies are missing, so it can easily go unnoticed (particularly when users follow manual instructions or use existing configurations rather than an automated tool that pays attention to LAYERDEPENDS).

Quoted 3 lines

The database
should obey local.conf when mismatches, otherwise, this would confuse users a lot when they want to update LAYERDEPENDS.

As I indicated, I agree that if LAYERDEPENDS gets updated then the dependencies in the database should get updated. My concern is updating them if LAYERDEPENDS has not changed and it simply happens to have a difference to what is in the database (most importantly if it has less dependencies specified).

Quoted 1 line

I'm glad to fix oe-classic's LAYERDEPENDS issues if they are wrong.

Well, it's not so much OE-Classic as every single layer in the index, maintained by a number of different maintainers. I agree in the long term we do want people to fix their dependencies. The trouble is we don't even know the size of the problem at the moment, and if it's widespread then we'll end up dropping dependencies potentially leading to errors during parsing.

How about this - could we start with a setting in the configuration that specifies that dependencies should be updated automatically, and if not set just gives a warning if the dependencies are not the same? We can then decide how to handle it when we see the warnings we're getting in the OE index.

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#9 lore raw

Re: [PATCH V4 03/10] utils.py: remove obsolete dependencies

Quoted 43 lines · Paul Eggleton
Quoted 8 lines · CEST Robert Yang
Quoted 4 lines · Paul Eggleton

So I do see the need to clear out old dependency values when your list of dependencies is coming entirely from LAYERDEPENDS, however if you have only manually added dependencies and LAYERDEPENDS is not complete (as is often the case in the OE layer index), will this automatically remove those? If so I

The dependencies will be removed if it is not in conf/layer.conf.

Quoted 8 lines
Quoted 5 lines

think we're going to have to come up with some mechanism to avoid that. Probably the easiest one would be to avoid touching the dependencies unless LAYERDEPENDS actually changes. Of course if that means we're going to store the previous LAYERDEPENDS value somewhere to compare to, we'd need to avoid it triggering initially when none of those values are set.

Why not fix conf/layer.conf rather than fix it from database ? If conf/layer.conf is wrong, we would meet problems elsewhere.

Up until now it hasn't caused any issues other than not getting an error up front from bitbake when a dependencies are missing, so it can easily go unnoticed (particularly when users follow manual instructions or use existing configurations rather than an automated tool that pays attention to LAYERDEPENDS).

Quoted 3 lines

The database
should obey local.conf when mismatches, otherwise, this would confuse users a lot when they want to update LAYERDEPENDS.

As I indicated, I agree that if LAYERDEPENDS gets updated then the dependencies in the database should get updated. My concern is updating them if LAYERDEPENDS has not changed and it simply happens to have a difference to what is in the database (most importantly if it has less dependencies specified).

Quoted 1 line

I'm glad to fix oe-classic's LAYERDEPENDS issues if they are wrong.

Well, it's not so much OE-Classic as every single layer in the index, maintained by a number of different maintainers. I agree in the long term we do want people to fix their dependencies. The trouble is we don't even know the size of the problem at the moment, and if it's widespread then we'll end up dropping dependencies potentially leading to errors during parsing.

Quoted 5 lines

How about this - could we start with a setting in the configuration that specifies that dependencies should be updated automatically, and if not set just gives a warning if the dependencies are not the same? We can then decide how to handle it when we see the warnings we're getting in the OE index.

Sounds good to me, I will fix it in V5.

// Robert

Quoted 4 lines

Cheers,
Paul

#10 lore raw

Re: [PATCH V4 03/10] utils.py: remove obsolete dependencies

Hi Paul,

Quoted 46 lines · Robert Yang
Quoted 43 lines · Paul Eggleton
Quoted 8 lines · CEST Robert Yang
Quoted 4 lines · Paul Eggleton

So I do see the need to clear out old dependency values when your list of dependencies is coming entirely from LAYERDEPENDS, however if you have only manually added dependencies and LAYERDEPENDS is not complete (as is often the case in the OE layer index), will this automatically remove those? If so I

The dependencies will be removed if it is not in conf/layer.conf.

Quoted 8 lines
Quoted 5 lines

think we're going to have to come up with some mechanism to avoid that. Probably the easiest one would be to avoid touching the dependencies unless LAYERDEPENDS actually changes. Of course if that means we're going to store the previous LAYERDEPENDS value somewhere to compare to, we'd need to avoid it triggering initially when none of those values are set.

Why not fix conf/layer.conf rather than fix it from database ? If conf/layer.conf is wrong, we would meet problems elsewhere.

Up until now it hasn't caused any issues other than not getting an error up front from bitbake when a dependencies are missing, so it can easily go unnoticed (particularly when users follow manual instructions or use existing configurations rather than an automated tool that pays attention to LAYERDEPENDS).

Quoted 3 lines

The database
should obey local.conf when mismatches, otherwise, this would confuse users a lot when they want to update LAYERDEPENDS.

As I indicated, I agree that if LAYERDEPENDS gets updated then the dependencies in the database should get updated. My concern is updating them if LAYERDEPENDS has not changed and it simply happens to have a difference to what is in the database (most importantly if it has less dependencies specified).

Quoted 1 line

I'm glad to fix oe-classic's LAYERDEPENDS issues if they are wrong.

Well, it's not so much OE-Classic as every single layer in the index, maintained by a number of different maintainers. I agree in the long term we do want people to fix their dependencies. The trouble is we don't even know the size of the problem at the moment, and if it's widespread then we'll end up dropping dependencies potentially leading to errors during parsing.

Quoted 7 lines
Quoted 5 lines

How about this - could we start with a setting in the configuration that specifies that dependencies should be updated automatically, and if not set just gives a warning if the dependencies are not the same? We can then decide how to handle it when we see the warnings we're getting in the OE index.

Sounds good to me, I will fix it in V5.

After more thinking, I will use:
# Remove layer dependencies if it is not in conf/layer.conf REMOVE_LAYER_DEPENDENCIES = False

Currently it does can add dependencies from conf/layer.conf, but no remove, so use REMOVE_LAYER_DEPENDENCIES makes it more clear.

// Robert

Quoted 7 lines

// Robert

Quoted 4 lines

Cheers,
Paul

#11 lore raw

Re: [PATCH V4 03/10] utils.py: remove obsolete dependencies

Quoted 8 lines · CEST Robert Yang
Quoted 7 lines · Robert Yang
Quoted 5 lines · Paul Eggleton

How about this - could we start with a setting in the configuration that specifies that dependencies should be updated automatically, and if not set just gives a warning if the dependencies are not the same? We can then decide how to handle it when we see the warnings we're getting in the OE index.

Quoted 8 lines
Quoted 1 line

Sounds good to me, I will fix it in V5.

After more thinking, I will use:
# Remove layer dependencies if it is not in conf/layer.conf REMOVE_LAYER_DEPENDENCIES = False

Currently it does can add dependencies from conf/layer.conf, but no remove, so use REMOVE_LAYER_DEPENDENCIES makes it more clear.

Sounds good to me.

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#12 lore raw

PATCH v4 4/10 update_layer.py: set layerbranch's collection before add_dependencies

In reply to Robert Yang

The _add_dependency() uses:
if layerbranch.collection:
var_name = layerbranch.collection

The layerbranch.collection is none if it is newly created, thus it can't get LAYERDEPENDS, because what defined in layer.conf is LAYERDEPENDS_<collection>, but what it would get is LAYERDEPENDS_<layer_name>, this patch can fix the problem.

Reproducer:

$ python3 update_layer.py -l mete-xfce -b <newbranch> --fullreload -d

It would get None LAYERDEPENDS.

---
 layerindex/update_layer.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
layerindex/update_layer.py +1 -1
index 05ab3e7..bcf7056 100644
@@ -360,9 +360,9 @@ def main():
                     logger.info("Skipping update of layer %s for branch %s - conf/layer.conf may have parse issues" % (layer.name, branchdesc))
                     layerconfparser.shutdown()
                     sys.exit(1)
+                utils.set_layerbranch_collection_version(layerbranch, layer_config_data, logger=logger)
                 utils.add_dependencies(layerbranch, layer_config_data, logger=logger)
                 utils.add_recommends(layerbranch, layer_config_data, logger=logger)
-                utils.set_layerbranch_collection_version(layerbranch, layer_config_data, logger=logger)
                 layerbranch.save()
 
                 try:
Signature · 1 line
2.10.2
#13 lore raw

PATCH v4 5/10 recipeparse.py: restore cwd when the parsing is done

In reply to Robert Yang

Othewise it may cause troubles to the function who calls it.

---
 layerindex/recipeparse.py | 3 +++
 1 file changed, 3 insertions(+)
layerindex/recipeparse.py +3 -0
index dd85bc3..f2a5235 100644
@@ -70,10 +70,13 @@ def init_parser(settings, branch, bitbakepath, enable_tracking=False, nocheckout
     if not os.path.exists(settings.TEMP_BASE_DIR):
         os.makedirs(settings.TEMP_BASE_DIR)
     tempdir = tempfile.mkdtemp(dir=settings.TEMP_BASE_DIR)
+    saved_cwd = os.getcwd()
     os.chdir(tempdir)
 
     tinfoil = utils.setup_tinfoil(bitbakepath, enable_tracking)
 
+    os.chdir(saved_cwd)
+
     # Ensure TMPDIR exists (or insane.bbclass will blow up trying to write to the QA log)
     oe_tmpdir = tinfoil.config_data.getVar('TMPDIR', True)
     if not os.path.exists(oe_tmpdir):
Signature · 1 line
2.10.2
#14 lore raw

PATCH v4 6/10 update.py: update layers orderly

In reply to Robert Yang

* Problems
The update.py couldn't handle new (not only new branch, in fact, but also existing branches, see below for more info) branch well, for example, there are 3 layers: layer_A, layer_B and layer_C, and create new branch "branch_1" for them, and they have depends:

layer_A -> layer_B -> layer_C

The "->" means depends on.

Then run "update.py -b branch_1", there would be errors like:

ERROR: Dependency layer_B of layer_A does not have branch record for branch branch_1

Though update.py runs "update_layer.py" twice, but it didn't help since layerbranch was None when it was failed to create in the first run.

The reason is if update.py updates layer_A firstly, it would fail since it can't find layer_B:branch_1 in database (not added to database yet), similarly, if add layer_B before layer_C, it would also fail. Only layer_C can be added (assume it has no dependencies). So we have to re-run update.py again and again to make it work, here we may have to run update.py 3 times, and more runs are needed if the dependency chain is longer.

* Solutions:
Make update.py pass layers orderly to update_layer.py according to dependencies can fix the problem, we can get intial dependencies from tinfoil, add an option "-i, --initial" to update_layer.py to get them.

Not only new branch, but also existing branches may have the problem, because collections and dependencies maybe changed in the coming update, so we can't trust database when the layer is going to be updated, for example, if there are 10 layers in database, and 3 of them will be updated (-l layer1,layer2,layer3), then we can not use the 3 layers' collections data from database, we need get them from tinfoil.

* Performance improvement:
It should be the same as before in theory, I have tested it with 97 layers:
- Before: 11m6.472s, but only 75 layers were added, 22 ones were failed, I have to re-run update.py again and again (maybe 4 times to make all of them added). So:
(11 * 60 + 6)/75*97/60 = 14m35s

- Now 12m10.350s, all the layers are added in the first run.

So about 2 minutes are saved.

---
 layerindex/update.py       | 101 +++++++++++++++++++++++++++++++--------------
 layerindex/update_layer.py |  16 +++++--
 layerindex/utils.py        |  38 +++++++++++++++++
 3 files changed, 121 insertions(+), 34 deletions(-)
layerindex/update.py +70 -31
index 7d2e367..0046f38 100755
@@ -18,6 +18,7 @@ import signal
 from datetime import datetime, timedelta
 from distutils.version import LooseVersion
 import utils
+import operator
 
 import warnings
 warnings.filterwarnings("ignore", category=DeprecationWarning)
@@ -62,15 +63,13 @@ def run_command_interruptible(cmd):
     return process.returncode, buf
 
 
-def prepare_update_layer_command(options, branch, layer, updatedeps=False):
+def prepare_update_layer_command(options, branch, layer, initial=False):
     """Prepare the update_layer.py command line"""
     if branch.update_environment:
         cmdprefix = branch.update_environment.get_command()
     else:
         cmdprefix = 'python3'
     cmd = '%s update_layer.py -l %s -b %s' % (cmdprefix, layer.name, branch.name)
-    if updatedeps:
-        cmd += ' --update-dependencies'
     if options.reload:
         cmd += ' --reload'
     if options.fullreload:
@@ -79,6 +78,8 @@ def prepare_update_layer_command(options, branch, layer, updatedeps=False):
         cmd += ' --nocheckout'
     if options.dryrun:
         cmd += ' -n'
+    if initial:
+        cmd += ' -i'
     if options.loglevel == logging.DEBUG:
         cmd += ' -d'
     elif options.loglevel == logging.ERROR:
@@ -288,8 +289,74 @@ def main():
             # they never get used during normal operation).
             last_rev = {}
             for branch in branches:
+                # If layer_A depends(or recommends) on layer_B, add layer_B before layer_A
+                deps_dict_all = {}
+                layerquery_sorted = []
+                collections_done = set()
                 branchobj = utils.get_branch(branch)
+                for layer in layerquery_all:
+                    # Get all collections from database, but we can't trust the
+                    # one which will be updated since its collections maybe
+                    # changed (different from database).
+                    if layer in layerquery:
+                        continue
+                    layerbranch = layer.get_layerbranch(branch)
+                    if layerbranch:
+                        collections_done.add((layerbranch.collection, layerbranch.version))
+
                 for layer in layerquery:
+                    cmd = prepare_update_layer_command(options, branchobj, layer, initial=True)
+                    logger.debug('Running layer update command: %s' % cmd)
+                    ret, output = run_command_interruptible(cmd)
+                    key_string = "Initial layer values: "
+                    if not key_string in output:
+                        continue
+                    initial_values = output.split(key_string)[1].split(',')
+                    logger.debug('Initial layer values: %s' % initial_values)
+                    col = (initial_values[0] or '').strip()
+                    ver = initial_values[1] or ''
+                    deps = initial_values[2] or ''
+                    recs = (initial_values[3] or '').rstrip('\n')
+
+                    deps_dict = utils.explode_dep_versions2(bitbakepath, deps + ' ' + recs)
+                    if len(deps_dict) == 0:
+                        # No depends, add it firstly
+                        layerquery_sorted.append(layer)
+                        collections_done.add((col, ver))
+                        continue
+                    deps_dict_all[layer] = {'requires': deps_dict, 'collection': col, 'version': ver}
+
+                # Move deps_dict_all to layerquery_sorted orderly
+                logger.info("Sorting layers for branch %s" % branch)
+                while True:
+                    deps_dict_all_copy = deps_dict_all.copy()
+                    for layer, value in deps_dict_all_copy.items():
+                        for req_col, req_ver_list in value['requires'].copy().items():
+                            matched = False
+                            if req_ver_list:
+                                req_ver = req_ver_list[0]
+                            else:
+                                req_ver = None
+                            if utils.is_deps_satisfied(req_col, req_ver, collections_done):
+                                del(value['requires'][req_col])
+                        if not value['requires']:
+                            # All the depends are in collections_done:
+                            del(deps_dict_all[layer])
+                            layerquery_sorted.append(layer)
+                            collections_done.add((value['collection'], value['version']))
+
+                    if not len(deps_dict_all):
+                        break
+
+                    # Something is wrong if nothing changed after a run
+                    if operator.eq(deps_dict_all_copy, deps_dict_all):
+                        logger.error("Cannot find required collections on branch %s:" % branch)
+                        for layer, value in deps_dict_all.items():
+                            logger.error('%s: %s' % (layer.name, value['requires']))
+                        logger.error("Known collections: %s" % collections_done)
+                        sys.exit(1)
+
+                for layer in layerquery_sorted:
                     layerupdate = LayerUpdate()
                     layerupdate.update = update
 
@@ -306,9 +373,6 @@ def main():
                                 layerupdate.save()
                         continue
 
-                    urldir = layer.get_fetch_dir()
-                    repodir = os.path.join(fetchdir, urldir)
-
                     cmd = prepare_update_layer_command(options, branchobj, layer)
                     logger.debug('Running layer update command: %s' % cmd)
                     layerupdate.started = datetime.now()
@@ -329,31 +393,6 @@ def main():
                     if ret == 254:
                         # Interrupted by user, break out of loop
                         break
-
-            # Since update_layer may not be called in the correct order to have the
-            # dependencies created before trying to link them, we now have to loop
-            # back through all the branches and layers and try to link in the
-            # dependencies that may have been missed.  Note that creating the
-            # dependencies is a best-effort and continues if they are not found.
-            for branch in branches:
-                branchobj = utils.get_branch(branch)
-                for layer in layerquery:
-                    layerbranch = layer.get_layerbranch(branch)
-                    if layerbranch:
-                        if not (options.reload or options.fullreload):
-                            # Skip layers that did not change.
-                            layer_last_rev = last_rev.get(layerbranch, None)
-                            if layer_last_rev is None or layer_last_rev == layerbranch.vcs_last_rev:
-                                continue
-
-                        logger.info('Updating layer dependencies for %s on branch %s' % (layer.name, branch))
-                        cmd = prepare_update_layer_command(options, branchobj, layer, updatedeps=True)
-                        logger.debug('Running update dependencies command: %s' % cmd)
-                        ret, output = run_command_interruptible(cmd)
-                        if ret == 254:
-                            # Interrupted by user, break out of loop
-                            break
-
         finally:
             utils.unlock_file(lockfile)
 
layerindex/update_layer.py +13 -3
index bcf7056..93da3c4 100644
@@ -188,6 +188,9 @@ def main():
     parser.add_option("", "--nocheckout",
             help = "Don't check out branches",
             action="store_true", dest="nocheckout")
+    parser.add_option("-i", "--initial",
+            help = "Print initial layer value",
+            action="store_true")
     parser.add_option("-d", "--debug",
             help = "Enable debug output",
             action="store_const", const=logging.DEBUG, dest="loglevel", default=logging.INFO)
@@ -336,7 +339,7 @@ def main():
             layerdistros = Distro.objects.filter(layerbranch=layerbranch)
             layerappends = BBAppend.objects.filter(layerbranch=layerbranch)
             layerclasses = BBClass.objects.filter(layerbranch=layerbranch)
-            if layerbranch.vcs_last_rev != topcommit.hexsha or options.reload:
+            if layerbranch.vcs_last_rev != topcommit.hexsha or options.reload or options.initial:
                 # Check out appropriate branch
                 if not options.nocheckout:
                     utils.checkout_layer_branch(layerbranch, repodir, logger=logger)
@@ -361,6 +364,14 @@ def main():
                     layerconfparser.shutdown()
                     sys.exit(1)
                 utils.set_layerbranch_collection_version(layerbranch, layer_config_data, logger=logger)
+                if options.initial:
+                    # Use print() rather than logger.info() since "-q" makes it print nothing.
+                    print('Initial layer values: %s,%s,%s,%s' % (
+                        utils.get_layer_var(layer_config_data, 'BBFILE_COLLECTIONS'), \
+                        utils.get_layer_var(layer_config_data, 'LAYERVERSION'), \
+                        utils.get_layer_var(layer_config_data, 'LAYERDEPENDS'), \
+                        utils.get_layer_var(layer_config_data, 'LAYERRECOMMENDS')))
+                    sys.exit(0)
                 utils.add_dependencies(layerbranch, layer_config_data, logger=logger)
                 utils.add_recommends(layerbranch, layer_config_data, logger=logger)
                 layerbranch.save()
@@ -716,8 +727,7 @@ def main():
         import traceback
         traceback.print_exc()
     finally:
-        if LooseVersion(bb.__version__) > LooseVersion("1.27"):
-            tinfoil.shutdown()
+        utils.shutdown_tinfoil(tinfoil)
 
     shutil.rmtree(tempdir)
     sys.exit(0)
layerindex/utils.py +38 -0
index b7165d0..01a6200 100644
@@ -27,6 +27,33 @@ def get_layer(layername):
         return res[0]
     return None
 
+def get_layer_var(config_data, var):
+    collection = config_data.getVar('BBFILE_COLLECTIONS', True)
+    if collection:
+        collection = collection.strip()
+    value = config_data.getVar('%s_%s' % (var, collection), True)
+    if not value:
+        value = config_data.getVar(var, True)
+    return value or ''
+
+def is_deps_satisfied(req_col, req_ver, collections):
+    """ Check whether required collection and version are in collections"""
+    for existed_col, existed_ver in collections:
+        if req_col == existed_col:
+            # If there is no version constraint, return True when collection matches
+            if not req_ver:
+                return True
+            else:
+                # If there is no version in the found layer, then don't use this layer.
+                if not existed_ver:
+                    continue
+                (op, dep_version) = req_ver.split()
+                success = bb.utils.vercmp_string_op(existed_ver, dep_version, op)
+                if success:
+                    return True
+    # Return False when not found
+    return False
+
 def get_dependency_layer(depname, version_str=None, logger=None):
     from layerindex.models import LayerItem, LayerBranch
 
@@ -156,6 +183,17 @@ def setup_tinfoil(bitbakepath, enable_tracking):
 
     return tinfoil
 
+def shutdown_tinfoil(tinfoil):
+    if tinfoil and hasattr(tinfoil, 'shutdown'):
+        tinfoil.shutdown()
+
+def explode_dep_versions2(bitbakepath, deps):
+    bblib = bitbakepath + '/lib'
+    if not bblib in sys.path:
+        sys.path.insert(0, bblib)
+    import bb.utils
+    return bb.utils.explode_dep_versions2(deps)
+
 def checkout_layer_branch(layerbranch, repodir, logger=None):
 
     branchname = layerbranch.branch.name
Signature · 1 line
2.10.2
#15 lore raw

Re: [PATCH V4 06/10] update.py: update layers orderly

Hi Robert,

Thanks for solving this long-standing bug! Just a few notes below.

layerindex/update_layer.py def main(): · line 192

Quoted 8 lines · CEST Robert Yang
layerindex/update_layer.py +2 -0
@@ -188,6 +188,9 @@ def main():
     parser.add_option("", "--nocheckout",
             help = "Don't check out branches",
             action="store_true", dest="nocheckout")
+    parser.add_option("-i", "--initial",
+            help = "Print initial layer value",

Can we change the help text to:

"Print initial values parsed from layer.conf only"

Quoted 15 lines
layerindex/update_layer.py +2 -1
+            action="store_true")
     parser.add_option("-d", "--debug",
             help = "Enable debug output",
             action="store_const", const=logging.DEBUG, dest="loglevel", default=logging.INFO)
@@ -336,7 +339,7 @@ def main():
             layerdistros = Distro.objects.filter(layerbranch=layerbranch)
             layerappends = BBAppend.objects.filter(layerbranch=layerbranch)
             layerclasses = BBClass.objects.filter(layerbranch=layerbranch)
-            if layerbranch.vcs_last_rev != topcommit.hexsha or options.reload:
+            if layerbranch.vcs_last_rev != topcommit.hexsha or options.reload or options.initial:
                 # Check out appropriate branch
                 if not options.nocheckout:
                     utils.checkout_layer_branch(layerbranch, repodir, logger=logger)
@@ -361,6 +364,14 @@ def main():
                     layerconfparser.shutdown()

layerindex/update_layer.py def main(): · line 374

Quoted 10 lines
layerindex/update_layer.py +8 -0
                     sys.exit(1)
                 utils.set_layerbranch_collection_version(layerbranch, layer_config_data, logger=logger)
+                if options.initial:
+                    # Use print() rather than logger.info() since "-q" makes it print nothing.
+                    print('Initial layer values: %s,%s,%s,%s' % (
+                        utils.get_layer_var(layer_config_data, 'BBFILE_COLLECTIONS'), \
+                        utils.get_layer_var(layer_config_data, 'LAYERVERSION'), \
+                        utils.get_layer_var(layer_config_data, 'LAYERDEPENDS'), \
+                        utils.get_layer_var(layer_config_data, 'LAYERRECOMMENDS')))
+                    sys.exit(0)

This isn't meant to be human-readable, it's for update.py. Therefore I'd suggest a plain VARIABLENAME = "value" with one variable/value pair per line, and then these can be read by variable name rather than by index.

layerindex/update_layer.py def main(): · line 730

Quoted 10 lines
layerindex/update_layer.py +1 -2
                 utils.add_dependencies(layerbranch, layer_config_data, logger=logger)
                 utils.add_recommends(layerbranch, layer_config_data, logger=logger)
                 layerbranch.save()
@@ -716,8 +727,7 @@ def main():
         import traceback
         traceback.print_exc()
     finally:
-        if LooseVersion(bb.__version__) > LooseVersion("1.27"):
-            tinfoil.shutdown()
+        utils.shutdown_tinfoil(tinfoil)

This change is not mentioned in the commit message and if you look at 19a559cfa665b1add51fad41b7db88237fa82a09 you'll see I check the version intentionally because fido has a broken shutdown() function.

layerindex/utils.py def get_layer(layername): · line 31

Quoted 12 lines
layerindex/update_layer.py +0 -0
     shutil.rmtree(tempdir)
     sys.exit(0)
layerindex/utils.py +2 -0
index b7165d0..01a6200 100644
@@ -27,6 +27,33 @@ def get_layer(layername):
         return res[0]
     return None
 
+def get_layer_var(config_data, var):
+    collection = config_data.getVar('BBFILE_COLLECTIONS', True)

This makes the assumption that the layer.conf only adds one collection to BBFILE_COLLECTIONS. Probably a valid assumption 99.9% of the time these days, but it is worth noting.

Quoted 29 lines
layerindex/utils.py +25 -0
+    if collection:
+        collection = collection.strip()
+    value = config_data.getVar('%s_%s' % (var, collection), True)
+    if not value:
+        value = config_data.getVar(var, True)
+    return value or ''
+
+def is_deps_satisfied(req_col, req_ver, collections):
+    """ Check whether required collection and version are in collections"""
+    for existed_col, existed_ver in collections:
+        if req_col == existed_col:
+            # If there is no version constraint, return True when collection matches
+            if not req_ver:
+                return True
+            else:
+                # If there is no version in the found layer, then don't use this layer.
+                if not existed_ver:
+                    continue
+                (op, dep_version) = req_ver.split()
+                success = bb.utils.vercmp_string_op(existed_ver, dep_version, op)
+                if success:
+                    return True
+    # Return False when not found
+    return False
+
 def get_dependency_layer(depname, version_str=None, logger=None):
     from layerindex.models import LayerItem, LayerBranch
 

layerindex/utils.py def setup_tinfoil(bitbakepath, enable_tracking): · line 188

Quoted 6 lines
layerindex/utils.py +3 -0
@@ -156,6 +183,17 @@ def setup_tinfoil(bitbakepath, enable_tracking):
 
     return tinfoil
 
+def shutdown_tinfoil(tinfoil):
+    if tinfoil and hasattr(tinfoil, 'shutdown'):
+        tinfoil.shutdown()

See above.

layerindex/utils.py def setup_tinfoil(bitbakepath, enable_tracking): · line 195

Quoted 6 lines
layerindex/utils.py +6 -0
+def explode_dep_versions2(bitbakepath, deps):
+    bblib = bitbakepath + '/lib'
+    if not bblib in sys.path:
+        sys.path.insert(0, bblib)
+    import bb.utils
+    return bb.utils.explode_dep_versions2(deps)

I'm not particularly happy with this (the wrapper nor calling bitbake code from update.py) but I suppose it's marginally better than duplicating explode_dep_versions2().

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#16 lore raw

Re: [PATCH V4 06/10] update.py: update layers orderly

Hi Paul,

Thanks for the review, I've fixed all your comments except the following ones.

Quoted 7 lines · Paul Eggleton

Hi Robert,

Thanks for solving this long-standing bug! Just a few notes below.

Quoted 2 lines · CEST Robert Yang
layerindex/update_layer.py +0 -0

...

Quoted 6 lines

layerindex/update_layer.py

Quoted 2 lines
layerindex/update_layer.py +2 -0
+def get_layer_var(config_data, var):
+    collection = config_data.getVar('BBFILE_COLLECTIONS', True)

This makes the assumption that the layer.conf only adds one collection to BBFILE_COLLECTIONS. Probably a valid assumption 99.9% of the time these days, but it is worth noting.

Should we check all BBFILE_COLLECTIONS, please ?

Quoted 9 lines
Quoted 9 lines
layerindex/update_layer.py +7 -0
+    if collection:
+        collection = collection.strip()
+    value = config_data.getVar('%s_%s' % (var, collection), True)
+    if not value:
+        value = config_data.getVar(var, True)
+    return value or ''
+

...

Quoted 10 lines

layerindex/update_layer.py

Quoted 8 lines
layerindex/update_layer.py +6 -0
+def explode_dep_versions2(bitbakepath, deps):
+    bblib = bitbakepath + '/lib'
+    if not bblib in sys.path:
+        sys.path.insert(0, bblib)
+    import bb.utils
+    return bb.utils.explode_dep_versions2(deps)

I'm not particularly happy with this (the wrapper nor calling bitbake code

update.py needs explode_dep_versions2 to get dependencies and then calculate them, I had tried to move the code into update_layer.py, and let update_layer.py print explode_dep_versions2() to update.py, but it is a OderedDic, and not easy to process it manually, so I leave the code in update.py.

I will send a V5 after we've made agreements.

// Robert

Quoted 6 lines

from update.py) but I suppose it's marginally better than duplicating explode_dep_versions2().

Cheers,
Paul

#17 lore raw

Re: [PATCH V4 06/10] update.py: update layers orderly

Hi Robert,

Quoted 5 lines · CEST Robert Yang
Quoted 3 lines · Paul Eggleton
Quoted 2 lines · CEST Robert Yang
layerindex/update_layer.py +0 -0

...

Quoted 8 lines
Quoted 6 lines

layerindex/update_layer.py

Quoted 2 lines
layerindex/update_layer.py +2 -0
+def get_layer_var(config_data, var):
+    collection = config_data.getVar('BBFILE_COLLECTIONS', True)

This makes the assumption that the layer.conf only adds one collection to BBFILE_COLLECTIONS. Probably a valid assumption 99.9% of the time these days, but it is worth noting.

Should we check all BBFILE_COLLECTIONS, please ?

I would say let's split the value and use the first one, but if there is more than one then warn e.g. "Multiple collections found, handling first one (%s) only" but otherwise continue processing. That way at least we'll know if this situation occurs.

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
layerindex/update_layer.py +6 -0
+def explode_dep_versions2(bitbakepath, deps):
+    bblib = bitbakepath + '/lib'
+    if not bblib in sys.path:
+        sys.path.insert(0, bblib)
+    import bb.utils
+    return bb.utils.explode_dep_versions2(deps)
Quoted 7 lines
Quoted 1 line

I'm not particularly happy with this (the wrapper nor calling bitbake code

update.py needs explode_dep_versions2 to get dependencies and then calculate them, I had tried to move the code into update_layer.py, and let update_layer.py print explode_dep_versions2() to update.py, but it is a OderedDic, and not easy to process it manually, so I leave the code in update.py.

Right, I understand the limitations and I don't really have a better alternative unfortunately, so we'll go with your current implementation. We may need to re-evaluate this in future at least structure-wise if we find the need to call other bitbake code in a similar way.

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#18 lore raw

Re: [PATCH V4 06/10] update.py: update layers orderly

Quoted 15 lines · Paul Eggleton

Hi Robert,

Quoted 12 lines · CEST Robert Yang
Quoted 3 lines · Paul Eggleton
Quoted 2 lines · CEST Robert Yang
layerindex/update_layer.py +0 -0

...

Quoted 6 lines

layerindex/update_layer.py

Quoted 2 lines
layerindex/update_layer.py +2 -0
+def get_layer_var(config_data, var):
+    collection = config_data.getVar('BBFILE_COLLECTIONS', True)

This makes the assumption that the layer.conf only adds one collection to BBFILE_COLLECTIONS. Probably a valid assumption 99.9% of the time these days, but it is worth noting.

Quoted 6 lines
Quoted 1 line

Should we check all BBFILE_COLLECTIONS, please ?

I would say let's split the value and use the first one, but if there is more than one then warn e.g. "Multiple collections found, handling first one (%s) only" but otherwise continue processing. That way at least we'll know if this situation occurs.

Thanks, good idea, I will update it in V5.

// Robert

Quoted 25 lines
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
layerindex/update_layer.py +6 -0
+def explode_dep_versions2(bitbakepath, deps):
+    bblib = bitbakepath + '/lib'
+    if not bblib in sys.path:
+        sys.path.insert(0, bblib)
+    import bb.utils
+    return bb.utils.explode_dep_versions2(deps)
Quoted 7 lines
Quoted 1 line

I'm not particularly happy with this (the wrapper nor calling bitbake code

update.py needs explode_dep_versions2 to get dependencies and then calculate them, I had tried to move the code into update_layer.py, and let update_layer.py print explode_dep_versions2() to update.py, but it is a OderedDic, and not easy to process it manually, so I leave the code in update.py.

Right, I understand the limitations and I don't really have a better alternative unfortunately, so we'll go with your current implementation. We may need to re-evaluate this in future at least structure-wise if we find the need to call other bitbake code in a similar way.

Cheers,
Paul

#19 lore raw

PATCH v4 7/10 update_layer.py: remove --update-dependencies

In reply to Robert Yang

It never works since it is in the middle of transaction.atomic() block, and update.py doesn't need it any more, so remove it.

---
 layerindex/update_layer.py | 27 ---------------------------
 1 file changed, 27 deletions(-)
layerindex/update_layer.py +0 -27
index 93da3c4..36a9025 100644
@@ -182,9 +182,6 @@ def main():
     parser.add_option("-n", "--dry-run",
             help = "Don't write any data back to the database",
             action="store_true", dest="dryrun")
-    parser.add_option("", "--update-dependencies",
-            help = "Update layer dependencies only",
-            action="store_true", dest="updatedeps")
     parser.add_option("", "--nocheckout",
             help = "Don't check out branches",
             action="store_true", dest="nocheckout")
@@ -256,30 +253,6 @@ def main():
                     branchname = layerbranch.actual_branch
                     branchdesc = "%s (%s)" % (options.branch, branchname)
 
-            if options.updatedeps:
-                # Update layer dependencies only
-                if not layerbranch:
-                    logger.debug('Skipping dependency update for layer %s on branch %s - no layerbranch record' % (layer, branchdesc))
-                    sys.exit(0)
-                if not options.nocheckout:
-                    utils.checkout_layer_branch(layerbranch, repodir, logger=logger)
-                layerdir = os.path.join(repodir, layerbranch.vcs_subdir)
-                if not os.path.exists(layerdir):
-                    # If this happens it was already flagged during the main update, so ignore it
-                    logger.debug('Skipping dependency update for layer %s on branch %s - layer directory not found' % (layer, branchdesc))
-                    sys.exit(0)
-
-                layerconfparser = layerconfparse.LayerConfParse(logger=logger, bitbakepath=bitbakepath, tinfoil=tinfoil)
-                config_data = layerconfparser.parse_layer(layerdir)
-                if not config_data:
-                    logger.debug("Layer %s does not appear to be valid for branch %s" % (layer.name, branchdesc))
-                    sys.exit(0)
-
-                utils.add_dependencies(layerbranch, config_data, logger=logger)
-                utils.add_recommends(layerbranch, config_data, logger=logger)
-
-                sys.exit(0)
-
             # Collect repo info
             repo = git.Repo(repodir)
             assert repo.bare == False
Signature · 1 line
2.10.2
#20 lore raw

PATCH v4 8/10 update_layer.py: move the location of transaction.atomic()

In reply to Robert Yang

It doesn't need to be so ahead since we only need it when writing database, and a following patch will remove layerbranch from database when the branch had been removed from the repo, it's not easy to do the work in transaction.atomic() block.

---
 layerindex/update_layer.py | 56 +++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 28 deletions(-)
layerindex/update_layer.py +28 -28
index 36a9025..3990ad6 100644
@@ -238,36 +238,36 @@ def main():
     # why won't they just fix that?!)
     tinfoil.config_data.setVar('LICENSE', '')
 
+    layer = utils.get_layer(options.layer)
+    urldir = layer.get_fetch_dir()
+    repodir = os.path.join(fetchdir, urldir)
+
+    layerbranch = layer.get_layerbranch(options.branch)
+
+    branchname = options.branch
+    branchdesc = options.branch
+    if layerbranch:
+        if layerbranch.actual_branch:
+            branchname = layerbranch.actual_branch
+            branchdesc = "%s (%s)" % (options.branch, branchname)
+
+    # Collect repo info
+    repo = git.Repo(repodir)
+    assert repo.bare == False
     try:
-        with transaction.atomic():
-            layer = utils.get_layer(options.layer)
-            urldir = layer.get_fetch_dir()
-            repodir = os.path.join(fetchdir, urldir)
-
-            layerbranch = layer.get_layerbranch(options.branch)
-
-            branchname = options.branch
-            branchdesc = options.branch
-            if layerbranch:
-                if layerbranch.actual_branch:
-                    branchname = layerbranch.actual_branch
-                    branchdesc = "%s (%s)" % (options.branch, branchname)
-
-            # Collect repo info
-            repo = git.Repo(repodir)
-            assert repo.bare == False
-            try:
-                if options.nocheckout:
-                    topcommit = repo.commit('HEAD')
-                else:
-                    topcommit = repo.commit('origin/%s' % branchname)
-            except:
-                if layerbranch:
-                    logger.error("Failed update of layer %s - branch %s no longer exists" % (layer.name, branchdesc))
-                else:
-                    logger.info("Skipping update of layer %s - branch %s doesn't exist" % (layer.name, branchdesc))
-                sys.exit(1)
+        if options.nocheckout:
+            topcommit = repo.commit('HEAD')
+        else:
+            topcommit = repo.commit('origin/%s' % branchname)
+    except:
+        if layerbranch:
+            logger.error("Failed update of layer %s - branch %s no longer exists" % (layer.name, branchdesc))
+        else:
+            logger.info("Skipping update of layer %s - branch %s doesn't exist" % (layer.name, branchdesc))
+        sys.exit(1)
 
+    try:
+        with transaction.atomic():
             newbranch = False
             if not layerbranch:
                 # LayerBranch doesn't exist for this branch, create it
Signature · 1 line
2.10.2
#21 lore raw

PATCH v4 9/10 update.py: add -p to git fetch

In reply to Robert Yang

-p, --prune
Before fetching, remove any remote-tracking references that no longer exist on the remote.

Fixed:

$ git push origin :test_branch
$ ./update.py

The test_branch was still in fetched local repo which was incorrect, it should be gone since it has been removed by upstream.

---
 layerindex/update.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
layerindex/update.py +2 -2
index 0046f38..1512743 100755
@@ -262,7 +262,7 @@ def main():
                             if not os.path.exists(repodir):
                                 out = utils.runcmd("git clone %s %s" % (layer.vcs_url, urldir), fetchdir, logger=logger, printerr=False)
                             else:
-                                out = utils.runcmd("git fetch", repodir, logger=logger, printerr=False)
+                                out = utils.runcmd("git fetch -p", repodir, logger=logger, printerr=False)
                         except subprocess.CalledProcessError as e:
                             logger.error("Fetch of layer %s failed: %s" % (layer.name, e.output))
                             failedrepos[layer.vcs_url] = e.output
@@ -277,7 +277,7 @@ def main():
                 if not os.path.exists(bitbakepath):
                     out = utils.runcmd("git clone %s %s" % (settings.BITBAKE_REPO_URL, 'bitbake'), fetchdir, logger=logger)
                 else:
-                    out = utils.runcmd("git fetch", bitbakepath, logger=logger)
+                    out = utils.runcmd("git fetch -p", bitbakepath, logger=logger)
 
             if options.actual_branch:
                 update_actual_branch(layerquery, fetchdir, branches[0], options, update_bitbake, bitbakepath)
Signature · 1 line
2.10.2
#22 lore raw

PATCH v4 10/10 update_layer.py: delete layerbranch for non-existed branch

In reply to Robert Yang

The branch is not needed any more when it has been removed from the repo, so we also need remove its layerbranch, otherwise it still can be got from the web, which causes confusions.

Note, we have to move the location of tinfiol's code to make it can be shutdown correctly.

---
 layerindex/update_layer.py | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)
layerindex/update_layer.py +18 -16
index 3990ad6..85e05b7 100644
@@ -223,21 +223,6 @@ def main():
 
     bitbakepath = os.path.join(fetchdir, 'bitbake')
 
-    try:
-        (tinfoil, tempdir) = recipeparse.init_parser(settings, branch, bitbakepath, nocheckout=options.nocheckout, logger=logger)
-    except recipeparse.RecipeParseError as e:
-        logger.error(str(e))
-        sys.exit(1)
-    # Clear the default value of SUMMARY so that we can use DESCRIPTION instead if it hasn't been set
-    tinfoil.config_data.setVar('SUMMARY', '')
-    # Clear the default value of DESCRIPTION so that we can see where it's not set
-    tinfoil.config_data.setVar('DESCRIPTION', '')
-    # Clear the default value of HOMEPAGE ('unknown')
-    tinfoil.config_data.setVar('HOMEPAGE', '')
-    # Set a blank value for LICENSE so that it doesn't cause the parser to die (e.g. with meta-ti -
-    # why won't they just fix that?!)
-    tinfoil.config_data.setVar('LICENSE', '')
-
     layer = utils.get_layer(options.layer)
     urldir = layer.get_fetch_dir()
     repodir = os.path.join(fetchdir, urldir)
@@ -261,12 +246,29 @@ def main():
             topcommit = repo.commit('origin/%s' % branchname)
     except:
         if layerbranch:
-            logger.error("Failed update of layer %s - branch %s no longer exists" % (layer.name, branchdesc))
+            logger.info("layer %s - branch %s no longer exists, removing it from database" % (layer.name, branchdesc))
+            if not options.dryrun:
+                layerbranch.delete()
         else:
             logger.info("Skipping update of layer %s - branch %s doesn't exist" % (layer.name, branchdesc))
         sys.exit(1)
 
     try:
+        (tinfoil, tempdir) = recipeparse.init_parser(settings, branch, bitbakepath, nocheckout=options.nocheckout, logger=logger)
+    except recipeparse.RecipeParseError as e:
+        logger.error(str(e))
+        sys.exit(1)
+    # Clear the default value of SUMMARY so that we can use DESCRIPTION instead if it hasn't been set
+    tinfoil.config_data.setVar('SUMMARY', '')
+    # Clear the default value of DESCRIPTION so that we can see where it's not set
+    tinfoil.config_data.setVar('DESCRIPTION', '')
+    # Clear the default value of HOMEPAGE ('unknown')
+    tinfoil.config_data.setVar('HOMEPAGE', '')
+    # Set a blank value for LICENSE so that it doesn't cause the parser to die (e.g. with meta-ti -
+    # why won't they just fix that?!)
+    tinfoil.config_data.setVar('LICENSE', '')
+
+    try:
         with transaction.atomic():
             newbranch = False
             if not layerbranch:
Signature · 1 line
2.10.2
#23 lore raw

Re: [PATCH V4 10/10] update_layer.py: delete layerbranch for non-existed branch

Quoted 3 lines · CEST Robert Yang

The branch is not needed any more when it has been removed from the repo, so we also need remove its layerbranch, otherwise it still can be got from the web, which causes confusions.

I think I'd previously avoided this since I was concerned we'd potentially lose data about a branch which was manually gathered (such as maintainer information) however I can't actually think of a scenario now where you would lose valuable data - if the branch is gone then there's not much point keeping a reference to it around, so this should be OK.

Quoted 2 lines

Note, we have to move the location of tinfiol's code to make it can be shutdown correctly.

How does this relate to the rest of this change? Also, can you explain the issue you were hitting? The try...finally is supposed to ensure it gets shut down.

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre
#24 lore raw

Re: [PATCH V4 10/10] update_layer.py: delete layerbranch for non-existed branch

Quoted 11 lines · Paul Eggleton
Quoted 3 lines · CEST Robert Yang

The branch is not needed any more when it has been removed from the repo, so we also need remove its layerbranch, otherwise it still can be got from the web, which causes confusions.

I think I'd previously avoided this since I was concerned we'd potentially lose data about a branch which was manually gathered (such as maintainer information) however I can't actually think of a scenario now where you would lose valuable data - if the branch is gone then there's not much point keeping a reference to it around, so this should be OK.

Quoted 6 lines
Quoted 2 lines

Note, we have to move the location of tinfiol's code to make it can be shutdown correctly.

How does this relate to the rest of this change? Also, can you explain the issue you were hitting? The try...finally is supposed to ensure it gets shut down.

I updated the commit message:

update_layer.py: delete layerbranch for non-existed branch

The branch is not needed any more when it has been removed from the repo, so we also need remove its layerbranch, otherwise it still can be got from the web, which causes confusions.

Note, we have to move the location of tinfiol's code to avoid creating tinfoil when not needed, otherewise, if tinfoil is created (but not needed), and the program exists earlier before "try ... finally" block, then tinfoil.shutdown() doesn't run so that it is not shutdown. Move the code back, right before where it is needed can fix the problem.

// Robert

Quoted 5 lines

Cheers,
Paul

#25 lore raw

Re: [PATCH V4 10/10] update_layer.py: delete layerbranch for non-existed branch

Quoted 17 lines · CEST Robert Yang
Quoted 7 lines · Paul Eggleton
Quoted 2 lines · CEST Robert Yang

Note, we have to move the location of tinfiol's code to make it can be shutdown correctly.

How does this relate to the rest of this change? Also, can you explain the issue you were hitting? The try...finally is supposed to ensure it gets shut down.

I updated the commit message:

update_layer.py: delete layerbranch for non-existed branch

The branch is not needed any more when it has been removed from the repo, so we also need remove its layerbranch, otherwise it still can be got from the web, which causes confusions.

Quoted 5 lines

Note, we have to move the location of tinfiol's code to avoid creating tinfoil when not needed, otherewise, if tinfoil is created (but not needed), and the program exists earlier before "try ... finally" block, then tinfoil.shutdown() doesn't run so that it is not shutdown. Move the code back, right before where it is needed can fix the problem.

OK, that explains the change, thanks.

Cheers,
Paul

Signature · 2 lines
Paul Eggleton
Intel Open Source Technology Centre