Bug 1661347 - bug in comparison/sorting of debian package epoch/version/revision attributes.
Summary: bug in comparison/sorting of debian package epoch/version/revision attributes.
Keywords:
Status: CLOSED INSUFFICIENT_DATA
Alias: None
Product: Spacewalk
Classification: Community
Component: Server
Version: 2.8
Hardware: x86_64
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Michael Mráka
QA Contact: Red Hat Satellite QA List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-12-20 23:37 UTC by ppanon-avi
Modified: 2020-04-23 06:35 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-03-09 13:33:44 UTC
Embargoed:


Attachments (Terms of Use)
Corrected rpmstrcmp function (4.23 KB, text/plain)
2018-12-20 23:37 UTC, ppanon-avi
no flags Details
Updated rpm.rpmstrcmp, changed to also avoid re-introducing bug 1249219 (4.19 KB, text/plain)
2019-01-04 03:49 UTC, ppanon-avi
no flags Details
Updated rpm.rpmstrcmp to handle comparisons of Ubuntu rc package versions (4.47 KB, application/sql)
2019-08-30 18:25 UTC, ppanon-avi
no flags Details
rpm.rpmstrcmp updated to better handle comparisons of ubuntu patch levels (4.55 KB, application/sql)
2019-08-31 00:00 UTC, ppanon-avi
no flags Details
Updated SW2.10 rpmstrcmp handling Ubuntu prerelease [gcc] packages (7.88 KB, text/plain)
2020-04-23 05:11 UTC, ppanon-avi
no flags Details

Description ppanon-avi 2018-12-20 23:37:55 UTC
Created attachment 1516003 [details]
Corrected rpmstrcmp function

Description of problem:
Ubuntu releases sometimes include pre-release versions of packages that don't strictly meet Debian package numbering standards. For example, the initial gcc package for Ubuntu 14.04 was gcc-4.9-base-4.9-20140406-0ubuntu1.amd64-deb, and the current version of the package is gcc-4.9-base-4.9.3-0ubuntu4.amd64-deb. Spacewalk 's evr sorting treats the 4.9-20140406 as newer than the 4.9.3 release and always indicates the new package needs to be upgraded to the older version.

Version-Release number of selected component (if applicable):

Still in 2.8.60-1.el7 , although it has been in older releases back to at least 2.6


How reproducible:
With either Ubuntu 14.04 or Ubuntu18.04 distributions that include pre-release versions of packages (gcc and friends is a good example)

Steps to Reproduce:
1. Populate base (main) channel and update channel from Ubuntu repos for Ubuntu 18.04

2. a)Prepare a new Ubuntu VM from the 18.04 install CD, including the gcc-8-base compiler and libraries. Do not allow updates to be applied.
   b)Join to Spacewalk associated to the two 18.04 channels, without first patching against the Ubuntu repos.
   c)Checking for packages to upgrade in Spacewalk will not list the gcc-8-base package and related packages as needing upgrades, even though there are newer versions in the bionic-update repo.

3. a) unregister/remove the Ubuntu VM from the Spacewalk group and re-enable the Ubuntu Internet repos. 
   b) Apply all pending updates
   c) re-register the VM with the Spacewalk repo
   d) check for updates. It will list the original pre-release gcc-8-base packages as newer than the latest versions.

Actual results:
Spacewalk treats version 8-20180414 as newer than 8.2.0

Expected results:
8.2.0 should be treated as more recent than 8-20180414 

Additional info:
The bug is in the rpm.rpmstrcmp stored procedure used by the evr_t comparison operators, which treats all separators between version portions as the equivalent. i.e. '.' == '-' . The comparison function therefore compares 2 against 20180414 and determines that 20180414 is greater/more recent. 

The attached file includes a change to that stored proc which fixes the comparison.

Comment 1 ppanon-avi 2019-01-04 03:08:43 UTC
In retrospect, the last line, setting ownership to the account our site uses, shouldn't be used as part of an update. I used it because I copied it from a schema dump and was running it using the postgres user, however an update script should be running it under whatever the Spacewalk installation is configured to use for connecting to the database.

It would be really good for this to make it into Spacewalk 2.9.

Comment 2 ppanon-avi 2019-01-04 03:33:09 UTC
OK looking back at Bug 1249219, there was a case made that . and _ should be treated as equivalent. However that approach breaks with the pre-release packages in the initial packages for Ubuntu 14.04 and 18.04.  A more specific test for this condition can be used to avoid breaking the bugfix for 1249219:

                -- if one of the separators is for a point subversion indicator and the other is for a pre-release, then the point subversion is considered more recent.
                if isnum and sep1 <> '' and sep2 <> ''
                then
                    if sep1 = '.' and sep2 == '-'
                    then
                        return 1;
		elsif sep2 = '.' and sep1 == '-'
                    then
                        return -1;
					end if;
                end if;

Comment 3 ppanon-avi 2019-01-04 03:37:11 UTC
Sorry, tabs crept in there somehow.

                -- if one of the separators is for a point subversion indicator and the other is for a pre-release, then the point subversion is considered more recent.
                if isnum and sep1 <> '' and sep2 <> ''
                then
                    if sep1 = '.' and sep2 == '-'
                    then
                        return 1;
                    elsif sep2 = '.' and sep1 == '-'
                    then
                        return -1;
                    end if;
                end if;

Comment 4 ppanon-avi 2019-01-04 03:38:54 UTC
Once more for luck, and without the ==

                -- if one of the separators is for a point subversion indicator and the other is for a pre-release, then the point subversion is considered more recent.
                if isnum and sep1 <> '' and sep2 <> ''
                then
                    if sep1 = '.' and sep2 = '-'
                    then
                        return 1;
                    elsif sep2 = '.' and sep1 = '-'
                    then
                        return -1;
                    end if;
                end if;

Comment 5 ppanon-avi 2019-01-04 03:49:47 UTC
Created attachment 1518292 [details]
Updated rpm.rpmstrcmp, changed to also avoid re-introducing bug 1249219

Comment 6 philippe.bidault 2019-01-04 14:45:08 UTC
I did have this same bug, as those packages was always flags as upgradable in Spacewalk for the Ubuntu 18.04 registered servers :

	gcc-8-base-8-20180414-1ubuntu2.amd64-deb 	gcc-8-base-8.2.0-1ubuntu2~18.04.amd64-deb 	
	lib32gcc1-8-20180414-1ubuntu2:1.amd64-deb 	lib32gcc1-8.2.0-1ubuntu2~18.04:1.amd64-deb 	
	libatomic1-8-20180414-1ubuntu2.amd64-deb 	libatomic1-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libcc1-0-8-20180414-1ubuntu2.amd64-deb 		libcc1-0-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libgcc1-8-20180414-1ubuntu2:1.amd64-deb 	libgcc1-8.2.0-1ubuntu2~18.04:1.amd64-deb 	
	libgomp1-8-20180414-1ubuntu2.amd64-deb 		libgomp1-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libitm1-8-20180414-1ubuntu2.amd64-deb 		libitm1-8.2.0-1ubuntu2~18.04.amd64-deb 	
	liblsan0-8-20180414-1ubuntu2.amd64-deb 		liblsan0-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libmpx2-8-20180414-1ubuntu2.amd64-deb 		libmpx2-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libquadmath0-8-20180414-1ubuntu2.amd64-deb 	libquadmath0-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libstdc++6-8-20180414-1ubuntu2.amd64-deb 	libstdc++6-8.2.0-1ubuntu2~18.04.amd64-deb 	
	libtsan0-8-20180414-1ubuntu2.amd64-deb 		libtsan0-8.2.0-1ubuntu2~18.04.amd64-deb

I have applied the PostgreSQL SP attached by ppanon-avi in a Spacewalk 2.8 installation, and as far as I can see this solved my issue.

Regards,
Philippe.

Comment 7 philippe.bidault 2019-07-31 06:21:02 UTC
It seems that finally that if I am not wrong the stored procedure is not working at 100% as Spacewalk wrongly detects the "~rc1" packages to be updated on some Ubuntu 18.04 servers :

Latest Package                                             Installed Package 
libpython2.7-2.7.15~rc1-1ubuntu0.1.amd64-deb               libpython2.7-2.7.15-4ubuntu4~18.04.amd64-deb	
libpython2.7-minimal-2.7.15~rc1-1ubuntu0.1.amd64-deb	   libpython2.7-minimal-2.7.15-4ubuntu4~18.04.amd64-deb	
libpython2.7-stdlib-2.7.15~rc1-1ubuntu0.1.amd64-deb        libpython2.7-stdlib-2.7.15-4ubuntu4~18.04.amd64-deb	
python2.7-2.7.15~rc1-1ubuntu0.1.amd64-deb                  python2.7-2.7.15-4ubuntu4~18.04.amd64-deb	
python2.7-minimal-2.7.15~rc1-1ubuntu0.1.amd64-deb          python2.7-minimal-2.7.15-4ubuntu4~18.04.amd64-deb

