On Thu, Nov 08, 2018 at 09:57:32AM +0100, Nicolas Morey-Chaisemartin wrote:
> Hi,
>
> One of the openSUSE reviewer as left some feedback on github as he is much more knowledgable in python packaging than I am.
> Here's the summary.
Thanks, pushed updated version.
It doesn't include bcond change.
>
> On 11/7/18 6:31 PM, Leon Romanovsky wrote:
> > From: Noa Osherovich <noaos@xxxxxxxxxxxx>
> >
> > Update spec file and cbuild dependencies to allow pyverbs to be
> > included in SUSE distributions.
> >
> > Signed-off-by: Noa Osherovich <noaos@xxxxxxxxxxxx>
> > Signed-off-by: Alaa Hleihel <alaa@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > ---
> > buildlib/cbuild | 2 ++
> > suse/rdma-core.spec | 26 ++++++++++++++++++++++++--
> > 2 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/buildlib/cbuild b/buildlib/cbuild
> > index 098bea23..33c03781 100755
> > --- a/buildlib/cbuild
> > +++ b/buildlib/cbuild
> > @@ -338,6 +338,8 @@ class leap(ZypperEnvironment):
> > 'rpm-build',
> > 'systemd-devel',
> > 'valgrind-devel',
> > + 'python3-Cython',
> > + 'python3-devel',
> > };
> > name = "opensuse-15.0";
> > aliases = {"leap"};
> > diff --git a/suse/rdma-core.spec b/suse/rdma-core.spec
> > index 3eafce39..5abb92b0 100644
> > --- a/suse/rdma-core.spec
> > +++ b/suse/rdma-core.spec
> > @@ -60,7 +60,13 @@ BuildRequires: pkgconfig(libsystemd)
> > BuildRequires: pkgconfig(libudev)
> > BuildRequires: pkgconfig(systemd)
> > BuildRequires: pkgconfig(udev)
> > +%define with_pyverbs %{?_with_pyverbs: 1} %{?!_with_pyverbs: 0}
>
> bcond would be nicer but we didn't do that for _with_static as RHEL/Centos6 seems to have some issue with it. SO we can keep it this way.
> It may be nicer to put them right under the with_static define so we can see all flags in one place
>
> > +%if %{with_pyverbs}
> > +BuildRequires: python3-devel
> > +BuildRequires: python3-Cython
> > +%else
> > BuildRequires: python3-base
> > +%endif
>
> This makes python3-base conditional. It breaks nothing as it's pulled by python3-devel but pulling it out of the %else
> will make it obvious that it is needed in both cases.
>
> > %ifnarch s390 s390x
> > BuildRequires: valgrind-devel
> > %endif
> > @@ -153,7 +159,7 @@ Obsoletes: ibacm-devel < %{version}-%{release}
> > BuildRequires: pkgconfig(libnl-3.0)
> > BuildRequires: pkgconfig(libnl-route-3.0)
> > %endif
> > -
> > +
> > %description devel
> > RDMA core development libraries and headers.
> >
> > @@ -325,6 +331,13 @@ rdma-ndd is a system daemon which watches for rdma device changes and/or
> > hostname changes and updates the Node Description of the rdma devices based
> > on those changes.
> >
> > +%package -n python3-pyverbs
> > +Summary: Python3 API over IB verbs
> This needs a Group: tag.
> Group: Development/Languages/Python
>
> The rest looks fine.
>
> Nicolas
>
Attachment:
signature.asc
Description: PGP signature
