diff mbox

OFED-4.8, rdma-core, and library paths

Message ID 20170208185446.GH6005@mtr-leonro.local (mailing list archive)
State Not Applicable
Headers show

Commit Message

Leon Romanovsky Feb. 8, 2017, 6:54 p.m. UTC
On Wed, Feb 08, 2017 at 11:18:20AM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 08, 2017 at 08:01:04PM +0200, Leon Romanovsky wrote:
> > On Wed, Feb 08, 2017 at 10:33:35AM -0700, Jason Gunthorpe wrote:
>
> > Thanks for the help, my final version which works correctly for build in place, install from
> > sources and packages for centos6/centos6 is below:
>
> Looks good
>
> > # Create a special provider with exported symbols in it
> > function(rdma_shared_provider DEST VERSION_SCRIPT SOVERSION VERSION)
> >   # Installed driver file
> >   file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/${DEST}.driver" "driver ${DEST}\n")
> >   install(FILES "${CMAKE_CURRENT_BINARY_DIR}/${DEST}.driver" DESTINATION "${CONFIG_DIR}")
> >
> >   # Uninstalled driver file
> >   file(MAKE_DIRECTORY "${BUILD_ETC}/libibverbs.d/")
> >   file(WRITE "${BUILD_ETC}/libibverbs.d/${DEST}.driver" "driver ${BUILD_LIB}/lib${DEST}\n")
> >
> >   # Create a static provider library
> >   if (ENABLE_STATIC)
> >     add_library(${DEST} STATIC ${ARGN})
> >     set_target_properties(${DEST} PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${BUILD_LIB}")
> >     install(TARGETS ${DEST} DESTINATION "${CMAKE_INSTALL_LIBDIR}")
> >
> >     list(APPEND RDMA_STATIC_LIBS ${DEST}-rdmav2 ${DEST})
> >     set(RDMA_STATIC_LIBS "${RDMA_STATIC_LIBS}" CACHE INTERNAL "")
> >   endif()
> >
> >   # Create the plugin shared library
> >   add_library(${DEST} SHARED ${ARGN})
> >   # Even though these are modules we still want to use Wl,--no-undefined
> >   set_target_properties(${DEST} PROPERTIES LINK_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
> >   rdma_set_library_map(${DEST} ${VERSION_SCRIPT})
> >
> >   target_link_libraries(${DEST} LINK_PRIVATE ${COMMON_LIBS_PIC})
> >   target_link_libraries(${DEST} LINK_PRIVATE ibverbs)
> >   target_link_libraries(${DEST} LINK_PRIVATE ${CMAKE_THREAD_LIBS_INIT})
> >   set_target_properties(${DEST} PROPERTIES
> >   	SOVERSION ${SOVERSION}
> >   	VERSION ${VERSION}
> >   	LIBRARY_OUTPUT_DIRECTORY "${BUILD_LIB}")
> >   add_custom_target(share_link ALL DEPENDS "${DEST}"  COMMAND ${CMAKE_COMMAND} -E create_symlink "lib${DEST}.so.${VERSION}"
> > 	  "${BUILD_LIB}/lib${DEST}-rdmav2.so")
> >   add_dependencies(share_link ${DEST})
>
> Except this really shouldn't be a rule.  The non-rule method is used
> everywhere else (eg man pages), so it must work here, it doesn't make
> sense that ninja vs make would be any different. When you get
> everything working put it back to execute_process..
>
> >   install(TARGETS ${DEST} DESTINATION "${CMAKE_INSTALL_LIBDIR}")
> >   execute_process(COMMAND python ${CMAKE_SOURCE_DIR}/buildlib/relpath
> >     ${CMAKE_INSTALL_LIBDIR}/lib${DEST}.so.${VERSION} ${VERBS_PROVIDER_DIR}
> >     OUTPUT_VARIABLE DEST_LINK_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
> >   rdma_install_symlink("${DEST_LINK_PATH}" "${VERBS_PROVIDER_DIR}/lib${DEST}-rdmav2.so")
>
> > and buildlib/relpath
> > import os
> > import sys
> >
> > print(os.path.relpath(sys.argv[1], sys.argv[2]))
>
> Sure, yes, doing the escaping right would probably be quite hard, that is the
> usual nightmare with shell stuff unfortunately.
>
> > But I still have issues with DEB package.
> > It creates absolute (and wrong)  symlink instead of relative one.
>
> Hmm. Can you update your github? I'll look for you.

I pushed latest code to m/dv-1 branch, it is not for inclusion yet.
https://github.com/rleon/rdma-core/commits/m/dv-v1
Updated cover letter with changelog:
https://github.com/rleon/rdma-core/commit/a332669511d5bbbd3dcdd977fbf95aa7ee47a69e
And all cmake/packages stuff in the patch:
https://github.com/rleon/rdma-core/commit/fcc4996c7fa5169fe599c2d568476314c1f65ddc

>
> > -rw-r--r-- 1 leonro leonro 46K Feb  8 17:13 libmlx4-rdmav2.so
> > lrwxrwxrwx 1 leonro leonro  65 Feb  8 17:13 libmlx5-rdmav2.so -> /home/leonro/src/build-deb/lib/x86_64-linux-gnu/libmlx5.so.1.0.13
>
> Most likely this means that DEST_LINK_PATH passed to
> rdma_install_symlink is not correct. Which suggests python relpath is
> not working..
>
> relpath in python is done textually, from these args:
>
> >     ${CMAKE_INSTALL_LIBDIR}/lib${DEST}.so.${VERSION} ${VERBS_PROVIDER_DIR}
>
> But
>
>  set(VERBS_PROVIDER_DIR "${CMAKE_INSTALL_FULL_LIBDIR}/libibverbs"
>
> So if CMAKE_INSTALL_FULL_LIBDIR != CMAKE_INSTALL_LIBDIR (which depends
> on how the distro build script configures cmake) then it will fail.
>
> Switch to:
>
>      ${CMAKE_INSTALL_FULL_LIBDIR}/lib${DEST}.so.${VERSION} ${VERBS_PROVIDER_DIR}

You are right, it fixed debian, I'm testing other packages now.

>
> Jason

Comments

Jason Gunthorpe Feb. 8, 2017, 8:56 p.m. UTC | #1
On Wed, Feb 08, 2017 at 08:54:46PM +0200, Leon Romanovsky wrote:

> > Hmm. Can you update your github? I'll look for you.
> 
> I pushed latest code to m/dv-1 branch, it is not for inclusion yet.
> https://github.com/rleon/rdma-core/commits/m/dv-v1
> Updated cover letter with changelog:
> https://github.com/rleon/rdma-core/commit/a332669511d5bbbd3dcdd977fbf95aa7ee47a69e
> And all cmake/packages stuff in the patch:
> https://github.com/rleon/rdma-core/commit/fcc4996c7fa5169fe599c2d568476314c1f65ddc

This should take care of most things

https://github.com/jgunthorpe/rdma-plumbing/commit/490b7fb184c414ab16b7056f1effa9136e8afe46

The debian packaging still needs some adjusting.

I suspect it is against Debian policy to include libmlx.so in the
providers package, it probably needs to have its own package and its
own -dev package.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Feb. 9, 2017, 5:31 a.m. UTC | #2
On Wed, Feb 08, 2017 at 01:56:22PM -0700, Jason Gunthorpe wrote:
> On Wed, Feb 08, 2017 at 08:54:46PM +0200, Leon Romanovsky wrote:
>
> > > Hmm. Can you update your github? I'll look for you.
> >
> > I pushed latest code to m/dv-1 branch, it is not for inclusion yet.
> > https://github.com/rleon/rdma-core/commits/m/dv-v1
> > Updated cover letter with changelog:
> > https://github.com/rleon/rdma-core/commit/a332669511d5bbbd3dcdd977fbf95aa7ee47a69e
> > And all cmake/packages stuff in the patch:
> > https://github.com/rleon/rdma-core/commit/fcc4996c7fa5169fe599c2d568476314c1f65ddc
>
> This should take care of most things
>
> https://github.com/jgunthorpe/rdma-plumbing/commit/490b7fb184c414ab16b7056f1effa9136e8afe46

Thanks a lot, we will add into our v1.

>
> The debian packaging still needs some adjusting.
>
> I suspect it is against Debian policy to include libmlx.so in the
> providers package, it probably needs to have its own package and its
> own -dev package.

I don't know, at the end, it is provider.

>
> Jason
Benjamin Drung Feb. 9, 2017, 9:53 a.m. UTC | #3
Am Mittwoch, den 08.02.2017, 13:56 -0700 schrieb Jason Gunthorpe:
> On Wed, Feb 08, 2017 at 08:54:46PM +0200, Leon Romanovsky wrote:
> 
> > > Hmm. Can you update your github? I'll look for you.
> > 
> > I pushed latest code to m/dv-1 branch, it is not for inclusion yet.
> > https://github.com/rleon/rdma-core/commits/m/dv-v1
> > Updated cover letter with changelog:
> > https://github.com/rleon/rdma-core/commit/a332669511d5bbbd3dcdd977f
> > bf95aa7ee47a69e
> > And all cmake/packages stuff in the patch:
> > https://github.com/rleon/rdma-core/commit/fcc4996c7fa5169fe599c2d56
> > 8476314c1f65ddc
> 
> This should take care of most things
> 
> https://github.com/jgunthorpe/rdma-plumbing/commit/490b7fb184c414ab16
> b7056f1effa9136e8afe46
> 
> The debian packaging still needs some adjusting.
> 
> I suspect it is against Debian policy to include libmlx.so in the
> providers package, it probably needs to have its own package and its
> own -dev package.

When other packages should be able to link against libmlx.so and need
header files for that, it is better to have separate binary packages
(libmlx and libmlx-dev) for it.
Leon Romanovsky Feb. 9, 2017, 12:32 p.m. UTC | #4
On Thu, Feb 09, 2017 at 10:53:26AM +0100, Benjamin Drung wrote:
> Am Mittwoch, den 08.02.2017, 13:56 -0700 schrieb Jason Gunthorpe:
> > On Wed, Feb 08, 2017 at 08:54:46PM +0200, Leon Romanovsky wrote:
> >
> > > > Hmm. Can you update your github? I'll look for you.
> > >
> > > I pushed latest code to m/dv-1 branch, it is not for inclusion yet.
> > > https://github.com/rleon/rdma-core/commits/m/dv-v1
> > > Updated cover letter with changelog:
> > > https://github.com/rleon/rdma-core/commit/a332669511d5bbbd3dcdd977f
> > > bf95aa7ee47a69e
> > > And all cmake/packages stuff in the patch:
> > > https://github.com/rleon/rdma-core/commit/fcc4996c7fa5169fe599c2d56
> > > 8476314c1f65ddc
> >
> > This should take care of most things
> >
> > https://github.com/jgunthorpe/rdma-plumbing/commit/490b7fb184c414ab16
> > b7056f1effa9136e8afe46
> >
> > The debian packaging still needs some adjusting.
> >
> > I suspect it is against Debian policy to include libmlx.so in the
> > providers package, it probably needs to have its own package and its
> > own -dev package.
>
> When other packages should be able to link against libmlx.so and need
> header files for that, it is better to have separate binary packages
> (libmlx and libmlx-dev) for it.

This libmlx is dependent on ibverbs-provider and libibverbs. It is useless
without them and will be always installed together, so why do we need to
complicate users life by adding new library?

And we can always spin-off it as a separate package once someone will need it.

Thanks

>
> --
> Benjamin Drung
> System Developer
> Debian & Ubuntu Developer
>
> ProfitBricks GmbH
> Greifswalder Str. 207
> D - 10405 Berlin
>
> Email: benjamin.drung@profitbricks.com
> URL:  http://www.profitbricks.com
>
> Sitz der Gesellschaft: Berlin.
> Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
> Geschäftsführer: Andreas Gauger, Achim Weiss.
diff mbox

Patch

diff --git a/buildlib/rdma_functions.cmake b/buildlib/rdma_functions.cmake
index 3d0683d0..3a1758f8 100644
--- a/buildlib/rdma_functions.cmake
+++ b/buildlib/rdma_functions.cmake
@@ -110,9 +110,15 @@  function(rdma_shared_provider DEST VERSION_SCRIPT SOVERSION VERSION)
   add_dependencies(share_link ${DEST})

   install(TARGETS ${DEST} DESTINATION "${CMAKE_INSTALL_LIBDIR}")
-  execute_process(COMMAND python ${CMAKE_SOURCE_DIR}/buildlib/relpath
-    ${CMAKE_INSTALL_LIBDIR}/lib${DEST}.so.${VERSION} ${VERBS_PROVIDER_DIR}
-    OUTPUT_VARIABLE DEST_LINK_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
+  if (NOT ${CMAKE_INSTALL_FULL_LIBDIR} STREQUAL ${CMAKE_INSTALL_LIBDIR})
+    execute_process(COMMAND python ${CMAKE_SOURCE_DIR}/buildlib/relpath
+	    ${CMAKE_INSTALL_FULL_LIBDIR}/lib${DEST}.so.${VERSION} ${VERBS_PROVIDER_DIR}
+      OUTPUT_VARIABLE DEST_LINK_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
+  else()
+    execute_process(COMMAND python ${CMAKE_SOURCE_DIR}/buildlib/relpath
+      ${CMAKE_INSTALL_LIBDIR}/lib${DEST}.so.${VERSION} ${VERBS_PROVIDER_DIR}
+      OUTPUT_VARIABLE DEST_LINK_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
+  endif()
   rdma_install_symlink("${DEST_LINK_PATH}" "${VERBS_PROVIDER_DIR}/lib${DEST}-rdmav2.so")
 endfunction()