[Yum-devel] [PATCH][RFC] Add skip-broken to YumBase

[Florian Festi]

Hi Tim!

Having a first look over you patches the following comes to my mind:

Tim Lauridsen wrote:
> I have created at patch to add skip-broken functionality to YumBase.
> 
> I works the following way:
> 1. I have added some code to the depsolver to store packages with 
> problems in the self.po_with_problems dict. So we have the po with 
> problems when the depsolving is completed.
  * This should be a set. These key:1 dicts are ugly IMHO.
> 2. I have added a extra 'skip-broken' parameter to 
> YumBase.buildTransaction to control if we want to skip packages with 
> problem from the transaction. (default = no)
May be this should be done by accessing a global config option.

> 3. if the skip-broken parameter is True, then we do
> 
>    While "There is problems"
>        Remove the packages from the transaction
>        Depsolve


  * From my understanding of the architecture of the depsolver the handling 
of po_with_problems should be done in the process* methods. But that might 
be my personal taste.
  * Your patch fails to make sure that the problematic packages are not 
readded by dependencies. If that happens you run into an endless loop. The 
right solution would be an excluded_pkgs set that is ignored for all 
depsolving operations. If pkgs are removed from the transaction they get put 
into that set. This way we can never get back to the same situation and can 
not run into an endless loop.
  * While your patch tries to keep the impact on the core code minimal 
(which is a good idea for a first proof of concept implementation) I think 
we need a deeper integration into the depsolver in the long run. In fact 
there is no need of an outer loop around the depsolver. It would be enough 
if the process* methods would just remove the problematic pkg and adds them 
to the excluded_pkgs set.
  * As your patch works on the packages that have problems and not on the 
packages that cause the problems there might be interesting side effects.
  * Like readding a package (by removing the erase txmbr) that has been 
updated - resulting in the old and the new packages beeing installed 
(welcome to the world of multilib file conflicts!)
  * Although I added some code to handle removing transaction_members a 
while ago this code still requires some testing to make sure everything 
works fine.
  * There's also still the Depsolver.cheaterlookup. You need to make sure 
that nothing goes wrong with it (your patch should be ok, but some of the 
stuff I sketched above might not)

Florian