Bug#944190: release.debian.org: Allow britney to consider installability of dependencies of essential packages

[Niels Thykier <niels@thykier.net>]

Mark Hindley:
> Neils,
> 
> On Fri, Nov 08, 2019 at 07:03:00AM +0000, Niels Thykier wrote:
>> Hi Mark
>>
>> Thanks for the investigative work and the patch.
>>
>> I have not had time to review the patch yet in details and hope to have
>> a look this weekend.
> 
> Thanks.
> 

Hi Mark,

Once again, thanks for trying to fix this.  I have had a look at the
code and your proposed patch.  As I understand it, the bug will still be
present but just "harder" to trigger and possibly harder to debug (I
suspect we will end up with a corrupted state in the essential set cache).

>> Could I convince you to add a small test case for this problem to our
>> britney2-tests repo (https://salsa.debian.org/debian/britney2-tests)
>> that fails with the current master but succeeds with your patch?  This
>> would ensure we do not inadvertently regress on this area when
>> refactoring code.
> 
> I will happily look at that. I am busy until Sunday, but will look at it 
> then.
> 
> Many thanks.
> 
> Mark
> 

I looking forward to your test case as it will make this issue a lot
easier to debug.

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.

 * The InstallabilityTester "should" consider that relation "unsolvable"
   when building the cache and defer it to later.  That is, it should
   keep the relation in "ess_choices" in _get_min_pseudo_ess_set until
   it returns.

 * 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 these are the 3 key points of assumptions to verify.  Honestly,
I *doubt* the first point fails (I would expect a lot more breakage),
but I have been wrong before.

The below is the loop responsible for pre-computing the essential set.

>             while ess_base:
>                 self._check_loop(universe, suite_contents, stats,
>                                  start, ess_never, cbroken,
>                                  ess_choices, ess_base)
>                 if ess_choices:
>                     # Try to break choices where possible
>                     nchoice = set()
>                     for choice in not_satisfied(ess_choices):
>                         b = False
>                         for c in choice:
>                             relations = universe.relations_of(c)
>                             if relations.negative_dependencies <= ess_never and \
>                                     not any(not_satisfied(relations.dependencies)):
>                                 ess_base.append(c)
>                                 b = True
>                                 break
>                         if not b:
>                             nchoice.add(choice)
>                     ess_choices = nchoice
>                 else:
>                     break

The crucial point here is that:

  relations.negative_dependencies <= ess_never

... is supposed to be false for *both* libsystemd0 *and* libelogin0, but
could become True for libsystemd0 if there is a bug that makes britney
pick another package in the pseudo-essential set that mandates (the
"real") libsytemd0.

Thanks,
~Niels