@simonmichal requested changes on this pull request.

  1. Are we sure that SSI should be part of xrootd-server-libs? We could also create a separate package for ssi, say xrootd-ssi-libs and xrootd-ssi-devel.
  2. libXrdSsi and libXrdSsiLog are not versioned and the unversioned libs are not packaged.
  3. IMHO it's better to keep all SSI specific staff in XrdSsi.cmake and don't split it between XrdSsi.cmake and XrdPlugins.cmake.

In src/CMakeLists.txt:

> @@ -29,6 +29,7 @@ include( XrdXml )
 include( XrdPosix )
 include( XrdFfs )
 include( XrdPlugins )
+include( XrdSsi )

Do we want to include XrdSsi in 4.7.0? If not this needs to be optional (similarly as for http and cepth).


In src/XrdPlugins.cmake:

> @@ -9,6 +9,8 @@ set( LIB_XRD_N2NO2P     XrdN2No2p-${PLUGIN_VERSION} )
 set( LIB_XRD_PSS        XrdPss-${PLUGIN_VERSION} )
 set( LIB_XRD_GPFS       XrdOssSIgpfsT-${PLUGIN_VERSION} )
 set( LIB_XRD_ZCRC32     XrdCksCalczcrc32-${PLUGIN_VERSION} )
+set( LIB_XRD_SSI        XrdSsi-${PLUGIN_VERSION} )

IMHO it would be better to keep all XrdSsi cmake staff in XrdSsi.cmake. Adding it partially to XrdPlugins.cmake is misleading and clutters the file.


In src/XrdPlugins.cmake:

> +  XrdSsi/XrdSsiDir.cc                    XrdSsi/XrdSsiDir.hh
+  XrdSsi/XrdSsiFile.cc                   XrdSsi/XrdSsiFile.hh
+  XrdSsi/XrdSsiFileReq.cc                XrdSsi/XrdSsiFileReq.hh
+  XrdSsi/XrdSsiFileSess.cc               XrdSsi/XrdSsiFileSess.hh
+  XrdSsi/XrdSsiSfs.cc                    XrdSsi/XrdSsiSfs.hh
+  XrdSsi/XrdSsiSfsConfig.cc              XrdSsi/XrdSsiSfsConfig.hh
+  XrdSsi/XrdSsiStat.cc
+)
+
+target_link_libraries(
+  ${LIB_XRD_SSI}
+  XrdSsiLib
+  XrdUtils
+  XrdServer )
+
+set_target_properties(

The library is not versioned!


In src/XrdPlugins.cmake:

> +#-------------------------------------------------------------------------------
+# The XrdSsiLog lib
+#-------------------------------------------------------------------------------
+add_library(
+  ${LIB_XRD_SSILOG}
+  SHARED
+  XrdSsi/XrdSsiLogging.cc
+)
+
+target_link_libraries(
+  ${LIB_XRD_SSILOG}
+  XrdSsiLib
+  XrdUtils
+  XrdServer )
+
+set_target_properties(

The library is not versioned!


In src/XrdPlugins.cmake:

> +  XrdSsi/XrdSsiDir.cc                    XrdSsi/XrdSsiDir.hh
+  XrdSsi/XrdSsiFile.cc                   XrdSsi/XrdSsiFile.hh
+  XrdSsi/XrdSsiFileReq.cc                XrdSsi/XrdSsiFileReq.hh
+  XrdSsi/XrdSsiFileSess.cc               XrdSsi/XrdSsiFileSess.hh
+  XrdSsi/XrdSsiSfs.cc                    XrdSsi/XrdSsiSfs.hh
+  XrdSsi/XrdSsiSfsConfig.cc              XrdSsi/XrdSsiSfsConfig.hh
+  XrdSsi/XrdSsiStat.cc
+)
+
+target_link_libraries(
+  ${LIB_XRD_SSI}
+  XrdSsiLib
+  XrdUtils
+  XrdServer )
+
+set_target_properties(

Also, the library is installed but is not packaged.
Note this creates libXrdSsi.so which is installed but not packaged. libXrdSsi.so.* are, in turn not created but they are packaged.


In packaging/rhel/xrootd.spec.in:

> @@ -733,6 +733,10 @@ fi
 %{_libdir}/libXrdN2No2p-4.so
 %{_libdir}/libXrdOssSIgpfsT-4.so
 %{_libdir}/libXrdServer.so.*
+%{_libdir}/libXrdSsi-4.so.*

Are we sure this should go to server-libs, and not to a new package, .e.g. ssi-libs ?
Actually, since libXrdSsi is not versioned this fails as there are no libXrdSsi-4.si.*, there is only the libXrdSsi-4.so, which in turn is installed but not packaged.


In packaging/rhel/xrootd.spec.in:

> @@ -733,6 +733,10 @@ fi
 %{_libdir}/libXrdN2No2p-4.so
 %{_libdir}/libXrdOssSIgpfsT-4.so
 %{_libdir}/libXrdServer.so.*
+%{_libdir}/libXrdSsi-4.so.*
+%{_libdir}/libXrdSsiLib.so.*
+%{_libdir}/libXrdSsiLog-4.so.*

Same here, since libXrdSsiLog is not versioned this fails as there are no libXrdSsiLog-4.si.*, there is only the libXrdSsiLog-4.so, which in turn is installed but not packaged.


In src/XrdHeaders.cmake:

> @@ -125,6 +125,18 @@ set( XROOTD_PRIVATE_HEADERS
   XrdOfs/XrdOfsHandle.hh
   XrdOfs/XrdOfsTrace.hh
   XrdOfs/XrdOfsTPCInfo.hh
+  XrdSsi/XrdSsiAtomics.hh

Are we sure we don't want this in a separate package, say ssi-devel (together with *.so files) ?


In src/XrdPlugins.cmake:

> +#-------------------------------------------------------------------------------
+# The XrdSsiLog lib
+#-------------------------------------------------------------------------------
+add_library(
+  ${LIB_XRD_SSILOG}
+  SHARED
+  XrdSsi/XrdSsiLogging.cc
+)
+
+target_link_libraries(
+  ${LIB_XRD_SSILOG}
+  XrdSsiLib
+  XrdUtils
+  XrdServer )
+
+set_target_properties(

Also, the library is installed but is not packaged.
Note this creates libXrdSsiLog.so which is installed but not packaged. libXrdSsiLog.so.* are, in turn not created but they are packaged.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/xrootd/xrootd","title":"xrootd/xrootd","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/xrootd/xrootd"}},"updates":{"snippets":[{"icon":"PERSON","message":"@simonmichal requested changes on #542"}],"action":{"name":"View Pull Request","url":"https://github.com/xrootd/xrootd/pull/542#pullrequestreview-50007603"}}}

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1