-
Notifications
You must be signed in to change notification settings - Fork 49
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
CMakeLists.txt: Make libnsl optional #255
base: next
Are you sure you want to change the base?
Conversation
@@ -197,13 +197,13 @@ set(SYSTEM_LIBRARIES | |||
${SYSTEM_LIBRARIES} | |||
) | |||
|
|||
if (NOT BSDBASED) | |||
if (USE_LIBNSL) |
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.
You need to also have an option() call. See USE_LTTNG or USE_PROFILE. You also need to have a conditional define in config-h.in.cmake for YP, which is set when this is enabled. However, it looks like YP is never defined anywhere, so maybe this is all dead code. Unfortunately, I can't fully test, since on Fedora, sudo depends on libnsl2, so I cannot remove it from my system.
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.
It is not dead code. If USE_LIBNSL(functionality) is used but the dependency is not available linking will fail. I will update the review as per your recommendation.
Although indeed there seems to be some dead code laying around. I had a patch removing it but wanted to have some feedback. Also keep in mind that some code is not built but function prototypes are provided in the library. Dead code on ntirpc build but not at the symbol level.
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.
I can't find any place where NSL is used that's not protected by YP, which is never defined. Can you point at the code that fails to link? As I said, I can't remove libnsl, so I can't find this myself.
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.
I have it on a cross compiled environment so i can see it getting my host library and failing to link:
[54/56] : && /home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native/usr/bin/arm-poky-linux-gnueabi/arm-poky-linux-gnueabi-gcc -fPIC -march=armv5te -marm -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot -O2 -pipe -g -feliminate-unused-debug-types -fmacro-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot= -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native= -fPIC -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fmacro-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot= -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native= -Wl,-z,relro,-z,now -Wl,--version-script=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/ntirpc-4.0/libntirpc.map -shared -Wl,-soname,libntirpc.so.4.0 -o src/libntirpc.so.4.0 src/CMakeFiles/ntirpc.dir/auth_none.c.o src/CMakeFiles/ntirpc.dir/auth_unix.c.o src/CMakeFiles/ntirpc.dir/authunix_prot.c.o src/CMakeFiles/ntirpc.dir/bindresvport.c.o src/CMakeFiles/ntirpc.dir/bsd_epoll.c.o src/CMakeFiles/ntirpc.dir/city.c.o src/CMakeFiles/ntirpc.dir/clnt_bcast.c.o src/CMakeFiles/ntirpc.dir/clnt_dg.c.o src/CMakeFiles/ntirpc.dir/clnt_generic.c.o src/CMakeFiles/ntirpc.dir/clnt_perror.c.o src/CMakeFiles/ntirpc.dir/clnt_raw.c.o src/CMakeFiles/ntirpc.dir/clnt_simple.c.o src/CMakeFiles/ntirpc.dir/clnt_vc.c.o src/CMakeFiles/ntirpc.dir/getnetconfig.c.o src/CMakeFiles/ntirpc.dir/getnetpath.c.o src/CMakeFiles/ntirpc.dir/getpeereid.c.o src/CMakeFiles/ntirpc.dir/getrpcent.c.o src/CMakeFiles/ntirpc.dir/mt_misc.c.o src/CMakeFiles/ntirpc.dir/pmap_prot.c.o src/CMakeFiles/ntirpc.dir/pmap_prot2.c.o src/CMakeFiles/ntirpc.dir/pmap_rmt.c.o src/CMakeFiles/ntirpc.dir/rbtree.c.o src/CMakeFiles/ntirpc.dir/rbtree_x.c.o src/CMakeFiles/ntirpc.dir/rpc_prot.c.o src/CMakeFiles/ntirpc.dir/rpc_callmsg.c.o src/CMakeFiles/ntirpc.dir/rpc_commondata.c.o src/CMakeFiles/ntirpc.dir/rpc_crc32.c.o src/CMakeFiles/ntirpc.dir/rpc_dplx_msg.c.o src/CMakeFiles/ntirpc.dir/rpc_dtablesize.c.o src/CMakeFiles/ntirpc.dir/rpc_generic.c.o src/CMakeFiles/ntirpc.dir/rpcb_clnt.c.o src/CMakeFiles/ntirpc.dir/rpcb_prot.c.o src/CMakeFiles/ntirpc.dir/rpcb_st_xdr.c.o src/CMakeFiles/ntirpc.dir/strlcpy.c.o src/CMakeFiles/ntirpc.dir/svc.c.o src/CMakeFiles/ntirpc.dir/svc_auth.c.o src/CMakeFiles/ntirpc.dir/svc_auth_unix.c.o src/CMakeFiles/ntirpc.dir/svc_auth_none.c.o src/CMakeFiles/ntirpc.dir/svc_dg.c.o src/CMakeFiles/ntirpc.dir/svc_generic.c.o src/CMakeFiles/ntirpc.dir/svc_raw.c.o src/CMakeFiles/ntirpc.dir/svc_rqst.c.o src/CMakeFiles/ntirpc.dir/svc_simple.c.o src/CMakeFiles/ntirpc.dir/svc_vc.c.o src/CMakeFiles/ntirpc.dir/svc_xprt.c.o src/CMakeFiles/ntirpc.dir/xdr.c.o src/CMakeFiles/ntirpc.dir/xdr_float.c.o src/CMakeFiles/ntirpc.dir/xdr_mem.c.o src/CMakeFiles/ntirpc.dir/xdr_reference.c.o src/CMakeFiles/ntirpc.dir/xdr_ioq.c.o src/CMakeFiles/ntirpc.dir/svc_ioq.c.o src/CMakeFiles/ntirpc.dir/work_pool.c.o -Wl,-rpath,/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native/usr/lib: -latomic -lurcu-bp /home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native/usr/lib/libnsl.so && :
| FAILED: src/libntirpc.so.4.0
No problem. I attempting to fix the issues with YP not being defined as well as making it a drop-in replacement for libtirpc.
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.
I have it on a cross compiled environment so i can see it getting my host library and failing to link:
[54/56] : && /home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native/usr/bin/arm-poky-linux-gnueabi/arm-poky-linux-gnueabi-gcc -fPIC -march=armv5te -marm -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot -O2 -pipe -g -feliminate-unused-debug-types -fmacro-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot= -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native= -fPIC -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fmacro-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0=/usr/src/debug/ntirpc/4.0-r0 -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot= -fdebug-prefix-map=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native= -Wl,-z,relro,-z,now -Wl,--version-script=/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/ntirpc-4.0/libntirpc.map -shared -Wl,-soname,libntirpc.so.4.0 -o src/libntirpc.so.4.0 src/CMakeFiles/ntirpc.dir/auth_none.c.o src/CMakeFiles/ntirpc.dir/auth_unix.c.o src/CMakeFiles/ntirpc.dir/authunix_prot.c.o src/CMakeFiles/ntirpc.dir/bindresvport.c.o src/CMakeFiles/ntirpc.dir/bsd_epoll.c.o src/CMakeFiles/ntirpc.dir/city.c.o src/CMakeFiles/ntirpc.dir/clnt_bcast.c.o src/CMakeFiles/ntirpc.dir/clnt_dg.c.o src/CMakeFiles/ntirpc.dir/clnt_generic.c.o src/CMakeFiles/ntirpc.dir/clnt_perror.c.o src/CMakeFiles/ntirpc.dir/clnt_raw.c.o src/CMakeFiles/ntirpc.dir/clnt_simple.c.o src/CMakeFiles/ntirpc.dir/clnt_vc.c.o src/CMakeFiles/ntirpc.dir/getnetconfig.c.o src/CMakeFiles/ntirpc.dir/getnetpath.c.o src/CMakeFiles/ntirpc.dir/getpeereid.c.o src/CMakeFiles/ntirpc.dir/getrpcent.c.o src/CMakeFiles/ntirpc.dir/mt_misc.c.o src/CMakeFiles/ntirpc.dir/pmap_prot.c.o src/CMakeFiles/ntirpc.dir/pmap_prot2.c.o src/CMakeFiles/ntirpc.dir/pmap_rmt.c.o src/CMakeFiles/ntirpc.dir/rbtree.c.o src/CMakeFiles/ntirpc.dir/rbtree_x.c.o src/CMakeFiles/ntirpc.dir/rpc_prot.c.o src/CMakeFiles/ntirpc.dir/rpc_callmsg.c.o src/CMakeFiles/ntirpc.dir/rpc_commondata.c.o src/CMakeFiles/ntirpc.dir/rpc_crc32.c.o src/CMakeFiles/ntirpc.dir/rpc_dplx_msg.c.o src/CMakeFiles/ntirpc.dir/rpc_dtablesize.c.o src/CMakeFiles/ntirpc.dir/rpc_generic.c.o src/CMakeFiles/ntirpc.dir/rpcb_clnt.c.o src/CMakeFiles/ntirpc.dir/rpcb_prot.c.o src/CMakeFiles/ntirpc.dir/rpcb_st_xdr.c.o src/CMakeFiles/ntirpc.dir/strlcpy.c.o src/CMakeFiles/ntirpc.dir/svc.c.o src/CMakeFiles/ntirpc.dir/svc_auth.c.o src/CMakeFiles/ntirpc.dir/svc_auth_unix.c.o src/CMakeFiles/ntirpc.dir/svc_auth_none.c.o src/CMakeFiles/ntirpc.dir/svc_dg.c.o src/CMakeFiles/ntirpc.dir/svc_generic.c.o src/CMakeFiles/ntirpc.dir/svc_raw.c.o src/CMakeFiles/ntirpc.dir/svc_rqst.c.o src/CMakeFiles/ntirpc.dir/svc_simple.c.o src/CMakeFiles/ntirpc.dir/svc_vc.c.o src/CMakeFiles/ntirpc.dir/svc_xprt.c.o src/CMakeFiles/ntirpc.dir/xdr.c.o src/CMakeFiles/ntirpc.dir/xdr_float.c.o src/CMakeFiles/ntirpc.dir/xdr_mem.c.o src/CMakeFiles/ntirpc.dir/xdr_reference.c.o src/CMakeFiles/ntirpc.dir/xdr_ioq.c.o src/CMakeFiles/ntirpc.dir/svc_ioq.c.o src/CMakeFiles/ntirpc.dir/work_pool.c.o -Wl,-rpath,/home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native/usr/lib: -latomic -lurcu-bp /home/pneves/Projects/yocto-superproject/poky/build/tmp/work/armv5e-poky-linux-gnueabi/ntirpc/4.0-r0/recipe-sysroot-native/usr/lib/libnsl.so && :
| FAILED: src/libntirpc.so.4.0
YP libnsl functionality is already protected by ifdef guards where necessary and there are uses where it is not required. Add USE_LIBNSL as a build system knob. Upstream-Status: Submitted [nfs-ganesha#255] Signed-off-by: Paulo Neves <[email protected]> %% original patch: 0001-CMakeLists.txt-Make-libnsl-optional.patch
490b2fd
to
c2d37a5
Compare
In the new commit you can just disable USE_LIBNSL and make the tests you wish even on Fedora. That is possible as the detection of libnsl is disabled with that setting. |
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.
This seems correct for the cmake parts, but we also need to find the last bits of code that depend on NSL but aren't protected by ifdef.
@dang i believe the parts that are dependent on NSL but are not protected, are not built at all. I found some manually but they were not even in the build list. I did not submit changes on that code as protecting on dead code does not make much sense to me. On the other hand i did not want to add removal of dead code to this PR. |
If the link is failing, then some code must be using NSL, and we need to find and protect it. |
Ah I understand. I think it is just the FindNSL code that automatically adds -lnsl2 to the linking command, regardless of code needing it. I get your point though and will have a closer look. |
YP libnsl functionality is already protected by ifdef guards where necessary and there are usecases where it is not required. Add USE_LIBNSL as a build system knob. Fixes #254
Signed-off-by: Paulo Neves [email protected]