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

Ensure we can load the native library or fail the build #11262

Merged
merged 1 commit into from May 18, 2021
Merged

Conversation

normanmaurer
Copy link
Member

Motivation:

We used assumeTrue(...) in some places before to detect if we could load the native library but this could lead to the sitation that we not notice if we break native loading.

Modifications:

Always fail if we cant load the native library

Result:

Ensure we not cause any regression in the native loading code in the future

@normanmaurer
Copy link
Member Author

/cc @violetagg also related to your PR... This will ensure we not get into trouble again later on.

Motivation:

We used assumeTrue(...) in some places before to detect if we could load the native library but this could lead to the sitation that we not notice if we break native loading.

Modifications:

Always fail if we cant load the native library

Result:

Ensure we not cause any regression in the native loading code in the future
@fredericBregier
Copy link
Member

fredericBregier commented May 17, 2021

@normanmaurer Maybe not related, maybe only due to "latency" of Maven repositories, but when I update just few minutes ago to the last version of Netty (4.1.64), in my dependencies, I've got the netty native library (2.0.39), I've got an issue on cannot find depdencies such as the following.

Could not resolve dependencies for...: The following artifacts could not be resolved:

  • io.netty:netty-tcnative:jar:${os.detected.classifier}:2.0.39.Final
  • io.netty:netty-transport-native-epoll:jar:${os.detected.name}-${os.detected.arch}:4.1.64.Final
    Could not find artifact io.netty:netty-tcnative:jar:${os.detected.classifier}:2.0.39.Final

I'm on Linux x64 (Ubundu based).
I will retest tomorrow morning, just in case it was only the latency on maven repositories.

@fredericBregier
Copy link
Member

@normanmaurer I will retry tomorrow, but just for note:

  • using 4.1.63-Final : dependecies are OK, including netty-tcnative version 2.0.39
  • using 4.1.64-Final: depdencies are KO, blocking on netty-tcnative version 2.0.39, like ${os.detected.classifier} is not resolved, neither io.netty:netty-transport-native-epoll:jar:${os.detected.name}-${os.detected.arch}:4.1.64.Final, like ${os.detected.name}-${os.detected.arch} not resolved too

@normanmaurer
Copy link
Member Author

@fredericBregier can you show me your pom.xml file ?

@normanmaurer normanmaurer merged commit 6e86631 into 4.1 May 18, 2021
@normanmaurer normanmaurer deleted the ensure branch May 18, 2021 06:14
normanmaurer added a commit that referenced this pull request May 18, 2021
Motivation:

We used assumeTrue(...) in some places before to detect if we could load the native library but this could lead to the sitation that we not notice if we break native loading.

Modifications:

Always fail if we cant load the native library

Result:

Ensure we not cause any regression in the native loading code in the future
@fredericBregier
Copy link
Member

@normanmaurer I opened an issue with a simple pom.xml with only Netty dependencies (no code, just a pom.xml) #11272
I can confirm that using 4.1.63 is ok, while 4.1.64 is ko.

raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
Motivation:

We used assumeTrue(...) in some places before to detect if we could load the native library but this could lead to the sitation that we not notice if we break native loading.

Modifications:

Always fail if we cant load the native library

Result:

Ensure we not cause any regression in the native loading code in the future
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

3 participants