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

Optimize the nacos-client/pom.xml for exclude google/**/*.proto #9982

Merged
merged 9 commits into from
Mar 1, 2023

Conversation

wangliang181230
Copy link
Contributor

@wangliang181230 wangliang181230 commented Feb 21, 2023

Optimize the nacos-client/pom.xml for exclude google/**/*.proto

  1. 优化 nacos-clientpom.xml,用于排除掉 google/**/*.proto
    同时,将shade插件在多个profile中的重复定义转移到pluginManagement 中统一定义;
  2. 添加了maven-jar-plugin 的版本号,不添加时,打包会有WARNING信息;
  3. 其他 pom.xml 中,移除了不需要引用的依赖 io.grpc:protoc-gen-grpc-java,该依赖为pom类型,里面并没有定义任何dependenies信息,不需要依赖它。
  4. 优化了一下其他 pom.xml

See:
图片

@CLAassistant
Copy link

CLAassistant commented Feb 21, 2023

CLA assistant check
All committers have signed the CLA.

@wangliang181230 wangliang181230 changed the title Optimize the pom.xml of the nacos-client for exclude /META-INF/native-image/* Optimize the nacos-client/pom.xml for exclude /META-INF/native-image/* Feb 21, 2023
@wangliang181230 wangliang181230 changed the title Optimize the nacos-client/pom.xml for exclude /META-INF/native-image/* Optimize the nacos-client/pom.xml for exclude /META-INF/native-image/* and *.proto Feb 21, 2023
@wangliang181230 wangliang181230 changed the title Optimize the nacos-client/pom.xml for exclude /META-INF/native-image/* and *.proto Optimize the nacos-client/pom.xml for exclude /META-INF/native-image/* and google/**/*.proto Feb 21, 2023
Tonibrown20
Tonibrown20 previously approved these changes Feb 22, 2023
Copy link

@Tonibrown20 Tonibrown20 left a comment

Choose a reason for hiding this comment

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

Great future ahead

Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

Please use nacos code style to reformat code

the indent in this pr has problem

@KomachiSion
Copy link
Collaborator

顺便问一下 native-image也移除掉会不会影响 java native(graalVM)的功能?

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Feb 22, 2023

顺便问一下 native-image也移除掉会不会影响 java native(graalVM)的功能?

需要时,应该去oracle仓库里加。之前和SCA那边沟通过,是说以后都统一去oracle仓库里提PR,而不是写在jar里。
我们seata这边也会去oracle仓库里提PR。

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Feb 22, 2023

顺便问一下 native-image也移除掉会不会影响 java native(graalVM)的功能?

另外,我本地排除掉后,也在native-image里试过,可以正常使用。
反而留着这些native-image配置文件时,需要添加更多的args才能正常打包,用户体验不好。

@wangliang181230
Copy link
Contributor Author

Please use nacos code style to reformat code

the indent in this pr has problem

done

@wangliang181230
Copy link
Contributor Author

顺便问一下 native-image也移除掉会不会影响 java native(graalVM)的功能?

另外,我仔细看了一下从 io.grpc:grpc-netty-shaded shade 到 nacos-client 的这些 native-image 文件,其实又是从 io.netty:* 的各模块里 shade 进 io.grpc:grpc-netty-shaded 的。
也就是说,这些配置文件并不属于 io.grpc:grpc-netty-shaded

nacos-client 并没有 shade io.netty:*,所以,这部分配置文件是不需要的。

但是,我的PR里,还是得修改一下。排除掉 io.grpc:grpc-netty-shaded/META-INF/native-image/io.netty/**,而不是 /META-INF/native-image/**。因为万一以后io.grpc:grpc-netty-shaded拥有自己的native-image时,估计得shade进来。

@wangliang181230
Copy link
Contributor Author

图片

@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Feb 23, 2023

@KomachiSion
我对比了一下 nacos-clientio.grpc:grpc-netty-shaded 两个jar中shade进去的 /META-INF/native-image,发现 nacos-client 里的,没有在包名前面添加上 com.alibaba.nacos.shaded. 的前缀,而 io.grpc:grpc-netty-shadedio.netty 包名的基础上拼接了 io.grpc.shaded. 作为前缀,因为shade进来的所有类,包名都变了。

目前,可能是 shade 插件的配置有问题。

@KomachiSion
Copy link
Collaborator

native-image是不是需要自己生成,直接shard应该不行?

@KomachiSion
Copy link
Collaborator

我觉得可以提个issue来讨论, PR先不提交。 讨论并测试清楚之后再提交。

我觉得这个natvie-image应该是jdk17的 graalVM所用到的, 不shard进去不影响普通的使用,但在graalVM中可能有影响。

@wangliang181230
Copy link
Contributor Author

native-image是不是需要自己生成,直接shard应该不行?

我看了下grpc那边的源码,由于是gradle,不是maven,很多我看的不是很懂,但是应该是使用了shade插件的功能,自动补充了前缀。

@wangliang181230
Copy link
Contributor Author

我觉得可以提个issue来讨论, PR先不提交。 讨论并测试清楚之后再提交。

我觉得这个natvie-image应该是jdk17的 graalVM所用到的, 不shard进去不影响普通的使用,但在graalVM中可能有影响。

嗯,可以的,那我这个PR先暂时只移除proto文件吧。

@wangliang181230
Copy link
Contributor Author

我另外提个issue

@wangliang181230 wangliang181230 changed the title Optimize the nacos-client/pom.xml for exclude /META-INF/native-image/* and google/**/*.proto Optimize the nacos-client/pom.xml for exclude google/**/*.proto Feb 24, 2023
@wangliang181230
Copy link
Contributor Author

wangliang181230 commented Feb 24, 2023

我觉得可以提个issue来讨论, PR先不提交。 讨论并测试清楚之后再提交。

我觉得这个natvie-image应该是jdk17的 graalVM所用到的, 不shard进去不影响普通的使用,但在graalVM中可能有影响。

我测试过,不影响,因为shade进来的配置本来就是错的,包名和shaded进来的类名对不上。
但是直接移除,可能也不对,应该和 grpc-shaded 的做法一样,把 com.alibaba.nacos.shaded. 前缀拼接上以后,再shade进来。

@wangliang181230
Copy link
Contributor Author

这个PR已经好了,排除native-image的代码撤回了。
@KomachiSion PTAL

@wangliang181230 wangliang181230 requested review from KomachiSion and Tonibrown20 and removed request for KomachiSion and Tonibrown20 February 27, 2023 01:36
@KomachiSion KomachiSion merged commit a397c10 into alibaba:develop Mar 1, 2023
@wangliang181230 wangliang181230 deleted the optimize-pom branch March 1, 2023 01:47
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.

4 participants