-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19611. Fix native profile build of hadoop-pipes on SLES 15. #7789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
On SLES 15, failing to add tirpc results in failures linking pthreads. tirpc is present and appears to be the newer RPC library option (with support for IPv6). Use it when available.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the Hadoop Pipes CMake logic to prefer and link against libtirpc when available, fixing native builds on SLES 15.
- Switches from manually finding
rpc/rpc.h
to usingpkg_check_modules(LIBTIRPC)
- Simplifies conditional blocks to use
LIBTIRPC_FOUND
for include-path detection and linking - Always links
tirpc
when found
Comments suppressed due to low confidence (1)
hadoop-tools/hadoop-pipes/src/CMakeLists.txt:33
- Removing the fallback
find_path(rpc/rpc.h)
means RPC_INCLUDE_DIRS is never set when libtirpc isn’t available. This will break builds on systems without libtirpc. Consider restoring a fallback detection ofrpc/rpc.h
in anelse()
branch.
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @MikaelSmith . Thank you for the patch. It appears that this removes the capability for the build environment to detect rpc.h directly, and instead always uses tirpc. Can we be confident that direct detection of rpc.h is no longer relevant in modern build environments, and the change is safe across distros (not just SLES)?
We've built and tested this on RedHat 8 (ARM and x86_64), RedHat 9, SLES 15, Ubuntu 20, and Ubuntu 22. Ran extensive tests on RedHat 8 and 9. Conceptually I think it makes sense to use tirpc if it's available. Part of the problem we observed was that on SLES 15, it detected rpc.h but that came from tirpc, so the build was not configured correctly. |
Description of PR
On SLES 15, failing to add tirpc results in failures linking pthreads. tirpc is present and appears and the newer RPC library option (with support for IPv6). Use it when available.
How was this patch tested?
Builds on RHEL 8, RHEL 9, Ubuntu 22, and SLES 15.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?