[ovs-dev] [RFC 00/55] Add Python 3 support.

Russell Bryant russell at ovn.org
Tue Dec 22 00:29:41 UTC 2015


On Mon, Dec 21, 2015 at 6:46 PM, Ben Pfaff <blp at ovn.org> wrote:

> On Mon, Dec 21, 2015 at 06:20:42PM -0500, Russell Bryant wrote:
> > On Mon, Dec 21, 2015 at 5:14 PM, Ben Pfaff <blp at ovn.org> wrote:
> >
> > > On Sat, Dec 19, 2015 at 11:37:00AM -0500, Russell Bryant wrote:
> > > > $ make check TESTSUITEFLAGS="-k pep8"
> > > >
> > > > Another option would be to run this part at ovs build time, similar
> to a
> > > > number of other checks currently being done.
> > > >
> > > > $ grep 'ALL_LOCAL.*\-check' Makefile.am
> > > > ALL_LOCAL += config-h-check
> > > > ALL_LOCAL += printf-check
> > > > ALL_LOCAL += static-check
> > > > ALL_LOCAL += thread-safety-check
> > > > ALL_LOCAL += manpage-check
> > > >
> > > > Here's what that would look like.  I don't have a strong opinion on
> > > > whether it belongs just in 'make check' or at build time.
> > >
> > > How fast does it run?  If it's quick then I'd lean toward doing it at
> > > build time.
> >
> > On my laptop, it takes a little over 4 seconds the first time and a
> little
> > over 2 seconds for any subsequent run.  The first time it has a little
> more
> > work to do to set up a virtual python environment.
> >
> > The patch I have for it makes it only run once and then again only if
> > Python files change.  I can append it to v2 of this series if the time
> > seems reasonable.
>
> OK.  Given that it runs only when Python files change, that sounds
> reasonable.
>

A downside I just thought of is that the way it's written requires internet
access.  tox downloads packages from PyPI (the Python Package Index) to
create the test environment.  That would probably be annoying sometimes if
it was the only part of the build that requirement internet access.

The benefits of the way it works is that people don't have to install the
test requirements on their system just to run tests.  In this case, that's
just flake8, though.  The 'six' library dependency introduced in this patch
series has to be installed on the system since the library is used by
scripts in the build process.  It also ensures we're all using the same
version of the dependencies.  The same internet access requirement also
applies to the current patch that just runs it in 'make check'.

It's effectively only one dependency, though, so I could just change it so
you have to install flake8 on your system if you want the checks to run.  I
think I've just convinced myself that's better for this simple case.

-- 
Russell Bryant



More information about the dev mailing list