Allow jnr-unixsocket version to be overridden or excluded by downstream dependenencies #182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Shuffle the code in the builder so that we don't load unix socket support classes unless the client is configured to use unix sockets. Use reflection in the client to test if the address is a UnixSocketAddress. This allows the client code to function at runtime in UDP mode without having jnr-unixsocket artifacts on the classpath.
Skip UDS tests if jnr-unixsocket classes are not available at runtime.
Add new profiles to test two additional configurations related to jnr-unixsocket: - with jnr-unixsocket not available at runtime - with newer version of jnr-unixsocket (either compile or runtime)
Add two new jobs that simulate different configurations at runtime: - no jnr-unixsocket package available at all; - latest version is supplied instead of the one we depend on; Compilation is done using the default version in all jobs.
|
Windows failure is caused by an unrelated concurrent modification issue, which will be fixed by #183 |
truthbk
reviewed
Jan 5, 2022
| <dependency> | ||
| <groupId>com.github.jnr</groupId> | ||
| <artifactId>jnr-unixsocket</artifactId> | ||
| <version>0.38.15</version> |
There was a problem hiding this comment.
I believe this version is not java7 compatible and per: eed7a1c, it appears to be the case. Let's check.
There was a problem hiding this comment.
It isn't, and we still depend on 0.36. This bit is in a profile that is only activated for testing. The idea is to validate the library using both 0.36 and 0.38.15, so we can provide a bit of assurance for users that don't need Java 7 compatibility and would like a later version.
truthbk
approved these changes
Jan 10, 2022
| <dependency> | ||
| <groupId>com.github.jnr</groupId> | ||
| <artifactId>jnr-unixsocket</artifactId> | ||
| <version>0.38.15</version> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With refactor from fb4cd25 the amount of changes necessary to make jnr-unixsocket optional at runtime is fairly small, and we don't need to rely on reflection at all.
Rest of the PR is adding tests for this scenario. UDS-related tests are adjusted to skip if UDS is not available at runtime; but everything else should run as before. A new maven profile is added that drops jnr-unixsocket from classpath only when running tests. This would be similar to how things are for our library if downstream excludes jnr-unixsocket dependency.
Latest jnr-unixsocket version, 0.38.15, appears to be backwards compatible, and can be used in-place of 0.36 which we depend on currently. To help users that want to use later version, run the test suite against a newer version as well.
Lastly, this adds two new CI jobs to run the above.