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

[fix][build] Remove io.grpc.netty relocation #23342

Closed
wants to merge 1 commit into from

Conversation

nodece
Copy link
Member

@nodece nodece commented Sep 24, 2024

Motivation

I'm trying to release the private pulsar 3.0.x, and encountered a compilation issue:

$ mvn -T 1C -B -ntp install deploy -Pxxxx-release -DskipTests -Dlicense.skip=true -Dspotbugs.skip=true -Drat.skip=true -Dcheckstyle.skip=true

....
Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project pulsar-metadata: Compilation failure
Error:  /pulsar/pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/EtcdMetadataStore.java:[147,34] cannot access io.grpc.netty.shaded.io.grpc.netty.shaded.io.netty.handler.ssl.SslContext
Error:    class file for io.grpc.netty.shaded.io.grpc.netty.shaded.io.netty.handler.ssl.SslContext not found
Error:  -> [Help 1]
Error:  

#22892 introduces grpc-netty-shaded, the maven-shade-plugin rewrites io.grpc.netty.shaded.io.netty.handler.ssl.SslContext to io.grpc.netty.shaded.io.grpc.netty.shaded.io.netty.handler.ssl.SslContext, which have broken the compilation.

The import of the io.grpc.netty.shaded.* should be excluded.

Modifications

  • Exclude io.grpc.netty.shaded.* in the io.grpc.netty relocation.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@nodece nodece self-assigned this Sep 24, 2024
@nodece nodece requested a review from lhotari September 24, 2024 10:29
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 24, 2024
@nodece nodece added ready-to-test and removed doc-not-needed Your PR changes do not impact docs labels Sep 24, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 24, 2024
@nodece
Copy link
Member Author

nodece commented Sep 24, 2024

/pulsarbot rerun-failure-checks

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

This would require upgrading grpc to a version that is compatible with Netty 4.1.111.Final, please check #22892 for some details.

@nodece nodece closed this Sep 24, 2024
@nodece nodece force-pushed the fix-compile-etcd-metadata branch from fb4b3c3 to 4ce0c75 Compare September 24, 2024 11:47
@nodece nodece deleted the fix-compile-etcd-metadata branch September 24, 2024 11:49
@nodece nodece restored the fix-compile-etcd-metadata branch September 24, 2024 11:49
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece reopened this Sep 24, 2024
@nodece
Copy link
Member Author

nodece commented Sep 24, 2024

/pulsarbot rerun-failure-checks

@nodece nodece marked this pull request as draft September 24, 2024 11:58
@nodece
Copy link
Member Author

nodece commented Sep 24, 2024

Converted to draft, some tests have failed.

@nodece nodece marked this pull request as ready for review September 24, 2024 12:02
@nodece
Copy link
Member Author

nodece commented Sep 24, 2024

/pulsarbot rerun-failure-checks

@nodece nodece marked this pull request as draft September 24, 2024 12:04
@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.54%. Comparing base (bbc6224) to head (14b1027).
Report is 599 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23342      +/-   ##
============================================
+ Coverage     73.57%   74.54%   +0.96%     
- Complexity    32624    34430    +1806     
============================================
  Files          1877     1934      +57     
  Lines        139502   144981    +5479     
  Branches      15299    15839     +540     
============================================
+ Hits         102638   108069    +5431     
+ Misses        28908    28635     -273     
- Partials       7956     8277     +321     
Flag Coverage Δ
inttests 27.58% <ø> (+3.00%) ⬆️
systests 24.59% <ø> (+0.26%) ⬆️
unittests 73.88% <ø> (+1.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 600 files with indirect coverage changes

@nodece
Copy link
Member Author

nodece commented Sep 24, 2024

The mvn deploy includes package phase, this causes the jetcd to relocate multiple times.

@nodece nodece closed this Sep 24, 2024
@lhotari
Copy link
Member

lhotari commented Sep 24, 2024

The mvn deploy includes package phase, this causes the jetcd to relocate multiple times.

@nodece is there a way to address that? It might get triggered in our releases too.

@nodece nodece deleted the fix-compile-etcd-metadata branch September 24, 2024 16:43
@nodece
Copy link
Member Author

nodece commented Sep 24, 2024

I checked the Pulsar release document, it is correct.

“mvn deploy” is correct, “mvn install deploy” is incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants