add ${NL_LIBRARY_DIRS} to link_directories()

[chuckcranor]

Handles case where nl-route-3 and nl-3 libs are not installed in
a standard location. pkg_check_modules() puts the location it
got from pkgconfig in the NL_LIBRARY_DIRS variable.

For example, I have a test copy of nl-route-3 and nl-3 in /tmp/test/lib. There is no other version of these libs installed on my system. I can configure rdma-core with:

cmake -DCMAKE_PREFIX_PATH=/tmp/test -DCMAKE_INSTALL_PATH=/tmp/test -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=YES ../rdma-core

Without the fix, the compile will fail with:

[  7%] Linking C executable ../bin/rdma_rename
/usr/bin/ld: cannot find -lnl-route-3
/usr/bin/ld: cannot find -lnl-3
collect2: error: ld returned 1 exit status

because the "-L" linking information returned by pkg_check_modules() in ${NL_LIBRARY_DIRS} is ignored by the rdma-core CMakeLists.txt. This is easy to fix by adding a call to link_directories().

(an alternate solution if you are using cmake 3.6 or newer to use an imported target created by pkg_check_modules() with the "IMPORTED_TARGET" argument that got added in cmake 3.6)

[rleon]

Yes, it seems like the most simple solution.
I wonder if the better variant will be to convert pkg_check_modules() should be converted to find_library()/find_package().

[jgunthorpe]

How did such an old kernel have support for this new EFA device?

[chuckcranor]

Yes, it seems like the most simple solution.
I wonder if the better variant will be to convert pkg_check_modules() should be converted to find_library()/find_package().

That's easy to do too, and then you can stop using pkgconfig as noted in the "FIXME" comment. If you did that, you'd use CMAKE_PREFIX_PATH cmake variable to point at the desired libnl3 to use rather than pkgconfig's PKG_CONFIG_PATH environment variable. I just tested the following and it seems to work. You won't need the link_directories() call anymore because find_library() returns the full path to the requested library (so no need for a "-L"). if this works better you can use the diff below or i can do another pullup request if you want.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index edab9b0..7cfa8a2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -167,6 +167,7 @@ include(CMakeParseArguments)
 include(CheckCCompilerFlag)
 include(CheckIncludeFile)
 include(CheckTypeSize)
+include(FindPackageHandleStandardArgs)
 include(RDMA_EnableCStd)
 include(RDMA_Sparse)
 include(RDMA_BuildType)
@@ -432,17 +433,21 @@ FIND_PACKAGE(rst2man)
 # pthread
 FIND_PACKAGE (Threads REQUIRED)
 
-FIND_PACKAGE(PkgConfig REQUIRED)
-
 # libnl
 if (NOT DEFINED ENABLE_RESOLVE_NEIGH)
   set(ENABLE_RESOLVE_NEIGH "ON" CACHE BOOL "Enable internal resolution of neigh
bours for Etherent")
 endif()
 if (ENABLE_RESOLVE_NEIGH)
-  # FIXME use of pkgconfig is discouraged
-  pkg_check_modules(NL libnl-3.0 libnl-route-3.0 REQUIRED)
+  find_path(NL_INCLUDE_DIRS netlink/netlink.h PATH_SUFFIXES libnl3)
+  find_library(NL_LIBNL3 nl-3)
+  find_library(NL_LIBNL_ROUTE3 nl-route-3)
+  find_package_handle_standard_args(NL DEFAULT_MSG NL_INCLUDE_DIRS
+    NL_LIBNL3 NL_LIBNL_ROUTE3)
   include_directories(${NL_INCLUDE_DIRS})
-  link_directories(${NL_LIBRARY_DIRS})
+  if (NOT NL_INCLUDE_DIRS OR NOT NL_LIBNL3 OR NOT NL_LIBNL_ROUTE3)
+    message(FATAL_ERROR "libnl-3 not found: disable it or fix the prefix path")
+  endif()
+  set(NL_LIBRARIES ${NL_LIBNL_ROUTE3} ${NL_LIBNL3})
   set(NL_KIND 3)
 else()
   set(NL_KIND 0)

[rleon]

I would love to see that.
@jgunthorpe are you ok with this change?

[jgunthorpe]

It turned out we needed the pkgconfig to make static linking working, IIRC