It might be a good idea to do a v2, but you can wait until I've had a better look through the other code if you like, in case there's anything else.
Elliot On 16 March 2016 at 15:51, Lerner, David M (Wind River) < [email protected]> wrote: > 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 > > -- Elliot Smith Software Engineer Intel Open Source Technology Centre
-- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
