RRS: Add deleted recipe handling

Now that we're using RecipeSymbols we have the complete list of recipes
that ever existed in a layer. We only want to see the ones that are
valid for the selected milestone, so when a recipe gets deleted (or
renamed or moved outside of the layer subdirectory, if any) we need to
record that - do so using a RecipeUpgrade record with a new field
upgrade_type set to 'R'. Additionally we need to store the file path so
that deletion events (where we don't parse the contents of the recipe,
thus we don't have PN) are easy to match up with RecipeUpgrade records;
naturally we need to keep the paths "up-to-date" when we notice recipe
files being moved around.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>

rrs/migrations/0023_recipeupgrade_deleted.py

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.22 on 2019-08-13 12:17
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('rrs', '0022_recipesymbol_finish'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='recipeupgrade',
+            name='filepath',
+            field=models.CharField(blank=True, max_length=512),
+        ),
+        migrations.AddField(
+            model_name='recipeupgrade',
+            name='upgrade_type',
+            field=models.CharField(choices=[('U', 'Upgrade'), ('N', 'Delete'), ('R', 'Delete (final)')], db_index=True, default='U', max_length=1),
+        ),
+    ]

rrs/models.py

@@ -430,6 +430,12 @@ class RecipeDistro(models.Model):
 
 
 class RecipeUpgrade(models.Model):
+    UPGRADE_TYPE_CHOICES = (
+        ('U', 'Upgrade'),
+        ('N', 'Delete'),
+        ('R', 'Delete (final)'),
+    )
+
     recipesymbol = models.ForeignKey(RecipeSymbol)
     maintainer = models.ForeignKey(Maintainer, blank=True)
     sha1 = models.CharField(max_length=40, blank=True)
@@ -437,6 +443,8 @@ class RecipeUpgrade(models.Model):
     version = models.CharField(max_length=100, blank=True)
     author_date = models.DateTimeField(db_index=True)
     commit_date = models.DateTimeField(db_index=True)
+    upgrade_type = models.CharField(max_length=1, choices=UPGRADE_TYPE_CHOICES, default='U', db_index=True)
+    filepath = models.CharField(max_length=512, blank=True)
 
     @staticmethod
     def get_by_recipe_and_date(recipe, end_date):
@@ -452,8 +460,15 @@ class RecipeUpgrade(models.Model):
         return self.recipesymbol.layerbranch.commit_url(self.sha1)
 
     def __str__(self):
-        return '%s: (%s, %s)' % (self.recipesymbol.pn, self.version,
-                        self.commit_date)
+        if self.upgrade_type == 'R':
+            return '%s: deleted [final] (%s)' % (self.recipesymbol.pn,
+                            self.commit_date)
+        elif self.upgrade_type == 'N':
+            return '%s: deleted (%s)' % (self.recipesymbol.pn,
+                            self.commit_date)
+        else:
+            return '%s: (%s, %s)' % (self.recipesymbol.pn, self.version,
+                            self.commit_date)
 
 
 class RecipeMaintenanceLink(models.Model):

rrs/tools/upgrade_history_internal.py

@@ -16,6 +16,7 @@ import optparse
 import logging
 import re
 from distutils.version import LooseVersion
+import git
 
 sys.path.insert(0, os.path.realpath(os.path.join(os.path.dirname(__file__))))
 from common import common_setup, get_pv_type, load_recipes, \
@@ -28,7 +29,7 @@ from layerindex.update_layer import split_recipe_fn
 """
     Store upgrade into RecipeUpgrade model.
 """
-def _save_upgrade(recipe_data, layerbranch, pv, commit, title, info, logger):
+def _save_upgrade(recipesymbol, layerbranch, pv, commit, title, info, filepath, logger, upgrade_type=None):
     from email.utils import parsedate_tz, mktime_tz
     from rrs.models import Maintainer, RecipeUpgrade, RecipeSymbol
 
@@ -40,8 +41,7 @@ def _save_upgrade(recipe_data, layerbranch, pv, commit, title, info, logger):
     maintainer = Maintainer.create_or_update(maintainer_name, maintainer_email)
 
     upgrade = RecipeUpgrade()
-    summary = recipe_data.getVar('SUMMARY', True) or recipe_data.getVar('DESCRIPTION', True)
-    upgrade.recipesymbol = RecipeSymbol.symbol(recipe_data.getVar('PN', True), layerbranch, summary=summary)
+    upgrade.recipesymbol = recipesymbol
     upgrade.maintainer = maintainer
     upgrade.author_date = datetime.utcfromtimestamp(mktime_tz(
                                     parsedate_tz(author_date)))
