-
Notifications
You must be signed in to change notification settings - Fork 449
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 Wind River Linux and Cisco's Nexus platforms #544
Conversation
@lamont-granquist the Travis test is failing on a unicode character being emitted when 'bundle exec ohai' runs on Ruby 2.0. The ohai output is causing ffi_yajl to choke, I don't think it has to do with my patch. Could you check it out please? It works fine on my laptop with the same Ruby. |
Looks like /proc/cpuinfo is returning something funky on travis that cant be encoded |
Yeah the tests actually pass, the ohai run itself that travis invokes is failing due to UTF-8 in /proc/cpuinfo, which is a whole lotta lolwut, but i'll fix that in an incoming PR... |
@@ -59,6 +59,15 @@ def get_redhatish_version(contents) | |||
contents = File.read("/etc/parallels-release").chomp | |||
platform get_redhatish_platform(contents) | |||
platform_version contents.match(/(\d\.\d\.\d)/)[0] | |||
elsif File.exists?('/etc/os-release') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know if this file will exist on any other platforms? Could break detection for those, if so. Probably safer to put this last.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to go before the Red Hat detection because Wind River has a /etc/redhat file despite not being a Red Hat-based distro. I could make it smarter and put the logic inside the Red Hat detection. I didn't do that initially because the CentOS and Ubuntu boxes I checked didn't have the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the new standard for this release information in the future, so you will break the world if you put this first and don't have detection for everyone else in here.
Travis is passing now that ffi-yajl 2.2 is out. |
The Cisco test boxes have both an /etc/redhat-release and an /etc/os-release file. We check if they have both before determining it's a Nexus box.
@lamont-granquist @danielsdeleo updated the patch to make it safer. |
# don't clobber existing os-release properties, point to a different cisco file | ||
contents = {} | ||
File.read('/etc/os-release').split.collect {|x| x.split('=')}.each {|x| contents[x[0]] = x[1]} | ||
if File.exists?(contents['CISCO_RELEASE_INFO']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[1] pry(main)> File.exists?(nil)
TypeError: no implicit conversion of nil into String
from (pry):1:in `exists?'
Probably should guard that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we should use exist?
instead of exists?
, which is deprecated. I think it will likely never actually be removed, but it spits out a warning if you run ruby -w
. I'm slowly trying to fix up warnings and enable them in the tests in our codebase as I can.
👍 once the |
@danielsdeleo added the nil check |
Cool, 👍 |
add support for Wind River Linux and Cisco's Nexus platforms
No description provided.