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

Include native-image.properties in Netty shaded #7657

Closed
wants to merge 2 commits into from

Conversation

amnox
Copy link
Contributor

@amnox amnox commented Nov 23, 2020

This is a temporary fix to #7540. This PR does not have a custom Transformer instead reads the configuration from a file and rewrites native-image.properties inside META-INF

Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

Sorry it took so long for review. I just noticed this.

@@ -25,9 +25,6 @@
import java.util.ServiceLoader;

final class ServiceProviders {
private ServiceProviders() {
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

# License for the specific language governing permissions and limitations
# under the License.

Args = --initialize-at-build-time=io.grpc.netty.shaded.io.netty \
Copy link
Member

Choose a reason for hiding this comment

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

We don't want this hard-coded in gRPC. If you really wanted to avoid making the transformer/relocator, then you could extract the file from the Netty jar dependency. But hard-coding it here is a dead-end.

@ejona86 ejona86 changed the title Netty shaded Include native-image.properties in Netty shaded Dec 9, 2020
@ejona86 ejona86 closed this Jan 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants