[pacman-dev] [PATCH] Add more information in conflicts question

[Sebastian Nowicki]

On Mon, Apr 18, 2011 at 12:49 AM, Rémy Oudompheng
<remyoudompheng at gmail.com> wrote:
> On Sun 17 April 2011 at 23:40 +0800, Sebastian Nowicki wrote:
>> On Sun, Apr 17, 2011 at 5:50 PM, Rémy Oudompheng
>> <remyoudompheng at gmail.com> wrote:
>> >
>> > I am totally against this. Really really totally.
>> >
>> > I personally think a formatting function for package information has
>> > strictly nothing to do in sync.c, and maybe not even in libalpm.
>> > Second, it would introduce additional parsing effort from front-ends to
>> > split against the passed string if they want to display information in a
>> > different way.
>>
>> Agreed, but the alternatives require changes to the API (char * -> pmpkg_t).
>
> I think we disagree in what we call an API change. All the suggested
> patches are API changes in the sens that they change the behaviour of a
> function. Even if its type signature does not change, the behaviour
> changes in the sense that the nature of what is returned changes.
>
> As you already noticed, going from a simple package name to a complex
> string changes a lot of things when considering what a user (developer)
> may want to do with it.
>
> The only advantage is keeping binary compatibility for several
> applications, e.g. pacman, but that is a coincidence.

We do agree. I mentioned that it's an API change in the patch. I guess
I was referring to ABI rather than API as only the semantics changed
in this patch.

>> > And what output do you expect if a conflict arises when running
>> > pacman -U ?? What do you think is the repository from a package loaded
>> > from a file.
>>
>> Just the package name and version. Having a "file/" prefix would be
>> meaningless, and if you're installing from a single file, you're
>> likely to know what you're installing anyway.
>
> Is there is a reason you don't know what you are installing in other
> cases? Possible from the point of view of the user, not very improbable
> from the point of view of the program using libalpm.

The best example is `pacman -Syu` in which case hundreds of packages
can be updated. For me this is the most common instance of replaces
taking affect.

>> > I don't even understand why we would want to display any sort of version
>> > information. I don't see how it would help me answer yes or no to that
>> > conflict question either.
>> >
>> > That is in my mind the job of the front-end: if you really
>> > want information, it should display the list of selected packages
>> > (repository/pkgname-pkgver) that comes between the parsing of the
>> > command-line and the beginning of the transaction.
>> >
>> > For example:
>> >
>> > cb_trans_conv (from pacman) should be patched to display :
>> >  :: target qemu-kvm conflicts with installed qemu. Remove qemu ?
>> >
>> > which is perfectly clear: qemu is installed and pacman wants you to
>> > remove it, "local/qemu" brings no additional information, and the version
>> > number is usually meaningless. The callback returns precisely one local
>> > package and one target package, it can be reflected in the user output.
>>
>> If the version of the local package is newer than the one being
>> installed, it would be good to inform the user (as would have been
>> likely with the libdrm-git package). The "local/" prefix can easily be
>> omitted.
>
> I don't see why the version information is particularly more relevant
> than other things. Personnally I'm interested in knowing whether my local
> package comes from an official repository, if so, whether it's in
> testing... Things that still not solved using such a partial approach.

I never stated it was more relevant. The origin is already shown. What
other information are you referring to?

> Does knowing the version really helps you in answering yes or no ???

This is what the feature request was for.

>> > If you need to display the version number, then I don't feel like
>> > messing with libalpm's internals is the right answer. I would probably
>> > prefer have process_targname() patched (in pacman/sync.c), so that
>> > after running alpm_find_dbs_satisfier(), it prints something.
>>
>> I think that changing the API of the callback to use pmpkg_t, (or even
>> pmconflict_t if that's exposed) rather than a string, is the right
>> thing to do. The front-end shouldn't have to find the package again to
>> get its metadata (what if there are multiple packages with the same
>> name?).
>
> The front-end doesn't have to do anything magic to find what packages are
> in the transaction. Simple calls to alpm_find_satisfier() and
> alpm_trans_get_add() suffice to find the culprit package (even if
> of course having the whole pmpkg_t structure would be better).

In my opinion this is only a workaround to prevent an API change
(which is reasonable). It still requires traversing through lists in
order to find the package. It may not be "magic", but it is
unnecessary effort to find what is already available in the
_alpm_sync_prepare() function. I'll write up a patch if this is the
preferred method though.