Several fixes for layerindex
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
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
@@ -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
@@ -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
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.Signed-off-by Robert Yang <[email protected]>--- layerindex/update.py | 81 ++++++++++++++++++++++++++++++++++++++++++++++------ layerindex/utils.py | 11 +++++++ 2 files changed, 83 insertions(+), 9 deletions(-)layerindex/update.py +3
-0@@ -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 whichlayerindex/utils.py +0
-0@@ -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
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
PATCH v4 2/10 layerconfparse.py: remove unused layerbranch from parse_layer()
In reply to Robert YangThe 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
@@ -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
@@ -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
@@ -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
PATCH v4 3/10 utils.py: remove obsolete dependencies
In reply to Robert YangFixed:
- 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
@@ -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
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.
Signed-off-by Robert Yang <[email protected]>--- layerindex/utils.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)layerindex/utils.py +0
-0@@ -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
-0logger.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
-0return + 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
-0logger.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
-0continue + # 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
-0if 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
-0continue@@ -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
-0layerdep.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
-0layerbranch.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
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.
Signed-off-by Robert Yang <[email protected]>--- layerindex/utils.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)layerindex/utils.py +0
-0@@ -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
-0logger.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
-0return + 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=Tlayerindex/utils.py def _add_dependency(var, name, layerbranch, · line 107
Quoted 1 line
layerindex/utils.py +0
-0logger.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
-0continue + # 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
-0if 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
-0continue@@ -121,6 +128,10 @@ def _add_dependency(var, name, layerbranch,
config_data, logger=None, required=Tlayerindex/utils.py def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T
Quoted 8 lines
layerindex/utils.py +4
-0layerdep.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
-0layerbranch.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
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.confrather than fix it from database ? Ifconf/layer.confis 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
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.confrather than fix it from database ? Ifconf/layer.confis 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
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.confrather than fix it from database ? Ifconf/layer.confis 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
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 inconf/layer.confREMOVE_LAYER_DEPENDENCIES= FalseCurrently it does can add dependencies from
conf/layer.conf, but no remove, so useREMOVE_LAYER_DEPENDENCIESmakes it more clear.
Sounds good to me.
Cheers,
Paul
Signature · 2 lines
Paul Eggleton Intel Open Source Technology Centre
PATCH v4 4/10 update_layer.py: set layerbranch's collection before add_dependencies
In reply to Robert YangThe _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
@@ -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
PATCH v4 5/10 recipeparse.py: restore cwd when the parsing is done
In reply to Robert YangOthewise it may cause troubles to the function who calls it.
--- layerindex/recipeparse.py | 3 +++ 1 file changed, 3 insertions(+)
layerindex/recipeparse.py +3 -0
@@ -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
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
@@ -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
@@ -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
@@ -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
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
-0sys.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
-2utils.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
-0shutil.rmtree(tempdir) sys.exit(0)layerindex/utils.py +2
-0@@ -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
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 duplicatingexplode_dep_versions2().Cheers,
Paul
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.pyneedsexplode_dep_versions2to get dependencies and then calculate them, I had tried to move the code intoupdate_layer.py, and letupdate_layer.pyprintexplode_dep_versions2()toupdate.py, but it is a OderedDic, and not easy to process it manually, so I leave the code inupdate.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
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.pyneedsexplode_dep_versions2to get dependencies and then calculate them, I had tried to move the code intoupdate_layer.py, and letupdate_layer.pyprintexplode_dep_versions2()toupdate.py, but it is a OderedDic, and not easy to process it manually, so I leave the code inupdate.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
PATCH v4 7/10 update_layer.py: remove --update-dependencies
In reply to Robert YangIt 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
@@ -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
PATCH v4 8/10 update_layer.py: move the location of transaction.atomic()
In reply to Robert YangIt 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
@@ -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
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
@@ -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
PATCH v4 10/10 update_layer.py: delete layerbranch for non-existed branch
In reply to Robert YangThe 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
@@ -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
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
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
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 branchThe 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