Skip to content
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

Add support for rpmdevtools returning < / > / == #36739

Closed
wants to merge 1 commit into from

Conversation

edhgoose
Copy link

@edhgoose edhgoose commented Oct 2, 2016

What does this PR do?

Fixes #36738, where rpmdevtools returns different syntax on AWS Linux

What issues does this PR fix or reference?

#36738

Tests written?

No

Please review Salt's Contributing Guide for best practices.

rpmdevtools versions can return "<", ">" or "==" instead of "is newer". This adds support for those and fixes #36738.

rpmdevtools versions can return "<", ">" or "==" instead of "is newer". This adds support for those and fixes saltstack#36738.
@cachedout
Copy link
Contributor

@terminalmage Can you take a look here please?

@terminalmage
Copy link
Contributor

This looks solid. I coded the initial vercmp support based on the versions available in CentOS 5 and 6, and never saw this output in either. I recall trying to see if we could just interpret the results via the retcode, but for some reason decided not to. Perhaps it's time to revisit this, though. Can you replicate the following commands, which I just ran on CentOS 7, on your AWS instance?

[root@cent7 /]# rpmdev-vercmp 1.2.3-1 1.2.3-1 >/dev/null; echo $?
0
[root@cent7 /]# rpmdev-vercmp 1.2.3-2 1.2.3-1 >/dev/null; echo $?
11
[root@cent7 /]# rpmdev-vercmp 1.2.3-1 1.2.3-2 >/dev/null; echo $?
12

@terminalmage terminalmage added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Oct 4, 2016
@terminalmage
Copy link
Contributor

terminalmage commented Oct 4, 2016

If the retcodes on your AWS box match the example above, then #36754 (which I just opened) might be a better alternative.

I tried this on Cent5, Cent6, and Cent7 and all have the same retcodes for each outcome.

@edhgoose
Copy link
Author

edhgoose commented Oct 4, 2016

@terminalmage Yes, I can confirm my version works the same.

[user@host current]$ rpmdev-vercmp 1.2.3-1 1.2.3-1 >/dev/null; echo $?
0
[user@host current]$ rpmdev-vercmp 1.2.3-2 1.2.3-1 >/dev/null; echo $?
11
[user@host current]$ rpmdev-vercmp 1.2.3-1 1.2.3-2 >/dev/null; echo $?
12

@terminalmage
Copy link
Contributor

OK, would you be able to test #36754 to see if it works for you?

@edhgoose
Copy link
Author

edhgoose commented Oct 5, 2016

I can confirm that #36754 works for me. I'll close this.

@edhgoose edhgoose closed this Oct 5, 2016
@cachedout
Copy link
Contributor

@edhgoose Thanks! I will go ahead and merge #36754 then. I appreciate you getting back to us so quickly. :]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rpmdev-vercmp throws lots of warnings on Amazon Linux
3 participants