[pacman-dev] [PATCH] Fix bug 9395 by allowing pacman to remove unresolvable packages from a transaction

[Bryan Ischo]

Nagy Gabor wrote:
>> In terms of a warning, pacman already prints out lines like this:
>>
>> warning: kernel26: ignoring package upgrade (2.6.27.8-1 => 2.6.27.10-1)
>> warning: pacman: ignoring package upgrade (3.2.1FIXBUG9395-1 => 3.2.1-2)
>> warning: pango: ignoring package upgrade (1.22.3-2 => 1.22.4-1)
>>
>> Is that not sufficient?
>
> No. If you check pacman/callback.c, you will see that 
> PM_TRANS_CONV_INSTALL_IGNOREPKG was used for asking the following 
> questions:
> ":: %s requires installing %s from IgnorePkg/IgnoreGroup. Install 
> anyway?" (by resolvedeps)
> ":: %s is in IgnorePkg/IgnoreGroup. Install anyway?" (Iirc, after 
> "pacman -S ignoredpkg").
>
> These differ from sysupgrade (-Su) messages.
>

OK, I agree with you - I will put the prompt back for the second case.

>> HOWEVER, in writing this very last line, I realize that my final test
>> should be:
>>
>> if (is_needed(infolist, findinfo(infolist, deppkg), marker)) {
>>    return(1);
>> }
>>
>> This is because we want to consider a package needed if any of its
>> dependent packages are needed.  The code as it was was mistakenly
>> considering a package needed only if all of its dependent packages are
>> needed.
>>
>> Does that make sense?
>>
>
> Yes. And probably you want to change the final ("fall-back") return(1) 
> to return(0) in the function.

Yes indeed!  This was caught by one of the new test cases I added and I 
fixed it already :)

>
>>> 2. The result of this:
>>> error: cannot resolve dependencies for:
>>> error:    foo
>>> error:    bar
>>> ...
>>>
>>> This is ugly, we use data list for this purpose, the front-end should
>>> build this message.
>>>
>> Please tell me exactly the format you want to see this error message
>> in, and I will make it so.  I don't know exactly what you are expecting
>> so I am not sure how to fix the problem you are pointing out.
>
> I guess we shouldn't print anything in alpm. Probably in the front-end 
> we will check pm_errno and print the correct error message using the 
> data list. I know that we had an error message here earlier, but I 
> don't like these redundant error messages. (Front-end always calls 
> _alpm_strerrorlast(), and processes data list).

OK, I'm very happy to remove that error logging.  I don't like it 
either, I was just trying to match what I thought the standard behavior 
was for these functions, which do seem to log errors in such cases, but 
I absolutely don't need or want to do it ...

>
> No. pkg->depends or alpm_pkg_get_depends(pkg) is used to get the 
> dependency list of pkg. In the old code it was easy to determine if a 
> dependency is broken. If we couldn't find a satisfier package in the 
> target list or in the local database (which won't be upgraded), we 
> simply detected that this dependency is unresolvable. (However, our 
> error message was often uninformative, user may got an error message 
> like this: "Cannot resolve foo dependency of pulledpkg". User asked: 
> Pulledpkg? What? I just did "pacman -S bar". [1])
>
> However, your patch makes things more complicated. We start to resolve 
> foo dependency of bar, which may be satisfied. But *later* it may turn 
> out, that we cannot resolve completely the _pulled_ foo satisfier, so 
> we simply remove it  from the list. Now it would be elegant if we 
> could say: We could not resolve foo dependency of bar. (At this point 
> we can clearly see, that the old method doesn't work, because 
> *locally* foo is a resolvable dependency.) But it is not so easy to 
> detect, probably we need a more sophisticated info (graph?) structure, 
> and imho we can catch this broken dependency when we "label" the 
> package as unresolvable.
I'm not sure I follow everything you are saying here, and in a later 
email you say that what you've written here is due to some 
misinterpretation, but in terms of the last part, I think you're saying 
that it would be nice if there were a more sophisticated data structure 
that could describe all of the dependencies that were calculated, 
including the ones that could not be resolved, and why, so that the 
front-end can print out a more informative message, indicating what 
could not be resolved and why, instead of just listing the packages that 
could not be resolved (without saying why).  I agree - but I have 
suggested that I would do this in a second checkin.  Already there has 
been push back on this change because people are saying it's too big (I 
disagree; it's as small as it can be to accomplish the goal, but anyway 
...).  I'd rather not take on even more stuff for this particular change.

Thanks,
Bryan