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 multiple shaded version of the same netty-tcnative artifact ca… #382

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

normanmaurer
Copy link
Member

…n be loaded as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty-tcnative artifact as netty-tcnative is often used in multiple dependencies.

This is related to netty/netty#7272.

Modifications:

  • Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
  • Ensure fields are declared as static so these are not exported
  • Ensure we pass the correct linker flags so functions of the static linked version of Boring|Libre|OpenSSL and APR are not visible

Result:

Be able to use multiple shaded versions of the same netty-tcnative artifact.

dnl TCN_CHECK_STATIC
dnl Will prepare more LDFLAGS that should be set to ensure we not export any functions from the static compiled APR / OpenSSL libs.
dnl
AC_DEFUN([TCN_CHECK_STATIC],[
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to do this as an autoconf function as otherwise the flags would have been picked up during running ./configure and so may fail the test to detect the linker / gcc as the filtered symbols did not exist there yet.

@normanmaurer
Copy link
Member Author

Also verified that you can still see all functions in a flame graph after the stripping etc :)

@@ -275,6 +275,32 @@ dnl Note: this define must be on one line so that it can be properly returned
dnl as the help string.
AC_DEFUN([TCN_HELP_STRING],[ifelse(regexp(AC_ACVERSION, 2\.1), -1, AC_HELP_STRING($1,$2),[ ]$1 substr([ ],len($1))$2)])dnl

dnl
dnl TCN_CHECK_STATIC
dnl Will prepare more LDFLAGS that should be set to ensure we not export any functions from the static compiled APR / OpenSSL libs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we not -> we do not

…n be loaded as long as the shaded prefix is different

Motivation:

We should support to load multiple shaded versions of the same netty-tcnative artifact as netty-tcnative is often used in multiple dependencies.

This is related to netty/netty#7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Ensure we pass the correct linker flags so functions of the static linked version of Boring|Libre|OpenSSL and APR are not visible

Result:

Be able to use multiple shaded versions of the same netty-tcnative artifact.
@normanmaurer normanmaurer merged commit def592b into master Aug 23, 2018
@normanmaurer normanmaurer deleted the export_functions branch August 23, 2018 15:46
Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Belated LGTM :-)

Copy link
Member

@carl-mastrangelo carl-mastrangelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fzakaria pushed a commit to fzakaria/netty-tcnative that referenced this pull request Feb 4, 2019
…n be loaded as long as the shaded prefix is different (netty#382)

Motivation:

We should support to load multiple shaded versions of the same netty-tcnative artifact as netty-tcnative is often used in multiple dependencies.

This is related to netty/netty#7272.

Modifications:

- Use -fvisibility=hidden when compiling and use JNIEXPORT for things we really want to have exported
- Ensure fields are declared as static so these are not exported
- Ensure we pass the correct linker flags so functions of the static linked version of Boring|Libre|OpenSSL and APR are not visible

Result:

Be able to use multiple shaded versions of the same netty-tcnative artifact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants