From 5540a84434c188b5f5bf350e1174db7e58762061 Mon Sep 17 00:00:00 2001 From: Paul Eggleton Date: Fri, 8 Mar 2019 15:08:23 +1300 Subject: [PATCH] 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 --- rrs/migrations/0023_recipeupgrade_deleted.py | 25 ++++ rrs/models.py | 19 ++- rrs/tools/upgrade_history_internal.py | 137 +++++++++++++++---- rrs/views.py | 17 ++- templates/rrs/recipedetail.html | 22 ++- 5 files changed, 178 insertions(+), 42 deletions(-) create mode 100644 rrs/migrations/0023_recipeupgrade_deleted.py diff --git a/rrs/migrations/0023_recipeupgrade_deleted.py b/rrs/migrations/0023_recipeupgrade_deleted.py new file mode 100644 index 0000000..28b958c --- /dev/null +++ b/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), + ), + ] diff --git a/rrs/models.py b/rrs/models.py index 3806662..bd37313 100644 --- a/rrs/models.py +++ b/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): diff --git a/rrs/tools/upgrade_history_internal.py b/rrs/tools/upgrade_history_internal.py index 02a0fbd..b838050 100644 --- a/rrs/tools/upgrade_history_internal.py +++ b/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: diff --git a/rrs/views.py b/rrs/views.py index 0d3986e..eb7622d 100644 --- a/rrs/views.py +++ b/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) diff --git a/templates/rrs/recipedetail.html b/templates/rrs/recipedetail.html index 84b3fbc..22e260c 100644 --- a/templates/rrs/recipedetail.html +++ b/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 @@ @@ -91,7 +95,7 @@ {% for rud in recipe_upgrade_details %} {{ rud.title }} - {{ rud.version }} + {% if rud.upgrade_type != 'R' %}{{ rud.version }}{% endif %} {% if rud.milestone_name %} @@ -123,6 +127,7 @@ {% endif %} + {% if recipe %}

Patches

{% if recipe.patch_set.exists %} @@ -144,6 +149,7 @@ {% else %}

None

{% endif %} + {% endif %} {% if otherbranch_recipes %}

Other branches

@@ -178,21 +184,25 @@
Summary
-
{{ recipe.summary }}
+
{% if recipe %}{{ recipe.summary }}{% else %}{{ recipesymbol.summary }}{% endif %}
Section
{{ recipe.section }}
License
{{ recipe.license }}
Recipe file
+ {% if recipe %} {% if recipe.vcs_web_url %} {{ recipe.full_path }} {% else %} {{ recipe.full_path }} {% endif %} + {% else %} + {{ last_filepath }} + {% endif %}
Layer -
{{ recipe.layerbranch.layer.name }} ({{ recipe.layerbranch.branch.name}} branch)
+
{{ recipesymbol.layerbranch.layer.name }} ({{ recipesymbol.layerbranch.branch.name}} branch)
{% if recipe.homepage %}
Homepage