Control: tags -1 confirmed patch Niels Thykier: > [...] > > I looking forward to your test case as it will make this issue a lot > easier to debug. > Hi, Thanks for the test case. :) I have pushed it to britney2-tests and flagged it as a known failure. > What is supposed to happen is that: > > * Britney "should" rewrite the relation on "libsystemd0" as > "libsystemd0 | libelogin0" when building the BinaryPackageUniverse > (actually as libsystemd0/<version>/arch | libsystemd0/<version>/arch > tuples). > > - This is also based on the assumption that the Conflicts/Provides > setup in libelogin0 is done correctly. I /think/ it is - I am just > being explicit about the assumption. > Indeed, I have looked through the state of the test and the relations are as I expected here. > [...]> * It /could/ be related to the caching of the pseudo-essential set. > AFAIUI, the cache should "just work(tm)" if the previous point > does not have a flaw. At least the current cache code assumes that > libsystemd0 and libelogin0 will not be resolved by > _get_min_pseudo_ess_set. Though, it could also be a point where the > bug hides. > > [...] I think this ends up being the winner. The _get_min_pseudo_ess_set method builds a set that includes libtest (libsystemd0) from the beginning. This is technically correct and valid at the start because libprovider (libelogind0) is *not* in testing at that point (so any selection for the essential set will always include libtest at the start). Unfortunately, the cache is never invalidated by adding the alternative and thus libprovider is immediately rejected as broken. I have attached the following patch that passes the provided test and (AFAICT) does what we want. Please feel free to review it; I will come back to this in a few days. Note: I do not expect to have a lot of spare time for Debian the coming week; please ping me on this bug if I have not replied by next Sunday. Thanks, ~Niels
From 0d5ea5eb8c0276e48f1394f233e1441ab87dcfbc Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Sun, 10 Nov 2019 23:08:47 +0000
Subject: [PATCH] Improve cache invalidation of (pseudo-)essential set
If a new pseudo-essential package was added to testing *and* it is
mutually-exclusive with an existing member, britney would incorrectly
flag it as uninstallable (unless another change triggered a cache
invalidation of the pseudo-essential set).
With this commit, the cache invalidation is now done based on whether
we add something that *might* be in the (effective) pseudo-essential
set. It is an overapproximation as there are cases where the
invalidation is unnecessary, but at the moment it is not a performance
concern.
Closes: https://bugs.debian.org/944190
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney2/installability/tester.py | 12 ++++++++----
britney2/utils.py | 21 ++++++++++++++++++++-
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/britney2/installability/tester.py b/britney2/installability/tester.py
index 5ac6ef2..4687cc0 100644
--- a/britney2/installability/tester.py
+++ b/britney2/installability/tester.py
@@ -17,7 +17,7 @@ from functools import partial
import logging
from itertools import chain, filterfalse
-from britney2.utils import iter_except
+from britney2.utils import iter_except, add_transitive_dependencies_flatten
class InstallabilityTester(object):
@@ -52,12 +52,16 @@ class InstallabilityTester(object):
# are essential and packages that will always follow.
#
# It may not be a complete essential set, since alternatives
- # are not always resolved. Noticably cases like "awk" may be
+ # are not always resolved. Noticeably cases like "awk" may be
# left out (since it could be either gawk, mawk or
# original-awk) unless something in this sets depends strictly
# on one of them
self._cache_ess = {}
+ essential_w_transitive_deps = set(universe.essential_packages)
+ add_transitive_dependencies_flatten(universe, essential_w_transitive_deps)
+ self._cache_essential_transitive_dependencies = essential_w_transitive_deps
+
def compute_installability(self):
"""Computes the installability of all the packages in the suite
@@ -137,8 +141,8 @@ class InstallabilityTester(object):
# Re-add broken packages as some of them may now be installable
self._suite_contents |= self._cache_broken
self._cache_broken = set()
- if pkg_id in self._universe.essential_packages and pkg_id.architecture in self._cache_ess:
- # Adds new essential => "pseudo-essential" set needs to be
+ if pkg_id in self._cache_essential_transitive_dependencies and pkg_id.architecture in self._cache_ess:
+ # Adds new possibly pseudo-essential => "pseudo-essential" set needs to be
# recomputed
del self._cache_ess[pkg_id.architecture]
diff --git a/britney2/utils.py b/britney2/utils.py
index a72ff5f..10d352c 100644
--- a/britney2/utils.py
+++ b/britney2/utils.py
@@ -30,7 +30,7 @@ import time
from collections import defaultdict
from datetime import datetime
from functools import partial
-from itertools import filterfalse
+from itertools import filterfalse, chain
import yaml
@@ -127,6 +127,25 @@ def compute_reverse_tree(pkg_universe, affected):
return None
+def add_transitive_dependencies_flatten(pkg_universe, initial_set):
+ """Find and include all transitive dependencies
+
+ This method updates the initial_set parameter to include all transitive
+ dependencies. The first argument is an instance of the BinaryPackageUniverse
+ and the second argument are a set of BinaryPackageId.
+
+ The set of initial packages will be updated in place and must
+ therefore be mutable.
+ """
+ remain = list(initial_set)
+ while remain:
+ pkg_id = remain.pop()
+ new_pkg_ids = [x for x in chain.from_iterable(pkg_universe.dependencies_of(pkg_id)) if x not in initial_set]
+ initial_set.update(new_pkg_ids)
+ remain.extend(new_pkg_ids)
+ return None
+
+
def write_nuninst(filename, nuninst):
"""Write the non-installable report
--
2.24.0
Attachment:
signature.asc
Description: OpenPGP digital signature