[ovs-dev] fedora: Add python3-openvswitch split package

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1412694
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
---
 Documentation/intro/install/fedora.rst |  1 +
 Vagrantfile                            |  1 +
 rhel/openvswitch-fedora.spec.in        | 54 ++++++++++++++++++++++++----------
 3 files changed, 40 insertions(+), 16 deletions(-)

On Tue, Feb 21, 2017 at 04:29:51PM +0100, Timothy Redaelli wrote:
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1412694
> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> ---

Looks good to me.
Acked-by: Flavio Leitner <fbl@sysclose.org>

On Tue, Feb 21, 2017 at 12:29 PM, Flavio Leitner <fbl@sysclose.org> wrote:

> On Tue, Feb 21, 2017 at 04:29:51PM +0100, Timothy Redaelli wrote:
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1412694
> > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> > ---
>
> Looks good to me.
> Acked-by: Flavio Leitner <fbl@sysclose.org>


Thanks to both of you.  I applied this to master and branch-2.7.

Just an FYI that this patch breaks CentOS 7 builds because of the
python3-devel dependency. Might need to wrap it in a conditional to check
for CentOS 7 platform builds.

CC: Russell and Flavio

Thanks!
Leif.

On Tue, Feb 21, 2017 at 10:29 AM, Timothy Redaelli <tredaelli@redhat.com>
wrote:

> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1412694
> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> ---
>  Documentation/intro/install/fedora.rst |  1 +
>  Vagrantfile                            |  1 +
>  rhel/openvswitch-fedora.spec.in        | 54
> ++++++++++++++++++++++++----------
>  3 files changed, 40 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/intro/install/fedora.rst
> b/Documentation/intro/install/fedora.rst
> index fe53214..0ecd255 100644
> --- a/Documentation/intro/install/fedora.rst
> +++ b/Documentation/intro/install/fedora.rst
> @@ -42,6 +42,7 @@ in the :doc:`general`. Specific packages (by package
> name) include:
>  - rpm-build
>  - autoconf automake libtool
>  - systemd-units openssl openssl-devel
> +- python-devel python3-devel
>  - python python-twisted-core python-zope-interface python-six
>  - desktop-file-utils
>  - groff graphviz
> diff --git a/Vagrantfile b/Vagrantfile
> index ef25c77..8439918 100644
> --- a/Vagrantfile
> +++ b/Vagrantfile
> @@ -8,6 +8,7 @@ Vagrant.require_version ">=1.7.0"
>  $bootstrap_fedora = <<SCRIPT
>  dnf -y update
>  dnf -y install autoconf automake openssl-devel libtool \
> +               python-devel python3-devel \
>                 python-twisted-core python-zope-interface \
>                 desktop-file-utils groff graphviz rpmdevtools nc curl \
>                 wget python-six pyftpdlib checkpolicy selinux-policy-devel
> \
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.
> spec.in
> index b395613..a1bf1c7 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -53,12 +53,16 @@ Source: http://openvswitch.org/
> releases/%{name}-%{version}.tar.gz
>
>  BuildRequires: autoconf automake libtool
>  BuildRequires: systemd-units openssl openssl-devel
> -BuildRequires: python python-twisted-core python-zope-interface python-six
> +BuildRequires: python2-devel
> +BuildRequires: python3-devel
>  BuildRequires: desktop-file-utils
>  BuildRequires: groff graphviz
>  BuildRequires: checkpolicy, selinux-policy-devel
>  # make check dependencies
> +%if %{with check}
> +BuildRequires: python2-twisted python2-zope-interface python2-six
>  BuildRequires: procps-ng
> +%endif
>  %if %{with libcapng}
>  BuildRequires: libcap-ng libcap-ng-devel
>  %endif
> @@ -94,22 +98,33 @@ Requires: selinux-policy-targeted
>  %description selinux-policy
>  Tailored Open vSwitch SELinux policy
>
> -%package -n python-openvswitch
> -Summary: Open vSwitch python bindings
> +%package -n python2-openvswitch
> +Summary: Open vSwitch python2 bindings
>  License: ASL 2.0
>  BuildArch: noarch
> -Requires: python
> -Requires: python-six
> +Requires: python2
> +Requires: python2-six
> +Provides: python-openvswitch = %{version}-%{release}
>
> -%description -n python-openvswitch
> +%description -n python2-openvswitch
> +Python bindings for the Open vSwitch database
> +
> +%package -n python3-openvswitch
> +Summary: Open vSwitch python3 bindings
> +License: ASL 2.0
> +BuildArch: noarch
> +Requires: python3
> +Requires: python3-six
> +
> +%description -n python3-openvswitch
>  Python bindings for the Open vSwitch database
>
>  %package test
>  Summary: Open vSwitch testing utilities
>  License: ASL 2.0
>  BuildArch: noarch
> -Requires: python-openvswitch = %{version}-%{release}
> -Requires: python python-twisted-core python-twisted-web
> +Requires: python2-openvswitch = %{version}-%{release}
> +Requires: python2 python2-twisted
>
>  %description test
>  Utilities that are useful to diagnose performance and connectivity
> @@ -167,7 +182,7 @@ Utilities that are use to diagnose and manage the OVN
> components.
>  %package ovn-docker
>  Summary: Open vSwitch - Open Virtual Network support
>  License: ASL 2.0
> -Requires: openvswitch openvswitch-ovn-common python-openvswitch
> +Requires: openvswitch openvswitch-ovn-common python2-openvswitch
>
>  %description ovn-docker
>  Docker network plugins for OVN.
> @@ -222,10 +237,13 @@ install -p -m 0755 rhel/etc_sysconfig_network-scripts_ifdown-ovs
> \
>  install -p -m 0755 rhel/etc_sysconfig_network-scripts_ifup-ovs \
>          $RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
>
> -install -d -m 0755 $RPM_BUILD_ROOT%{python_sitelib}
> -mv $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/* \
> -   $RPM_BUILD_ROOT%{python_sitelib}
> -rmdir $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/
> +install -d -m 0755 $RPM_BUILD_ROOT%{python2_sitelib}
> +install -d -m 0755 $RPM_BUILD_ROOT%{python3_sitelib}
> +cp -a $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/* \
> +   $RPM_BUILD_ROOT%{python2_sitelib}
> +cp -a $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/ovs \
> +   $RPM_BUILD_ROOT%{python3_sitelib}
> +rm -rf $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/
>
>  install -d -m 0755 $RPM_BUILD_ROOT/%{_sharedstatedir}/openvswitch
>
> @@ -411,8 +429,12 @@ fi
>  %defattr(-,root,root)
>  %{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp
>
> -%files -n python-openvswitch
> -%{python_sitelib}/ovs
> +%files -n python2-openvswitch
> +%{python2_sitelib}/ovs
> +%doc COPYING
> +
> +%files -n python3-openvswitch
> +%{python3_sitelib}/ovs
>  %doc COPYING
>
>  %files test
> @@ -428,7 +450,7 @@ fi
>  %{_mandir}/man1/ovs-pcap.1*
>  %{_mandir}/man8/ovs-tcpdump.8*
>  %{_mandir}/man1/ovs-tcpundump.1*
> -%{python_sitelib}/ovstest
> +%{python2_sitelib}/ovstest
>
>  %files devel
>  %{_libdir}/*.a
> --
> 2.9.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

On Thu, Mar 2, 2017 at 3:42 PM, Leif Madsen <lmadsen@redhat.com> wrote:

> Just an FYI that this patch breaks CentOS 7 builds because of the
> python3-devel dependency. Might need to wrap it in a conditional to check
> for CentOS 7 platform builds.
>
> CC: Russell and Flavio
>
> Thanks!
> Leif.
>

Thanks for letting us know.  A conditional sounds fine to me.  Maybe make
it an option you can specify, on by default except for centos/rhel ?

On Thu, Mar 2, 2017 at 3:44 PM, Russell Bryant <russell@ovn.org> wrote:

>
> On Thu, Mar 2, 2017 at 3:42 PM, Leif Madsen <lmadsen@redhat.com> wrote:
>
>> Just an FYI that this patch breaks CentOS 7 builds because of the
>> python3-devel dependency. Might need to wrap it in a conditional to check
>> for CentOS 7 platform builds.
>>
>> CC: Russell and Flavio
>>
>> Thanks!
>> Leif.
>>
>
> Thanks for letting us know.  A conditional sounds fine to me.  Maybe make
> it an option you can specify, on by default except for centos/rhel ?
>
>
Working on a patch now. I was just thinking of wrapping the python3 parts
in %if 0%{?fedora}

Leif.

On Thu, Mar 02, 2017 at 04:13:17PM -0500, Leif Madsen wrote:
> On Thu, Mar 2, 2017 at 3:44 PM, Russell Bryant <russell@ovn.org> wrote:
> 
> >
> > On Thu, Mar 2, 2017 at 3:42 PM, Leif Madsen <lmadsen@redhat.com> wrote:
> >
> >> Just an FYI that this patch breaks CentOS 7 builds because of the
> >> python3-devel dependency. Might need to wrap it in a conditional to check
> >> for CentOS 7 platform builds.
> >>
> >> CC: Russell and Flavio
> >>
> >> Thanks!
> >> Leif.
> >>
> >
> > Thanks for letting us know.  A conditional sounds fine to me.  Maybe make
> > it an option you can specify, on by default except for centos/rhel ?
> >
> >
> Working on a patch now. I was just thinking of wrapping the python3 parts
> in %if 0%{?fedora}

Perhaps you could check if %{python3_pkgversion} or %{__python3} is
defined instead, so in case someone install python3 support on older
distros the spec will do the right thing.

On Fri, Mar 3, 2017 at 6:55 AM, Flavio Leitner <fbl@redhat.com> wrote:

> Perhaps you could check if %{python3_pkgversion} or %{__python3} is
> defined instead, so in case someone install python3 support on older
> distros the spec will do the right thing.
>

I'm following some python2 to python3 porting guides, and it seems like the
big thing to do is to make sure things are split out into subpackages (like
what was being done). There are some Provides macros that I'm going to add
instead of leaving them hard coded like now (which is apparently the new
right thing to do).

I think checking for whether the python3 macros are defined is a good way
to make this a bit more dynamic for distros that don't install py3 by
default, but have the option to have it installed post-install.

Thanks for the feedback. I'm continuing to work on this today and clean it
up based on some additional documentation that I've found.

Leif.