Skip to content

Add metadata for Netty MacOS specific nameserver resolution#103

Merged
dnestoro merged 1 commit into
oracle:masterfrom
violetagg:gh-100
Nov 14, 2022
Merged

Add metadata for Netty MacOS specific nameserver resolution#103
dnestoro merged 1 commit into
oracle:masterfrom
violetagg:gh-100

Conversation

@violetagg

@violetagg violetagg commented Nov 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 java.base@19.0.1/java.lang.Class.getConstructor0(DynamicHub.java:3641) ~[client:na]
        at java.base@19.0.1/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.

@violetagg
Add metadata for Netty MacOS specific nameserver resolution
196be0c 
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 java.base@19.0.1/java.lang.Class.getConstructor0(DynamicHub.java:3641) ~[client:na]
        at java.base@19.0.1/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

violetagg commented Nov 14, 2022

Copy link
Copy Markdown
Contributor Author

@dnestoro Thanks a lot!

@violetagg violetagg deleted the gh-100 branch November 14, 2022 13:49
@linghengqian linghengqian mentioned this pull request Nov 21, 2022
Closed
@violetagg violetagg mentioned this pull request Jan 26, 2023
Merged
@violetagg violetagg mentioned this pull request Aug 31, 2023
Closed
@dnestoro

dnestoro commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

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

violetagg commented Nov 6, 2023

Copy link
Copy Markdown
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

dnestoro commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

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

violetagg commented Nov 6, 2023

Copy link
Copy Markdown
Contributor Author

ok I'll try

@violetagg

violetagg commented Nov 24, 2023

Copy link
Copy Markdown
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

Reviewers

1 more reviewer

@dnestoro dnestoro dnestoro approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add MacOSDnsServerAddressStreamProvider hints

2 participants

@violetagg @dnestoro