diff mbox

[rdma-core,2/4] glue/redhat: add udev/systemd/etc infrastructure bits

Message ID 20161019185920.GA20600@obsidianresearch.com (mailing list archive)
State Superseded
Headers show

Commit Message

Jason Gunthorpe Oct. 19, 2016, 6:59 p.m. UTC
On Wed, Oct 19, 2016 at 06:38:52PM +0000, Weiny, Ira wrote:

> I have the C code ported to rdma-core but how do I do this in cmake?

Attached, you will also need

target_link_libraries(rdma-ndd ${UDEV_LIBRARIES})

>         PKG_CHECK_EXISTS(libudev >= 218, [with_dev_logging=no],
>                         [with_udev_logging=yes])
>         if test "$with_udev_logging" = "yes"; then
>                 AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1,
>                                 [whether libudev logging can be used])

Dump HAVE_UDEV_LOGGING from the code, upstream deleted support for it:

https://www.redhat.com/archives/libvir-list/2014-December/msg00749.html

src.c:3:2: warning: 'udev_set_log_fn' is deprecated (declared at /usr/include/libudev.h:41) [-Wdeprecated-declarations]
  int main(int argc,const char *argv[]) {udev_set_log_fn(NULL, NULL); return 0;}
  ^

> I've found a "modules" file which looks like it has a compatible BSD
> license and could be added but is there a better way?

Basically the right idea, but I prefer to avoid pkgconfig whenever
possible as it screws up cross compiling.

> I also have to convert the man page from *.rst to man in some way...
> Would it be ok if I put a dependency on rst2man in the repo?

We have talked about it, maybe for now just include both the .rst and
the rst2man output and we can revisit it..

Jason

From b0062acc238e06289aa946dacd5c534cf6c68d7d Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date: Wed, 19 Oct 2016 12:57:25 -0600
Subject: [PATCH] Add a dependency on libudev

incomplete, needs the RH stuff too.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 .travis.yml             |  1 +
 CMakeLists.txt          |  4 ++++
 README.md               |  2 +-
 buildlib/FindUDev.cmake | 10 ++++++++++
 debian/control          |  1 +
 5 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 buildlib/FindUDev.cmake

Comments

Ira Weiny Oct. 19, 2016, 9:35 p.m. UTC | #1
Udev below works great.  Thanks!

But I have another cmake question.

I have specified the following:

install(FILES rdma-ndd.conf DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}")
set(rdma_ndd_CONFIG_PATH "${CMAKE_INSTALL_SYSCONFDIR}")
configure_file ("${PROJECT_SOURCE_DIR}/rdma-ndd/rdma-config.h.in"
        "${PROJECT_BINARY_DIR}/rdma-ndd/rdma-config.h")
include_directories("${PROJECT_BINARY_DIR}/rdma-ndd")

The idea here is to have an rdma-ndd.conf file which allows the user to configure their names.  (previously this was the infiniband-diags.conf file). But I have ported over the code.

The value in rdma-config.h ends up being:

17:30:34 > cat build/rdma-ndd/rdma-config.h 
/* Pick up configuration items from cmake */
#define RDMA_NDD_CONFIG_PATH "etc"
#define PACKAGE_VERSION 11

Which seems fine but after a "make install" I'm left with an incorrect path:

[root@phcppriv13 sbin]# pwd && ./rdma-ndd -f
/usr/local/sbin
rdma-ndd[91656]: Using config file (etc/rdma-ndd.conf)
...

So it is not reading the correct config file.

How do I link the install to this variable in the code?

Ira


> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com]
> Sent: Wednesday, October 19, 2016 11:59 AM
> To: Weiny, Ira <ira.weiny@intel.com>
> Cc: Jarod Wilson <jarod@redhat.com>; linux-rdma@vger.kernel.org; Doug
> Ledford <dledford@redhat.com>; Hefty, Sean <sean.hefty@intel.com>
> Subject: Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc
> infrastructure bits
> 
> On Wed, Oct 19, 2016 at 06:38:52PM +0000, Weiny, Ira wrote:
> 
> > I have the C code ported to rdma-core but how do I do this in cmake?
> 
> Attached, you will also need
> 
> target_link_libraries(rdma-ndd ${UDEV_LIBRARIES})
> 
> >         PKG_CHECK_EXISTS(libudev >= 218, [with_dev_logging=no],
> >                         [with_udev_logging=yes])
> >         if test "$with_udev_logging" = "yes"; then
> >                 AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1,
> >                                 [whether libudev logging can be used])
> 
> Dump HAVE_UDEV_LOGGING from the code, upstream deleted support for it:
> 
> https://www.redhat.com/archives/libvir-list/2014-December/msg00749.html
> 
> src.c:3:2: warning: 'udev_set_log_fn' is deprecated (declared at
> /usr/include/libudev.h:41) [-Wdeprecated-declarations]
>   int main(int argc,const char *argv[]) {udev_set_log_fn(NULL, NULL); return 0;}
>   ^
> 
> > I've found a "modules" file which looks like it has a compatible BSD
> > license and could be added but is there a better way?
> 
> Basically the right idea, but I prefer to avoid pkgconfig whenever possible as it
> screws up cross compiling.
> 
> > I also have to convert the man page from *.rst to man in some way...
> > Would it be ok if I put a dependency on rst2man in the repo?
> 
> We have talked about it, maybe for now just include both the .rst and the
> rst2man output and we can revisit it..
> 
> Jason
> 
> From b0062acc238e06289aa946dacd5c534cf6c68d7d Mon Sep 17 00:00:00
> 2001
> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Date: Wed, 19 Oct 2016 12:57:25 -0600
> Subject: [PATCH] Add a dependency on libudev
> 
> incomplete, needs the RH stuff too.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  .travis.yml             |  1 +
>  CMakeLists.txt          |  4 ++++
>  README.md               |  2 +-
>  buildlib/FindUDev.cmake | 10 ++++++++++
>  debian/control          |  1 +
>  5 files changed, 17 insertions(+), 1 deletion(-)  create mode 100644
> buildlib/FindUDev.cmake
> 
> diff --git a/.travis.yml b/.travis.yml
> index d81b699294eb..d9c36cc9c649 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -24,6 +24,7 @@ addons:
>        - gcc-6
>        - libnl-3-dev
>        - libnl-route-3-dev
> +      - libudev-dev
>        - make
>        - ninja-build
>        - pkg-config
> diff --git a/CMakeLists.txt b/CMakeLists.txt index a23aa860e6d3..9402bacf70ce
> 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -237,6 +237,10 @@ if (NOT NL_KIND EQUAL 0)
>    set(CMAKE_REQUIRED_INCLUDES "${SAFE_CMAKE_REQUIRED_INCLUDES}")
>  endif()
> 
> +# udev
> +find_package(UDev REQUIRED)
> +include_directories(${UDEV_INCLUDE_DIRS})
> +
>  # Statically determine sizeof(long), this is largely unnecessary, no new code  #
> should rely on this.
>  check_type_size("long" SIZEOF_LONG BUILTIN_TYPES_ONLY LANGUAGE C) diff
> --git a/README.md b/README.md index 66aee3f49f00..d24fd0bf2606 100644
> --- a/README.md
> +++ b/README.md
> @@ -48,7 +48,7 @@ only load from the system path.
>  ### Debian Derived
> 
>  ```sh
> -$ apt-get install build-essential cmake gcc libnl-3-dev libnl-route-3-dev ninja-
> build pkg-config valgrind
> +$ apt-get install build-essential cmake gcc libudev-dev libnl-3-dev
> +libnl-route-3-dev ninja-build pkg-config valgrind
>  ```
> 
>  ### Fedora
> diff --git a/buildlib/FindUDev.cmake b/buildlib/FindUDev.cmake new file mode
> 100644 index 000000000000..ce05ddf991a1
> --- /dev/null
> +++ b/buildlib/FindUDev.cmake
> @@ -0,0 +1,10 @@
> +# COPYRIGHT (c) 2016 Obsidian Research Corporation. See COPYING file
> +
> +find_library(LIBUDEV_LIBRARY NAMES udev libudev)
> +
> +set(UDEV_LIBRARIES ${LIBUDEV_LIBRARY})
> +
> +include(FindPackageHandleStandardArgs)
> +find_package_handle_standard_args(UDev REQUIRED_VARS
> LIBUDEV_LIBRARY)
> +
> +mark_as_advanced(LIBUDEV_LIBRARY)
> diff --git a/debian/control b/debian/control index
> 2335d1f4814d..ed9850a348be 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -8,6 +8,7 @@ Build-Depends: build-essential,
>  	       dh-systemd,
>  	       dpkg-dev (>= 1.17),
>  	       gcc,
> +	       libudev-dev,
>  	       libnl-3-dev,
>  	       libnl-route-3-dev,
>  	       make,
> --
> 2.1.4

--
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
Ira Weiny Oct. 19, 2016, 10:55 p.m. UTC | #2
Nevermind I think I have it...

Just need to test with an rpm build...

Ira




> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Weiny, Ira
> Sent: Wednesday, October 19, 2016 2:35 PM
> To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Cc: Jarod Wilson <jarod@redhat.com>; linux-rdma@vger.kernel.org; Doug
> Ledford <dledford@redhat.com>; Hefty, Sean <sean.hefty@intel.com>
> Subject: RE: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc
> infrastructure bits
> 
> Udev below works great.  Thanks!
> 
> But I have another cmake question.
> 
> I have specified the following:
> 
> install(FILES rdma-ndd.conf DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}")
> set(rdma_ndd_CONFIG_PATH "${CMAKE_INSTALL_SYSCONFDIR}")
> configure_file ("${PROJECT_SOURCE_DIR}/rdma-ndd/rdma-config.h.in"
>         "${PROJECT_BINARY_DIR}/rdma-ndd/rdma-config.h")
> include_directories("${PROJECT_BINARY_DIR}/rdma-ndd")
> 
> The idea here is to have an rdma-ndd.conf file which allows the user to
> configure their names.  (previously this was the infiniband-diags.conf file). But I
> have ported over the code.
> 
> The value in rdma-config.h ends up being:
> 
> 17:30:34 > cat build/rdma-ndd/rdma-config.h
> /* Pick up configuration items from cmake */ #define
> RDMA_NDD_CONFIG_PATH "etc"
> #define PACKAGE_VERSION 11
> 
> Which seems fine but after a "make install" I'm left with an incorrect path:
> 
> [root@phcppriv13 sbin]# pwd && ./rdma-ndd -f /usr/local/sbin
> rdma-ndd[91656]: Using config file (etc/rdma-ndd.conf) ...
> 
> So it is not reading the correct config file.
> 
> How do I link the install to this variable in the code?
> 
> Ira
> 
> 
> > -----Original Message-----
> > From: Jason Gunthorpe [mailto:jgunthorpe@obsidianresearch.com]
> > Sent: Wednesday, October 19, 2016 11:59 AM
> > To: Weiny, Ira <ira.weiny@intel.com>
> > Cc: Jarod Wilson <jarod@redhat.com>; linux-rdma@vger.kernel.org; Doug
> > Ledford <dledford@redhat.com>; Hefty, Sean <sean.hefty@intel.com>
> > Subject: Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc
> > infrastructure bits
> >
> > On Wed, Oct 19, 2016 at 06:38:52PM +0000, Weiny, Ira wrote:
> >
> > > I have the C code ported to rdma-core but how do I do this in cmake?
> >
> > Attached, you will also need
> >
> > target_link_libraries(rdma-ndd ${UDEV_LIBRARIES})
> >
> > >         PKG_CHECK_EXISTS(libudev >= 218, [with_dev_logging=no],
> > >                         [with_udev_logging=yes])
> > >         if test "$with_udev_logging" = "yes"; then
> > >                 AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1,
> > >                                 [whether libudev logging can be
> > > used])
> >
> > Dump HAVE_UDEV_LOGGING from the code, upstream deleted support for it:
> >
> > https://www.redhat.com/archives/libvir-list/2014-December/msg00749.htm
> > l
> >
> > src.c:3:2: warning: 'udev_set_log_fn' is deprecated (declared at
> > /usr/include/libudev.h:41) [-Wdeprecated-declarations]
> >   int main(int argc,const char *argv[]) {udev_set_log_fn(NULL, NULL); return
> 0;}
> >   ^
> >
> > > I've found a "modules" file which looks like it has a compatible BSD
> > > license and could be added but is there a better way?
> >
> > Basically the right idea, but I prefer to avoid pkgconfig whenever
> > possible as it screws up cross compiling.
> >
> > > I also have to convert the man page from *.rst to man in some way...
> > > Would it be ok if I put a dependency on rst2man in the repo?
> >
> > We have talked about it, maybe for now just include both the .rst and
> > the rst2man output and we can revisit it..
> >
> > Jason
> >
> > From b0062acc238e06289aa946dacd5c534cf6c68d7d Mon Sep 17 00:00:00
> > 2001
> > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Date: Wed, 19 Oct 2016 12:57:25 -0600
> > Subject: [PATCH] Add a dependency on libudev
> >
> > incomplete, needs the RH stuff too.
> >
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > ---
> >  .travis.yml             |  1 +
> >  CMakeLists.txt          |  4 ++++
> >  README.md               |  2 +-
> >  buildlib/FindUDev.cmake | 10 ++++++++++
> >  debian/control          |  1 +
> >  5 files changed, 17 insertions(+), 1 deletion(-)  create mode 100644
> > buildlib/FindUDev.cmake
> >
> > diff --git a/.travis.yml b/.travis.yml index
> > d81b699294eb..d9c36cc9c649 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -24,6 +24,7 @@ addons:
> >        - gcc-6
> >        - libnl-3-dev
> >        - libnl-route-3-dev
> > +      - libudev-dev
> >        - make
> >        - ninja-build
> >        - pkg-config
> > diff --git a/CMakeLists.txt b/CMakeLists.txt index
> > a23aa860e6d3..9402bacf70ce
> > 100644
> > --- a/CMakeLists.txt
> > +++ b/CMakeLists.txt
> > @@ -237,6 +237,10 @@ if (NOT NL_KIND EQUAL 0)
> >    set(CMAKE_REQUIRED_INCLUDES
> "${SAFE_CMAKE_REQUIRED_INCLUDES}")
> >  endif()
> >
> > +# udev
> > +find_package(UDev REQUIRED)
> > +include_directories(${UDEV_INCLUDE_DIRS})
> > +
> >  # Statically determine sizeof(long), this is largely unnecessary, no
> > new code  # should rely on this.
> >  check_type_size("long" SIZEOF_LONG BUILTIN_TYPES_ONLY LANGUAGE C)
> > diff --git a/README.md b/README.md index 66aee3f49f00..d24fd0bf2606
> > 100644
> > --- a/README.md
> > +++ b/README.md
> > @@ -48,7 +48,7 @@ only load from the system path.
> >  ### Debian Derived
> >
> >  ```sh
> > -$ apt-get install build-essential cmake gcc libnl-3-dev
> > libnl-route-3-dev ninja- build pkg-config valgrind
> > +$ apt-get install build-essential cmake gcc libudev-dev libnl-3-dev
> > +libnl-route-3-dev ninja-build pkg-config valgrind
> >  ```
> >
> >  ### Fedora
> > diff --git a/buildlib/FindUDev.cmake b/buildlib/FindUDev.cmake new
> > file mode
> > 100644 index 000000000000..ce05ddf991a1
> > --- /dev/null
> > +++ b/buildlib/FindUDev.cmake
> > @@ -0,0 +1,10 @@
> > +# COPYRIGHT (c) 2016 Obsidian Research Corporation. See COPYING file
> > +
> > +find_library(LIBUDEV_LIBRARY NAMES udev libudev)
> > +
> > +set(UDEV_LIBRARIES ${LIBUDEV_LIBRARY})
> > +
> > +include(FindPackageHandleStandardArgs)
> > +find_package_handle_standard_args(UDev REQUIRED_VARS
> > LIBUDEV_LIBRARY)
> > +
> > +mark_as_advanced(LIBUDEV_LIBRARY)
> > diff --git a/debian/control b/debian/control index
> > 2335d1f4814d..ed9850a348be 100644
> > --- a/debian/control
> > +++ b/debian/control
> > @@ -8,6 +8,7 @@ Build-Depends: build-essential,
> >  	       dh-systemd,
> >  	       dpkg-dev (>= 1.17),
> >  	       gcc,
> > +	       libudev-dev,
> >  	       libnl-3-dev,
> >  	       libnl-route-3-dev,
> >  	       make,
> > --
> > 2.1.4
> 
> --
> 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
--
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
Jason Gunthorpe Oct. 20, 2016, 3:50 a.m. UTC | #3
On Wed, Oct 19, 2016 at 09:35:15PM +0000, Weiny, Ira wrote:

> set(rdma_ndd_CONFIG_PATH "${CMAKE_INSTALL_SYSCONFDIR}")
> configure_file ("${PROJECT_SOURCE_DIR}/rdma-ndd/rdma-config.h.in"
>         "${PROJECT_BINARY_DIR}/rdma-ndd/rdma-config.h")
> include_directories("${PROJECT_BINARY_DIR}/rdma-ndd")

Please just use the existing buildlib/config.h.in and put in:

#define RDMA_NDD_CONFIG_FILE "@CMAKE_INSTALL_FULL_SYSCONFDIR@/rdma-ndd.conf"

Nothing else is needed, please don't add more config.h's.

> #define RDMA_NDD_CONFIG_PATH "etc"

This is the difference between the 'FULL' and normal path versions in
the cmake scheme. The FULL version must be used everyplace needing an
absolute path, the other verions are used with install and have to do
with making DESTDIR work properly.

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
Ira Weiny Oct. 20, 2016, 4:20 a.m. UTC | #4
> 
> On Wed, Oct 19, 2016 at 09:35:15PM +0000, Weiny, Ira wrote:
> 
> > set(rdma_ndd_CONFIG_PATH "${CMAKE_INSTALL_SYSCONFDIR}")
> configure_file
> > ("${PROJECT_SOURCE_DIR}/rdma-ndd/rdma-config.h.in"
> >         "${PROJECT_BINARY_DIR}/rdma-ndd/rdma-config.h")
> > include_directories("${PROJECT_BINARY_DIR}/rdma-ndd")
> 
> Please just use the existing buildlib/config.h.in and put in:
> 
> #define RDMA_NDD_CONFIG_FILE
> "@CMAKE_INSTALL_FULL_SYSCONFDIR@/rdma-ndd.conf"
> 
> Nothing else is needed, please don't add more config.h's.

