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

Use host_os from RbConfig to detect host OS. #44

Merged
merged 1 commit into from
Jul 28, 2019

Conversation

headius
Copy link
Contributor

@headius headius commented Jul 27, 2019

RUBY_PLATFORM on JRuby is always "java", so it will not reflect the host operating system. This regex appears to be the consensus way to detect Windows based on a search of Ruby code on Github:

https://github.com/search?q=%2Fmswin%7Cmsys%7Cmingw%7Ccygwin%7Cbccwin%7Cwince%7Cemc%2F&type=Code

The regex appears to date back to at least 2012, judging by @enebo's comment here: https://stackoverflow.com/questions/11784109/detecting-operating-systems-in-ruby

The "os" gem is mentioned there as well, but probably is overkill (and would also need to be shipped with Ruby).

RUBY_PLATFORM on JRuby is always "java", so it will not reflect
the host operating system. This regex appears to be the consensus
way to detect Windows based on a search of Ruby code on Github:

https://github.com/search?q=%2Fmswin%7Cmsys%7Cmingw%7Ccygwin%7Cbccwin%7Cwince%7Cemc%2F&type=Code
@headius
Copy link
Contributor Author

headius commented Jul 27, 2019

Link to jruby/jruby#5802.

@headius
Copy link
Contributor Author

headius commented Jul 27, 2019

Oh, I guess I have push rights now? @aycabta please review and I can push this.

@headius headius requested a review from aycabta July 27, 2019 06:28
@headius
Copy link
Contributor Author

headius commented Jul 27, 2019

Oh right...I'm a ruby-core committer 😁

Copy link
Member

@aycabta aycabta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! It's a great improvement! Thank you!

@aycabta aycabta merged commit 11fded2 into ruby:master Jul 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants