[pacman-dev] Some comments on Bryan patches (bugs)

[Bryan Ischo]

Nagy Gabor wrote:
>> The new code resolves each package individually and independently of the 
>> target-list.  It is for this reason that the first package found during 
>> resolve, the one in the sync database, is being used instead of the one 
>> on that target-list, so pkg2 is getting pulled in to satisfy a 
>> dependency of pkg1 that pkg3 would also have satisfied, and pkg3 is also 
>> getting installed because it's in the target-list.
>>     
>
> The "already-resolved" target list (== *packages list) is checked first.
> So the resolving is not independent from the target list, however, it is
> true that resolvedeps cannot see the *whole* target list now. For
> example, if you change the command line to "-S pkg3 pkg1" in my pactest,
> it will pass. (Btw, the order of packages in the target list should not
> affect the result of resolvedeps!)
>   


I agree, but this is very tricky now that we allow for some packages in 
the target-list to be removed from the transaction by the user *after* 
the resolve step is done.  Because now not all of the packages in the 
target-list are guaranteed to survive the resolve step, so it's not a 
good idea to pick one of them during the resolve.


>   
>> Unfortunately, the obvious fix of first looking through the target list 
>> for packages which can resolve dependencies does not work - because it 
>> might find packages which satisfy the dependency but which themselves 
>> have not yet been resolved.  And if it then uses those unresolved 
>> packages to satisfy dependencies, and later on it is discovered that 
>> those packages are not themselves resolvable (because of 
>> IgnorePkg/IgnoreGroup), then the result will be the inclusion of 
>> unresolvable packages in the transaction, which then fails.
>>
>> A shorter way to say this is: we cannot resolve dependencies just by 
>> looking in the target-list to find them, because the target-list 
>> packages have not necessarily been resolved yet, and we can only resolve 
>> dependencies with packages which themselves have been resolved.
>>     
>
> I don't see the problem. Then resolvedeps will "recursively" resolve the
> pulled package from target list without any code modification, and when
> we want to resolve that package (in a new resolvedeps call from
> sync_prepare), resolvedeps will detect that the package is already in
> *packages list.
>
> I mean, you may want to add a new param, "lookfirst" (== target list).
> However, I hoped that you have a nicer idea, that's why I didn't send a
> patch.
>
>   

Your new param "lookfirst" is exactly the approach that I tried to fix 
this problem (although I called the parameter "preferred", but it's the 
same idea), and it didn't work.

It solved the problem for your sync993.py test, but it caused failures 
in other tests, most notably my ignoreXXX.py tests.

However, I believe that the cause was a mistake in my patch, which did 
not add the package that it found in the "lookfirst" list to the 
"*packages" list, and continue to try to resolve those.  I will try my 
patch again with this fix and see if it works properly.

>> A question: is the behavior of first searching the target-list for 
>> packages to satisfy dependencies a documented and expected behavior of 
>> pacman?
>>     
>
> Not documented, but expected :). Moreover, see my comment above about
> the current asymmetric behavior, that is quite odd imho.
>
> -- new suggestions :) [optional] --
>
> 3. Now it can happen, that a missing dependency is added to the *data
> list many times. For example, if there is a popular unresolvable package
> in our repo, let's say gtk2, and we have 10 packages in the target list
> depend on that, then we get "gtk2 requires foo" error 10 times (if user
> doesn't want to remove unresolvable packages). So we may want to check
> *data list before adding a new missing dependency to avoid duplication.
>   

Definitely.  I will submit a patch for this too.

> 4. One more comment (I've just realised this during writing the previous
> one). I never liked our unresolvable dependencies errors:
> User does "pacman -S foo1 foo2 foo3" and he may get "error: bar requires
> baz", and he says "What?". Now, after your patch we can easily say, what
> package "induced" that error (pkg param of resolvedeps). We have a field
> causingpkg in pmdepmissing_t, we may want to use that for this purpose.
> (With -R that field has an other meaning, so this may be not a good
> idea.) Or we can print this info via PM_TRANS_CONV_REMOVE_PKGS.
>   

That sounds good.

Thanks,
Bryan