Fair enough, I missed that as it was buried in buildlib...

> 
> > #define RDMA_NDD_CONFIG_PATH "etc"
> 
> This is the difference between the 'FULL' and normal path versions in the cmake
> scheme. The FULL version must be used everyplace needing an absolute path,
> the other verions are used with install and have to do with making DESTDIR
> work properly.

Good to know...

I think the biggest issue I have with cmake is that I finally feel like I understand autotools and now I have to learn another system...  :-P

Ira

--
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
diff mbox

Patch

diff --git a/.travis.yml b/.travis.yml
index d81b699294eb..d9c36cc9c649 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -24,6 +24,7 @@  addons:
       - gcc-6
       - libnl-3-dev
       - libnl-route-3-dev
+      - libudev-dev
       - make
       - ninja-build
       - pkg-config
diff --git a/CMakeLists.txt b/CMakeLists.txt
index a23aa860e6d3..9402bacf70ce 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -237,6 +237,10 @@  if (NOT NL_KIND EQUAL 0)
   set(CMAKE_REQUIRED_INCLUDES "${SAFE_CMAKE_REQUIRED_INCLUDES}")
 endif()
 
+# udev
+find_package(UDev REQUIRED)
+include_directories(${UDEV_INCLUDE_DIRS})
+
 # Statically determine sizeof(long), this is largely unnecessary, no new code
 # should rely on this.
 check_type_size("long" SIZEOF_LONG BUILTIN_TYPES_ONLY LANGUAGE C)
diff --git a/README.md b/README.md
index 66aee3f49f00..d24fd0bf2606 100644
--- a/README.md
+++ b/README.md
@@ -48,7 +48,7 @@  only load from the system path.
 ### Debian Derived
 
 ```sh
-$ apt-get install build-essential cmake gcc libnl-3-dev libnl-route-3-dev ninja-build pkg-config valgrind
+$ apt-get install build-essential cmake gcc libudev-dev libnl-3-dev libnl-route-3-dev ninja-build pkg-config valgrind
 ```
 
 ### Fedora
diff --git a/buildlib/FindUDev.cmake b/buildlib/FindUDev.cmake
new file mode 100644
index 000000000000..ce05ddf991a1
--- /dev/null
+++ b/buildlib/FindUDev.cmake
@@ -0,0 +1,10 @@ 
+# COPYRIGHT (c) 2016 Obsidian Research Corporation. See COPYING file
+
+find_library(LIBUDEV_LIBRARY NAMES udev libudev)
+
+set(UDEV_LIBRARIES ${LIBUDEV_LIBRARY})
+
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(UDev REQUIRED_VARS LIBUDEV_LIBRARY)
+
+mark_as_advanced(LIBUDEV_LIBRARY)
diff --git a/debian/control b/debian/control
index 2335d1f4814d..ed9850a348be 100644
--- a/debian/control
+++ b/debian/control
@@ -8,6 +8,7 @@  Build-Depends: build-essential,
 	       dh-systemd,
 	       dpkg-dev (>= 1.17),
 	       gcc,
+	       libudev-dev,
 	       libnl-3-dev,
 	       libnl-route-3-dev,
 	       make,