HBASE-30194 [thirdparty] Onboard libthrift to hbase-thirdparty

[apurtell]

Onboard libthrift to hbase-thirdparty so we can consume libthrift security fixes while maintaining Java 8 compatibility for HBase branch-2, branch-2.5, and branch-2.6.

[Apache-HBase]

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 0s		Docker mode activated.
-1 ❌	patch	0m 3s		#160 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help.

Subsystem	Report/Notes
Console output	https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/1/console
versions	git=2.17.1
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

[ndimiduk]

What's the strategy for this one, hard fork? We're going to make changes inline, rather than maintain a stack of patches?

[Apache9]

Is it better to follow the protobuf way, where we depend on the specific version and while building, we pull in the source tarball, extract it, apply our patches and then compile it?

[ndimiduk]

Is it better to follow the protobuf way, where we depend on the specific version and while building, we pull in the source tarball, extract it, apply our patches and then compile it?

I considered suggesting that approach, but i'm afraid that significant refactoring will be necessary to change this dependency. Managing that through patch files will be painful.

[apurtell]

Is it better to follow the protobuf way, where we depend on the specific version and while building, we pull in the source tarball, extract it, apply our patches and then compile it?

I allowed the modified source to be checked in but we don't need it. When I looked over at hbase-shaded-protobuf in tree I saw the sources expanded in tree there and thought maybe things had changed in thirdparty since I was last here.

Now there are only 7 modified files.

[Apache-HBase]

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	2m 4s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 1s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+0 🆗	shelldocs	0m 0s		Shelldocs was not available.
+0 🆗	spotbugs	0m 0s		spotbugs executables are not available.
+0 🆗	markdownlint	0m 0s		markdownlint was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-0 ⚠️	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.
			_ master Compile Tests _
+1 💚	mvninstall	1m 1s		master passed
+1 💚	compile	0m 16s		master passed
+1 💚	checkstyle	0m 28s		master passed
+1 💚	javadoc	0m 7s		master passed
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 7s		Maven dependency ordering for patch
+1 💚	mvninstall	0m 52s		the patch passed
+1 💚	compile	0m 26s		the patch passed
+1 💚	javac	0m 26s		the patch passed
-1 ❌	blanks	0m 0s	/blanks-eol.txt	The patch has 8 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-1 ❌	checkstyle	0m 26s	/results-checkstyle-hbase-shaded-thrift.txt	hbase-shaded-thrift: The patch generated 6068 new + 0 unchanged - 0 fixed = 6068 total (was 0)
-1 ❌	checkstyle	0m 42s	/results-checkstyle-root.txt	root: The patch generated 6068 new + 0 unchanged - 0 fixed = 6068 total (was 0)
+1 💚	shellcheck	0m 0s		No new issues.
+1 💚	xmllint	0m 0s		No new issues.
+1 💚	javadoc	0m 12s		the patch passed
			_ Other Tests _
+1 💚	unit	0m 7s		hbase-shaded-thrift in the patch passed.
+1 💚	unit	0m 50s		root in the patch passed.
-1 ❌	asflicense	0m 11s	/results-asflicense.txt	The patch generated 10 ASF License warnings.
		8m 50s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/2/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR	#160
Optional Tests	dupname asflicense codespell detsecrets javac javadoc unit xmllint compile shellcheck shelldocs spotbugs checkstyle markdownlint
uname	Linux 78abbb6af45d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-home/workspace/Base-Thirdparty-PreCommit_PR-160/yetus-precommit-check/src/.yetus/personality.sh
git revision	master / b1b7485
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/2/testReport/
Max. process+thread count	420 (vs. ulimit of 2000)
modules	C: hbase-shaded-thrift . U: .
Console output	https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/2/console
versions	git=2.34.1 maven=called shellcheck=0.8.0 xmllint=20913
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

[apurtell]

Because of the change history of this PR the asflicense and checkstyle checks are posting false positives, apologies for that.
The blanks check is finding real blanks in the patch files, but that seems fine here.

[PDavid]

Very cool, many thanks. 👍

[Apache-HBase]

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 5s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+0 🆗	shelldocs	0m 0s		Shelldocs was not available.
+0 🆗	spotbugs	0m 1s		spotbugs executables are not available.
+0 🆗	markdownlint	0m 1s		markdownlint was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-0 ⚠️	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.
			_ master Compile Tests _
+1 💚	mvninstall	0m 52s		master passed
+1 💚	compile	0m 12s		master passed
+1 💚	checkstyle	0m 21s		master passed
+1 💚	javadoc	0m 6s		master passed
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 5s		Maven dependency ordering for patch
+1 💚	mvninstall	0m 38s		the patch passed
+1 💚	compile	0m 25s		the patch passed
+1 💚	javac	0m 25s		the patch passed
-1 ❌	blanks	0m 0s	/blanks-eol.txt	The patch has 8 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-1 ❌	checkstyle	0m 19s	/results-checkstyle-hbase-shaded-thrift.txt	hbase-shaded-thrift: The patch generated 6068 new + 0 unchanged - 0 fixed = 6068 total (was 0)
-1 ❌	checkstyle	0m 30s	/results-checkstyle-root.txt	root: The patch generated 6068 new + 0 unchanged - 0 fixed = 6068 total (was 0)
+1 💚	shellcheck	0m 0s		No new issues.
+1 💚	xmllint	0m 0s		No new issues.
+1 💚	javadoc	0m 10s		the patch passed
			_ Other Tests _
+1 💚	unit	0m 7s		hbase-shaded-thrift in the patch passed.
+1 💚	unit	0m 41s		root in the patch passed.
-1 ❌	asflicense	0m 10s	/results-asflicense.txt	The patch generated 10 ASF License warnings.
		6m 25s

Subsystem	Report/Notes
Docker	ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/3/artifact/yetus-precommit-check/output/Dockerfile
GITHUB PR	#160
Optional Tests	dupname asflicense codespell detsecrets javac javadoc unit xmllint compile shellcheck shelldocs spotbugs checkstyle markdownlint
uname	Linux 10b1865cdae7 6.8.0-1024-aws #26~22.04.1-Ubuntu SMP Wed Feb 19 06:54:57 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/workspace/Base-Thirdparty-PreCommit_PR-160/yetus-precommit-check/src/.yetus/personality.sh
git revision	master / 809eac6
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/3/testReport/
Max. process+thread count	418 (vs. ulimit of 2000)
modules	C: hbase-shaded-thrift . U: .
Console output	https://ci-hbase.apache.org/job/HBase-Thirdparty-PreCommit/job/PR-160/3/console
versions	git=2.34.1 maven=called shellcheck=0.8.0 xmllint=20913
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

[apurtell]

Will merge in a few hours unless objection. Thanks everyone for the reviews and approvals.

[Apache9]

Because of the change history of this PR the asflicense and checkstyle checks are posting false positives, apologies for that. The blanks check is finding real blanks in the patch files, but that seems fine here.

We can exclude these files from rat license check.

hbase-thirdparty/pom.xml

Line 243 in 9907761

<exclude>**/src/main/patches/**</exclude>

And on the patch file, when processing protobuf, it is fine to just strip the ending blanks, it does not break the patching process.

Thanks.

[PDavid]

Many thanks! 👍