@@ -50,12 +50,15 @@ def _save_upgrade(recipe_data, layerbranch, pv, commit, title, info, logger):
     upgrade.version = pv
     upgrade.sha1 = commit
     upgrade.title = title.strip()
+    upgrade.filepath = filepath
+    if upgrade_type:
+        upgrade.upgrade_type = upgrade_type
     upgrade.save()
 
 """
     Create upgrade receives new recipe_data and cmp versions.
 """
-def _create_upgrade(recipe_data, layerbranch, ct, title, info, logger, initial=False):
+def _create_upgrade(recipe_data, layerbranch, ct, title, info, filepath, logger, initial=False):
     from rrs.models import RecipeUpgrade, RecipeSymbol
     from bb.utils import vercmp_string
 
@@ -66,11 +69,12 @@ def _create_upgrade(recipe_data, layerbranch, ct, title, info, logger, initial=F
         logger.warn('Invalid version for recipe %s in commit %s, ignoring' % (recipe_data.getVar('FILE', True), ct))
         return
 
-    rsym = RecipeSymbol.objects.filter(pn=pn, layerbranch=layerbranch)
+    summary = recipe_data.getVar('SUMMARY', True) or recipe_data.getVar('DESCRIPTION', True)
+    recipesymbol = RecipeSymbol.symbol(recipe_data.getVar('PN', True), layerbranch, summary=summary)
 
     try:
         latest_upgrade = RecipeUpgrade.objects.filter(
-                recipesymbol=rsym).order_by('-commit_date')[0]
+                recipesymbol=recipesymbol).order_by('-commit_date')[0]
         prev_pv = latest_upgrade.version
     except KeyboardInterrupt:
         raise
@@ -79,7 +83,7 @@ def _create_upgrade(recipe_data, layerbranch, ct, title, info, logger, initial=F
 
     if prev_pv is None:
         logger.debug("%s: Initial upgrade ( -> %s)." % (pn, pv))
-        _save_upgrade(recipe_data, layerbranch, pv, ct, title, info, logger)
+        _save_upgrade(recipesymbol, layerbranch, pv, ct, title, info, filepath, logger)
     else:
         from common import get_recipe_pv_without_srcpv
 
@@ -95,12 +99,13 @@ def _create_upgrade(recipe_data, layerbranch, ct, title, info, logger, initial=F
                 if initial is True:
                     logger.debug("%s: Update initial upgrade ( -> %s)." % \
                             (pn, pv)) 
+                    latest_upgrade.filepath = filepath
                     latest_upgrade.version = pv
                     latest_upgrade.save()
                 else:
                     logger.debug("%s: detected upgrade (%s -> %s)" \
                             " in ct %s." % (pn, prev_pv, pv, ct))
-                    _save_upgrade(recipe_data, layerbranch, pv, ct, title, info, logger)
+                    _save_upgrade(recipesymbol, layerbranch, pv, ct, title, info, filepath, logger)
         except KeyboardInterrupt:
             raise
         except Exception as e:
@@ -111,39 +116,46 @@ def _create_upgrade(recipe_data, layerbranch, ct, title, info, logger, initial=F
 """
     Returns a list containing the fullpaths to the recipes from a commit.
 """
-def _get_recipes_filenames(ct, repodir, layerdir, logger):
+def _get_recipes_filenames(ct, repo, repodir, layersubdir_start, logger):
     import glob
     ct_files = []
-    layerdir_start = os.path.normpath(layerdir) + os.sep
-
-    files = utils.runcmd(['git', 'log', '--name-only', '--format=%n', '-n', '1', ct],
-                            repodir, logger=logger)
+    deleted = []
+    moved_files = []
 
     incdirs = []
-    for f in files.split("\n"):
-        if f != "":
-            fullpath = os.path.join(repodir, f)
-            # Skip deleted files in commit
-            if not os.path.exists(fullpath):
+    commitobj = repo.commit(ct)
+    for parent in commitobj.parents:
+        diff = parent.diff(commitobj)
+        for diffitem in diff:
+            if layersubdir_start and not (diffitem.a_path.startswith(layersubdir_start) or diffitem.b_path.startswith(layersubdir_start)):
+                # Not in this layer, skip it
                 continue
-            if not fullpath.startswith(layerdir_start):
-                # Ignore files in repo that are outside of the layer
+
+            (typename, _, _) = recipeparse.detect_file_type(diffitem.a_path,
+                                        layersubdir_start)
+
+            if not diffitem.b_path or diffitem.deleted_file or not diffitem.b_path.startswith(layersubdir_start):
+                # Deleted, or moved out of the layer (which we treat as a delete)
+                if typename == 'recipe':
+                    deleted.append(diffitem.a_path)
                 continue
-            (typename, _, filename) = recipeparse.detect_file_type(fullpath,
-                                        layerdir_start)
+
             if typename == 'recipe':
-                ct_files.append(fullpath)
-            elif fullpath.endswith('.inc'):
-                fpath = os.path.dirname(fullpath)
+                ct_files.append(os.path.join(repodir, diffitem.b_path))
+                if diffitem.a_path != diffitem.b_path:
+                    moved_files.append((diffitem.a_path, diffitem.b_path))
+            elif typename == 'incfile':
+                fpath = os.path.dirname(os.path.join(repodir, diffitem.a_path))
                 if not fpath in incdirs:
                     incdirs.append(fpath)
+
     for fpath in incdirs:
         # Let's just assume that all .bb files next to a .inc need to be checked
         for f in glob.glob(os.path.join(fpath, '*.bb')):
             if not f in ct_files:
                 ct_files.append(f)
 
-    return ct_files
+    return ct_files, deleted, moved_files
 
 
 def checkout_layer_deps(layerbranch, commit, fetchdir, logger):
@@ -174,7 +186,7 @@ def checkout_layer_deps(layerbranch, commit, fetchdir, logger):
 
 def generate_history(options, layerbranch_id, commit, logger):
     from layerindex.models import LayerBranch
-    from rrs.models import Release
+    from rrs.models import Release, RecipeUpgrade
     layerbranch = LayerBranch.objects.get(id=layerbranch_id)
 
     fetchdir = settings.LAYER_FETCH_DIR
@@ -187,13 +199,27 @@ def generate_history(options, layerbranch_id, commit, logger):
     repodir = os.path.join(fetchdir, urldir)
     layerdir = os.path.join(repodir, str(layerbranch.vcs_subdir))
 
+    if layerbranch.vcs_subdir:
+        layersubdir_start = layerbranch.vcs_subdir
+        if not layersubdir_start.endswith('/'):
+            layersubdir_start += '/'
+    else:
+        layersubdir_start = ''
+
+    repo = git.Repo(repodir)
+    if repo.bare:
+        logger.error('Repository %s is bare, not supported' % repodir)
+        sys.exit(1)
+
     commitdate = checkout_layer_deps(layerbranch, commit, fetchdir, logger)
 
     if options.initial:
         fns = None
+        deleted = []
+        moved = []
     else:
-        fns = _get_recipes_filenames(commit, repodir, layerdir, logger)
-        if not fns:
+        fns, deleted, moved = _get_recipes_filenames(commit, repo, repodir, layersubdir_start, logger)
+        if not (fns or deleted or moved):
             return
 
     # setup bitbake
@@ -223,11 +249,62 @@ def generate_history(options, layerbranch_id, commit, logger):
             info = utils.runcmd(['git', 'log', '--format=%an;%ae;%ad;%cd', '--date=rfc', '-n', '1', commit], destdir=repodir, logger=logger)
             recordcommit = commit
 
+        fn_data = {}
+        for recipe_data in recipes:
+            fn = os.path.relpath(recipe_data.getVar('FILE', True), repodir)
+            fn_data[fn] = recipe_data
+
+        seen_pns = []
         try:
             with transaction.atomic():
+                for a, b in moved:
+                    logger.debug('Move %s -> %s' % (a,b))
+                    rus = RecipeUpgrade.objects.filter(recipesymbol__layerbranch=layerbranch, filepath=a).order_by('-commit_date')
+                    recipe_data = fn_data.get(b, None)
+                    if recipe_data:
+                        pn = recipe_data.getVar('PN', True)
+                        ru = rus.first()
+                        if ru and ru.recipesymbol.pn != pn:
+                            # PN has been changed! We need to mark the old record as deleted
+                            logger.debug('PN changed: %s -> %s' % (ru.recipesymbol.pn, pn))
+                            if a not in deleted:
+                                deleted.append(a)
+                    else:
+                        logger.warning('Unable to find parsed data for recipe %s' % b)
+
+                    if a not in deleted:
+                        # Need to keep filepath up-to-date, otherwise we won't be able to
+                        # find the record if we need to mark it as deleted later
+                        for ru in rus:
+                            ru.filepath = b
+                            ru.save()
+
                 for recipe_data in recipes:
+                    filepath = os.path.relpath(recipe_data.getVar('FILE', True), repodir)
                     _create_upgrade(recipe_data, layerbranch, recordcommit, title,
-                            info, logger, initial=options.initial)
+                            info, filepath, logger, initial=options.initial)
+                    seen_pns.append(recipe_data.getVar('PN', True))
+
+                for df in deleted:
+                    rus = RecipeUpgrade.objects.filter(recipesymbol__layerbranch=layerbranch, filepath=df).order_by('-commit_date')
+                    for ru in rus:
+                        other_rus = RecipeUpgrade.objects.filter(recipesymbol=ru.recipesymbol, commit_date__gt=ru.commit_date).exclude(filepath=df).order_by('-commit_date')
+                        # We make a distinction between deleting just one version and the entire recipe being deleted
+                        upgrade_type = 'R'
+                        for other_ru in other_rus:
+                            if other_ru.upgrade_type == 'R':
+                                logger.debug('There is a delete: %s' % other_ru)
+                                upgrade_type = ''
+                                break
+                            if os.path.exists(os.path.join(repodir, other_ru.filepath)):
+                                upgrade_type = 'N'
+                        if not upgrade_type:
+                            continue
+                        if ru.upgrade_type != upgrade_type and ru.recipesymbol.pn not in seen_pns:
+                            logger.debug("%s: marking as deleted (%s)" % (ru.recipesymbol.pn, ru.filepath))
+                            _save_upgrade(ru.recipesymbol, layerbranch, ru.version, recordcommit, title, info, df, logger, upgrade_type=upgrade_type)
+                            break
+
                 if options.dry_run:
                     raise DryRunRollbackException
         except DryRunRollbackException:

rrs/views.py

@@ -195,15 +195,17 @@ class Raw():
         """ Get info for Recipes for the milestone """
         cur = connection.cursor()
         cur.execute("""SELECT rs.id, rs.pn, rs.summary, te.version, rownum FROM (
-                            SELECT recipesymbol_id, version, commit_date, ROW_NUMBER() OVER(
+                            SELECT recipesymbol_id, version, commit_date, upgrade_type, ROW_NUMBER() OVER(
                                 PARTITION BY recipesymbol_id
                                 ORDER BY commit_date DESC
                             ) AS rownum
                         FROM rrs_recipeupgrade
-                        WHERE commit_date <= %s) AS te
+                        WHERE commit_date <= %s
+                        AND upgrade_type <> 'N') AS te
                         INNER JOIN rrs_recipesymbol AS rs
                         ON te.recipesymbol_id = rs.id
                         WHERE rownum = 1
+                        AND te.upgrade_type <> 'R'
                         AND rs.layerbranch_id = %s
                         ORDER BY rs.pn;
                         """, [date, layerbranch_id])
@@ -621,9 +623,10 @@ class RecipeUpgradeDetail():
     is_recipe_maintainer = None
     commit = None
     commit_url = None
+    upgrade_type = None
 
     def __init__(self, title, version, maintplan_name, release_name, milestone_name, date, 
-            maintainer_name, is_recipe_maintainer, commit, commit_url):
+            maintainer_name, is_recipe_maintainer, commit, commit_url, upgrade_type):
         self.title = title
         self.version = version
         self.maintplan_name = maintplan_name
@@ -634,6 +637,7 @@ class RecipeUpgradeDetail():
         self.is_recipe_maintainer = is_recipe_maintainer
         self.commit = commit
         self.commit_url = commit_url
+        self.upgrade_type = upgrade_type
 
 def _get_recipe_upgrade_detail(maintplan, recipe_upgrade):
     release_name = ''
@@ -668,7 +672,7 @@ def _get_recipe_upgrade_detail(maintplan, recipe_upgrade):
 
     rud = RecipeUpgradeDetail(recipe_upgrade.title, recipe_upgrade.version, \
             maintplan.name, release_name, milestone_name, commit_date, maintainer_name, \
-            is_recipe_maintainer, commit, commit_url)
+            is_recipe_maintainer, commit, commit_url, recipe_upgrade.upgrade_type)
 
     return rud
 
