diff mbox

[ovs-dev] rhel: Support building python ovs package with native json impl

Message ID f9a25255-97a2-ae97-0320-4835709641a5@redhat.com
State Changes Requested
Delegated to: Russell Bryant
Headers show

Commit Message

Numan Siddique Nov. 28, 2016, 9:37 a.m. UTC
Since building python-openvswitch _json.so requires libopenvswitch.so
a separate spec file is added which configures Open vSwitch with
--enable-shared flag. The resulting RPM also includes the Open vSwitch
shared library.

$ rpm -qlp python-openvswitch-2.6.90-1.fc25.x86_64.rpm
/usr/lib64/libopenvswitch.so
/usr/lib64/libopenvswitch.so.1
/usr/lib64/libopenvswitch.so.1.0.0
/usr/lib64/python2.7/site-packages/ovs
/usr/lib64/python2.7/site-packages/ovs-2.6.90-py2.7.egg-info
...
/usr/lib64/python2.7/site-packages/ovs/_json.so
...

CC: Terry Wilson <twilson@redhat.com>
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 INSTALL.Fedora.rst                     |  13 +++++
 python/setup.py                        |   9 ++-
 rhel/automake.mk                       |   9 +++
 rhel/python-openvswitch-fedora.spec.in | 103 +++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 rhel/python-openvswitch-fedora.spec.in

Comments

Ben Pfaff Dec. 12, 2016, 10:19 p.m. UTC | #1
Russell and Terry, I think that probably one of you should review this.

Thanks,

Ben.

