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

jruby fixes #821

Merged
merged 1 commit into from Aug 30, 2020
Merged

jruby fixes #821

merged 1 commit into from Aug 30, 2020

Conversation

ahorek
Copy link
Contributor

@ahorek ahorek commented Aug 30, 2020

@@ -35,4 +35,4 @@ module LibC
expect(t[:tv_usec]).to be_kind_of(Numeric)
end
end
end
end unless RUBY_ENGINE == 'jruby'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

on Windows, gettimeofday doesn't exist in the standard libc library, but cruby redefines it. That's why it cannot be supported on jruby.

@@ -128,7 +128,7 @@ def self.is_os(os)
end

LIBC = if IS_WINDOWS
if RbConfig::CONFIG['host_os'] =~ /mingw/i
if RbConfig::CONFIG['host_os'] =~ /mingw|mswin/i
Copy link
Member

Choose a reason for hiding this comment

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

It's intentional that mswin uses "ucrtbase.dll". See f259139 and #779 . Is there a specific reason for this change on jruby?

Copy link
Contributor Author

@ahorek ahorek Aug 30, 2020

Choose a reason for hiding this comment

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

sry, I forget about it. You're right. On JRuby, the Windows platform is always "mswin", but I use GCC as a compiler, so "ucrtbase.dll" library is wrong. Unlike CRuby, JRuby doesn't depend on a C compiler at all, it's optional. We would probably need some kind of runtime check for what the correct library is, it's hard to guess based just on the platform.

I was trying to figure out why's "gettimeofday" test failing, but it's broken anyway (see my other comment), so I just revert the change. Thanks for pointing it out.

@@ -64,7 +64,7 @@ class StructUCDP < FFI::Struct
end
end

if RbConfig::CONFIG['host_os'] =~ /mingw/
if RbConfig::CONFIG['host_os'] =~ /mswin|mingw/
Copy link
Member

Choose a reason for hiding this comment

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

Similary, this is intended to apply to MINGW only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants