Fix rpm version comparison

[vzhestkov]

What does this PR do?

With #68342 the logic of version comparison was improved, but one addition of omitting the release part if it's missing for one of the parameters is breaking the previous behaviour what is causing some issues.

Some vendors are releasing rpm packages with blank release, but after could add release to the same version, what should be treated as new, but due to the change in the PR mentioned it's showing the version with blank release and with the specified release as identical.

The problematic part is:

salt/salt/modules/rpm_lowpkg.py

Lines 707 to 712 in 4ce634d

	# If one EVR is missing a release but not the other and they
	# otherwise would be equal, ignore the release. This can happen if
	# e.g. you are checking if a package version 3.2 is satisfied by
	# 3.2-1.
	if not ver1_r or not ver2_r:
	ver1_r = ver2_r = ""

The case described in the comment must be handled explicitly if really needed with no breaking previous logic.

EXTRA NOTE: looks like tests/pytests/unit/modules/test_rpm_lowpkg.py::test_version_cmp_matches_rpm was skipped due to missing rpm module in the testing environment, what was hiding some failing tests. With this change it's still skipped, but should work fine. It requires adding rpm module.

Previous Behavior

Wrong result of pkg.version_cmp call could be returned if the release part of any of the paramiters is omitted.

New Behavior

Return to the original behaviour.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

Yes/No

[bdrx312]

I agree that the comparisons changes you made seem more correct.

Note however that this behavior

The case described in the comment must be handled explicitly if really needed with no breaking previous logic.

and code was not added as part of #68342. It was just moved around. It was previously on lines 725-733.

https://github.com/saltstack/salt/pull/68342/changes#diff-ab812a87d31b49af2a8f96e0cbb4c89eda9eee8536dde06e8f34911de4002a67L725-L733

[bdrx312]

According to rpmdev-vercmp, these should be 0 not 1 and -1

$ rpmdev-vercmp '2.0-' '2.0'
2.0 == 2.0

[vzhestkov]

And it's wrong just to call it this way. I mean rpmdev-vercmp shold compare separately either epoch or version or release. In case of using 2.0- in reality it should be treated as the release with empty string value, while 2.0 is just a version with no release at all. In this particular call actually 2.0- as version value is compared to 2.0 as a version value only, assuming there is no release in both cases. rpmdev-vercmp must be called separatelly for epoch and if it's equal or omitted, versions should be compared and if identical, releases. But for 2.0- in this context it's version where - is just dropped by rpm logic as it shouldn't be there.

> zypper vcmp '2.0-' '2.0'
2.0- is newer than 2.0

zypper here is intended to compare full [epoch]:version-release strings.

For some reason different version comparison libs are using slightly different logic causing some mess. In this exable above zypper is using libsolv internally which is using exact copy of logic from rpm itself.

[bdrx312]

From all research that I have done, an rpm without a release is not a valid case. An rpm that does not have a release field is invalid and I believe the tool should be rejecting it.

https://rpm.org/docs/latest/man/rpm-version.7#Comparing

Only the VERSION component is mandatory in an EVR label. All RPM packages have a VERSION and a RELEASE, however.

The release is not required to be in the EVR but the RPM information is still required to have the release field.

What is the actual use case where you are even running into this problem? It seems these RPMs may be invalid and whatever tool is building them needs to be fixed?

The lua rpm.vercmp does seem to agree with your comparison though:

 $ rpm --eval '%{lua:print(rpm.vercmp("2.0-","2.0"))}'
1