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 metadata for Netty MacOS specific nameserver resolution #103

Merged
merged 1 commit into from
Nov 14, 2022
Merged

Add metadata for Netty MacOS specific nameserver resolution #103

merged 1 commit into from
Nov 14, 2022

Conversation

violetagg
Copy link
Contributor

@violetagg violetagg commented Nov 4, 2022

What does this PR do?

Netty provides MacOS specific nameserver resolution - resolver-dns-classes-macos and resolver-dns-native-macos.

The PR adds metadata for Netty MacOS specific nameserver resolution.

The exception below is observed when there is no metadata for this functionality:

java.lang.NoSuchMethodException: io.netty.resolver.dns.macos.MacOSDnsServerAddressStreamProvider.<init>()
        at [email protected]/java.lang.Class.getConstructor0(DynamicHub.java:3641) ~[client:na]
        at [email protected]/java.lang.Class.getConstructor(DynamicHub.java:2324) ~[client:na]
        at io.netty.resolver.dns.DnsServerAddressStreamProviders.<clinit>(DnsServerAddressStreamProviders.java:63) ~[na:na]
        at io.netty.resolver.dns.DnsNameResolverBuilder.<init>(DnsNameResolverBuilder.java:60) ~[na:na]

Closes #100

Checklist before merging

  • I have properly formatted metadata files (see CONTRIBUTING document)
  • I have added thorough tests. (see this)

Test is not provided because the fix is relevant only for MacOS.

Netty provides MacOS specific nameserver resolution -
resolver-dns-classes-macos and resolver-dns-native-macos.

The exception below is observed when there is no metadata for this functionality:

```
java.lang.NoSuchMethodException: io.netty.resolver.dns.macos.MacOSDnsServerAddressStreamProvider.<init>()
        at [email protected]/java.lang.Class.getConstructor0(DynamicHub.java:3641) ~[client:na]
        at [email protected]/java.lang.Class.getConstructor(DynamicHub.java:2324) ~[client:na]
        at io.netty.resolver.dns.DnsServerAddressStreamProviders.<clinit>(DnsServerAddressStreamProviders.java:63) ~[na:na]
        at io.netty.resolver.dns.DnsNameResolverBuilder.<init>(DnsNameResolverBuilder.java:60) ~[na:na]
```

Closes #100
@dnestoro dnestoro merged commit a88c71e into oracle:master Nov 14, 2022
@violetagg
Copy link
Contributor Author

@dnestoro Thanks a lot!

@dnestoro
Copy link
Member

dnestoro commented Nov 6, 2023

Hey, during some checking I realized that some of the entries in jni-config.json from this PR, don't have adequate typeReachable. For example, this entry has io.netty.util.internal.NativeLibraryUtil typeReachable which is reachable from any platform, and it includes MacOS specific type (therefore this MacOS specific type will be pulled into native image on any platform). Instead of using io.netty.util.internal.NativeLibraryUtil, can't we use MacOSDnsServerAddressStreamProvider class as typeReachable? Can you give it a try and open a PR if it works correctly?

Entries: both this and this typeReachable should be changed as described above.

@violetagg
Copy link
Contributor Author

@dnestoro We do have in Reactor Netty a smoke test for all OS. What would be the best way for testing the proposed change? Provide a local metadata repository instead of the original?
https://github.com/reactor/reactor-netty/blob/main/reactor-netty-graalvm-smoke-tests/build.gradle#L64-L66

@dnestoro
Copy link
Member

dnestoro commented Nov 6, 2023

Can you manually change typeReachable locally and check whether netty tests that are in metadata repo are passing or not (if I understood correctly, you didn't provide tests in this PR because you ran existing netty tests from this repo on MacOS and therefore no new test is needed)? Also you can run smoke tests with changed metadata to see if it works there as well.

@violetagg
Copy link
Contributor Author

ok I'll try

@violetagg
Copy link
Contributor Author

@dnestoro The proposal seems to be working
violetagg@55316be

I made some changes to the test in order to test Mac OS natives.
The current test is made to be successful on all OSes so it does not uses Linux/Mac OS natives

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.

Add MacOSDnsServerAddressStreamProvider hints
2 participants