Hi Elliot, I realized this morning that a line in _get_all_dependents() is benign but not required. The impact is performance - an unnecessary pass through the list of reverse dependendencies. I don't know if that justifies a V2 patch or not.
See the details below, inline. Dave > -----Original Message----- > From: Dave Lerner [mailto:[email protected]] > Sent: Friday, March 11, 2016 10:29 AM > To: [email protected]; [email protected]; WOOD, MICHAEL > Cc: Lerner, Dave > Subject: [PATCH 3/3] toaster: get all dependents for pkg for removal > > For customised image package removal change behavior. > From: > Only display the immediate dependents of the requested package > to remove, not the full dependent list, that is dependents of > dependents ... > Do not remove the displayed dependents, just notify the user > of the list. > To: > Display the complete dependent tree, traversing all reverse > dependencies starting from the package to be removed and then it's > dependents. > Change the modal dialog to note that all of these dependents will > be removed automatically. > > [YOCTO #9121] > > Signed-off-by: Dave Lerner <[email protected]> > --- > bitbake/lib/toaster/toastergui/views.py | 98 > +++++++++++++++++++++++++++++---- > 1 file changed, 86 insertions(+), 12 deletions(-) > > diff --git a/bitbake/lib/toaster/toastergui/views.py > b/bitbake/lib/toaster/toastergui/views.py > index 85ca9be..4442d33 100755 > --- a/bitbake/lib/toaster/toastergui/views.py > +++ b/bitbake/lib/toaster/toastergui/views.py > @@ -2538,6 +2538,67 @@ if True: > > return response > > + def _traverse_dependents(next_package_id, list, all_current_packages, > tree_level): > + """ > + Recurse through dependents (reverse dependency) tree up to a > + depth of MAX_DEPENDENCY_TREE_DEPTH just in case their are circular > + dependencies. A large system has 500 packages, exceeding a > + dependency chain that long is unlikely. > + Don't scan for dependencies for a package already in list. > + Return the updated list with new dependencies. > + """ > + MAX_DEPENDENCY_TREE_DEPTH = 500 > + if tree_level >= MAX_DEPENDENCY_TREE_DEPTH: > + logger.warning( > + "The number of dependents in the dependency tree" > + "for this package exceeds " + MAX_DEPENDENDENCY_TREE_DEPTH + > + " and the remaining dependents will not be removed") > + return list, True > + > + package = CustomImagePackage.objects.get(id=next_package_id) > + dependents = \ > + package.package_dependencies_target.annotate( > + name=F('package__name'), > + pk=F('package__pk'), > + size=F('package__size'), > + ).values("name", "pk", "size").exclude( > + ~Q(pk__in=all_current_packages) > + ) > + > + truncated = False > + for pkg in dependents: > + if pkg in list: > + # already seen, skip dependent search > + continue The scan prevents duplicate packages in the list and therefor the groupby list line below to make list entries unique is not required. > + list.append(pkg) > + extension, truncated = _traverse_dependents( > + pkg["pk"], list, all_current_packages, tree_level+1) > + if truncated: > + break > + > + return list, truncated > + > + def _get_all_dependents(package_id, all_current_packages, flat): > + """ > + Returns the full list of dependents, also known as reverse > + dependencies, for package_id, recursing through dependency > + relationships. > + Returns either list of dictionary items or a list of > CustomImagePackage > + model objects depending on arg flat=False > + """ > + import itertools > + list = [] > + list, truncated = _traverse_dependents( > + package_id, list, all_current_packages, 0) > + > + # sort and remove duplicates > + sortedlist = sorted(list, key=lambda x: x["name"]) > + list = [x[0] for x in itertools.groupby(sortedlist)] The above line is should not be necessary, the list entries are already unique. > + > + > + if flat: > + list = [CustomImagePackage.objects.get(id=entry["pk"]) for entry > in list] > + return list > > @xhr_response > def xhr_customrecipe_packages(request, recipe_id, package_id): > @@ -2606,15 +2667,10 @@ if True: > ) > > # Reverse dependencies which are needed by packages that are > - # in the image > - reverse_deps = package.package_dependencies_target.annotate( > - name=F('package__name'), > - pk=F('package__pk'), > - size=F('package__size'), > - ).values("name", "pk", "size").exclude( > - ~Q(pk__in=all_current_packages) > - ) > - > + # in the image. Recursive search providing all dependents, > + # not just immediate dependents. > + reverse_deps = _get_all_dependents( > + package_id, all_current_packages, flat=False) > total_size_deps = 0 > total_size_reverse_deps = 0 > > @@ -2658,6 +2714,11 @@ if True: > > else: > recipe.appends_set.add(package) > + # Make sure that package is not in the excludes set > + try: > + recipe.excludes_set.remove(package) > + except: > + pass > # Add the dependencies we think will be added to the recipe > # as a result of appending this package. > # TODO this should recurse down the entire deps tree > @@ -2668,11 +2729,12 @@ if True: > > recipe.includes_set.add(cust_package) > try: > - # when adding the pre-requisite package make > sure it's not > in the > - # excluded list from a prior removal. > + # When adding the pre-requisite package, make > + # sure it's not in the excluded list from a > + # prior removal. > recipe.excludes_set.remove(cust_package) > except Package.DoesNotExist: > - # Don't care if the package had never been > excluded > + # Don't care if the package had never been > excluded > pass > except: > logger.warning("Could not add package's suggested" > @@ -2688,6 +2750,18 @@ if True: > recipe.excludes_set.add(package) > else: > recipe.appends_set.remove(package) > + all_current_packages = recipe.get_all_packages() > + reverse_deps = _get_all_dependents( > + package.pk, all_current_packages, flat=True) > + for r in reverse_deps: > + try: > + if r.id in included_packages: > + recipe.excludes_set.add(r) > + else: > + recipe.appends_set.remove(r) > + except: > + pass > + > return {"error": "ok"} > except CustomImageRecipe.DoesNotExist: > return {"error": "Tried to remove package that wasn't > present"} > -- > 1.9.1 -- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
