You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2024/08/02 14:01:37 UTC
[PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
pan3793 opened a new pull request, #6975:
URL: https://github.com/apache/hadoop/pull/6975
<!--
Thanks for sending a pull request!
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
2. Make sure your PR title starts with JIRA issue id, e.g., 'HADOOP-17799. Your PR title ...'.
-->
### Description of PR
As part of HADOOP-16596, it switched from the vanilla protobuf to the shaded one, thus the generated code by `protobuf` should be modified (replace package name) before compiling and generating Javadoc.
Currently, the protobuf code generate happens in `generate-sources`(and other similar phases), while the replace happens in `process-sources`, this is OK for compiling but has problems for Javadoc.
```
$ mvn clean install -DskipTests
$ mvn javadoc:javadoc
```
The above commands won't trigger replace, thus the final generated Javadoc refers to vanilla protobuf.
<img width="1428" alt="image" src="https://github.com/user-attachments/assets/58dd666e-4c91-497c-ba7e-ffa272393378">
The `javadoc`(Java 8) also complains that
```
[INFO] --- javadoc:3.0.1:javadoc (default-cli) @ hadoop-common ---
[INFO]
ExcludePrivateAnnotationsStandardDoclet
100 warnings
[WARNING] Javadoc Warnings
[WARNING] /Users/chengpan/Projects/apache-hadoop/hadoop-common-project/hadoop-common/target/generated-sources/java/org/apache/hadoop/ipc/protobuf/RpcHeaderProtos.java:3887: error: cannot find symbol
[WARNING] com.google.protobuf.GeneratedMessageV3 implements
[WARNING] ^
[WARNING] symbol: class GeneratedMessageV3
[WARNING] location: package com.google.protobuf
```
Seems things become worse in Java 17 ...
### How was this patch tested?
Tested with Java 8, macOS aarch64.
```
ā apache-hadoop git:(trunk) mvn clean install -DskipTests
```
```
ā apache-hadoop git:(trunk) mvn javadoc:javadoc | grep '@ hadoop-common '
[INFO] >>> javadoc:3.0.1:javadoc (default-cli) > generate-sources @ hadoop-common >>>
[INFO] --- antrun:1.7:run (create-testdirs) @ hadoop-common ---
[INFO] --- protobuf:0.5.1:compile (src-compile-protoc) @ hadoop-common ---
[INFO] --- build-helper:1.9:add-source (add-source-legacy-protobuf) @ hadoop-common ---
[INFO] <<< javadoc:3.0.1:javadoc (default-cli) < generate-sources @ hadoop-common <<<
[INFO] --- javadoc:3.0.1:javadoc (default-cli) @ hadoop-common ---
```
```
ā apache-hadoop git:(HADOOP-19248) mvn javadoc:javadoc | grep '@ hadoop-common '
[INFO] >>> javadoc:3.0.1:javadoc (default-cli) > generate-sources @ hadoop-common >>>
[INFO] --- antrun:1.7:run (create-testdirs) @ hadoop-common ---
[INFO] --- protobuf:0.5.1:compile (src-compile-protoc) @ hadoop-common ---
[INFO] --- replacer:1.5.3:replace (replace-generated-sources) @ hadoop-common ---
[INFO] --- replacer:1.5.3:replace (replace-sources) @ hadoop-common ---
[INFO] --- build-helper:1.9:add-source (add-source-legacy-protobuf) @ hadoop-common ---
[INFO] <<< javadoc:3.0.1:javadoc (default-cli) < generate-sources @ hadoop-common <<<
[INFO] --- javadoc:3.0.1:javadoc (default-cli) @ hadoop-common ---
```
### For code changes:
- [x] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2265722043
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 48m 55s | | trunk passed |
| +1 :green_heart: | compile | 0m 20s | | trunk passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | compile | 0m 20s | | trunk passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | mvnsite | 0m 26s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 26s | | trunk passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 0m 23s | | trunk passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | shadedclient | 86m 34s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 13s | | the patch passed |
| +1 :green_heart: | compile | 0m 12s | | the patch passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javac | 0m 12s | | the patch passed |
| +1 :green_heart: | compile | 0m 12s | | the patch passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | javac | 0m 12s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | mvnsite | 0m 15s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 13s | | the patch passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 |
| +1 :green_heart: | javadoc | 0m 13s | | the patch passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| +1 :green_heart: | shadedclient | 37m 7s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 16s | | hadoop-project in the patch passed. |
| +1 :green_heart: | asflicense | 0m 36s | | The patch does not generate ASF License warnings. |
| | | 127m 23s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6975/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6975 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint |
| uname | Linux cc0f6c476722 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 9c51b6cfeabac515f7e015eecbfc27b551c6e694 |
| Default Java | Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6975/1/testReport/ |
| Max. process+thread count | 691 (vs. ulimit of 5500) |
| modules | C: hadoop-project U: hadoop-project |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6975/1/console |
| versions | git=2.25.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2268382423
also cc @vinayakumarb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2285979475
LGTM +1.
Iām currently organizing the JDK 17 build scripts and will verify this functionality again. Let's see if there are any other comments from other members. (wait an additional 1-2 days)
@pan3793 Thanks for the contribution! @steveloughran Thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2316418124
@steveloughran Thanks for reviewing pr! @pan3793 Thank for your contribution!
Pan is a highly experienced developer with extensive community development experience. Although he is not yet a Hadoop committer, he has great potential to become one in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2274308687
@steveloughran I understand your concerns, without explicitly setting dependencies between executions, the final execution set and order will always seem fragile.
> do we have any guarantees that it will continue to work?
According to https://maven.apache.org/plugins/maven-javadoc-plugin/javadoc-mojo.html, `mvn javadoc:javadoc` would
> Invokes the execution of the following [lifecycle phase](https://maven.apache.org/ref/current/maven-core/lifecycles.html) prior to executing itself: `generate-sources`.
I think the answer is yes, assuming all executions within a goal run sequentially based on the declared order. (I didn't find such a statement in the Maven documentation, but it seems to be true)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2273813010
Looking at the maven stuff I'd consider the replacement part of the generation. The problem we have is that within a life-cycle phase I am not sure we can guarantee that operations happen. Having no specific phases is designed to eliminate the ambiguity. As it is: even if it currently works for maven locally, do we have any guarantees that it will continue to work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2309276084
kindly ping @slfan1989, can we move this ahead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2265478907
there is no issue with `mvn package -Pdocs -DskipTests`, but I also suppose `mvn javadoc:javadoc` should work because it is listed at https://github.com/apache/hadoop/blob/c593c17255c06a32b01055e2f4bb2394009bd94a/BUILDING.txt#L148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2265482551
cc @steveloughran @slfan1989 @aajisaka
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2266279589
> cc @steveloughran @slfan1989 @aajisaka
Can we explain the difference between these two options?
> cc @steveloughran @slfan1989 @aajisaka
Thank you for contribution! Personally, I'm not sure if this modification is feasible; I noticed differences between these two options on Maven's lifecycle page. I need other members to help verify.
https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html
Phase|Description
-- | --
generate-sources | generate any source code for inclusion in compilation.
process-sources| process the source code, for example to filter any values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #6975:
URL: https://github.com/apache/hadoop/pull/6975#issuecomment-2316085824
@pan3793 merged...sorry, thought you had the permissions to do this.
please can you do a PR for branch-3.4, push it up to see what yetus does there and I'll merge afterwards. thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19248. Protobuf code generate and replace should happen together [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran merged PR #6975:
URL: https://github.com/apache/hadoop/pull/6975
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org