@@ -731,6 +735,11 @@ class RecipeDetailView(DetailView):
             context['recipe_upgrade_details'].append(_get_recipe_upgrade_detail(maintplan, ru))
         context['recipe_upgrade_detail_count'] = len(context['recipe_upgrade_details'])
 
+        if not recipe:
+            ru = RecipeUpgrade.objects.filter(recipesymbol=recipesymbol).order_by('-commit_date').first()
+            if ru:
+                context['last_filepath'] = ru.filepath
+
         context['recipe_layer_branch_url'] = _get_layer_branch_url(
                 recipesymbol.layerbranch.branch.name, recipesymbol.layerbranch.layer.name)
 

templates/rrs/recipedetail.html

@@ -12,7 +12,7 @@
 {% endcomment %}
 
 {% autoescape on %}
-{% block title_append %} - {{ recipe.pn }}{% endblock %}
+{% block title_append %} - {% if recipe %}{{ recipe.name }}{% else %}{{ recipesymbol.pn }}{% endif %}{% endblock %}
 {% endautoescape %}
 
 {% block topfunctions %}
@@ -26,13 +26,17 @@
 
 <ul class="breadcrumb">
     <li><a href="{% url 'rrs_maintplan' maintplan_name %}">{{ maintplan_name }}</a></li>
-    <li><a href="{% url 'layer_item' recipe.layerbranch.branch.name recipe.layerbranch.layer.name %}">{{ recipe.layerbranch.layer.name }}</a></li>
-    <li class="active">{{ recipe.name }}</li>
+    <li><a href="{% url 'layer_item' recipesymbol.layerbranch.branch.name recipesymbol.layerbranch.layer.name %}">{{ recipesymbol.layerbranch.layer.name }}</a></li>
+    <li class="active">{% if recipe %}{{ recipe.name }}{% else %}{{ recipesymbol.pn }}{% endif %}</li>
 </ul>
 
 <div class="page-header">
     <h1>
+        {% if recipe %}
         {{ recipe.name }} {{ recipe.pv }}
+        {% else %}
+        {{ recipesymbol.pn }} <span class="label label-default">deleted</span>
+        {% endif %}
     </h1>
 </div>
 
@@ -91,7 +95,7 @@
                 {% for rud in recipe_upgrade_details %}
                 <tr>
                     <td>{{ rud.title }}</td>
-                    <td>{{ rud.version }}</td>
+                    <td>{% if rud.upgrade_type != 'R' %}{{ rud.version }}{% endif %}</td>
                     {% if rud.milestone_name %}
                     <td>
                         <a href="{% url 'rrs_recipes' rud.maintplan_name rud.release_name rud.milestone_name %}">
@@ -123,6 +127,7 @@
         </table>
         {% endif %}
 
+        {% if recipe %}
         <h2>Patches</h2>
         {% if recipe.patch_set.exists %}
         <table class="table table-striped table-bordered">
@@ -144,6 +149,7 @@
         {% else %}
         <p>None</p>
         {% endif %}
+        {% endif %}
 
         {% if otherbranch_recipes %}
         <h2>Other branches</h2>
@@ -178,21 +184,25 @@
         <div class="well well-transparent">
             <dl>
                 <dt>Summary</dt>
-                <dd>{{ recipe.summary }}</dd>
+                <dd>{% if recipe %}{{ recipe.summary }}{% else %}{{ recipesymbol.summary }}{% endif %}</dd>
                 <dt>Section</dt>
                 <dd>{{ recipe.section }}</dd>
                 <dt>License</dt>
                 <dd>{{ recipe.license }}</dd>
                 <dt>Recipe file</dt>
                 <dd>
+                {% if recipe %}
                 {% if recipe.vcs_web_url %}
                     <a href="{{ recipe.vcs_web_url }}">{{ recipe.full_path }}</a>
                 {% else %}
                     {{ recipe.full_path }}
                 {% endif %}
+                {% else %}
+                    {{ last_filepath }}
+                {% endif %}
                 </dd>
                 <dt>Layer</dd>
-                <dd><a href="{{ recipe_layer_branch_url }}">{{ recipe.layerbranch.layer.name }} ({{ recipe.layerbranch.branch.name}} branch)</a></dd>
+                <dd><a href="{{ recipe_layer_branch_url }}">{{ recipesymbol.layerbranch.layer.name }} ({{ recipesymbol.layerbranch.branch.name}} branch)</a></dd>
 
                 {% if recipe.homepage %}
                 <dt>Homepage</dt>