Skip to content
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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

ptsneves
Copy link

@ptsneves ptsneves commented Sep 7, 2022

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 ptsneves@gmail.com

@@ -197,13 +197,13 @@ set(SYSTEM_LIBRARIES
${SYSTEM_LIBRARIES}
)

if (NOT BSDBASED)
if (USE_LIBNSL)
Copy link

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.

Copy link
Author

@ptsneves ptsneves Sep 7, 2022

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.

Copy link

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.

Copy link
Author

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.

Copy link
Author

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 <ptsneves@gmail.com>

%% original patch: 0001-CMakeLists.txt-Make-libnsl-optional.patch
@ptsneves
Copy link
Author

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.

Copy link

@dang dang left a 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.

@ptsneves
Copy link
Author

@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.

@dang
Copy link

dang commented Sep 15, 2022

If the link is failing, then some code must be using NSL, and we need to find and protect it.

@ptsneves
Copy link
Author

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.

@ffilz ffilz mentioned this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libnsl dependency
2 participants