New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assume two EVRs are equal if E and V are equal but one R is missing. #35119

Merged
merged 3 commits into from Aug 3, 2016

Conversation

Projects
None yet
3 participants
@dere
Copy link
Contributor

dere commented Aug 2, 2016

What does this PR do?

Changes the behavior of RPM version checking such that if one release is missing but they are otherwise equal, assume they are equal. Installed packages will always have a release, but a desired version specification may not, and in that case we should assume the installed version matches the desired version, as yum does.

What issues does this PR fix or reference?

#34861

Previous Behavior

Assume somepkg-3.2-1 is installed and you want installed somepkg-3.2. Previously, somepkg-3.2-1 would not satisfy somepkg-3.2 and 3.2 would be slated for install. This would fail because yum considers 3.2 to already satisfy 3.2-1, and since no changes have been made, the state would fail entirely.

New Behavior

somepkg-3.2-1 will satisfy somepkg-3.2.

Tests written?

No

@dere

This comment has been minimized.

Copy link
Contributor Author

dere commented Aug 2, 2016

In #34861, pkg.installed fails when the package is not previously installed, and fails when it is previously installed. This PR fixes the latter, I assume it fixes the former but I am not currently certain.

)

return cmp_result
else:

This comment has been minimized.

@terminalmage

terminalmage Aug 3, 2016

Contributor

This "else" directly follows the previous "else". There's no way this wouldn't raise an exception, was this tested?

'Comparison result \'{0}\' is invalid'.format(tmp_cmp)
)
if tmp_cmp == 0:
return 0

This comment has been minimized.

@terminalmage

terminalmage Aug 3, 2016

Contributor

This entire block of code is an unnecessary duplication of what comes below. The better solution here would be to replace the entire block after salt.utils.str_version_to_evr(ver2) with the following:

if not ver1_r or not ver2_r:
    ver1_r = ver2_r = ''

Then we can let the call to the cmp_func proceed below as normal.

@dere dere force-pushed the dere:develop branch from 163b6dd to f6eb5d0 Aug 3, 2016

@dere

This comment has been minimized.

Copy link
Contributor Author

dere commented Aug 3, 2016

You're right, it is unnecessary. In the case that the EVRs sans release are not equal, one package is older or newer than the other, and in that case it doesn't matter what the release is.

@terminalmage

This comment has been minimized.

Copy link
Contributor

terminalmage commented Aug 3, 2016

Awesome. This looks great!

@terminalmage terminalmage merged commit fbede2a into saltstack:develop Aug 3, 2016

1 of 5 checks passed

jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt Linode Ubuntu14.04 #3599 — FAILURE
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #3891 — FAILURE
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #4038 — FAILURE
Details
default Build started sha1 is merged.
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #3980 — SUCCESS
Details
@terminalmage

This comment has been minimized.

Copy link
Contributor

terminalmage commented Aug 3, 2016

@rallytime this should be backported to 2016.3 at minimum.

@rallytime

This comment has been minimized.

Copy link
Contributor

rallytime commented Aug 3, 2016

@terminalmage Can do!

@rallytime

This comment has been minimized.

Copy link
Contributor

rallytime commented Aug 5, 2016

Back-ported to 2015.8 in #35236.

cachedout pushed a commit that referenced this pull request Aug 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment