On Fri, Oct 8, 2010 at 10:31 PM, Allan McRae <allan@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