Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20336 )
Change subject: [thirdparty] fix TSAN build on Ubuntu 22.04 ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Overall looks good to me. Please address Ashwani's feedback regarding the information about the original changelist(s) from the LLVM upstream repo and maybe add the information about the TSAN build failure message. http://gerrit.cloudera.org:8080/#/c/20336/1/thirdparty/patches/llvm-Address-D100645-comment-eliminate-double-stacksize-calc.patch File thirdparty/patches/llvm-Address-D100645-comment-eliminate-double-stacksize-calc.patch: PS1: > No, it is just a speedup . I will merge the 2 patches, there is no reason t Ah, I see. Calling the same function that returns the same result twice doesn't make much sense, so this update looks reasonable if that was the intent, indeed. Thank you for the clarification. Yep, merging these into a single patch would make more sense. http://gerrit.cloudera.org:8080/#/c/20336/2/thirdparty/patches/llvm-Sanitizer-built-against-glibc-2_34-doesnt-work.patch File thirdparty/patches/llvm-Sanitizer-built-against-glibc-2_34-doesnt-work.patch: http://gerrit.cloudera.org:8080/#/c/20336/2/thirdparty/patches/llvm-Sanitizer-built-against-glibc-2_34-doesnt-work.patch@6 PS2, Line 6: > nit: Remove whitespaces here and at lines 15, 30. Ashwani, it's a patch file, and removing these spaces could break the patch utility when trying to apply this. http://gerrit.cloudera.org:8080/#/c/20336/2/thirdparty/patches/llvm-Sanitizer-built-against-glibc-2_34-doesnt-work.patch@8 PS2, Line 8: // TODO(glider): > Better to add information (commit id, gcc branch info, etc) about the patch +1, but please add that information into the commit description, not this code itself. It's better to keep this code as close to the upstream version as possible. -- To view, visit http://gerrit.cloudera.org:8080/20336 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9493a9f7212c26a9cfc1e6a6015340cc5fbfdc5 Gerrit-Change-Number: 20336 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Martonka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 14 Aug 2023 18:22:22 +0000 Gerrit-HasComments: Yes
