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

fix resolv on windows #5158

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@ahorek
Contributor

ahorek commented May 5, 2018

bug #4755

@kares

This comment has been minimized.

Member

kares commented May 10, 2018

sun.net.dns.ResolverConfiguration OpenJDK specific right? and it will get hidden away under Java 9+
definitely better than nothing I am just asking if you have any input or ideas on doing this in a different way.

@ahorek

This comment has been minimized.

Contributor

ahorek commented May 10, 2018

sun.net.dns.ResolverConfiguration is marked as deprecated, but it still works
jruby 9.2.0.0-SNAPSHOT (2.5.0) 2018-05-10 00c19bb Java HotSpot(TM) 64-Bit Server VM 10.0.1+10 on 10.0.1+10 +jit [mswin32-x86_64]
btw. with this patch the behaviour is the same as 9.1.17.0

I think the only way to do this right and fix https://bugs.ruby-lang.org/issues/12604 is to reimplement this
https://github.com/ruby/ruby/blob/202bbda2bf5f25343e286099140fb9282880ecba/ext/win32/resolv/resolv.c using JNI.

What do you think? Should I try it or is there a better way?

@headius

This comment has been minimized.

Member

headius commented May 14, 2018

@ahorek Is this fix perhaps "good enough" to merge now in case we can't do the "right" fix before the 9.2 release (within a week from now)?

@headius headius added this to the JRuby 9.2.0.0 milestone May 14, 2018

@ahorek

This comment has been minimized.

Contributor

ahorek commented May 14, 2018

my only concern is that sun.net.dns.ResolverConfiguration will become deprecated (I tested it on Oracle's Java 8 and Java 10 and it works fine for now)
it should be fixed before 9.2 release, otherwise bundler won't work on Windows.

@headius

This comment has been minimized.

Member

headius commented May 14, 2018

my only concern is that sun.net.dns.ResolverConfiguration will become deprecated

A valid concern. There are builds of 11, can you try to test one of them too? That would get us working into the next LTS version of OpenJDK.

it should be fixed before 9.2 release, otherwise bundler won't work on Windows.

Yikes! 😨 cc @enebo

@ahorek ahorek referenced this pull request May 19, 2018

Merged

WIP: resolv ffi prototype #5178

@ahorek

This comment has been minimized.

Contributor

ahorek commented May 19, 2018

I've never used jnr.ffi before and I'm a little bit stucked :). Maybe someone could help me with it? #5178

@ahorek

This comment has been minimized.

Contributor

ahorek commented May 23, 2018

daeb255 fixed the issue, thanks @enebo

@ahorek ahorek closed this May 23, 2018

@enebo

This comment has been minimized.

Member

enebo commented May 23, 2018

@ahorek thanks for the first stab too. I agree that jnr/ffi and friends are super confusing in this case. Using the previous call for number of bytes to next call is weird. In Ruby FFI it was a bit easier to figure out. Plus Ruby...

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