[layerindex-web][PATCH 0/7] Layer index fixes

[layerindex-web][PATCH 0/7] Layer index fixes

[Paul Eggleton]

Some bug fixes as well as site-wide notice support.


The following changes since commit 5cfdfdca8b9200b5a6e4aa14661b14059a799310:

  rrs_upstream_history.py: fix set_regexes function (2018-06-06 11:25:46 +1200)

are available in the Git repository at:

  git://git.yoctoproject.org/layerindex-web paule/fixes3
  http://git.yoctoproject.org/cgit.cgi/layerindex-web/log/?h=paule/fixes3

Paul Eggleton (7):
  settings: allow disabling layer publishing emails
  update_layer: drop debug message for deleting recipe file dependencies
  update_layer: avoid errors on modified & renamed files
  recipeparse: don't error out on missing layer recommends
  update: ignore recommends when ordering layers
  utils: fix missing dependency logic in _add_dependency()
  Add site-wide notice support

 TODO                                     |  1 -
 layerindex/admin.py                      |  1 +
 layerindex/context_processors.py         |  7 ++-
 layerindex/migrations/0014_sitenotice.py | 24 +++++++++
 layerindex/models.py                     | 25 ++++++++-
 layerindex/recipeparse.py                |  6 ++-
 layerindex/update.py                     |  2 +-
 layerindex/update_layer.py               |  3 +-
 layerindex/utils.py                      | 21 +++++++-
 layerindex/views.py                      | 66 ++++++++++++------------
 requirements.txt                         |  1 +
 settings.py                              |  3 ++
 templates/base.html                      |  5 ++
 13 files changed, 123 insertions(+), 42 deletions(-)
 create mode 100644 layerindex/migrations/0014_sitenotice.py

-- 
2.17.1

[layerindex-web][PATCH 1/7] settings: allow disabling layer publishing emails

[Paul Eggleton]

If you're running a testing / internal instance then you really don't
want to be emailing maintainers on publish, so provide a setting you can
use to disable that.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 layerindex/views.py | 66 +++++++++++++++++++++++----------------------
 settings.py         |  3 +++
 2 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/layerindex/views.py b/layerindex/views.py
index a905c347..d02e4f91 100644
--- a/layerindex/views.py
+++ b/layerindex/views.py
@@ -259,38 +259,40 @@ def _check_url_branch(kwargs):
 def publish_view(request, name):
     if not (request.user.is_authenticated() and request.user.has_perm('layerindex.publish_layer')):
         raise PermissionDenied
-    layeritem = get_object_or_404(LayerItem, name=name)
-    layerbranch = get_object_or_404(LayerBranch, layer=layeritem)
-    layer_url = request.build_absolute_uri(reverse('layer_item', args=(layerbranch.branch, layeritem.name)))
-    maintainers = get_list_or_404(LayerMaintainer, layerbranch=layerbranch)
-    from_email = settings.SUBMIT_EMAIL_FROM
-    subjecttext = get_template('layerindex/publishemailsubject.txt')
-    bodytext = get_template('layerindex/publishemail.txt')
-    maintainer_names = [m.name for m in maintainers]
-    # find appropriate help contact
-    help_contact = None
-    for user in User.objects.all():
-        if user.username != 'root' and (user.is_staff or user.is_superuser) and user.is_active:
-            help_contact = user
-            break
-
-    # create subject from subject template
-    d = {
-        'layer_name': layeritem.name,
-        'site_name': request.META['HTTP_HOST'],
-    }
-    subject = subjecttext.render(d).rstrip()
-
-    #create body from body template
-    d = {
-        'maintainers': maintainer_names,
-        'layer_name': layeritem.name,
-        'layer_url': layer_url,
-        'help_contact': help_contact,
-    }
-    body = bodytext.render(d)
-
-    tasks.send_email.apply_async((subject, body, from_email, [m.email for m in maintainers]))
+
+    if getattr(settings, 'SEND_PUBLISH_EMAIL', True):
+        layeritem = get_object_or_404(LayerItem, name=name)
+        layerbranch = get_object_or_404(LayerBranch, layer=layeritem)
+        layer_url = request.build_absolute_uri(reverse('layer_item', args=(layerbranch.branch, layeritem.name)))
+        maintainers = get_list_or_404(LayerMaintainer, layerbranch=layerbranch)
+        from_email = settings.SUBMIT_EMAIL_FROM
+        subjecttext = get_template('layerindex/publishemailsubject.txt')
+        bodytext = get_template('layerindex/publishemail.txt')
+        maintainer_names = [m.name for m in maintainers]
+        # find appropriate help contact
+        help_contact = None
+        for user in User.objects.all():
+            if user.username != 'root' and (user.is_staff or user.is_superuser) and user.is_active:
+                help_contact = user
+                break
+
+        # create subject from subject template
+        d = {
+            'layer_name': layeritem.name,
+            'site_name': request.META['HTTP_HOST'],
+        }
+        subject = subjecttext.render(d).rstrip()
+
+        #create body from body template
+        d = {
+            'maintainers': maintainer_names,
+            'layer_name': layeritem.name,
+            'layer_url': layer_url,
+            'help_contact': help_contact,
+        }
+        body = bodytext.render(d)
+
+        tasks.send_email.apply_async((subject, body, from_email, [m.email for m in maintainers]))
 
     return _statuschange(request, name, 'P')
 
diff --git a/settings.py b/settings.py
index c3075a5e..3d70aaa5 100644
--- a/settings.py
+++ b/settings.py
@@ -227,6 +227,9 @@ FORCE_REVIEW_HTTPS = False
 SUBMIT_EMAIL_FROM = 'noreply@example.com'
 SUBMIT_EMAIL_SUBJECT = 'OE Layerindex layer submission'
 
+# Send email to maintainer(s) when their layer is published
+SEND_PUBLISH_EMAIL = True
+
 # RabbitMQ settings
 RABBIT_BROKER = 'amqp://'
 RABBIT_BACKEND = 'rpc://'
-- 
2.17.1

[layerindex-web][PATCH 2/7] update_layer: drop debug message for deleting recipe file dependencies

[Paul Eggleton]

This was too noisy and not particularly useful.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 layerindex/update_layer.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/layerindex/update_layer.py b/layerindex/update_layer.py
index bbfaba9a..018d7bcd 100644
--- a/layerindex/update_layer.py
+++ b/layerindex/update_layer.py
@@ -228,7 +228,6 @@ def update_recipe_file(tinfoil, data, path, recipe, layerdir_start, repodir, ski
             recipedep.save()
 
         for filedep in recipedeps_delete:
-            logger.debug('%s: removing %s' % (recipe.layerbranch, filedep))
             recipedeps.filter(path=filedep).delete()
 
     except KeyboardInterrupt:
-- 
2.17.1

[layerindex-web][PATCH 3/7] update_layer: avoid errors on modified & renamed files

[Paul Eggleton]

If a file is modified and renamed it will show up in both
iter_change_type('M') and iter_change_type('R'), however naturally the
file that will exist will be the b path and not the a one, so we should
be looking at the b path or we will get errors.

FYI you can reproduce this with OE-Core (in a scratch database) using
the following procedure:
1) (in the OE-Core layer directory):
   git checkout 59285b324f6d9ed270b0bef209ef5da22a620a83
2) update.py -l openembedded-core -b master -x --nofetch -r --fullreload
3) (in the OE-Core layer directory):
   git checkout 086308aa2a5e332de6f00ed397c4a55d132f158f
4) update.py -l openembedded-core -b master -x --nofetch

Without this change you'll see the following error:

ERROR: Unable to read /opt/layerindex/layers/git___git_openembedded_org_openembedded-core/meta/recipes-devtools/python-numpy/python-numpy_1.13.1.bb: Traceback (most recent call last):
  File "/opt/layerindex/layers/bitbake/lib/bb/command.py", line 84, in runCommand
    result = command_method(self, commandline)
  File "/opt/layerindex/layers/bitbake/lib/bb/command.py", line 568, in parseRecipeFile
    envdata = bb.cache.parse_recipe(config_data, fn, appendfiles)['']
  File "/opt/layerindex/layers/bitbake/lib/bb/cache.py", line 315, in parse_recipe
    bb_data = bb.parse.handle(bbfile, bb_data)
  File "/opt/layerindex/layers/bitbake/lib/bb/parse/__init__.py", line 117, in handle
    return h['handle'](fn, data, include)
  File "/opt/layerindex/layers/bitbake/lib/bb/parse/parse_py/BBHandler.py", line 132, in handle
    abs_fn = resolve_file(fn, d)
  File "/opt/layerindex/layers/bitbake/lib/bb/parse/__init__.py", line 141, in resolve_file
    raise IOError(errno.ENOENT, "file %s not found" % fn)
FileNotFoundError: [Errno 2] file /opt/layerindex/layers/git___git_openembedded_org_openembedded-core/meta/recipes-devtools/python-numpy/python-numpy_1.13.1.bb not found

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 TODO                       | 1 -
 layerindex/update_layer.py | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/TODO b/TODO
index 96d4feb8..7d522f84 100644
--- a/TODO
+++ b/TODO
@@ -10,7 +10,6 @@ TODO:
 
 Bugs
 * Duplication of first maintainer when editing to add a second?
-* GitPython sometimes reports renamed files as type M but we don't handle that properly e.g. OE-Core 59285b324f6d9ed270b0bef209ef5da22a620a83 086308aa2a5e332de6f00ed397c4a55d132f158f
 
 Other
 * Full-text search on layer contents
diff --git a/layerindex/update_layer.py b/layerindex/update_layer.py
index 018d7bcd..272fe51a 100644
--- a/layerindex/update_layer.py
+++ b/layerindex/update_layer.py
@@ -636,7 +636,7 @@ def main():
                                 bbclass.save()
 
                     for diffitem in diff.iter_change_type('M'):
-                        path = diffitem.a_blob.path
+                        path = diffitem.b_blob.path
                         if path.startswith(subdir_start):
                             skip = False
                             for removedir in removedirs:
-- 
2.17.1

[layerindex-web][PATCH 4/7] recipeparse: don't error out on missing layer recommends

[Paul Eggleton]

If a LAYERRECOMMENDS relationship is not satisfied, we shouldn't be
erroring out - it's a recommendation, not a hard dependency. Just show a
warning and allow processing to continue.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 layerindex/recipeparse.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/layerindex/recipeparse.py b/layerindex/recipeparse.py
index f1c1bd32..bc8a19ef 100644
--- a/layerindex/recipeparse.py
+++ b/layerindex/recipeparse.py
@@ -102,7 +102,11 @@ def setup_layer(config_data, fetchdir, layerdir, layer, layerbranch):
         deprepodir = os.path.join(fetchdir, depurldir)
         deplayerbranch = dep.dependency.get_layerbranch(layerbranch.branch.name)
         if not deplayerbranch:
-            raise RecipeParseError('Dependency %s of layer %s does not have branch record for branch %s' % (dep.dependency.name, layer.name, layerbranch.branch.name))
+            if dep.required:
+                raise RecipeParseError('Dependency %s of layer %s does not have branch record for branch %s' % (dep.dependency.name, layer.name, layerbranch.branch.name))
+            else:
+                logger.warning('Recommends %s of layer %s does not have branch record for branch %s - ignoring' % (dep.dependency.name, layer.name, layerbranch.branch.name))
+                continue
         deplayerdir = os.path.join(deprepodir, deplayerbranch.vcs_subdir)
         utils.parse_layer_conf(deplayerdir, config_data_copy)
     config_data_copy.delVar('LAYERDIR')
-- 
2.17.1

[layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Paul Eggleton]

We don't actually need to consider recommended layers when preparing the
order - just the depends. If we do then we can get into circular
dependency situation e.g. currently with meta-intel and meta-intel-qat
where meta-intel recommends meta-intel-qat and meta-intel-qat depends on
meta-intel. (Likely the latter dependency is erroneous since the content
of meta-intel-qat doesn't appear to depend on meta-intel, but there
could be other scenarios where it is legitimate).

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 layerindex/update.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/layerindex/update.py b/layerindex/update.py
index 06c61a79..a4b96e24 100755
--- a/layerindex/update.py
+++ b/layerindex/update.py
@@ -399,7 +399,7 @@ def main():
                     deps = re.search("^LAYERDEPENDS = \"(.*)\"", output, re.M).group(1) or ''
                     recs = re.search("^LAYERRECOMMENDS = \"(.*)\"", output, re.M).group(1) or ''
 
-                    deps_dict = utils.explode_dep_versions2(bitbakepath, deps + ' ' + recs)
+                    deps_dict = utils.explode_dep_versions2(bitbakepath, deps)
                     if len(deps_dict) == 0:
                         # No depends, add it firstly
                         layerquery_sorted.append(layer)
-- 
2.17.1

[layerindex-web][PATCH 6/7] utils: fix missing dependency logic in _add_dependency()

[Paul Eggleton]

* If a missing dependency is not required, show a warning instead of an
  error
