HADOOP-19605. (3.4) Upgrade Protobuf 3.25.5 for docker images (#7780) #8078
Conversation
…#7780) * HADOOP-19605. Upgrade Protobuf 3.25.5 for docker images. Reviewed-by: Steve Loughran <stevel@apache.org> Signed-off-by: Shilun Fan <slfan1989@apache.org>
|
(!) A patch to the testing environment has been detected. |
|
@pan3793 This PR is not a clean backport, so I can’t give it a +1. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
@slfan1989, there are a few differences in trunk and branch-3.4, but the essential change is same, I manually resolved the conflicts and verified again to ensure the backport works. Since the Java side protobuf upgrading happens in branch-3.4, we should land this in 3.4 too. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@steveloughran do you want to include this in 3.4.3, to make the protobuf cpp version match Java's one? Native tests in both https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8078/3/artifact/out/patch-unit-root.txt |
|
-1 According to our previous code merge guidelines, we should keep trunk and branch-3.4 as consistent as possible. From what I can see, this PR introduces quite a large difference. Are all of these changes strictly necessary? Would it be possible to meet the requirements without adding these extra modifications? I’d prefer not to let personal preferences affect our overall standards. Thanks again for your support and cooperation! |
|
@slfan1989 I'm not sure what you mean "quite a large difference", compared to the original patch (#7780), except for comments, the only difference is that this backport does not include The backport decision is based on that HADOOP-19289 goes to branch-3.4 but has not completed the work, it only upgrades the Java side, keeping that branch-3.4 uses two different version of protobuf libs. I'd like to know the technical justification behind the veto. |
|
@pan3793 What I want to convey is that, compared to the trunk branch, we've missed several earlier PRs during the compilation of this part. Many of these PRs were provided by you. Should we consider incorporating these earlier changes before proceeding with further modifications? The relevant PRs are as follows:
|
|
@slfan1989 none of these are required. IIRC, previously we had a conclusion on how to maintain those
In branch-3.4, there are 4
BTW, disabling |
|
@pan3793 Thank you for the effort you've put into this PR, but I would prefer a clear merge path and an executable specification, rather than relying on verbal explanations about whether this PR is viable. From my understanding, we need a clean backport. Currently, the differences in the base are significant, and forcing a backport in this manner is hard for me to agree with. I also hope this does not set a precedent for the future. |
If you think this is reasonable, why force the upgrade to Debian 11 on the trunk?
As with the previous question, why push for an upgrade to RockLinux 8? |
First of all, I suppose upgrading the building OS, e.g., Debian from 10 to 11, is kind of a riskier change than a normal dependency upgrade (e.g., protobuf, jackson), for those changes, trunk should move forward, but feature branches should stay at the cut point. I think this is a common policy. |
3 participants
Backport #7780 to branch-3.4
Reviewed-by: Steve Loughran stevel@apache.org
Note: I verified it again locally in both x86 and arm64 env.