[pacman-dev] [PATCH 1/5] Add epoch support to pacman/libalpm

[Dan McGee]

On Fri, Oct 8, 2010 at 10:31 PM, Allan McRae <allan at archlinux.org> wrote:
> On 09/10/10 01:02, Dan McGee wrote:
> <snip>
>
>> -/* Is spkg an upgrade for locapkg? */
>> +/* Is spkg an upgrade for localpkg? */
>>  int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg)
>>  {
>> -       int cmp = 0;
>> +       int spkg_epoch, localpkg_epoch;
>>
>>        ALPM_LOG_FUNC;
>>
>> -       cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg),
>> -                       alpm_pkg_get_version(localpkg));
>> +       spkg_epoch = alpm_pkg_get_epoch(spkg);
>> +       localpkg_epoch = alpm_pkg_get_epoch(localpkg);
>>
>> -       if(cmp<  0&&  alpm_pkg_has_force(spkg)) {
>> -               cmp = 1;
>> +       if(spkg_epoch>  localpkg_epoch) {
>> +               return(1);
>> +       } else if(spkg_epoch<  localpkg_epoch) {
>> +               return(-1);
>>        }
>>
>> -       return(cmp);
>> +       /* equal epoch values, move on to version comparison */
>> +       return alpm_pkg_vercmp(alpm_pkg_get_version(spkg),
>> +                       alpm_pkg_get_version(localpkg));
>>  }
>
>
> How about:
>
> /* Is spkg an upgrade for localpkg? */
> int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg)
> {
>        int cmp, spkg_epoch, localpkg_epoch;
>
>        ALPM_LOG_FUNC;
>
>        cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg),
>                        alpm_pkg_get_version(localpkg));
>
>        spkg_epoch = alpm_pkg_get_epoch(spkg);
>        localpkg_epoch = alpm_pkg_get_epoch(localpkg);
>
>        if(spkg_epoch > localpkg_epoch && cmp != 0) {
>                return(1);
>        } else if(spkg_epoch < localpkg_epoch) {
>                return(-1);
>        }
>
>        return(cmp);
> }
>
>
> So, when 'force' option is in effect, spkg_epoch = INT_MAX, localpkg_epoch =
> 0, but the versions are the same so we ignore it.

Works great now, but what if there is a legit reason to deem this as
an upgrade? Throw out the 'force' and look at it as epochs 1 and 2 or
something, so you don't get caught up just thinking this is an old
problem. Epoch *always* has to win or it doesn't work correctly.

Do we not record the force option in the local database right now?
That is quite unfortunate. I think we need to make an exception for
the force case, and in the future, we will always have epoch recorded
in both sync and local DBs.

Counterexample to your code above:

Project releases version 1.0, 1.1, and 2.0. Project decides it wasn't
ready for 2.0, so abandons that branch and goes back to 1.3. We need
to use epoch here because we were quick to package it. Later, because
someone wasn't thinking clearly, they release 2.0 again. Contrived,
sure, but it would break your code (if the person still had 2.0
installed they would never see the new 2.0).

-Dan