* If logger isn't specified we still need to skip to the next item, so
  move the continue statement out of the conditional block. (In practice
  I don't think this function is currently called anywhere in the code
  without a logger specified, but let's fix it anyway).

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 layerindex/utils.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/layerindex/utils.py b/layerindex/utils.py
index f8c5fd45..8f652da7 100644
--- a/layerindex/utils.py
+++ b/layerindex/utils.py
@@ -140,8 +140,11 @@ def _add_dependency(var, name, layerbranch, config_data, logger=None, required=T
         # No layer found.
         if not dep_layer:
             if logger:
-                logger.error('Cannot resolve %s %s (version %s) for %s' % (name, dep, ver_str, layer_name))
-                continue
+                if required:
+                    logger.error('Cannot resolve %s %s (version %s) for %s' % (name, dep, ver_str, layer_name))
+                else:
+                    logger.warning('Cannot resolve %s %s (version %s) for %s' % (name, dep, ver_str, layer_name))
+            continue
 
         # Preparing to remove obsolete ones
         if need_remove:
-- 
2.17.1

[layerindex-web][PATCH 7/7] Add site-wide notice support

[Paul Eggleton]

Add the ability to show a notice at the top of every page; this provides
the ability for admins to display a message to visitors in the case of
infrastructure or index data issues. Notices can have an expiry date and
can be disabled and re-enabled if needed. A subset of HTML can be used
for formatting the text, URLs will be made into clickable links, and
four "levels" are supported (info, success, warning and error).

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 layerindex/admin.py                      |  1 +
 layerindex/context_processors.py         |  7 +++++--
 layerindex/migrations/0014_sitenotice.py | 24 +++++++++++++++++++++++
 layerindex/models.py                     | 25 +++++++++++++++++++++++-
 layerindex/utils.py                      | 14 +++++++++++++
 requirements.txt                         |  1 +
 templates/base.html                      |  5 +++++
 7 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 layerindex/migrations/0014_sitenotice.py

diff --git a/layerindex/admin.py b/layerindex/admin.py
index 3cb59691..312f7a30 100644
--- a/layerindex/admin.py
+++ b/layerindex/admin.py
@@ -198,3 +198,4 @@ admin.site.register(Patch)
 admin.site.register(RecipeChangeset, RecipeChangesetAdmin)
 admin.site.register(ClassicRecipe, ClassicRecipeAdmin)
 admin.site.register(PythonEnvironment)
+admin.site.register(SiteNotice)
diff --git a/layerindex/context_processors.py b/layerindex/context_processors.py
index db8e3fa3..7cf20ede 100644
--- a/layerindex/context_processors.py
+++ b/layerindex/context_processors.py
@@ -1,11 +1,13 @@
 # layerindex-web - custom context processor
 #
-# Copyright (C) 2013 Intel Corporation
+# Copyright (C) 2013, 2018 Intel Corporation
 #
 # Licensed under the MIT license, see COPYING.MIT for details
 
-from layerindex.models import Branch, LayerItem
+from layerindex.models import Branch, LayerItem, SiteNotice
 from django.contrib.sites.models import Site
+from django.db.models import Q
+from datetime import datetime
 
 def layerindex_context(request):
     import settings
@@ -20,4 +22,5 @@ def layerindex_context(request):
         'oe_classic': Branch.objects.filter(name='oe-classic'),
         'site_name': site_name,
         'rrs_enabled': 'rrs' in settings.INSTALLED_APPS,
+        'notices': SiteNotice.objects.filter(disabled=False).filter(Q(expires__isnull=True) | Q(expires__gte=datetime.now())),
     }
diff --git a/layerindex/migrations/0014_sitenotice.py b/layerindex/migrations/0014_sitenotice.py
new file mode 100644
index 00000000..630700cf
--- /dev/null
+++ b/layerindex/migrations/0014_sitenotice.py
@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('layerindex', '0013_patch'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='SiteNotice',
+            fields=[
+                ('id', models.AutoField(auto_created=True, serialize=False, verbose_name='ID', primary_key=True)),
+                ('text', models.TextField(help_text='Text to show in the notice. A limited subset of HTML is supported for formatting.')),
+                ('level', models.CharField(choices=[('I', 'Info'), ('S', 'Success'), ('W', 'Warning'), ('E', 'Error')], help_text='Level of notice to display', default='I', max_length=1)),
+                ('disabled', models.BooleanField(verbose_name='Disabled', help_text='Use to temporarily disable this notice', default=False)),
+                ('expires', models.DateTimeField(blank=True, help_text='Optional date/time when this notice will stop showing', null=True)),
+            ],
+        ),
+    ]
diff --git a/layerindex/models.py b/layerindex/models.py
index ff164baa..891f5dfb 100644
--- a/layerindex/models.py
+++ b/layerindex/models.py
@@ -1,6 +1,6 @@
 # layerindex-web - model definitions
 #
-# Copyright (C) 2013-2016 Intel Corporation
+# Copyright (C) 2013-2018 Intel Corporation
 #
 # Licensed under the MIT license, see COPYING.MIT for details
 
@@ -658,3 +658,26 @@ class RecipeChange(models.Model):
     def reset_fields(self):
         for fieldname in self.RECIPE_VARIABLE_MAP:
             setattr(self, fieldname, getattr(self.recipe, fieldname))
+
+class SiteNotice(models.Model):
+    NOTICE_LEVEL_CHOICES = [
+        ('I', 'Info'),
+        ('S', 'Success'),
+        ('W', 'Warning'),
+        ('E', 'Error'),
+    ]
+    text = models.TextField(help_text='Text to show in the notice. A limited subset of HTML is supported for formatting.')
+    level = models.CharField(max_length=1, choices=NOTICE_LEVEL_CHOICES, default='I', help_text='Level of notice to display')
+    disabled = models.BooleanField('Disabled', default=False, help_text='Use to temporarily disable this notice')
+    expires = models.DateTimeField(blank=True, null=True, help_text='Optional date/time when this notice will stop showing')
+
+    def __str__(self):
+        prefix = ''
+        if self.expires and datetime.now() >= self.expires:
+            prefix = '[expired] '
+        elif self.disabled:
+            prefix = '[disabled] '
+        return '%s%s' % (prefix, self.text)
+
+    def text_sanitised(self):
+        return utils.sanitise_html(self.text)
diff --git a/layerindex/utils.py b/layerindex/utils.py
index 8f652da7..e86eff0a 100644
--- a/layerindex/utils.py
+++ b/layerindex/utils.py
@@ -14,6 +14,7 @@ import time
 import fcntl
 import signal
 import codecs
