Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23740 )
Change subject: Fix clean TSAN build. ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/23740/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23740/1//COMMIT_MSG@9 PS1, Line 9: If you run: : git clean -xfd (or start with a clean vm). : BUILD_TYPE=TSAN ./build-support/jenkins/build-and-test.sh : : It fails, because thirdparty/installed/uninstrumented/bin/flatc : is hardwired in java/kudu-flatbuffers/build.gradle. > It uses a binary from maven, in java/gradle/protobuf.gradle: Indeed: probably we could use similar approach here, but it would be a theme for a separate patch. Meanwhile, I realized that flatbuffers/flatc is different from protobuf/protoc, and I think we might have a simpler resolution in this case. Please see the comment at https://gerrit.cloudera.org/#/c/23740/1/thirdparty/build-thirdparty.sh@403 http://gerrit.cloudera.org:8080/#/c/23740/1//COMMIT_MSG@16 PS1, Line 16: There is no reason to include the tsan version instead. > So in case of TSAN build we should generate the java classes with a TSAN in I think you are right -- it doesn't matter whether to use sanitized or non-sanitized flatc executable to generate Java bindings. So, from this standpoint we can use any binary for this purpose, I agree. After looking at this closely, I think we should move flatbuffers into the 'COMMON' group, removing it from the 'TSAN' group. Please check my latest comment at https://gerrit.cloudera.org/#/c/23740/1/thirdparty/build-thirdparty.sh@403 Thanks! http://gerrit.cloudera.org:8080/#/c/23740/1/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/23740/1/thirdparty/build-thirdparty.sh@402 PS1, Line 402: # Java build always uses the uninstrumented version. : if [ -n "$F_COMMON" -o -n "$F_FLATBUFFERS" ]; then > Its fine: OK, but now it's becoming a bit confusing because 'common' means there isn't uninstrumented and tsan versions, but rather a single version that's used everywhere. At least that's so for all the existing 3rd party components now, IIUC. I took a closer look into the flatbuffers. It seems the sanitizer-related provisions are only for flatc and the generated tests themselves. Also, we don't use the flatbuffers library as dependency for Kudu C++ bits: it's header-only library. So, I'd rather do following: move flatbuffers into the COMMON group and remove it from the TSAN group, and also update the path to the flatc binary in the java/kudu-flatbuffers/build.gradle file. I think that should address the issue this patch is about. -- To view, visit http://gerrit.cloudera.org:8080/23740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id45e8c98e43df7d769a87b41e50f4781ca234061 Gerrit-Change-Number: 23740 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Martonka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Sun, 14 Dec 2025 07:58:29 +0000 Gerrit-HasComments: Yes
