[pacman-dev] PATCH: Fix bug 9395 (allow sync to remove unresolvable packages if user wishes)

[Bryan Ischo]

Nagy Gabor wrote:
>
> Basically this is a neat (and readable) job imho.
>   
Thanks for your kind words and your input.  Comments inline below.
> Some comments:
> 1. There is a typo in the description of unresolvable field.
>   
Check, will fix.

> 2. A new list for pkginfos looks ugly imho.
>   

What do you mean by 'ugly'?  Is it redundant given other data structures 
in libalpm?  If so, I will be happy to switch to another data 
structure.  Xavier already pointed out that pmgraph_t may fit the bill.  
I'll definitely look into it.  Or maybe you meant something else by 
'ugly'?  I'm not sure ...

> 3. I like the concept of pulled, maybe (pmpkg_t *) pulledby would be
> even better, then we could print more informative (error) messages (see
> FS#12536 for example). So we again reached to a ~graph structure.
> (Hm. Maybe dependents can be also used instead of pulledby...)
>   
This sounds like a good idea.  But it would involve greater changes to 
libalpm then I felt comfortable doing.  Unfortunately, when one is just 
a patch contributor rather than a real developer, one feels the need to 
keep all changes as minimal as possible, instead of reaching into lots 
of parts of the code and doing cleanups and refactoring and such.  But I 
would be happy to do some of that, once I feel more comfortable with the 
code base.

> 4. It is not easy to determine which package is unresolvable. If a
> pulled dependency satisfier of foo is unresolvable we may could find an
> other (resolvable) satisfier. But this is not handled in the current
> code neither, so your _alpm_mark_unresolvable() is OK.
>   
I see what you mean.  Is this something that should/could be fixed in 
_alpm_resolvedeps?  If so, perhaps it can be deferred to a separate bug 
incident and fixed separately from my change?

> 5. I may overlook something, but as I see, the following situation can
> happen:
> # pacman -S foo bar, where foo depends on bar.
> During resolvedeps foo may become unresolvable, but bar not. In
> this case pacman asks the user whether he wants to remove foo from
> target list. User answers yes. Since your _alpm_is_needed code (btw,
> _alpm prefix is not needed here) doesn't check pulled flag, it will
> determine that bar is not needed (which is not true, bar was an explicit
> package), and pacman will also remove bar as well, and the user wasn't
> informed about this.
>   
I will remove _alpm prefix from my static functions, I hadn't read the 
code guidelines carefully enough to realize the convention, mea culpa.

As to your problem situation: good catch, I missed that case in my 
thought process and in testing.  Thanks for pointing it out, I will fix 
it (should be an easy fix in _alpm_is_needed).

> 6. I think a dependency cycle can lead to an infinite loop due to
> _alpm_is_needed.
>   
I didn't realize that dependency cycles were possible.  But you are 
right, they would lead to an infinite loop.  I'll have to figure out 
some way to detect and break such cycles.  Thanks for pointing that out 
to me.

Best wishes,
Bryan