+from bs4 import BeautifulSoup
 
 def get_branch(branchname):
     from layerindex.models import Branch
@@ -379,6 +380,7 @@ def setup_core_layer_sys_path(settings, branchname):
     core_layerdir = os.path.join(core_repodir, core_layerbranch.vcs_subdir)
     sys.path.insert(0, os.path.join(core_layerdir, 'lib'))
 
+
 def run_command_interruptible(cmd):
     """
     Run a command with output displayed on the console, but ensure any Ctrl+C is
@@ -407,3 +409,15 @@ def run_command_interruptible(cmd):
     finally:
         signal.signal(signal.SIGINT, signal.SIG_DFL)
     return process.returncode, buf
+
+
+def sanitise_html(html):
+    soup = BeautifulSoup(html, "html.parser")
+    for tag in soup.findAll(True):
+        if tag.name not in ['strong', 'em', 'b', 'i', 'p', 'ul', 'ol', 'li', 'br', 'p']:
+            tag.hidden = True
+        elif tag.attrs:
+            tag.attrs = []
+
+    return soup.renderContents()
+
diff --git a/requirements.txt b/requirements.txt
index 5ca7cf6e..7facd2b6 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,5 +1,6 @@
 amqp==2.2.2
 anyjson==0.3.3
+beautifulsoup4==4.6.0
 billiard==3.5.0.3
 celery==4.1.0
 confusable-homoglyphs==3.0.0
diff --git a/templates/base.html b/templates/base.html
index 38373ab8..a5e97cd5 100644
--- a/templates/base.html
+++ b/templates/base.html
@@ -89,6 +89,11 @@
     {% endblock %}
 
 {% block contenttag %}<div id="content" class="container top-padded">{% endblock %}
+        {% if notices %}
+            {% for notice in notices %}
+            <div class="alert {% if notice.level == 'I' %}alert-info{% elif notice.level == 'S' %}alert-success{% elif notice.level == 'W' %}{% elif notice.level == 'E' %}alert-error{% endif %}">{{ notice.text_sanitised|safe|urlize }}</div>
+            {% endfor %}
+        {% endif %}
         {% if messages %}
             {% for message in messages %}
             <div{% if message.tags %} class="alert {{ message.tags }}"{% endif %}>{{ message }}</div>
-- 
2.17.1

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Robert Yang]

Hi Paul,

Thanks for let me know this, this patch might be incorrect, suppose we have two
layers: core and hello:

1) LAYERRECOMMENDS_core = "hello"
2) $ update.py -l hello,core

Then core maybe added before hello layer since it ignores recs on hello, and if
hello is a new layer, it would not be in core's recs in database since core
knows nothing about hello, I think that this is incorrect.

If we really need this, I think that we should not ignore recs when the
layer is present, but only ignore it when the layer is not present, for
example, ignore it when hello layer doesn't exist, otherwise, don't ignore it.

But I'm not sure about patch 4 (error -> warning) either, since layerindex is
a central database, whether add recs to conf/bblayers.conf should depend on
end user rather than ignore it in database, otherwise the end user (especially
the api user) would have no way to choice, for example, we use api to make
conf/bblayers.conf have all or no recs layers according to user's choice,
if the database is wrong, then there might be only part of recs layers.
Though we can check update.py's warnings to fix the problem.

// Robert

On 07/03/2018 06:58 AM, Paul Eggleton wrote:
> We don't actually need to consider recommended layers when preparing the
> order - just the depends. If we do then we can get into circular
> dependency situation e.g. currently with meta-intel and meta-intel-qat
> where meta-intel recommends meta-intel-qat and meta-intel-qat depends on
> meta-intel. (Likely the latter dependency is erroneous since the content
> of meta-intel-qat doesn't appear to depend on meta-intel, but there
> could be other scenarios where it is legitimate).
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>   layerindex/update.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/layerindex/update.py b/layerindex/update.py
> index 06c61a79..a4b96e24 100755
> --- a/layerindex/update.py
> +++ b/layerindex/update.py
> @@ -399,7 +399,7 @@ def main():
>                       deps = re.search("^LAYERDEPENDS = \"(.*)\"", output, re.M).group(1) or ''
>                       recs = re.search("^LAYERRECOMMENDS = \"(.*)\"", output, re.M).group(1) or ''
>   
> -                    deps_dict = utils.explode_dep_versions2(bitbakepath, deps + ' ' + recs)
> +                    deps_dict = utils.explode_dep_versions2(bitbakepath, deps)
>                       if len(deps_dict) == 0:
>                           # No depends, add it firstly
>                           layerquery_sorted.append(layer)
> 

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Paul Eggleton]

Hi Robert

On Tuesday, 3 July 2018 2:45:11 PM NZST Robert Yang wrote:
> Thanks for let me know this, this patch might be incorrect, suppose we have two
> layers: core and hello:
> 
> 1) LAYERRECOMMENDS_core = "hello"
> 2) $ update.py -l hello,core
> 
> Then core maybe added before hello layer since it ignores recs on hello, and if
> hello is a new layer, it would not be in core's recs in database since core
> knows nothing about hello, I think that this is incorrect.
>
> If we really need this, I think that we should not ignore recs when the
> layer is present, but only ignore it when the layer is not present, for
> example, ignore it when hello layer doesn't exist, otherwise, don't ignore it.

Can you come up with an alternative fix that doesn't break parsing like it does now?
 
> But I'm not sure about patch 4 (error -> warning) either, since layerindex is
> a central database, whether add recs to conf/bblayers.conf should depend on
> end user rather than ignore it in database, otherwise the end user (especially
> the api user) would have no way to choice, for example, we use api to make
> conf/bblayers.conf have all or no recs layers according to user's choice,
> if the database is wrong, then there might be only part of recs layers.
> Though we can check update.py's warnings to fix the problem.

Recommends are just that - recommendations. Someone might legitimately
submit a layer that recommends another which they don't submit (perhaps a
commercial layer?). The system shouldn't refuse to handle it or indicate that
it's broken (it isn't, there might be reduced functionality but the layer will still
be parseable by bitbake).

I'll leave these changes unmerged for a bit in case you have a better fix for
the current problems.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Robert Yang]

On 07/03/2018 10:58 AM, Paul Eggleton wrote:
> Hi Robert
> 
> On Tuesday, 3 July 2018 2:45:11 PM NZST Robert Yang wrote:
>> Thanks for let me know this, this patch might be incorrect, suppose we have two
>> layers: core and hello:
>>
>> 1) LAYERRECOMMENDS_core = "hello"
>> 2) $ update.py -l hello,core
>>
>> Then core maybe added before hello layer since it ignores recs on hello, and if
>> hello is a new layer, it would not be in core's recs in database since core
>> knows nothing about hello, I think that this is incorrect.
>>
>> If we really need this, I think that we should not ignore recs when the
>> layer is present, but only ignore it when the layer is not present, for
>> example, ignore it when hello layer doesn't exist, otherwise, don't ignore it.
> 
> Can you come up with an alternative fix that doesn't break parsing like it does now?

OK, I will, maybe I can send you a patch by tomorrow, but I'm not sure since
the implementation might be a little complicated.

// Robert

>   
>> But I'm not sure about patch 4 (error -> warning) either, since layerindex is
>> a central database, whether add recs to conf/bblayers.conf should depend on
>> end user rather than ignore it in database, otherwise the end user (especially
>> the api user) would have no way to choice, for example, we use api to make
>> conf/bblayers.conf have all or no recs layers according to user's choice,
>> if the database is wrong, then there might be only part of recs layers.
>> Though we can check update.py's warnings to fix the problem.
> 
> Recommends are just that - recommendations. Someone might legitimately
> submit a layer that recommends another which they don't submit (perhaps a
> commercial layer?). The system shouldn't refuse to handle it or indicate that
> it's broken (it isn't, there might be reduced functionality but the layer will still
> be parseable by bitbake).
> 
> I'll leave these changes unmerged for a bit in case you have a better fix for
> the current problems.
> 
> Cheers,
> Paul
> 

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Robert Yang]

Hi Paul,

I'm sorry to say that I met layerindex' loaddata problems yesterday and today,
I still didn't find the root cause. Have you tried dumpdata and loaddata
recently, please ?

What I did was:

$ python3 manage.py dumpdata --settings settings --exclude=contenttypes 
--exclude=auth.Permission --    exclude=corsheaders >dumped.json

On another environment:
Setup database to sqlite3 in settings.py.
$ python3 manage.py loaddata --settings settings dumped.json

The first problem I got was:
[snip]
   File 
"/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/revisions.py", 
line 410, in _assert_registered
     model=model,
reversion.errors.RegistrationError: Problem installing fixture 
'/buildarea1/lyang1/layerindex-web/dumped.json': <class 
'layerindex.models.Distro'> has not been registered with django-reversion
[snip]

I think it is because we didn't use @reversion.register() for the class, so I 
added them to layerindex/models.py, then I got other errors:

[snip]
   File 
"/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/models.py", 
line 272, in _local_field_dict
     field_dict[field.attname] = getattr(obj, field.attname)
AttributeError: Problem installing fixture 
'/buildarea1/lyang1/layerindex-web/dumped.json': 'Branch' object has no 
attribute 'layerbranch_id'

I'm not sure what's wrong atm, need more investigations.

I need loaddata on my localhost to do development testing, so I can't start
work on update.py until I fix the loaddata problem.

// Robert

On 07/03/2018 11:08 AM, Robert Yang wrote:
> 
> 
> On 07/03/2018 10:58 AM, Paul Eggleton wrote:
>> Hi Robert
>>
>> On Tuesday, 3 July 2018 2:45:11 PM NZST Robert Yang wrote:
>>> Thanks for let me know this, this patch might be incorrect, suppose we have two
>>> layers: core and hello:
>>>
>>> 1) LAYERRECOMMENDS_core = "hello"
>>> 2) $ update.py -l hello,core
>>>
>>> Then core maybe added before hello layer since it ignores recs on hello, and if
>>> hello is a new layer, it would not be in core's recs in database since core
>>> knows nothing about hello, I think that this is incorrect.
>>>
>>> If we really need this, I think that we should not ignore recs when the
>>> layer is present, but only ignore it when the layer is not present, for
>>> example, ignore it when hello layer doesn't exist, otherwise, don't ignore it.
>>
>> Can you come up with an alternative fix that doesn't break parsing like it 
>> does now?
> 
> OK, I will, maybe I can send you a patch by tomorrow, but I'm not sure since
> the implementation might be a little complicated.
> 
> // Robert
> 
>>> But I'm not sure about patch 4 (error -> warning) either, since layerindex is
>>> a central database, whether add recs to conf/bblayers.conf should depend on
>>> end user rather than ignore it in database, otherwise the end user (especially
>>> the api user) would have no way to choice, for example, we use api to make
>>> conf/bblayers.conf have all or no recs layers according to user's choice,
>>> if the database is wrong, then there might be only part of recs layers.
>>> Though we can check update.py's warnings to fix the problem.
>>
>> Recommends are just that - recommendations. Someone might legitimately
>> submit a layer that recommends another which they don't submit (perhaps a
>> commercial layer?). The system shouldn't refuse to handle it or indicate that
>> it's broken (it isn't, there might be reduced functionality but the layer will 
>> still
>> be parseable by bitbake).
>>
>> I'll leave these changes unmerged for a bit in case you have a better fix for
>> the current problems.
>>
>> Cheers,
>> Paul
>>

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Paul Eggleton]

Hi Robert

On Wednesday, 4 July 2018 7:52:05 PM NZST you wrote:
> I'm sorry to say that I met layerindex' loaddata problems yesterday and
> today,
> I still didn't find the root cause. Have you tried dumpdata and loaddata
> recently, please ?
> 
> What I did was:
> 
> $ python3 manage.py dumpdata --settings settings --exclude=contenttypes 
> --exclude=auth.Permission --    exclude=corsheaders >dumped.json
> 
> On another environment:
> Setup database to sqlite3 in settings.py.
> $ python3 manage.py loaddata --settings settings dumped.json
> 
> The first problem I got was:
> [snip]
>    File 
> "/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/revisions.py", 
> line 410, in _assert_registered
>      model=model,
> reversion.errors.RegistrationError: Problem installing fixture 
> '/buildarea1/lyang1/layerindex-web/dumped.json': <class 
> 'layerindex.models.Distro'> has not been registered with django-reversion
> [snip]
> 
> I think it is because we didn't use @reversion.register() for the class, so I 
> added them to layerindex/models.py, then I got other errors:
> 
> [snip]
>    File 
> "/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/models.py", 
> line 272, in _local_field_dict
>      field_dict[field.attname] = getattr(obj, field.attname)
> AttributeError: Problem installing fixture 
> '/buildarea1/lyang1/layerindex-web/dumped.json': 'Branch' object has no 
> attribute 'layerbranch_id'

Hmm, that's odd. Branch shouldn't have layerbranch_id, it's the other way around - 
LayerBranch has a branch_id.

> I'm not sure what's wrong atm, need more investigations.
> 
> I need loaddata on my localhost to do development testing, so I can't start
> work on update.py until I fix the loaddata problem.

I have used loaddata and dumpdata here (a couple of times) but not recently.
I did not experience these issues before though. However these don't seem like 
issues that would have started as a result of this patchset (or indeed recent
changes, other than perhaps an upgrade of django-reversion), have you been 
using loaddata/dumpdata prior to this?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Robert Yang]

On 07/06/2018 01:28 PM, Paul Eggleton wrote:
> Hi Robert
> 
> On Wednesday, 4 July 2018 7:52:05 PM NZST you wrote:
>> I'm sorry to say that I met layerindex' loaddata problems yesterday and
>> today,
>> I still didn't find the root cause. Have you tried dumpdata and loaddata
>> recently, please ?
>>
>> What I did was:
>>
>> $ python3 manage.py dumpdata --settings settings --exclude=contenttypes
>> --exclude=auth.Permission --    exclude=corsheaders >dumped.json
>>
>> On another environment:
>> Setup database to sqlite3 in settings.py.
>> $ python3 manage.py loaddata --settings settings dumped.json
>>
>> The first problem I got was:
>> [snip]
>>     File
>> "/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/revisions.py",
>> line 410, in _assert_registered
>>       model=model,
>> reversion.errors.RegistrationError: Problem installing fixture
>> '/buildarea1/lyang1/layerindex-web/dumped.json': <class
>> 'layerindex.models.Distro'> has not been registered with django-reversion
>> [snip]
>>
>> I think it is because we didn't use @reversion.register() for the class, so I
>> added them to layerindex/models.py, then I got other errors:
>>
>> [snip]
>>     File
>> "/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/models.py",
>> line 272, in _local_field_dict
>>       field_dict[field.attname] = getattr(obj, field.attname)
>> AttributeError: Problem installing fixture
>> '/buildarea1/lyang1/layerindex-web/dumped.json': 'Branch' object has no
>> attribute 'layerbranch_id'
> 
> Hmm, that's odd. Branch shouldn't have layerbranch_id, it's the other way around -
> LayerBranch has a branch_id.
> 
>> I'm not sure what's wrong atm, need more investigations.
>>
>> I need loaddata on my localhost to do development testing, so I can't start
>> work on update.py until I fix the loaddata problem.
> 
> I have used loaddata and dumpdata here (a couple of times) but not recently.
> I did not experience these issues before though. However these don't seem like
> issues that would have started as a result of this patchset (or indeed recent
> changes, other than perhaps an upgrade of django-reversion), have you been
> using loaddata/dumpdata prior to this?

dumpdata/loaddata worked well before March, Konrad (in CC) worked it around by:

dumpdata --exclude=corsheaders --exclude=reversion.version 
--exclude=reversion.revision --exclude=captcha.captchastore 
--exclude=sessions.session

So I can loaddata now.

I've finished patch for 5/7, but I met other problems when testing on completely
new branch which caused by recently changes, I will fix them and send out
patches later

// Robert

> 
> Cheers,
> Paul
> 

Re: [layerindex-web][PATCH 5/7] update: ignore recommends when ordering layers

[Robert Yang]

Hi Paul,

I've sent the patches to mailing list:

[yocto] [layerindex-web][PATCH 0/4] update.py: several fixes

The one for recommends is:
   update.py: add layers when RECOMMENDS isn't satisfied

Now you can drop [PATCH 5/7], others are still needed.

// Robert

On 07/06/2018 02:49 PM, Robert Yang wrote:
> 
> 
> On 07/06/2018 01:28 PM, Paul Eggleton wrote:
>> Hi Robert
>>
>> On Wednesday, 4 July 2018 7:52:05 PM NZST you wrote:
>>> I'm sorry to say that I met layerindex' loaddata problems yesterday and
>>> today,
>>> I still didn't find the root cause. Have you tried dumpdata and loaddata
>>> recently, please ?
>>>
>>> What I did was:
>>>
>>> $ python3 manage.py dumpdata --settings settings --exclude=contenttypes
>>> --exclude=auth.Permission --    exclude=corsheaders >dumped.json
>>>
>>> On another environment:
>>> Setup database to sqlite3 in settings.py.
>>> $ python3 manage.py loaddata --settings settings dumped.json
>>>
>>> The first problem I got was:
>>> [snip]
>>>     File
>>> "/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/revisions.py", 
>>>
>>> line 410, in _assert_registered
>>>       model=model,
>>> reversion.errors.RegistrationError: Problem installing fixture
>>> '/buildarea1/lyang1/layerindex-web/dumped.json': <class
>>> 'layerindex.models.Distro'> has not been registered with django-reversion
>>> [snip]
>>>
>>> I think it is because we didn't use @reversion.register() for the class, so I
>>> added them to layerindex/models.py, then I got other errors:
>>>
>>> [snip]
>>>     File
>>> "/buildarea1/lyang1/layerindex-web/.venv/lib/python3.5/site-packages/reversion/models.py", 
>>>
>>> line 272, in _local_field_dict
>>>       field_dict[field.attname] = getattr(obj, field.attname)
>>> AttributeError: Problem installing fixture
>>> '/buildarea1/lyang1/layerindex-web/dumped.json': 'Branch' object has no
>>> attribute 'layerbranch_id'
>>
>> Hmm, that's odd. Branch shouldn't have layerbranch_id, it's the other way 
>> around -
>> LayerBranch has a branch_id.
>>
>>> I'm not sure what's wrong atm, need more investigations.
>>>
>>> I need loaddata on my localhost to do development testing, so I can't start
>>> work on update.py until I fix the loaddata problem.
>>
>> I have used loaddata and dumpdata here (a couple of times) but not recently.
>> I did not experience these issues before though. However these don't seem like
>> issues that would have started as a result of this patchset (or indeed recent
>> changes, other than perhaps an upgrade of django-reversion), have you been
>> using loaddata/dumpdata prior to this?
> 
> dumpdata/loaddata worked well before March, Konrad (in CC) worked it around by:
> 
> dumpdata --exclude=corsheaders --exclude=reversion.version 
> --exclude=reversion.revision --exclude=captcha.captchastore 
> --exclude=sessions.session
> 
> So I can loaddata now.
> 
> I've finished patch for 5/7, but I met other problems when testing on completely
> new branch which caused by recently changes, I will fix them and send out
> patches later
> 
> // Robert
> 
>>
>> Cheers,
>> Paul
>>