Regards,
Philippe.

Comment 8 ppanon-avi 2019-07-31 17:17:38 UTC
Yes, I've noticed that recently as well. Also with

Latest Package                                                          Installed Package 
xserver-xorg-core-hwe-16.04-1.19.6-1ubuntu4~16.04.1:2.amd64-deb	        xserver-xorg-core-hwe-16.04-1.19.6-1ubuntu4.1~16.04.2:2.amd64-deb
xserver-xorg-legacy-hwe-16.04-1.19.6-1ubuntu4~16.04.1:2.amd64-deb	xserver-xorg-legacy-hwe-16.04-1.19.6-1ubuntu4.1~16.04.2:2.amd64-deb
 
It's a challenge to have something in a SQL function, with no context on whether a package name is for Ubuntu or CentOS, that will meet the needs of CentOS/RedHat (bug 1249219) and not break with Ubuntu's contortionist bending of the Debian naming/numbering conventions.

Comment 9 ppanon-avi 2019-07-31 17:37:12 UTC
And there really isn't a way to change that either. The same user-defined data type/object class applies to both types of packages because they're used in the same columns in the data model package entity. That UDT encapsulates the comparison function/SP, which is used in SELECT queries when comparing rows to find the "most recent/newest" row. That column/field is in the alternate key for a primary entity, and splitting it into two entities would have repercussions through the data model and would require massive changes/duplication in the GUI. I just can't see it happening.

Comment 10 ppanon-avi 2019-08-30 18:25:49 UTC
Created attachment 1609945 [details]
Updated rpm.rpmstrcmp to handle comparisons of Ubuntu rc package versions

Comment 11 ppanon-avi 2019-08-30 18:33:24 UTC
That still doesn't fix the issue with the Ubuntu 16 xserver-xorg-core-hwe versions. But it does fix the comparison of release candidate versions.

Comment 12 ppanon-avi 2019-08-31 00:00:49 UTC
Created attachment 1610033 [details]
rpm.rpmstrcmp updated to better handle comparisons of ubuntu patch levels

This should now allow correct comparisons between
xserver-xorg-core-hwe-16.04-1.19.6-1ubuntu4~16.04.1:2.amd64-deb
and
xserver-xorg-core-hwe-16.04-1.19.6-1ubuntu4.1~16.04.2:2.amd64-deb 
by determining that the latter is more recent because 1ubuntu4.1 > 1ubuntu4

Comment 13 philippe.bidault 2019-09-06 15:22:16 UTC
Thanks, tested on my Spacewalk, and all working the false-positive upgradable packages are gone for Ubuntu 18.04 clients.

Comment 14 Michael Mráka 2019-09-10 13:58:00 UTC
We have recently implemented tilde and caret sorting, see
https://github.com/spacewalkproject/spacewalk/commit/0df225487e572bf9bb1b9fe751f919318903c0cd

Does it fix your issue or are there still some differences?

Comment 15 ppanon-avi 2019-09-10 20:05:32 UTC
Not fully, no. While I think it does deal correctly with the cases identified in comments 7 and 8 above, it fails to correctly deal with the cases in comment 6.

I tested this by renaming the tip version of the function as testrpmstrcmp, adding it to our database, and checking the results with 

spaceschema=# select testrpmstrcmp('gcc-8-base-8-20180414-1ubuntu2.amd64-deb', 'gcc-8-base-8.2.0-1ubuntu2~18.04.amd64-deb')
;
 testrpmstrcmp
---------------
             1
(1 row)


I would expect that result to be the same as 


spaceschema=# select testrpmstrcmp('libpython2.7-2.7.15~rc1-1ubuntu0.1.amd64-deb', 'libpython2.7-2.7.15-4ubuntu4~18.04.amd64-deb');
 testrpmstrcmp
---------------
            -1
(1 row)

spaceschema=# select testrpmstrcmp('xserver-xorg-core-hwe-16.04-1.19.6-1ubuntu4~16.04.1:2.amd64-deb', 'xserver-xorg-core-hwe-16.04-1.19.6-1ubuntu4.1~16.04.2:2.amd64-deb');
 testrpmstrcmp
---------------
            -1
(1 row)

and I believe that the latter two are the correct ones. My version doesn't do caret handling and only works for Postgres, but perhaps you can take a look at what I did and merge it with your changes or modify it to add caret handling. I didn't look in detail but I think they perform similarly since they both do tilde handling.

Comment 16 Michael Mráka 2019-09-11 08:26:56 UTC
The main issue in comparison
  gcc-8-base-8-20180414-1ubuntu2.amd64-deb > gcc-8-base-8.2.0-1ubuntu2~18.04.amd64-deb
is that '8-20180414' is not a correct version.
Version must not contain dash (-) therefore the function actually compares
  name:gcc-8-base-8 version:20180414 release:1ubuntu2 arch:amd64-deb
vs.
  name:gcc-8-base version:8.2.0 release:1ubuntu2~18.04 arch:amd64-deb
and this correctly gives 20180414 > 8.2.0.

Even replacing '-' with allowed '.' or '_' still would give
  8.20180414 > 8.2.0
  8_20180414 > 8.2.0

So IMO this is not a bug in version comparison but ubuntu package versioning.

Pre-release packages should use '~' version as
  8~20180414 < 8.2.0

Comment 17 Michael Mráka 2020-03-09 13:33:44 UTC
This issue has been closed because it is missing requested data for a long time.
If you are still able to see it on your setup you are encouraged
to update the reproducer and reopen it.

Comment 18 ppanon-avi 2020-04-23 05:11:18 UTC
Created attachment 1681011 [details]
Updated SW2.10 rpmstrcmp handling Ubuntu prerelease [gcc] packages

Comment 19 ppanon-avi 2020-04-23 06:30:44 UTC
I've modified the Spacewalk 2.10 rpmstrcmp (and added a sub-function for it) to deal with the Ubuntu 18.04 initial/pre-release gcc-related packages.

I ran a test of the modified stored procedure logic, comparing it against the stock Spacewalk 2.10 rpmstrcmp for all the packages in our copies of CentOS 7, Ubuntu 16.04, Ubuntu 18.04, SUSE 11, and SUSE 15. The only combinations of related packages for which different results were returned were pre-release packages as expected.

select 
 pn1.name,
 p1.id, 
 pv1.evr,
 c1.label, 
 COALESCE( pc1.label, c1.label),
 p2.id, 
 pv2.evr,
 c2.label, 
 COALESCE( pc2.label, c2.label),
 rpm.vercmp(pv1.epoch, pv1.version, pv1.release, pv2.epoch, pv2.version, pv2.release),
 vercmp_sw2101(pv1.epoch, pv1.version, pv1.release, pv2.epoch, pv2.version, pv2.release)
from (rhnpackage p1 
  inner join rhnpackagename pn1 on pn1.id = p1.name_id
  inner join rhnpackageevr pv1 on pv1.id = p1.evr_id
  inner join rhnchannelpackage cp1 on p1.id = cp1.package_id
  inner join rhnchannel c1 on c1.id = cp1.channel_id
  left join rhnchannel pc1 on pc1.id = c1.parent_channel)
JOIN (rhnpackage p2 
  inner join rhnpackageevr pv2 on pv2.id = p2.evr_id
  inner join rhnchannelpackage cp2 on p2.id = cp2.package_id
  inner join rhnchannel c2 on c2.id = cp2.channel_id
  left join rhnchannel pc2 on pc2.id = c2.parent_channel)
on p1.name_id = p2.name_id 
  and p2.evr_id >= p1.evr_id
  and COALESCE( pc1.label, c1.label) =  COALESCE( pc2.label, c2.label)
where 
 rpm.vercmp(pv1.epoch, pv1.version, pv1.release, pv2.epoch, pv2.version, pv2.release) <>
 vercmp_sw2101(pv1.epoch, pv1.version, pv1.release, pv2.epoch, pv2.version, pv2.release)
order by pn1.name, p1.evr_id, p2.evr_id;

The above query is what I used for the comparison prior to replacing the stock rpm.rpmstrcmp, where vercmp_sw2101 is a modified rpm.vercmp I used to drive the (also temporarily renamed) modified rpmstrcmp.

Comment 20 ppanon-avi 2020-04-23 06:35:16 UTC
In case I was unclear, the actual modified SW 2.10 rpmstrcmp is the second bug attachment, and the query in the previous comment is the query used to verify the functionality of the modified rpmstrmp


Note You need to log in before you can comment on or make changes to this bug.