On Mon, Nov 28, 2016 at 03:07:05PM +0530, Numan Siddique wrote:
> Since building python-openvswitch _json.so requires libopenvswitch.so
> a separate spec file is added which configures Open vSwitch with
> --enable-shared flag. The resulting RPM also includes the Open vSwitch
> shared library.
> 
> $ rpm -qlp python-openvswitch-2.6.90-1.fc25.x86_64.rpm
> /usr/lib64/libopenvswitch.so
> /usr/lib64/libopenvswitch.so.1
> /usr/lib64/libopenvswitch.so.1.0.0
> /usr/lib64/python2.7/site-packages/ovs
> /usr/lib64/python2.7/site-packages/ovs-2.6.90-py2.7.egg-info
> ...
> /usr/lib64/python2.7/site-packages/ovs/_json.so
> ...
> 
> CC: 
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---
>  INSTALL.Fedora.rst                     |  13 +++++
>  python/setup.py                        |   9 ++-
>  rhel/automake.mk                       |   9 +++
>  rhel/python-openvswitch-fedora.spec.in | 103 +++++++++++++++++++++++++++++++++
>  4 files changed, 133 insertions(+), 1 deletion(-)
>  create mode 100644 rhel/python-openvswitch-fedora.spec.in
> 
> diff --git a/INSTALL.Fedora.rst b/INSTALL.Fedora.rst
> index b9be0ed..40eacfc 100644
> --- a/INSTALL.Fedora.rst
> +++ b/INSTALL.Fedora.rst
> @@ -83,6 +83,8 @@ This will create the RPMs `openvswitch`, `python-openvswitch`,
>  `openvswitch-ovn-central`, `openvswitch-ovn-host`, `openvswitch-ovn-vtep`,
>  `openvswitch-ovn-docker`, and `openvswitch-debuginfo`.
>  
> +`python-openvswitch` RPM doesn't include the native json library.
> +
>  To enable DPDK support in the openvswitch package, the ``--with dpdk`` option
>  can be added:
>  
> @@ -98,6 +100,17 @@ these tests, the ``--without check`` option can be added:
>  
>      $ make rpm-fedora RPMBUILD_OPT="--without check"
>  
> +Open vSwitch python binding RPM with native json library
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +To build the `python-openvswitch` RPM with native json library, run:
> +
> +
> +::
> +    $ make rpm-fedora-python-ovs
> +
> +This also builds the Open vSwitch shared library and includes it in the RPM.
> +
>  Kernel OVS Tree Datapath RPM
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/python/setup.py b/python/setup.py
> index 19c1f18..5070c9b 100644
> --- a/python/setup.py
> +++ b/python/setup.py
> @@ -12,6 +12,7 @@
>  
>  from __future__ import print_function
>  import sys
> +import os
>  
>  from distutils.command.build_ext import build_ext
>  from distutils.errors import CCompilerError, DistutilsExecError, \
> @@ -33,6 +34,10 @@ ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
>  if sys.platform == 'win32':
>      ext_errors += (IOError, ValueError)
>  
> +# Get the include path if defined
> +include_dirs = os.environ.get('OVS_INCLUDE_DIR', '.')
> +library_dirs = os.environ.get('OVS_LIB_DIR', '.')
> +
>  
>  class BuildFailed(Exception):
>      pass
> @@ -77,7 +82,9 @@ setup_args = dict(
>          'Programming Language :: Python :: 3.4',
>      ],
>      ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
> -                                      libraries=['openvswitch'])],
> +                                      libraries=['openvswitch'],
> +                                      include_dirs=[include_dirs],
> +                                      library_dirs=[library_dirs])],
>      cmdclass={'build_ext': try_build_ext},
>  )
>  
> diff --git a/rhel/automake.mk b/rhel/automake.mk
> index 45aa9b1..1113fd8 100644
> --- a/rhel/automake.mk
> +++ b/rhel/automake.mk
> @@ -23,6 +23,7 @@ EXTRA_DIST += \
>  	rhel/openvswitch.spec.in \
>  	rhel/openvswitch-fedora.spec \
>  	rhel/openvswitch-fedora.spec.in \
> +	rhel/python-openvswitch-fedora.spec.in \
>  	rhel/usr_share_openvswitch_scripts_sysconfig.template \
>  	rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>  	rhel/usr_lib_systemd_system_openvswitch.service \
> @@ -62,6 +63,14 @@ rpm-fedora: dist $(srcdir)/rhel/openvswitch-fedora.spec
>                   -D "_topdir ${RPMBUILD_TOP}" \
>                   -ba $(srcdir)/rhel/openvswitch-fedora.spec
>  
> +# Build Python binding RPM with native json
> +rpm-fedora-python-ovs: dist $(srcdir)/rhel/python-openvswitch-fedora.spec
> +	${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
> +	cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES
> +	rpmbuild ${RPMBUILD_OPT} \
> +                 -D "_topdir ${RPMBUILD_TOP}" \
> +                 -ba $(srcdir)/rhel/python-openvswitch-fedora.spec
> +
>  # Build kernel datapath RPM
>  rpm-fedora-kmod: dist $(srcdir)/rhel/openvswitch-kmod-fedora.spec
>  	${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
> diff --git a/rhel/python-openvswitch-fedora.spec.in b/rhel/python-openvswitch-fedora.spec.in
> new file mode 100644
> index 0000000..cd863d8
> --- /dev/null
> +++ b/rhel/python-openvswitch-fedora.spec.in
> @@ -0,0 +1,103 @@
> +# Spec file for Python Open vSwitch with native json library
> +
> +# Copyright (C) 2016 Red Hat, Inc.
> +#
> +# Copying and distribution of this file, with or without modification,
> +# are permitted in any medium without royalty provided the copyright
> +# notice and this notice are preserved.  This file is offered as-is,
> +# without warranty of any kind.
> +#
> +
> +# If libcap-ng isn't available and there is no need for running OVS
> +# as regular user, specify the '--without libcapng'
> +%bcond_without libcapng
> +
> +# Enable PIE, bz#955181
> +%global _hardened_build 1
> +
> +# some distros (e.g: RHEL-7) don't define _rundir macro yet
> +# Fedora 15 onwards uses /run as _rundir
> +%if 0%{!?_rundir:1}
> +%define _rundir /run
> +%endif
> +
> +%global pkgname openvswitch
> +%global srcname openvswitch
> +
> +Name: python-%{pkgname}
> +Summary: Open vSwitch python bindings
> +Group: System Environment/Daemons
> +URL: http://www.openvswitch.org/
> +Version: @VERSION@
> +
> +# Nearly all of openvswitch is ASL 2.0.  The bugtool is LGPLv2+, and the
> +# lib/sflow*.[ch] files are SISSL
> +License: ASL 2.0 and LGPLv2+ and SISSL
> +Release: 1%{?dist}
> +Source: http://openvswitch.org/releases/%{pkgname}-%{version}.tar.gz
> +
> +BuildRequires: autoconf automake libtool
> +BuildRequires: systemd-units openssl openssl-devel
> +BuildRequires: python python-twisted-core python-zope-interface python-six
> +BuildRequires: desktop-file-utils
> +BuildRequires: groff graphviz
> +BuildRequires: checkpolicy, selinux-policy-devel
> +# make check dependencies
> +BuildRequires: procps-ng
> +%if %{with libcapng}
> +BuildRequires: libcap-ng libcap-ng-devel
> +%endif
> +
> +Requires: openssl iproute module-init-tools
> +Requires: python
> +Requires: python-six
> +
> +%description
> +Python bindings for the Open vSwitch database
> +
> +%prep
> +rm -rf $RPM_BUILD_DIR/%{pkgname}-%{version}
> +cp $RPM_SOURCE_DIR/%{pkgname}-%{version}.tar.gz .
> +tar xzvf %{pkgname}-%{version}.tar.gz
> +
> +%build
> +cd %{pkgname}-%{version}
> +%configure \
> +%if %{with libcapng}
> +	--enable-libcapng \
> +%else
> +	--disable-libcapng \
> +%endif
> +%if %{with dpdk}
> +	--with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
> +%endif
> +	--enable-ssl \
> +	--with-pkidir=%{_sharedstatedir}/openvswitch/pki \
> +        --enable-shared
> +
> +make %{?_smp_mflags}
> +
> +export OVS_INCLUDE_DIR=$PWD/include/
> +export OVS_LIB_DIR=$PWD/lib/.libs/
> +cd python
> +%{__python2} setup.py build
> +
> +%install
> +rm -rf $RPM_BUILD_ROOT
> +
> +cd %{pkgname}-%{version}/python
> +%{__python2} setup.py install -O1 --skip-build --root %{buildroot}
> +cd ..
> +cp -d lib/.libs/libopenvswitch.so* %{buildroot}/%{_libdir}/
> +
> +%clean
> +rm -rf $RPM_BUILD_ROOT
> +
> +%files
> +%{python2_sitearch}/ovs
> +%{python2_sitearch}/ovs-%{version}-py?.?.egg-info
> +%{_libdir}/*.so*
> +
> +%changelog
> +* Fri Nov 25 2016 Numan Siddique <nusiddiq@redhat.com>
> +- First build on F25
> -- 
> 2.9.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Russell Bryant Dec. 13, 2016, 4:22 p.m. UTC | #2
My main concern with this was that it implements a new python-ovs package
and leaves the existing one alone.  I'd like to figure out how to change
the existing package so that we always use this approach.

Added Flavio to CC.

On Mon, Dec 12, 2016 at 5:19 PM, Ben Pfaff <blp@ovn.org> wrote:

> Russell and Terry, I think that probably one of you should review this.
>
> Thanks,
>
> Ben.
>
> On Mon, Nov 28, 2016 at 03:07:05PM +0530, Numan Siddique wrote:
> > Since building python-openvswitch _json.so requires libopenvswitch.so
> > a separate spec file is added which configures Open vSwitch with
> > --enable-shared flag. The resulting RPM also includes the Open vSwitch
> > shared library.
> >
> > $ rpm -qlp python-openvswitch-2.6.90-1.fc25.x86_64.rpm
> > /usr/lib64/libopenvswitch.so
> > /usr/lib64/libopenvswitch.so.1
> > /usr/lib64/libopenvswitch.so.1.0.0
> > /usr/lib64/python2.7/site-packages/ovs
> > /usr/lib64/python2.7/site-packages/ovs-2.6.90-py2.7.egg-info
> > ...
> > /usr/lib64/python2.7/site-packages/ovs/_json.so
> > ...
> >
> > CC:
> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> > ---
> >  INSTALL.Fedora.rst                     |  13 +++++
> >  python/setup.py                        |   9 ++-
> >  rhel/automake.mk                       |   9 +++
> >  rhel/python-openvswitch-fedora.spec.in | 103
> +++++++++++++++++++++++++++++++++
> >  4 files changed, 133 insertions(+), 1 deletion(-)
> >  create mode 100644 rhel/python-openvswitch-fedora.spec.in
> >
> > diff --git a/INSTALL.Fedora.rst b/INSTALL.Fedora.rst
> > index b9be0ed..40eacfc 100644
> > --- a/INSTALL.Fedora.rst
> > +++ b/INSTALL.Fedora.rst
> > @@ -83,6 +83,8 @@ This will create the RPMs `openvswitch`,
> `python-openvswitch`,
> >  `openvswitch-ovn-central`, `openvswitch-ovn-host`,
> `openvswitch-ovn-vtep`,
> >  `openvswitch-ovn-docker`, and `openvswitch-debuginfo`.
> >
> > +`python-openvswitch` RPM doesn't include the native json library.
> > +
> >  To enable DPDK support in the openvswitch package, the ``--with dpdk``
> option
> >  can be added:
> >
> > @@ -98,6 +100,17 @@ these tests, the ``--without check`` option can be
> added:
> >
> >      $ make rpm-fedora RPMBUILD_OPT="--without check"
> >
> > +Open vSwitch python binding RPM with native json library
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +To build the `python-openvswitch` RPM with native json library, run:
> > +
> > +
> > +::
> > +    $ make rpm-fedora-python-ovs
> > +
> > +This also builds the Open vSwitch shared library and includes it in the
> RPM.
> > +
> >  Kernel OVS Tree Datapath RPM
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > diff --git a/python/setup.py b/python/setup.py
> > index 19c1f18..5070c9b 100644
> > --- a/python/setup.py
> > +++ b/python/setup.py
> > @@ -12,6 +12,7 @@
> >
> >  from __future__ import print_function
> >  import sys
> > +import os
> >
> >  from distutils.command.build_ext import build_ext
> >  from distutils.errors import CCompilerError, DistutilsExecError, \
> > @@ -33,6 +34,10 @@ ext_errors = (CCompilerError, DistutilsExecError,
> DistutilsPlatformError)
> >  if sys.platform == 'win32':
> >      ext_errors += (IOError, ValueError)
> >
> > +# Get the include path if defined
> > +include_dirs = os.environ.get('OVS_INCLUDE_DIR', '.')
> > +library_dirs = os.environ.get('OVS_LIB_DIR', '.')
> > +
> >
> >  class BuildFailed(Exception):
> >      pass
> > @@ -77,7 +82,9 @@ setup_args = dict(
> >          'Programming Language :: Python :: 3.4',
> >      ],
> >      ext_modules=[setuptools.Extension("ovs._json",
> sources=["ovs/_json.c"],
> > -                                      libraries=['openvswitch'])],
> > +                                      libraries=['openvswitch'],
> > +                                      include_dirs=[include_dirs],
> > +                                      library_dirs=[library_dirs])],
> >      cmdclass={'build_ext': try_build_ext},
> >  )
> >
> > diff --git a/rhel/automake.mk b/rhel/automake.mk
> > index 45aa9b1..1113fd8 100644
> > --- a/rhel/automake.mk
> > +++ b/rhel/automake.mk
> > @@ -23,6 +23,7 @@ EXTRA_DIST += \
> >       rhel/openvswitch.spec.in \
> >       rhel/openvswitch-fedora.spec \
> >       rhel/openvswitch-fedora.spec.in \
> > +     rhel/python-openvswitch-fedora.spec.in \
> >       rhel/usr_share_openvswitch_scripts_sysconfig.template \
> >       rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
> >       rhel/usr_lib_systemd_system_openvswitch.service \
> > @@ -62,6 +63,14 @@ rpm-fedora: dist $(srcdir)/rhel/openvswitch-
> fedora.spec
> >                   -D "_topdir ${RPMBUILD_TOP}" \
> >                   -ba $(srcdir)/rhel/openvswitch-fedora.spec
> >
> > +# Build Python binding RPM with native json
> > +rpm-fedora-python-ovs: dist $(srcdir)/rhel/python-
> openvswitch-fedora.spec
> > +     ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
> > +     cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES
> > +     rpmbuild ${RPMBUILD_OPT} \
> > +                 -D "_topdir ${RPMBUILD_TOP}" \
> > +                 -ba $(srcdir)/rhel/python-openvswitch-fedora.spec
> > +
> >  # Build kernel datapath RPM
> >  rpm-fedora-kmod: dist $(srcdir)/rhel/openvswitch-kmod-fedora.spec
> >       ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
> > diff --git a/rhel/python-openvswitch-fedora.spec.in b/rhel/
> python-openvswitch-fedora.spec.in
> > new file mode 100644
> > index 0000000..cd863d8
> > --- /dev/null
> > +++ b/rhel/python-openvswitch-fedora.spec.in
> > @@ -0,0 +1,103 @@
> > +# Spec file for Python Open vSwitch with native json library
> > +
> > +# Copyright (C) 2016 Red Hat, Inc.
> > +#
> > +# Copying and distribution of this file, with or without modification,
> > +# are permitted in any medium without royalty provided the copyright
> > +# notice and this notice are preserved.  This file is offered as-is,
> > +# without warranty of any kind.
> > +#
> > +
> > +# If libcap-ng isn't available and there is no need for running OVS
> > +# as regular user, specify the '--without libcapng'
> > +%bcond_without libcapng
> > +
> > +# Enable PIE, bz#955181
> > +%global _hardened_build 1
> > +
> > +# some distros (e.g: RHEL-7) don't define _rundir macro yet
> > +# Fedora 15 onwards uses /run as _rundir
> > +%if 0%{!?_rundir:1}
> > +%define _rundir /run
> > +%endif
> > +
> > +%global pkgname openvswitch
> > +%global srcname openvswitch
> > +
> > +Name: python-%{pkgname}
> > +Summary: Open vSwitch python bindings
> > +Group: System Environment/Daemons
> > +URL: http://www.openvswitch.org/
> > +Version: @VERSION@
> > +
> > +# Nearly all of openvswitch is ASL 2.0.  The bugtool is LGPLv2+, and the
> > +# lib/sflow*.[ch] files are SISSL
> > +License: ASL 2.0 and LGPLv2+ and SISSL
> > +Release: 1%{?dist}
> > +Source: http://openvswitch.org/releases/%{pkgname}-%{version}.tar.gz
> > +
> > +BuildRequires: autoconf automake libtool
> > +BuildRequires: systemd-units openssl openssl-devel
> > +BuildRequires: python python-twisted-core python-zope-interface
> python-six
> > +BuildRequires: desktop-file-utils
> > +BuildRequires: groff graphviz
> > +BuildRequires: checkpolicy, selinux-policy-devel
> > +# make check dependencies
> > +BuildRequires: procps-ng
> > +%if %{with libcapng}
> > +BuildRequires: libcap-ng libcap-ng-devel
> > +%endif
> > +
> > +Requires: openssl iproute module-init-tools
> > +Requires: python
> > +Requires: python-six
> > +
> > +%description
> > +Python bindings for the Open vSwitch database
> > +
> > +%prep
> > +rm -rf $RPM_BUILD_DIR/%{pkgname}-%{version}
> > +cp $RPM_SOURCE_DIR/%{pkgname}-%{version}.tar.gz .
> > +tar xzvf %{pkgname}-%{version}.tar.gz
> > +
> > +%build
> > +cd %{pkgname}-%{version}
> > +%configure \
> > +%if %{with libcapng}
> > +     --enable-libcapng \
> > +%else
> > +     --disable-libcapng \
> > +%endif
> > +%if %{with dpdk}
> > +     --with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
> > +%endif
> > +     --enable-ssl \
> > +     --with-pkidir=%{_sharedstatedir}/openvswitch/pki \
> > +        --enable-shared
> > +
> > +make %{?_smp_mflags}
> > +
> > +export OVS_INCLUDE_DIR=$PWD/include/
> > +export OVS_LIB_DIR=$PWD/lib/.libs/
> > +cd python
> > +%{__python2} setup.py build
> > +
> > +%install
> > +rm -rf $RPM_BUILD_ROOT
> > +
> > +cd %{pkgname}-%{version}/python
> > +%{__python2} setup.py install -O1 --skip-build --root %{buildroot}
> > +cd ..
> > +cp -d lib/.libs/libopenvswitch.so* %{buildroot}/%{_libdir}/
> > +
> > +%clean
> > +rm -rf $RPM_BUILD_ROOT
> > +
> > +%files
> > +%{python2_sitearch}/ovs
> > +%{python2_sitearch}/ovs-%{version}-py?.?.egg-info
> > +%{_libdir}/*.so*
> > +
> > +%changelog
> > +* Fri Nov 25 2016 Numan Siddique <nusiddiq@redhat.com>
> > +- First build on F25
> > --
> > 2.9.3
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Aaron Conole Dec. 13, 2016, 6:45 p.m. UTC | #3
Although I'm not Flavio, I do have some quick thoughts on this.

Russell Bryant <russell@ovn.org> writes:

> My main concern with this was that it implements a new python-ovs package
> and leaves the existing one alone.  I'd like to figure out how to change
> the existing package so that we always use this approach.

+1.  I don't think this should be split out the way it is here.

Additionally, I'm concerned about a possible future issue with conflicting
files (the openvswitch library file, itself), and ABI versioning.

Rather than build and ship the openvswitch library, would it be possible
to just try and detect it in the python code, and fallback if it doesn't
exist/load?  That would give us a way to handle mismatched ABIs in the
.so, AND prevent possible conflicts with a package that contains the
libopenvswitch.so (which I would assume would just be the openvswitch
package).

Also, FYI (and this is sortof a side-note) the libtool ABI versioning
information hasn't changed since 2014;  bumping / managing that may
become an additional release burden if we start building on top of it,
and expect to ship it around.  Imagine an install of this python package
from ovs 2.7.0 and a future openvswitch package 3.0.0; they currently
both see libtool version tag of 1:0:0

Thanks,
-Aaron

> Added Flavio to CC.
>
> On Mon, Dec 12, 2016 at 5:19 PM, Ben Pfaff <blp@ovn.org> wrote:
>
>> Russell and Terry, I think that probably one of you should review this.
>>
>> Thanks,
>>
>> Ben.
>>
>> On Mon, Nov 28, 2016 at 03:07:05PM +0530, Numan Siddique wrote:
>> > Since building python-openvswitch _json.so requires libopenvswitch.so
>> > a separate spec file is added which configures Open vSwitch with
>> > --enable-shared flag. The resulting RPM also includes the Open vSwitch
>> > shared library.
>> >
>> > $ rpm -qlp python-openvswitch-2.6.90-1.fc25.x86_64.rpm
>> > /usr/lib64/libopenvswitch.so
>> > /usr/lib64/libopenvswitch.so.1
>> > /usr/lib64/libopenvswitch.so.1.0.0
>> > /usr/lib64/python2.7/site-packages/ovs
>> > /usr/lib64/python2.7/site-packages/ovs-2.6.90-py2.7.egg-info
>> > ...
>> > /usr/lib64/python2.7/site-packages/ovs/_json.so
>> > ...
>> >
>> > CC:
>> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
>> > ---
>> >  INSTALL.Fedora.rst                     |  13 +++++
>> >  python/setup.py                        |   9 ++-
>> >  rhel/automake.mk                       |   9 +++
>> >  rhel/python-openvswitch-fedora.spec.in | 103
>> +++++++++++++++++++++++++++++++++
>> >  4 files changed, 133 insertions(+), 1 deletion(-)
>> >  create mode 100644 rhel/python-openvswitch-fedora.spec.in
>> >
>> > diff --git a/INSTALL.Fedora.rst b/INSTALL.Fedora.rst
>> > index b9be0ed..40eacfc 100644
>> > --- a/INSTALL.Fedora.rst
>> > +++ b/INSTALL.Fedora.rst
>> > @@ -83,6 +83,8 @@ This will create the RPMs `openvswitch`,
>> `python-openvswitch`,
>> >  `openvswitch-ovn-central`, `openvswitch-ovn-host`,
>> `openvswitch-ovn-vtep`,
>> >  `openvswitch-ovn-docker`, and `openvswitch-debuginfo`.
>> >
>> > +`python-openvswitch` RPM doesn't include the native json library.
>> > +
>> >  To enable DPDK support in the openvswitch package, the ``--with dpdk``
>> option
>> >  can be added:
>> >
>> > @@ -98,6 +100,17 @@ these tests, the ``--without check`` option can be
>> added:
>> >
>> >      $ make rpm-fedora RPMBUILD_OPT="--without check"
>> >
>> > +Open vSwitch python binding RPM with native json library
>> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > +
>> > +To build the `python-openvswitch` RPM with native json library, run:
>> > +
>> > +
>> > +::
>> > +    $ make rpm-fedora-python-ovs
>> > +
>> > +This also builds the Open vSwitch shared library and includes it in the
>> RPM.
>> > +
>> >  Kernel OVS Tree Datapath RPM
>> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> >
>> > diff --git a/python/setup.py b/python/setup.py
>> > index 19c1f18..5070c9b 100644
>> > --- a/python/setup.py
>> > +++ b/python/setup.py
>> > @@ -12,6 +12,7 @@
>> >
>> >  from __future__ import print_function
>> >  import sys
>> > +import os
>> >
>> >  from distutils.command.build_ext import build_ext
>> >  from distutils.errors import CCompilerError, DistutilsExecError, \
>> > @@ -33,6 +34,10 @@ ext_errors = (CCompilerError, DistutilsExecError,
>> DistutilsPlatformError)
>> >  if sys.platform == 'win32':
>> >      ext_errors += (IOError, ValueError)
>> >
>> > +# Get the include path if defined
>> > +include_dirs = os.environ.get('OVS_INCLUDE_DIR', '.')
>> > +library_dirs = os.environ.get('OVS_LIB_DIR', '.')
>> > +
>> >
>> >  class BuildFailed(Exception):
>> >      pass
>> > @@ -77,7 +82,9 @@ setup_args = dict(
>> >          'Programming Language :: Python :: 3.4',
>> >      ],
>> >      ext_modules=[setuptools.Extension("ovs._json",
>> sources=["ovs/_json.c"],
>> > -                                      libraries=['openvswitch'])],
>> > +                                      libraries=['openvswitch'],
>> > +                                      include_dirs=[include_dirs],
>> > +                                      library_dirs=[library_dirs])],
>> >      cmdclass={'build_ext': try_build_ext},
>> >  )
>> >
>> > diff --git a/rhel/automake.mk b/rhel/automake.mk
>> > index 45aa9b1..1113fd8 100644
>> > --- a/rhel/automake.mk
>> > +++ b/rhel/automake.mk
>> > @@ -23,6 +23,7 @@ EXTRA_DIST += \
>> >       rhel/openvswitch.spec.in \
>> >       rhel/openvswitch-fedora.spec \
>> >       rhel/openvswitch-fedora.spec.in \
>> > +     rhel/python-openvswitch-fedora.spec.in \
>> >       rhel/usr_share_openvswitch_scripts_sysconfig.template \
>> >       rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
>> >       rhel/usr_lib_systemd_system_openvswitch.service \
>> > @@ -62,6 +63,14 @@ rpm-fedora: dist $(srcdir)/rhel/openvswitch-
>> fedora.spec
>> >                   -D "_topdir ${RPMBUILD_TOP}" \
>> >                   -ba $(srcdir)/rhel/openvswitch-fedora.spec
>> >
>> > +# Build Python binding RPM with native json
>> > +rpm-fedora-python-ovs: dist $(srcdir)/rhel/python-
>> openvswitch-fedora.spec
>> > +     ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
>> > +     cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES
>> > +     rpmbuild ${RPMBUILD_OPT} \
>> > +                 -D "_topdir ${RPMBUILD_TOP}" \
>> > +                 -ba $(srcdir)/rhel/python-openvswitch-fedora.spec
>> > +
>> >  # Build kernel datapath RPM
>> >  rpm-fedora-kmod: dist $(srcdir)/rhel/openvswitch-kmod-fedora.spec
>> >       ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
>> > diff --git a/rhel/python-openvswitch-fedora.spec.in b/rhel/
>> python-openvswitch-fedora.spec.in
>> > new file mode 100644
>> > index 0000000..cd863d8
>> > --- /dev/null
>> > +++ b/rhel/python-openvswitch-fedora.spec.in

One suggestion I have is to make this a bcond_with option to set the
--enable-shared flag in the existing spec file.  That removes the need
for this spec.  Make sense?

>> > @@ -0,0 +1,103 @@
>> > +# Spec file for Python Open vSwitch with native json library
>> > +
>> > +# Copyright (C) 2016 Red Hat, Inc.
>> > +#
>> > +# Copying and distribution of this file, with or without modification,
>> > +# are permitted in any medium without royalty provided the copyright
>> > +# notice and this notice are preserved.  This file is offered as-is,
>> > +# without warranty of any kind.
>> > +#
>> > +
>> > +# If libcap-ng isn't available and there is no need for running OVS
>> > +# as regular user, specify the '--without libcapng'
>> > +%bcond_without libcapng
>> > +
>> > +# Enable PIE, bz#955181
>> > +%global _hardened_build 1
>> > +
>> > +# some distros (e.g: RHEL-7) don't define _rundir macro yet
>> > +# Fedora 15 onwards uses /run as _rundir
>> > +%if 0%{!?_rundir:1}
>> > +%define _rundir /run
>> > +%endif
>> > +
>> > +%global pkgname openvswitch
>> > +%global srcname openvswitch
>> > +
>> > +Name: python-%{pkgname}
>> > +Summary: Open vSwitch python bindings
>> > +Group: System Environment/Daemons
>> > +URL: http://www.openvswitch.org/
>> > +Version: @VERSION@
>> > +
>> > +# Nearly all of openvswitch is ASL 2.0.  The bugtool is LGPLv2+, and the
>> > +# lib/sflow*.[ch] files are SISSL
>> > +License: ASL 2.0 and LGPLv2+ and SISSL
>> > +Release: 1%{?dist}
>> > +Source: http://openvswitch.org/releases/%{pkgname}-%{version}.tar.gz
>> > +
>> > +BuildRequires: autoconf automake libtool
>> > +BuildRequires: systemd-units openssl openssl-devel
>> > +BuildRequires: python python-twisted-core python-zope-interface
>> python-six
>> > +BuildRequires: desktop-file-utils
>> > +BuildRequires: groff graphviz
>> > +BuildRequires: checkpolicy, selinux-policy-devel
>> > +# make check dependencies
>> > +BuildRequires: procps-ng
>> > +%if %{with libcapng}
>> > +BuildRequires: libcap-ng libcap-ng-devel
>> > +%endif
>> > +
>> > +Requires: openssl iproute module-init-tools
>> > +Requires: python
>> > +Requires: python-six
>> > +
>> > +%description
>> > +Python bindings for the Open vSwitch database
>> > +
>> > +%prep
>> > +rm -rf $RPM_BUILD_DIR/%{pkgname}-%{version}
>> > +cp $RPM_SOURCE_DIR/%{pkgname}-%{version}.tar.gz .
>> > +tar xzvf %{pkgname}-%{version}.tar.gz
>> > +
>> > +%build
>> > +cd %{pkgname}-%{version}
>> > +%configure \
>> > +%if %{with libcapng}
>> > +     --enable-libcapng \
>> > +%else
>> > +     --disable-libcapng \
>> > +%endif
>> > +%if %{with dpdk}
>> > +     --with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
>> > +%endif
>> > +     --enable-ssl \
>> > +     --with-pkidir=%{_sharedstatedir}/openvswitch/pki \
>> > +        --enable-shared
>> > +
>> > +make %{?_smp_mflags}
>> > +
>> > +export OVS_INCLUDE_DIR=$PWD/include/
>> > +export OVS_LIB_DIR=$PWD/lib/.libs/
>> > +cd python
>> > +%{__python2} setup.py build
>> > +
>> > +%install
>> > +rm -rf $RPM_BUILD_ROOT
>> > +
>> > +cd %{pkgname}-%{version}/python
>> > +%{__python2} setup.py install -O1 --skip-build --root %{buildroot}
>> > +cd ..
>> > +cp -d lib/.libs/libopenvswitch.so* %{buildroot}/%{_libdir}/
>> > +
>> > +%clean
>> > +rm -rf $RPM_BUILD_ROOT
>> > +
>> > +%files
>> > +%{python2_sitearch}/ovs
>> > +%{python2_sitearch}/ovs-%{version}-py?.?.egg-info
>> > +%{_libdir}/*.so*
>> > +
>> > +%changelog
>> > +* Fri Nov 25 2016 Numan Siddique <nusiddiq@redhat.com>
>> > +- First build on F25
>> > --
>> > 2.9.3
Russell Bryant Dec. 13, 2016, 8:09 p.m. UTC | #4
On Tue, Dec 13, 2016 at 1:45 PM, Aaron Conole <aconole@redhat.com> wrote:

> Although I'm not Flavio, I do have some quick thoughts on this.
>
> Russell Bryant <russell@ovn.org> writes:
>
> > My main concern with this was that it implements a new python-ovs package
> > and leaves the existing one alone.  I'd like to figure out how to change
> > the existing package so that we always use this approach.
>
> +1.  I don't think this should be split out the way it is here.
>
> Additionally, I'm concerned about a possible future issue with conflicting
> files (the openvswitch library file, itself), and ABI versioning.
>
> Rather than build and ship the openvswitch library, would it be possible
> to just try and detect it in the python code, and fallback if it doesn't
> exist/load?  That would give us a way to handle mismatched ABIs in the
> .so, AND prevent possible conflicts with a package that contains the
> libopenvswitch.so (which I would assume would just be the openvswitch
> package).
>

I think the Python code works this way now.  it will try to import the C
based json parser, and will fall back to the Python implementation if
importing it fails for any reason.

We don't actually build the openvswitch lib right now, though.  We also
need to make sure the ovs headers are available at build time of the python
package.


>
> Also, FYI (and this is sortof a side-note) the libtool ABI versioning
> information hasn't changed since 2014;  bumping / managing that may
> become an additional release burden if we start building on top of it,
> and expect to ship it around.  Imagine an install of this python package
> from ovs 2.7.0 and a future openvswitch package 3.0.0; they currently
> both see libtool version tag of 1:0:0
>
> Thanks,
> -Aaron
>
> > Added Flavio to CC.
> >
> > On Mon, Dec 12, 2016 at 5:19 PM, Ben Pfaff <blp@ovn.org> wrote:
> >
> >> Russell and Terry, I think that probably one of you should review this.
> >>
> >> Thanks,
> >>
> >> Ben.
> >>
> >> On Mon, Nov 28, 2016 at 03:07:05PM +0530, Numan Siddique wrote:
> >> > Since building python-openvswitch _json.so requires libopenvswitch.so
> >> > a separate spec file is added which configures Open vSwitch with
> >> > --enable-shared flag. The resulting RPM also includes the Open vSwitch
> >> > shared library.
> >> >
> >> > $ rpm -qlp python-openvswitch-2.6.90-1.fc25.x86_64.rpm
> >> > /usr/lib64/libopenvswitch.so
> >> > /usr/lib64/libopenvswitch.so.1
> >> > /usr/lib64/libopenvswitch.so.1.0.0
> >> > /usr/lib64/python2.7/site-packages/ovs
> >> > /usr/lib64/python2.7/site-packages/ovs-2.6.90-py2.7.egg-info
> >> > ...
> >> > /usr/lib64/python2.7/site-packages/ovs/_json.so
> >> > ...
> >> >
> >> > CC:
> >> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> >> > ---
> >> >  INSTALL.Fedora.rst                     |  13 +++++
> >> >  python/setup.py                        |   9 ++-
> >> >  rhel/automake.mk                       |   9 +++
> >> >  rhel/python-openvswitch-fedora.spec.in | 103
> >> +++++++++++++++++++++++++++++++++
> >> >  4 files changed, 133 insertions(+), 1 deletion(-)
> >> >  create mode 100644 rhel/python-openvswitch-fedora.spec.in
> >> >
> >> > diff --git a/INSTALL.Fedora.rst b/INSTALL.Fedora.rst
> >> > index b9be0ed..40eacfc 100644
> >> > --- a/INSTALL.Fedora.rst
> >> > +++ b/INSTALL.Fedora.rst
> >> > @@ -83,6 +83,8 @@ This will create the RPMs `openvswitch`,
> >> `python-openvswitch`,
> >> >  `openvswitch-ovn-central`, `openvswitch-ovn-host`,
> >> `openvswitch-ovn-vtep`,
> >> >  `openvswitch-ovn-docker`, and `openvswitch-debuginfo`.
> >> >
> >> > +`python-openvswitch` RPM doesn't include the native json library.
> >> > +
> >> >  To enable DPDK support in the openvswitch package, the ``--with
> dpdk``
> >> option
> >> >  can be added:
> >> >
> >> > @@ -98,6 +100,17 @@ these tests, the ``--without check`` option can be
> >> added:
> >> >
> >> >      $ make rpm-fedora RPMBUILD_OPT="--without check"
> >> >
> >> > +Open vSwitch python binding RPM with native json library
> >> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> > +
> >> > +To build the `python-openvswitch` RPM with native json library, run:
> >> > +
> >> > +
> >> > +::
> >> > +    $ make rpm-fedora-python-ovs
> >> > +
> >> > +This also builds the Open vSwitch shared library and includes it in
> the
> >> RPM.
> >> > +
> >> >  Kernel OVS Tree Datapath RPM
> >> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> >
> >> > diff --git a/python/setup.py b/python/setup.py
> >> > index 19c1f18..5070c9b 100644
> >> > --- a/python/setup.py
> >> > +++ b/python/setup.py
> >> > @@ -12,6 +12,7 @@
> >> >
> >> >  from __future__ import print_function
> >> >  import sys
> >> > +import os
> >> >
> >> >  from distutils.command.build_ext import build_ext
> >> >  from distutils.errors import CCompilerError, DistutilsExecError, \
> >> > @@ -33,6 +34,10 @@ ext_errors = (CCompilerError, DistutilsExecError,
> >> DistutilsPlatformError)
> >> >  if sys.platform == 'win32':
> >> >      ext_errors += (IOError, ValueError)
> >> >
> >> > +# Get the include path if defined
> >> > +include_dirs = os.environ.get('OVS_INCLUDE_DIR', '.')
> >> > +library_dirs = os.environ.get('OVS_LIB_DIR', '.')
> >> > +
> >> >
> >> >  class BuildFailed(Exception):
> >> >      pass
> >> > @@ -77,7 +82,9 @@ setup_args = dict(
> >> >          'Programming Language :: Python :: 3.4',
> >> >      ],
> >> >      ext_modules=[setuptools.Extension("ovs._json",
> >> sources=["ovs/_json.c"],
> >> > -                                      libraries=['openvswitch'])],
> >> > +                                      libraries=['openvswitch'],
> >> > +                                      include_dirs=[include_dirs],
> >> > +                                      library_dirs=[library_dirs])],
> >> >      cmdclass={'build_ext': try_build_ext},
> >> >  )
> >> >
> >> > diff --git a/rhel/automake.mk b/rhel/automake.mk
> >> > index 45aa9b1..1113fd8 100644
> >> > --- a/rhel/automake.mk
> >> > +++ b/rhel/automake.mk
> >> > @@ -23,6 +23,7 @@ EXTRA_DIST += \
> >> >       rhel/openvswitch.spec.in \
> >> >       rhel/openvswitch-fedora.spec \
> >> >       rhel/openvswitch-fedora.spec.in \
> >> > +     rhel/python-openvswitch-fedora.spec.in \
> >> >       rhel/usr_share_openvswitch_scripts_sysconfig.template \
> >> >       rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
> >> >       rhel/usr_lib_systemd_system_openvswitch.service \
> >> > @@ -62,6 +63,14 @@ rpm-fedora: dist $(srcdir)/rhel/openvswitch-
> >> fedora.spec
> >> >                   -D "_topdir ${RPMBUILD_TOP}" \
> >> >                   -ba $(srcdir)/rhel/openvswitch-fedora.spec
> >> >
> >> > +# Build Python binding RPM with native json
> >> > +rpm-fedora-python-ovs: dist $(srcdir)/rhel/python-
> >> openvswitch-fedora.spec
> >> > +     ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
> >> > +     cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES
> >> > +     rpmbuild ${RPMBUILD_OPT} \
> >> > +                 -D "_topdir ${RPMBUILD_TOP}" \
> >> > +                 -ba $(srcdir)/rhel/python-openvswitch-fedora.spec
> >> > +
> >> >  # Build kernel datapath RPM
> >> >  rpm-fedora-kmod: dist $(srcdir)/rhel/openvswitch-kmod-fedora.spec
> >> >       ${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
> >> > diff --git a/rhel/python-openvswitch-fedora.spec.in b/rhel/
> >> python-openvswitch-fedora.spec.in
> >> > new file mode 100644
> >> > index 0000000..cd863d8
> >> > --- /dev/null
> >> > +++ b/rhel/python-openvswitch-fedora.spec.in
>
> One suggestion I have is to make this a bcond_with option to set the
> --enable-shared flag in the existing spec file.  That removes the need
> for this spec.  Make sense?
>
> >> > @@ -0,0 +1,103 @@
> >> > +# Spec file for Python Open vSwitch with native json library
> >> > +
> >> > +# Copyright (C) 2016 Red Hat, Inc.
> >> > +#
> >> > +# Copying and distribution of this file, with or without
> modification,
> >> > +# are permitted in any medium without royalty provided the copyright
> >> > +# notice and this notice are preserved.  This file is offered as-is,
> >> > +# without warranty of any kind.
> >> > +#
> >> > +
> >> > +# If libcap-ng isn't available and there is no need for running OVS
> >> > +# as regular user, specify the '--without libcapng'
> >> > +%bcond_without libcapng
> >> > +
> >> > +# Enable PIE, bz#955181
> >> > +%global _hardened_build 1
> >> > +
> >> > +# some distros (e.g: RHEL-7) don't define _rundir macro yet
> >> > +# Fedora 15 onwards uses /run as _rundir
> >> > +%if 0%{!?_rundir:1}
> >> > +%define _rundir /run
> >> > +%endif
> >> > +
> >> > +%global pkgname openvswitch
> >> > +%global srcname openvswitch
> >> > +
> >> > +Name: python-%{pkgname}
> >> > +Summary: Open vSwitch python bindings
> >> > +Group: System Environment/Daemons
> >> > +URL: http://www.openvswitch.org/
> >> > +Version: @VERSION@
> >> > +
> >> > +# Nearly all of openvswitch is ASL 2.0.  The bugtool is LGPLv2+, and
> the
> >> > +# lib/sflow*.[ch] files are SISSL
> >> > +License: ASL 2.0 and LGPLv2+ and SISSL
> >> > +Release: 1%{?dist}
> >> > +Source: http://openvswitch.org/releases/%{pkgname}-%{version}.tar.gz
> >> > +
> >> > +BuildRequires: autoconf automake libtool
> >> > +BuildRequires: systemd-units openssl openssl-devel
> >> > +BuildRequires: python python-twisted-core python-zope-interface
> >> python-six
> >> > +BuildRequires: desktop-file-utils
> >> > +BuildRequires: groff graphviz
> >> > +BuildRequires: checkpolicy, selinux-policy-devel
> >> > +# make check dependencies
> >> > +BuildRequires: procps-ng
> >> > +%if %{with libcapng}
> >> > +BuildRequires: libcap-ng libcap-ng-devel
> >> > +%endif
> >> > +
> >> > +Requires: openssl iproute module-init-tools
> >> > +Requires: python
> >> > +Requires: python-six
> >> > +
> >> > +%description
> >> > +Python bindings for the Open vSwitch database
> >> > +
> >> > +%prep
> >> > +rm -rf $RPM_BUILD_DIR/%{pkgname}-%{version}
> >> > +cp $RPM_SOURCE_DIR/%{pkgname}-%{version}.tar.gz .
> >> > +tar xzvf %{pkgname}-%{version}.tar.gz
> >> > +
> >> > +%build
> >> > +cd %{pkgname}-%{version}
> >> > +%configure \
> >> > +%if %{with libcapng}
> >> > +     --enable-libcapng \
> >> > +%else
> >> > +     --disable-libcapng \
> >> > +%endif
> >> > +%if %{with dpdk}
> >> > +     --with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
> >> > +%endif
> >> > +     --enable-ssl \
> >> > +     --with-pkidir=%{_sharedstatedir}/openvswitch/pki \
> >> > +        --enable-shared
> >> > +
> >> > +make %{?_smp_mflags}
> >> > +
> >> > +export OVS_INCLUDE_DIR=$PWD/include/
> >> > +export OVS_LIB_DIR=$PWD/lib/.libs/
> >> > +cd python
> >> > +%{__python2} setup.py build
> >> > +
> >> > +%install
> >> > +rm -rf $RPM_BUILD_ROOT
> >> > +
> >> > +cd %{pkgname}-%{version}/python
> >> > +%{__python2} setup.py install -O1 --skip-build --root %{buildroot}
> >> > +cd ..
> >> > +cp -d lib/.libs/libopenvswitch.so* %{buildroot}/%{_libdir}/
> >> > +
> >> > +%clean
> >> > +rm -rf $RPM_BUILD_ROOT
> >> > +
> >> > +%files
> >> > +%{python2_sitearch}/ovs
> >> > +%{python2_sitearch}/ovs-%{version}-py?.?.egg-info
> >> > +%{_libdir}/*.so*
> >> > +
> >> > +%changelog
> >> > +* Fri Nov 25 2016 Numan Siddique <nusiddiq@redhat.com>
> >> > +- First build on F25
> >> > --
> >> > 2.9.3
>
Ben Pfaff Dec. 23, 2016, 5:30 p.m. UTC | #5
On Tue, Dec 13, 2016 at 01:45:05PM -0500, Aaron Conole wrote:
> Also, FYI (and this is sortof a side-note) the libtool ABI versioning
> information hasn't changed since 2014;  bumping / managing that may
> become an additional release burden if we start building on top of it,
> and expect to ship it around.  Imagine an install of this python package
> from ovs 2.7.0 and a future openvswitch package 3.0.0; they currently
> both see libtool version tag of 1:0:0

Should we bump the libtool version now?  We're nearing a release, so
it's about the right time.

libtool version numbering rules:
https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
Aaron Conole Dec. 30, 2016, 3:48 p.m. UTC | #6
Ben Pfaff <blp@ovn.org> writes:

> On Tue, Dec 13, 2016 at 01:45:05PM -0500, Aaron Conole wrote:
>> Also, FYI (and this is sortof a side-note) the libtool ABI versioning
>> information hasn't changed since 2014;  bumping / managing that may
>> become an additional release burden if we start building on top of it,
>> and expect to ship it around.  Imagine an install of this python package
>> from ovs 2.7.0 and a future openvswitch package 3.0.0; they currently
>> both see libtool version tag of 1:0:0
>
> Should we bump the libtool version now?  We're nearing a release, so
> it's about the right time.

Sorry for the late response.  I think it's probably appropriate to bump
at least the libtool 'age'.  I'm not sure what kinds of ABI changes have
happened since introduction though - the interface 'major' version may
need to change.

> libtool version numbering rules:
> https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html

Thanks for the refresher.  It's always a pain to remember which field
means what in the libtool parlance.

-Aaron
Flavio Leitner Dec. 30, 2016, 4:41 p.m. UTC | #7
On Tue, 13 Dec 2016 13:45:05 -0500
Aaron Conole <aconole@redhat.com> wrote:

> Although I'm not Flavio, I do have some quick thoughts on this.
> 
> Russell Bryant <russell@ovn.org> writes:
> 
> > My main concern with this was that it implements a new python-ovs package
> > and leaves the existing one alone.  I'd like to figure out how to change
> > the existing package so that we always use this approach.  
> 
> +1.  I don't think this should be split out the way it is here.

The preference is for a sub-package and if it's something that most
people don't care but gets impacted, then we can use a conditional to
enable the build.

Creating another package causes headaches for all distros maintainers
because one has import in the distro and deal with two packages from
now on.
Ben Pfaff Jan. 4, 2017, 11:43 p.m. UTC | #8
On Fri, Dec 30, 2016 at 10:48:39AM -0500, Aaron Conole wrote:
> Ben Pfaff <blp@ovn.org> writes:
> 
> > On Tue, Dec 13, 2016 at 01:45:05PM -0500, Aaron Conole wrote:
> >> Also, FYI (and this is sortof a side-note) the libtool ABI versioning
> >> information hasn't changed since 2014;  bumping / managing that may
> >> become an additional release burden if we start building on top of it,
> >> and expect to ship it around.  Imagine an install of this python package
> >> from ovs 2.7.0 and a future openvswitch package 3.0.0; they currently
> >> both see libtool version tag of 1:0:0
> >
> > Should we bump the libtool version now?  We're nearing a release, so
> > it's about the right time.
> 
> Sorry for the late response.  I think it's probably appropriate to bump
> at least the libtool 'age'.  I'm not sure what kinds of ABI changes have
> happened since introduction though - the interface 'major' version may
> need to change.

I'm pretty sure we've busted ABIs left and right since 2014, so a major
version update should be appropriate.

> > libtool version numbering rules:
> > https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
> 
> Thanks for the refresher.  It's always a pain to remember which field
> means what in the libtool parlance.

Yes.  I can remember the Unix rules, but not the libtool ones.
Aaron Conole Jan. 5, 2017, 3:52 p.m. UTC | #9
<snip>

>> > Should we bump the libtool version now?  We're nearing a release, so
>> > it's about the right time.
>> 
>> Sorry for the late response.  I think it's probably appropriate to bump
>> at least the libtool 'age'.  I'm not sure what kinds of ABI changes have
>> happened since introduction though - the interface 'major' version may
>> need to change.
>
> I'm pretty sure we've busted ABIs left and right since 2014, so a major
> version update should be appropriate.

I was about to post a patch that simply bumped this version, but it sent
me off onto a tangent leading to this question:

Will Open vSwitch support the libopenvswitch.so as an externally
linkable library?

I know there was some work done in this direction, but nothing for ABI
versioning or API compatibility has been done yet.

I've CCd folks who were on the original discussions for the various
series I've found (linked below).

I'm uncomfortable with bumping this and just 'let the chips fall where
they may,' since having a version that hasn't changed is the virtually
the same as not having a version.  The instant we bump, we state that
the version means something, so I'm not comfortable just shipping a
patch that changes the version without some accompanying documentation
explaining *what* that means.  It also means we need to be more diligent
with reviews and watch for potential ABI breakages, with a compatibility
strategy in place (when ABI/API changes can be made, how they are made,
etc.).  I think it's got more implication than just a single line change
to the configure.ac file.

Below are a few snippets I found;  there may be more discussions I've
missed, so please feel free to include links (or summaries).

Versioning patch (which mentions that it isn't yet intended to be for
  3rd party linking):
  https://mail.openvswitch.org/pipermail/ovs-dev/2014-November/291959.html

The first non-rfc 'third-party linking' series I could find (with note
  from Panu at least warning about this):
  https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/310703.html
  https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/310773.html

The series 'third party linking' series has all parts accepted:
  https://mail.openvswitch.org/pipermail/ovs-dev/2016-April/313183.html

-Aaron
Ben Pfaff Jan. 5, 2017, 4:51 p.m. UTC | #10
On Thu, Jan 05, 2017 at 10:52:00AM -0500, Aaron Conole wrote:
> I'm uncomfortable with bumping this and just 'let the chips fall where
> they may,' since having a version that hasn't changed is the virtually
> the same as not having a version.  The instant we bump, we state that
> the version means something, so I'm not comfortable just shipping a
> patch that changes the version without some accompanying documentation
> explaining *what* that means.  It also means we need to be more diligent
> with reviews and watch for potential ABI breakages, with a compatibility
> strategy in place (when ABI/API changes can be made, how they are made,
> etc.).  I think it's got more implication than just a single line change
> to the configure.ac file.

My intent has been that each release branch is a separate major version,
so that we maintain ABI compatibility across all 2.5.x releases, all
2.6.x releases, and so on.  I don't know whether we've succeeded at
this, since it hasn't been a focus, but that was my original intent.
Li,Rongqing via dev Jan. 5, 2017, 5:44 p.m. UTC | #11
Hi Aaron,

> On Jan 5, 2017, at 7:52 AM, Aaron Conole <aconole@bytheb.org> wrote:
> 
> <snip>
> 
>>>> Should we bump the libtool version now?  We're nearing a release, so
>>>> it's about the right time.
>>> 
>>> Sorry for the late response.  I think it's probably appropriate to bump
>>> at least the libtool 'age'.  I'm not sure what kinds of ABI changes have
>>> happened since introduction though - the interface 'major' version may
>>> need to change.
>> 
>> I'm pretty sure we've busted ABIs left and right since 2014, so a major
>> version update should be appropriate.
> 
> I was about to post a patch that simply bumped this version, but it sent
> me off onto a tangent leading to this question:
> 
> Will Open vSwitch support the libopenvswitch.so as an externally
> linkable library?
> 
Yes, as an example Debian now has a ‘openvswitch-dev’ package that bundles the library and header files.  Here’s how it currently looks (Debian ‘sid’):

ben@ben-lab-sc1:~$ dpkg -c openvswitch-dev_2.6.2-pre+git20161223-2_amd64.deb | grep "lib.*\.so"
lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libofproto.so -> libofproto.so.1.0.0
lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libopenvswitch.so -> libopenvswitch.so.1.0.0
lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libovn.so -> libovn.so.1.0.0
lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libovsdb.so -> libovsdb.so.1.0.0
lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libsflow.so -> libsflow.so.1.0.0
lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libvtep.so -> libvtep.so.1.0.0

> I know there was some work done in this direction, but nothing for ABI
> versioning or API compatibility has been done yet.
> 
As you can see, the library naming isn’t very useful.  As for ABI versioning, I don’t think anything meaningful has been done.  IMHO we should probably change this so the major release is part of the name, since that’s where I believe we should guarantee compatibility, along the l lines of ‘libopenvswitch_2.5.so’ or whatever.  I think there are libtool directives for managing this naming, but we haven’t done that.

> I've CCd folks who were on the original discussions for the various
> series I've found (linked below).
> 
> I'm uncomfortable with bumping this and just 'let the chips fall where
> they may,' since having a version that hasn't changed is the virtually
> the same as not having a version.  The instant we bump, we state that
> the version means something, so I'm not comfortable just shipping a
> patch that changes the version without some accompanying documentation
> explaining *what* that means.  It also means we need to be more diligent
> with reviews and watch for potential ABI breakages, with a compatibility
> strategy in place (when ABI/API changes can be made, how they are made,
> etc.).  I think it's got more implication than just a single line change
> to the configure.ac file.
> 
> Below are a few snippets I found;  there may be more discussions I've
> missed, so please feel free to include links (or summaries).
> 
> Versioning patch (which mentions that it isn't yet intended to be for
>  3rd party linking):
>  https://mail.openvswitch.org/pipermail/ovs-dev/2014-November/291959.html
> 
> The first non-rfc 'third-party linking' series I could find (with note
>  from Panu at least warning about this):
>  https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/310703.html
>  https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/310773.html
> 
> The series 'third party linking' series has all parts accepted:
>  https://mail.openvswitch.org/pipermail/ovs-dev/2016-April/313183.html
> 
> -Aaron
Ben Pfaff Jan. 5, 2017, 6:37 p.m. UTC | #12
On Thu, Jan 05, 2017 at 09:44:33AM -0800, Ben Warren wrote:
> > I know there was some work done in this direction, but nothing for ABI
> > versioning or API compatibility has been done yet.
> > 
> As you can see, the library naming isn’t very useful.  As for ABI
> versioning, I don’t think anything meaningful has been done.  IMHO we
> should probably change this so the major release is part of the name,
> since that’s where I believe we should guarantee compatibility, along
> the l lines of ‘libopenvswitch_2.5.so’ or whatever.  I think there are
> libtool directives for managing this naming, but we haven’t done that.

That's a good idea, I hadn't thought of that approach.
Aaron Conole Jan. 10, 2017, 4:32 p.m. UTC | #13
Ben Warren via dev <ovs-dev@openvswitch.org> writes:

> Hi Aaron,
>
>> On Jan 5, 2017, at 7:52 AM, Aaron Conole <aconole@bytheb.org> wrote:
>> 
>> <snip>
>> 
>>>>> Should we bump the libtool version now?  We're nearing a release, so
>>>>> it's about the right time.
>>>> 
>>>> Sorry for the late response.  I think it's probably appropriate to bump
>>>> at least the libtool 'age'.  I'm not sure what kinds of ABI changes have
>>>> happened since introduction though - the interface 'major' version may
>>>> need to change.
>>> 
>>> I'm pretty sure we've busted ABIs left and right since 2014, so a major
>>> version update should be appropriate.
>> 
>> I was about to post a patch that simply bumped this version, but it sent
>> me off onto a tangent leading to this question:
>> 
>> Will Open vSwitch support the libopenvswitch.so as an externally
>> linkable library?
>> 
> Yes, as an example Debian now has a ‘openvswitch-dev’ package that
> bundles the library and header files.  Here’s how it currently looks
> (Debian ‘sid’):
>
> ben@ben-lab-sc1:~$ dpkg -c
> openvswitch-dev_2.6.2-pre+git20161223-2_amd64.deb | grep "lib.*\.so"
> lrwxrwxrwx root/root 0 2016-12-23 16:35 ./usr/lib/libofproto.so ->
> libofproto.so.1.0.0
> lrwxrwxrwx root/root 0 2016-12-23 16:35 ./usr/lib/libopenvswitch.so ->
> libopenvswitch.so.1.0.0
> lrwxrwxrwx root/root         0 2016-12-23 16:35 ./usr/lib/libovn.so -> libovn.so.1.0.0
> lrwxrwxrwx root/root 0 2016-12-23 16:35 ./usr/lib/libovsdb.so ->
> libovsdb.so.1.0.0
> lrwxrwxrwx root/root 0 2016-12-23 16:35 ./usr/lib/libsflow.so ->
> libsflow.so.1.0.0
> lrwxrwxrwx root/root 0 2016-12-23 16:35 ./usr/lib/libvtep.so ->
> libvtep.so.1.0.0
>
>> I know there was some work done in this direction, but nothing for ABI
>> versioning or API compatibility has been done yet.
>> 
> As you can see, the library naming isn’t very useful.  As for ABI
> versioning, I don’t think anything meaningful has been done.  IMHO we
> should probably change this so the major release is part of the name,
> since that’s where I believe we should guarantee compatibility, along
> the l lines of ‘libopenvswitch_2.5.so’ or whatever.  I think there are
> libtool directives for managing this naming, but we haven’t done that.

Posted an RFC patch with this type of functionality at:

  https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327472.html

Thanks everyone for the input.

-Aaron
diff mbox

Patch

diff --git a/INSTALL.Fedora.rst b/INSTALL.Fedora.rst
index b9be0ed..40eacfc 100644
--- a/INSTALL.Fedora.rst
+++ b/INSTALL.Fedora.rst
@@ -83,6 +83,8 @@  This will create the RPMs `openvswitch`, `python-openvswitch`,
 `openvswitch-ovn-central`, `openvswitch-ovn-host`, `openvswitch-ovn-vtep`,
 `openvswitch-ovn-docker`, and `openvswitch-debuginfo`.
 
+`python-openvswitch` RPM doesn't include the native json library.
+
 To enable DPDK support in the openvswitch package, the ``--with dpdk`` option
 can be added:
 
@@ -98,6 +100,17 @@  these tests, the ``--without check`` option can be added:
 
     $ make rpm-fedora RPMBUILD_OPT="--without check"
 
+Open vSwitch python binding RPM with native json library
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To build the `python-openvswitch` RPM with native json library, run:
+
+
+::
+    $ make rpm-fedora-python-ovs
+
+This also builds the Open vSwitch shared library and includes it in the RPM.
+
 Kernel OVS Tree Datapath RPM
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/python/setup.py b/python/setup.py
index 19c1f18..5070c9b 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -12,6 +12,7 @@ 
 
 from __future__ import print_function
 import sys
+import os
 
 from distutils.command.build_ext import build_ext
 from distutils.errors import CCompilerError, DistutilsExecError, \
@@ -33,6 +34,10 @@  ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
 if sys.platform == 'win32':
     ext_errors += (IOError, ValueError)
 
+# Get the include path if defined
+include_dirs = os.environ.get('OVS_INCLUDE_DIR', '.')
+library_dirs = os.environ.get('OVS_LIB_DIR', '.')
+
 
 class BuildFailed(Exception):
     pass
@@ -77,7 +82,9 @@  setup_args = dict(
         'Programming Language :: Python :: 3.4',
     ],
     ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
-                                      libraries=['openvswitch'])],
+                                      libraries=['openvswitch'],
+                                      include_dirs=[include_dirs],
+                                      library_dirs=[library_dirs])],
     cmdclass={'build_ext': try_build_ext},
 )
 
diff --git a/rhel/automake.mk b/rhel/automake.mk
index 45aa9b1..1113fd8 100644
--- a/rhel/automake.mk
+++ b/rhel/automake.mk
@@ -23,6 +23,7 @@  EXTRA_DIST += \
 	rhel/openvswitch.spec.in \
 	rhel/openvswitch-fedora.spec \
 	rhel/openvswitch-fedora.spec.in \
+	rhel/python-openvswitch-fedora.spec.in \
 	rhel/usr_share_openvswitch_scripts_sysconfig.template \
 	rhel/usr_share_openvswitch_scripts_systemd_sysconfig.template \
 	rhel/usr_lib_systemd_system_openvswitch.service \
@@ -62,6 +63,14 @@  rpm-fedora: dist $(srcdir)/rhel/openvswitch-fedora.spec
                  -D "_topdir ${RPMBUILD_TOP}" \
                  -ba $(srcdir)/rhel/openvswitch-fedora.spec
 
+# Build Python binding RPM with native json
+rpm-fedora-python-ovs: dist $(srcdir)/rhel/python-openvswitch-fedora.spec
+	${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
+	cp ${DIST_ARCHIVES} ${RPMBUILD_TOP}/SOURCES
+	rpmbuild ${RPMBUILD_OPT} \
+                 -D "_topdir ${RPMBUILD_TOP}" \
+                 -ba $(srcdir)/rhel/python-openvswitch-fedora.spec
+
 # Build kernel datapath RPM
 rpm-fedora-kmod: dist $(srcdir)/rhel/openvswitch-kmod-fedora.spec
 	${MKDIR_P} ${RPMBUILD_TOP}/SOURCES
diff --git a/rhel/python-openvswitch-fedora.spec.in b/rhel/python-openvswitch-fedora.spec.in
new file mode 100644
index 0000000..cd863d8
--- /dev/null
+++ b/rhel/python-openvswitch-fedora.spec.in
@@ -0,0 +1,103 @@ 
+# Spec file for Python Open vSwitch with native json library
+
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.  This file is offered as-is,
+# without warranty of any kind.
+#
+
+# If libcap-ng isn't available and there is no need for running OVS
+# as regular user, specify the '--without libcapng'
+%bcond_without libcapng
+
+# Enable PIE, bz#955181
+%global _hardened_build 1
+
+# some distros (e.g: RHEL-7) don't define _rundir macro yet
+# Fedora 15 onwards uses /run as _rundir
+%if 0%{!?_rundir:1}
+%define _rundir /run
+%endif
+
+%global pkgname openvswitch
+%global srcname openvswitch
+
+Name: python-%{pkgname}
+Summary: Open vSwitch python bindings
+Group: System Environment/Daemons
+URL: http://www.openvswitch.org/
+Version: @VERSION@
+
+# Nearly all of openvswitch is ASL 2.0.  The bugtool is LGPLv2+, and the
+# lib/sflow*.[ch] files are SISSL
+License: ASL 2.0 and LGPLv2+ and SISSL
+Release: 1%{?dist}
+Source: http://openvswitch.org/releases/%{pkgname}-%{version}.tar.gz
+
+BuildRequires: autoconf automake libtool
+BuildRequires: systemd-units openssl openssl-devel
+BuildRequires: python python-twisted-core python-zope-interface python-six
+BuildRequires: desktop-file-utils
+BuildRequires: groff graphviz
+BuildRequires: checkpolicy, selinux-policy-devel
+# make check dependencies
+BuildRequires: procps-ng
+%if %{with libcapng}
+BuildRequires: libcap-ng libcap-ng-devel
+%endif
+
+Requires: openssl iproute module-init-tools
+Requires: python
+Requires: python-six
+
+%description
+Python bindings for the Open vSwitch database
+
+%prep
+rm -rf $RPM_BUILD_DIR/%{pkgname}-%{version}
+cp $RPM_SOURCE_DIR/%{pkgname}-%{version}.tar.gz .
+tar xzvf %{pkgname}-%{version}.tar.gz
+
+%build
+cd %{pkgname}-%{version}
+%configure \
+%if %{with libcapng}
+	--enable-libcapng \
+%else
+	--disable-libcapng \
+%endif
+%if %{with dpdk}
+	--with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
+%endif
+	--enable-ssl \
+	--with-pkidir=%{_sharedstatedir}/openvswitch/pki \
+        --enable-shared
+
+make %{?_smp_mflags}
+
+export OVS_INCLUDE_DIR=$PWD/include/
+export OVS_LIB_DIR=$PWD/lib/.libs/
+cd python
+%{__python2} setup.py build
+
+%install
+rm -rf $RPM_BUILD_ROOT
+
+cd %{pkgname}-%{version}/python
+%{__python2} setup.py install -O1 --skip-build --root %{buildroot}
+cd ..
+cp -d lib/.libs/libopenvswitch.so* %{buildroot}/%{_libdir}/
+
+%clean
+rm -rf $RPM_BUILD_ROOT
+
+%files
+%{python2_sitearch}/ovs
+%{python2_sitearch}/ovs-%{version}-py?.?.egg-info
+%{_libdir}/*.so*
+
+%changelog
+* Fri Nov 25 2016 Numan Siddique <nusiddiq@redhat.com>
+- First build on F25