| Message ID | 20171205234005.10904-1-tatyana.e.nikolova@intel.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
Le 06/12/2017 à 00:40, Tatyana Nikolova a écrit : > Without systemd-devel, all rdma-core daemons are using shim version of > systemd include file (in-turn, shim version of systemd functions) > causing them to not communicate with systemd. > > Add the missing required systemd-devel package to SUSE's rdma-core.spec > so the correct header file is used and correct functions are linked > with the daemons. > > Fixes: 8854eefa60b5 ("suse: Add spec file for SUSE") > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> > --- > suse/rdma-core.spec | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/suse/rdma-core.spec b/suse/rdma-core.spec > index 0eba0507..912a77c5 100644 > --- a/suse/rdma-core.spec > +++ b/suse/rdma-core.spec > @@ -57,6 +57,7 @@ BuildRequires: pkgconfig > BuildRequires: pkgconfig(libsystemd) > BuildRequires: pkgconfig(libudev) > BuildRequires: pkgconfig(systemd) > +BuildRequires: pkgconfig(systemd-devel) > BuildRequires: pkgconfig(udev) > BuildRequires: python3-base > %ifnarch s390 s390x You should not need that. This is what this line is for: BuildRequires: pkgconfig(systemd) We don't expressly name systemd-devel and use the pkgconfig macro to allow building using systemd-mini-devel when needed. Nicolas -- 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
On Wed, Dec 06, 2017 at 09:00:50AM +0100, Nicolas Morey-Chaisemartin wrote: > > > Le 06/12/2017 à 00:40, Tatyana Nikolova a écrit : > > Without systemd-devel, all rdma-core daemons are using shim version of > > systemd include file (in-turn, shim version of systemd functions) > > causing them to not communicate with systemd. > > > > Add the missing required systemd-devel package to SUSE's rdma-core.spec > > so the correct header file is used and correct functions are linked > > with the daemons. > > > > Fixes: 8854eefa60b5 ("suse: Add spec file for SUSE") > > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> > > suse/rdma-core.spec | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/suse/rdma-core.spec b/suse/rdma-core.spec > > index 0eba0507..912a77c5 100644 > > +++ b/suse/rdma-core.spec > > @@ -57,6 +57,7 @@ BuildRequires: pkgconfig > > BuildRequires: pkgconfig(libsystemd) > > BuildRequires: pkgconfig(libudev) > > BuildRequires: pkgconfig(systemd) > > +BuildRequires: pkgconfig(systemd-devel) > > BuildRequires: pkgconfig(udev) > > BuildRequires: python3-base > > %ifnarch s390 s390x > > You should not need that. > This is what this line is for: > > BuildRequires: pkgconfig(systemd) I actually think pkgconfig(systemd) is a mistake in the spec file.. This brings in the libraries: BuildRequires: pkgconfig(libsystemd) This is does something else and isn't needed, AFAIK: BuildRequires: pkgconfig(systemd) Same with udev: BuildRequires: pkgconfig(libudev) vs BuildRequires: pkgconfig(udev) But you explained in the past suse needed actual udev installed for some reason I can't remember.. > We don't expressly name systemd-devel and use the pkgconfig macro to > allow building using systemd-mini-devel when needed. I suspect Tatyana has an old version of suse where 'pkgconfig(libsystemd)' doesn't do the right thing. I suspect the issue is that old versions of suse probably have the old arrangement of systemd libraries that were split into three libraries (libsystemd-journal, libsystemd-id128, and libsystemd-daemon) so pkgconfig(libsystemd) isn't going to work (and rpmbuild should fail fail, why doesn't it??) Probably something like BuildRequires: pkgconfig(libsystemd) | (pkgconfig(libsystemd-journal), pkgconfig(libsystemd-id128), pkgconfig(libsystemd-daemon)) If RPM can do that, otherwise a suse version conditional? 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
Le 06/12/2017 à 18:13, Jason Gunthorpe a écrit : > On Wed, Dec 06, 2017 at 09:00:50AM +0100, Nicolas Morey-Chaisemartin wrote: >> >> Le 06/12/2017 à 00:40, Tatyana Nikolova a écrit : >>> Without systemd-devel, all rdma-core daemons are using shim version of >>> systemd include file (in-turn, shim version of systemd functions) >>> causing them to not communicate with systemd. >>> >>> Add the missing required systemd-devel package to SUSE's rdma-core.spec >>> so the correct header file is used and correct functions are linked >>> with the daemons. >>> >>> Fixes: 8854eefa60b5 ("suse: Add spec file for SUSE") >>> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> >>> suse/rdma-core.spec | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/suse/rdma-core.spec b/suse/rdma-core.spec >>> index 0eba0507..912a77c5 100644 >>> +++ b/suse/rdma-core.spec >>> @@ -57,6 +57,7 @@ BuildRequires: pkgconfig >>> BuildRequires: pkgconfig(libsystemd) >>> BuildRequires: pkgconfig(libudev) >>> BuildRequires: pkgconfig(systemd) >>> +BuildRequires: pkgconfig(systemd-devel) >>> BuildRequires: pkgconfig(udev) >>> BuildRequires: python3-base >>> %ifnarch s390 s390x >> You should not need that. >> This is what this line is for: >> >> BuildRequires: pkgconfig(systemd) > I actually think pkgconfig(systemd) is a mistake in the spec file.. > > This brings in the libraries: > > BuildRequires: pkgconfig(libsystemd) > > This is does something else and isn't needed, AFAIK: > > BuildRequires: pkgconfig(systemd) This was requested by the main OpenSUSE maintainer so I blindly trusted him with this :) I'll still ask him if both are really needed (and if yes, what's the diff) > Same with udev: > BuildRequires: pkgconfig(libudev) > vs > BuildRequires: pkgconfig(udev) > > But you explained in the past suse needed actual udev installed for some > reason I can't remember.. Our build systems checks that scriptlets call existing binaries. Without udev, udevadm does not exists which gets some errors in the build logs. >> We don't expressly name systemd-devel and use the pkgconfig macro to >> allow building using systemd-mini-devel when needed. > I suspect Tatyana has an old version of suse where > 'pkgconfig(libsystemd)' doesn't do the right thing. > > I suspect the issue is that old versions of suse probably have the old > arrangement of systemd libraries that were split into three libraries > (libsystemd-journal, libsystemd-id128, and libsystemd-daemon) > so pkgconfig(libsystemd) isn't going to work (and rpmbuild should fail > fail, why doesn't it??) The current works with version down to SLE12. I doubt anyone would run rdma-core on older releases as it very ancient. Tatyana, could you give some more info on what prompted this patch ? > Probably something like > > BuildRequires: pkgconfig(libsystemd) | (pkgconfig(libsystemd-journal), pkgconfig(libsystemd-id128), pkgconfig(libsystemd-daemon)) > > If RPM can do that, otherwise a suse version conditional? Version conditionnal is better. But I'd like to know what the exact problem is before tweaking that. rdma-core is at the middle of huge build dependencies that involve systemd, boost and a bunch of other stuff. I have to be very careful with these dependencies not to mess everything up. Nicolas -- 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
On Wed, Dec 06, 2017 at 06:24:20PM +0100, Nicolas Morey-Chaisemartin wrote: > > This is does something else and isn't needed, AFAIK: > > > > BuildRequires: pkgconfig(systemd) > > This was requested by the main OpenSUSE maintainer so I blindly > trusted him with this :) I'll still ask him if both are really > needed (and if yes, what's the diff) I would anticipate this is the reason: Our build systems checks that scriptlets call existing binaries. Without udev, udevadm does not exists which gets some errors in the build logs. As we also call systemd binaries in various places. > Tatyana, could you give some more info on what prompted this patch ? +1 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
> -----Original Message----- > From: Jason Gunthorpe [mailto:jgg@mellanox.com] > Sent: Wednesday, December 06, 2017 11:28 AM > To: Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> > Cc: Nikolova, Tatyana E <tatyana.e.nikolova@intel.com>; > dledford@redhat.com; leonro@mellanox.com; linux-rdma@vger.kernel.org; > e1000-rdma@lists.sourceforge.net > Subject: Re: [PATCH] rdma-core: Add missing systemd-devel dependency to > the SUSE rdma-core.spec > > On Wed, Dec 06, 2017 at 06:24:20PM +0100, Nicolas Morey-Chaisemartin > wrote: > > > > This is does something else and isn't needed, AFAIK: > > > > > > BuildRequires: pkgconfig(systemd) > > > > This was requested by the main OpenSUSE maintainer so I blindly > > trusted him with this :) I'll still ask him if both are really needed > > (and if yes, what's the diff) > > I would anticipate this is the reason: > > Our build systems checks that scriptlets call existing binaries. > Without udev, udevadm does not exists which gets some errors in the > build logs. > > As we also call systemd binaries in various places. > > > Tatyana, could you give some more info on what prompted this patch ? > > +1 > The patch was prompted when the rdma-core daemons were unable to start on a SLES12SP2 system, because the shim version of the systemd functions were used instead. Unfortunately I don't have the details on how rdma-core was installed on this system. Checking the current SUSE spec file on the same system (without systemd-devel installed), it indeed detects a missing dependency: "error: Failed build dependencies: pkgconfig(libsystemd) is needed by rdma-core-15-0.x86_64" However libsystemd packages (libsystemd0-228-117.12.x86_64 and libsystemd0-32bit-228-117.12.x86_64) are installed on this system, but systemd-devel is necessary to install the required headers, for example "sd-daemon.h" needed by the daemons. Maybe it would be better to replace pkgconfig(libsystemd) with pkgconfig(systemd-devel) in the spec file, since systemd-devel requires libsystemd? Thank you, Tatyana > 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
On Wed, Dec 06, 2017 at 10:06:57PM +0000, Nikolova, Tatyana E wrote: > The patch was prompted when the rdma-core daemons were unable to > start on a SLES12SP2 system, because the shim version of the systemd > functions were used instead. Unfortunately I don't have the details > on how rdma-core was installed on this system. Well, are you even sure it was built with rpmbuild? > Checking the current SUSE spec file on the same system (without > systemd-devel installed), it indeed detects a missing dependency: > "error: Failed build dependencies: pkgconfig(libsystemd) is needed > by rdma-core-15-0.x86_64" Because rpmbuild shouldn't have succeeded, right? > However libsystemd packages (libsystemd0-228-117.12.x86_64 and > libsystemd0-32bit-228-117.12.x86_64) are installed on this system, > but systemd-devel is necessary to install the required headers, for > example "sd-daemon.h" needed by the daemons. Yes, that is right. But pkgconfig(XXX) is a magic notation to 'bring in -devel packages for XXX' So pkgconfig(libsystemd) is supposed to bring in systemd-devel, for instance. You'd need to reproduce a failing rpmbuild where the deps are met and the shims are used to understand this issue.. 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
Le 06/12/2017 à 18:13, Jason Gunthorpe a écrit : > On Wed, Dec 06, 2017 at 09:00:50AM +0100, Nicolas Morey-Chaisemartin wrote: >> >> Le 06/12/2017 à 00:40, Tatyana Nikolova a écrit : >>> Without systemd-devel, all rdma-core daemons are using shim version of >>> systemd include file (in-turn, shim version of systemd functions) >>> causing them to not communicate with systemd. >>> >>> Add the missing required systemd-devel package to SUSE's rdma-core.spec >>> so the correct header file is used and correct functions are linked >>> with the daemons. >>> >>> Fixes: 8854eefa60b5 ("suse: Add spec file for SUSE") >>> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> >>> suse/rdma-core.spec | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/suse/rdma-core.spec b/suse/rdma-core.spec >>> index 0eba0507..912a77c5 100644 >>> +++ b/suse/rdma-core.spec >>> @@ -57,6 +57,7 @@ BuildRequires: pkgconfig >>> BuildRequires: pkgconfig(libsystemd) >>> BuildRequires: pkgconfig(libudev) >>> BuildRequires: pkgconfig(systemd) >>> +BuildRequires: pkgconfig(systemd-devel) >>> BuildRequires: pkgconfig(udev) >>> BuildRequires: python3-base >>> %ifnarch s390 s390x >> You should not need that. >> This is what this line is for: >> >> BuildRequires: pkgconfig(systemd) > I actually think pkgconfig(systemd) is a mistake in the spec file.. > > This brings in the libraries: > > BuildRequires: pkgconfig(libsystemd) > > This is does something else and isn't needed, AFAIK: > > BuildRequires: pkgconfig(systemd) I just checked: This pulls in systemd (or systemd-mini) And pkgconfig(udev) pulls udev or udev-mini. So they are not compulsory build requirements (as we require the -devel they should be installed by dependency) but they don't hurt. > > Probably something like > > BuildRequires: pkgconfig(libsystemd) | (pkgconfig(libsystemd-journal), pkgconfig(libsystemd-id128), pkgconfig(libsystemd-daemon)) On a recent systemd, systemd-devel provides all those (probably for compat reason during migration): $ rpm -q --provides systemd-devel pkgconfig(libsystemd) = 228 pkgconfig(libsystemd-daemon) = 228 pkgconfig(libsystemd-id128) = 228 pkgconfig(libsystemd-journal) = 228 pkgconfig(libsystemd-login) = 228 So I can porobably safely changes the BuildRequires to the libsystemd-XXXX. If I'm not mistaken, we just need the -daemon and -journal ones. -- 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
On Thu, Dec 07, 2017 at 10:57:08AM +0100, Nicolas Morey-Chaisemartin wrote: > On a recent systemd, systemd-devel provides all those (probably for compat reason during migration): > $ rpm -q --provides systemd-devel > pkgconfig(libsystemd) = 228 > pkgconfig(libsystemd-daemon) = 228 > pkgconfig(libsystemd-id128) = 228 > pkgconfig(libsystemd-journal) = 228 > pkgconfig(libsystemd-login) = 228 > > > So I can porobably safely changes the BuildRequires to the libsystemd-XXXX. > If I'm not mistaken, we just need the -daemon and -journal ones. cmake checks for all id128 as well when it runs in compat mode, so you need all three to make it happy. I wouldn't change the main spec file, maybe protect the 2nd option with a ifdef for old suse versions? 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
Le 07/12/2017 à 15:59, Jason Gunthorpe a écrit : > On Thu, Dec 07, 2017 at 10:57:08AM +0100, Nicolas Morey-Chaisemartin wrote: > >> On a recent systemd, systemd-devel provides all those (probably for compat reason during migration): >> $ rpm -q --provides systemd-devel >> pkgconfig(libsystemd) = 228 >> pkgconfig(libsystemd-daemon) = 228 >> pkgconfig(libsystemd-id128) = 228 >> pkgconfig(libsystemd-journal) = 228 >> pkgconfig(libsystemd-login) = 228 >> >> >> So I can porobably safely changes the BuildRequires to the libsystemd-XXXX. >> If I'm not mistaken, we just need the -daemon and -journal ones. > cmake checks for all id128 as well when it runs in compat mode, so you > need all three to make it happy. > > I wouldn't change the main spec file, maybe protect the 2nd option > with a ifdef for old suse versions? I don't really mind either way. However is there any chance that it will work ? I'm pretty sure there are some (if not many) ABI changes from ibverbs 1.2.0 to rdma-core v16. This means that all dependent packages needs to be rebuilt, and some probably updated too. From my SUSE package maintainer perspective, that is something that should not be done and SUSE will not provide any official support for that. Regarding upstream we can't really forbid people to do this, just try to point them in a "sane" direction :) Nicolas -- 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
On Thu, Dec 07, 2017 at 04:22:00PM +0100, Nicolas Morey-Chaisemartin wrote: > I'm pretty sure there are some (if not many) ABI changes from > ibverbs 1.2.0 to rdma-core v16. This means that all dependent > packages needs to be rebuilt, and some probably updated too. I expect full compatability. That is our design goal. 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
diff --git a/suse/rdma-core.spec b/suse/rdma-core.spec index 0eba0507..912a77c5 100644 --- a/suse/rdma-core.spec +++ b/suse/rdma-core.spec @@ -57,6 +57,7 @@ BuildRequires: pkgconfig BuildRequires: pkgconfig(libsystemd) BuildRequires: pkgconfig(libudev) BuildRequires: pkgconfig(systemd) +BuildRequires: pkgconfig(systemd-devel) BuildRequires: pkgconfig(udev) BuildRequires: python3-base %ifnarch s390 s390x
Without systemd-devel, all rdma-core daemons are using shim version of systemd include file (in-turn, shim version of systemd functions) causing them to not communicate with systemd. Add the missing required systemd-devel package to SUSE's rdma-core.spec so the correct header file is used and correct functions are linked with the daemons. Fixes: 8854eefa60b5 ("suse: Add spec file for SUSE") Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> --- suse/rdma-core.spec | 1 + 1 file changed, 1 insertion(+)