Sigh. And of course I had libopenvswitch installed on the system as
well and removing it breaks building the extensions with the above
patch. This is the kind of thing that would be much easier if the
Python lib was its own project with its own test suite and it could
just always assume libopenvswitch was installed on the system as a
build dependency.
There is much to say about why all of this is a pain, but the main
takeaway is that I'm going to split out the serialization part of this
patch from the build system stuff because there is no reason for one
part to hold up the other.
Terry
On Fri, Jul 22, 2016 at 9:57 PM, Terry Wilson <twil...@redhat.com> wrote:
> There is no particularly good reason to use our own Python JSON
> serialization implementation when serialization can be done faster
> with Python's built-in JSON library.
>
> A few tests were changed due to Python's default JSON library
> returning slightly more precise floating point numbers.
>
> Signed-off-by: Terry Wilson <twil...@redhat.com>
> ---
> configure.ac | 2 +
> m4/openvswitch.m4 | 47 ++++++++++++++++++++++++
> python/automake.mk | 27 +++++++++++++-
> python/ovs/json.py | 106
> +++++------------------------------------------------
> tests/json.at | 26 ++++++++++---
> 5 files changed, 104 insertions(+), 104 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 05d80d5..5472a52 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -96,6 +96,8 @@ OVS_CHECK_LIBCAPNG
> OVS_CHECK_LOGDIR
> OVS_CHECK_PYTHON
> OVS_CHECK_PYTHON3
> +OVS_CHECK_PYTHON_HEADERS
> +OVS_CHECK_PYTHON3_HEADERS
> OVS_CHECK_FLAKE8
> OVS_CHECK_DOT
> OVS_CHECK_IF_PACKET
> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
> index a448223..60ae114 100644
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -589,3 +589,50 @@ AC_DEFUN([OVS_CHECK_PRAGMA_MESSAGE],
> [AC_DEFINE(HAVE_PRAGMA_MESSAGE,1,[Define if compiler supports #pragma
> message directive])])
> ])
> +
> +dnl OVS_CHECK_PYTHON_HEADERS
> +AC_DEFUN([OVS_CHECK_PYTHON_HEADERS],
> + [AC_REQUIRE([OVS_CHECK_PYTHON])
> + AC_PATH_PROG([PYTHON_CONFIG], python-config, no)
> + if test "$PYTHON_CONFIG" != no; then
> + PYTHON_INCLUDES=`$PYTHON_CONFIG --includes`
> + PYTHON_LIBS=`$PYTHON_CONFIG --libs`
> + PYTHON_LDFLAGS=`$PYTHON_CONFIG --ldflags`
> + save_LIBS="$LIBS"
> + save_CPPFLAGS="$CPPFLAGS"
> + save_LDFLAGS="$LDFLAGS"
> + LIBS="$PYTHON_LIBS"
> + LDFLAGS="$PYTHON_LDFLAGS"
> + CPPFLAGS="$PYTHON_INCLUDES"
> + AC_LINK_IFELSE(
> + [AC_LANG_PROGRAM([#include <Python.h>],[])],
> + [have_py_headers=true])
> + LIBS="$save_LIBS"
> + CPPFLAGS="$save_CPPFLAGS"
> + LDFLAGS="$save_LDFLAGS"
> + fi
> + AM_CONDITIONAL([HAVE_PYTHON_HEADERS], [test "$have_py_headers" =
> "true"])])
> + ])
> +
> +AC_DEFUN([OVS_CHECK_PYTHON3_HEADERS],
> + [AC_REQUIRE([OVS_CHECK_PYTHON3])
> + AC_PATH_PROG([PYTHON3_CONFIG], python3-config, no)
> + if test "$PYTHON3_CONFIG" != no; then
> + PYTHON3_INCLUDES=`$PYTHON3_CONFIG --includes`
> + PYTHON3_LIBS=`$PYTHON3_CONFIG --libs`
> + PYTHON3_LDFLAGS=`$PYTHON3_CONFIG --ldflags`
> + save_LIBS="$LIBS"
> + save_CPPFLAGS="$CPPFLAGS"
> + save_LDFLAGS="$LDFLAGS"
> + LIBS="$PYTHON3_LIBS"
> + LDFLAGS="$PYTHON3_LDFLAGS"
> + CPPFLAGS="$PYTHON3_INCLUDES"
> + AC_LINK_IFELSE(
> + [AC_LANG_PROGRAM([#include <Python.h>],[])],
> + [have_py3_headers=true])
> + LIBS="$save_LIBS"
> + CPPFLAGS="$save_CPPFLAGS"
> + LDFLAGS="$save_LDFLAGS"
> + fi
> + AM_CONDITIONAL([HAVE_PYTHON3_HEADERS], [test "$have_py3_headers" =
> "true"])])
> + ])
> diff --git a/python/automake.mk b/python/automake.mk
> index ecad39d..874a178 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -79,10 +79,10 @@ ovs-install-data-local:
> rm python/ovs/dirs.py.tmp
>
> python-sdist: $(srcdir)/python/ovs/version.py $(ovs_pyfiles)
> python/ovs/dirs.py
> - (cd python/ && $(PYTHON) setup.py sdist)
> + cd $(srcdir)/python/ && $(PYTHON) setup.py sdist
>
> pypi-upload: $(srcdir)/python/ovs/version.py $(ovs_pyfiles)
> python/ovs/dirs.py
> - (cd python/ && $(PYTHON) setup.py sdist upload)
> + cd $(srcdir)/python/ && $(PYTHON) setup.py sdist upload
> else
> ovs-install-data-local:
> @:
> @@ -112,3 +112,26 @@ $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
> < $? > $@.tmp && \
> mv $@.tmp $@
> EXTRA_DIST += python/ovs/dirs.py.template
> +
> +.PHONY : clean_python_extensions
> +clean_python_extensions:
> + cd $(srcdir)/python/ && rm -f ovs/*.so
> +
> +SETUPPY_CFLAGS = -I$(abs_top_srcdir)/include -I$(abs_top_builddir)/include
> +if HAVE_PYTHON_HEADERS
> +$(srcdir)/python/ovs/_json.so: lib/libopenvswitch.la
> + cd $(srcdir)/python/ && CFLAGS="$(SETUPPY_CFLAGS)" $(PYTHON) setup.py
> build_ext --inplace
> +
> +ALL_LOCAL += $(srcdir)/python/ovs/_json.so
> +endif
> +
> +if HAVE_PYTHON3_HEADERS
> +PY3_EXT_NAME=$(srcdir)/python/ovs/_json$(shell $(PYTHON3) -c \
> + "from distutils import
> sysconfig;print(sysconfig.get_config_var('EXT_SUFFIX'))")
> +$(PY3_EXT_NAME): lib/libopenvswitch.la
> + cd $(srcdir)/python/ && CFLAGS="$(SETUPPY_CFLAGS)" $(PYTHON3)
> setup.py build_ext --inplace
> +
> +ALL_LOCAL += $(PY3_EXT_NAME)
> +endif
> +
> +CLEAN_LOCAL += clean_python_extensions
> diff --git a/python/ovs/json.py b/python/ovs/json.py
> index ea0400a..ddf5dd2 100644
> --- a/python/ovs/json.py
> +++ b/python/ovs/json.py
> @@ -12,11 +12,13 @@
> # See the License for the specific language governing permissions and
> # limitations under the License.
>
> +from __future__ import absolute_import
> +import functools
> +import json
> import re
> import sys
>
> import six
> -from six.moves import range
>
> try:
> import ovs._json
> @@ -25,112 +27,24 @@ except ImportError:
>
> __pychecker__ = 'no-stringiter'
>
> -escapes = {ord('"'): u"\\\"",
> - ord("\\"): u"\\\\",
> - ord("\b"): u"\\b",
> - ord("\f"): u"\\f",
> - ord("\n"): u"\\n",
> - ord("\r"): u"\\r",
> - ord("\t"): u"\\t"}
> -for esc in range(32):
> - if esc not in escapes:
> - escapes[esc] = u"\\u%04x" % esc
> -
> SPACES_PER_LEVEL = 2
> -
> -
> -class _Serializer(object):
> - def __init__(self, stream, pretty, sort_keys):
> - self.stream = stream
> - self.pretty = pretty
> - self.sort_keys = sort_keys
> - self.depth = 0
> -
> - def __serialize_string(self, s):
> - self.stream.write(u'"%s"' % ''.join(escapes.get(ord(c), c) for c in
> s))
> -
> - def __indent_line(self):
> - if self.pretty:
> - self.stream.write('\n')
> - self.stream.write(' ' * (SPACES_PER_LEVEL * self.depth))
> -
> - def serialize(self, obj):
> - if obj is None:
> - self.stream.write(u"null")
> - elif obj is False:
> - self.stream.write(u"false")
> - elif obj is True:
> - self.stream.write(u"true")
> - elif isinstance(obj, six.integer_types):
> - self.stream.write(u"%d" % obj)
> - elif isinstance(obj, float):
> - self.stream.write("%.15g" % obj)
> - elif isinstance(obj, six.text_type):
> - # unicode() on Python 2, or str() in Python 3 (always unicode)
> - self.__serialize_string(obj)
> - elif isinstance(obj, str):
> - # This is for Python 2, where this comes out to unicode(str()).
> - # For Python 3, it's str(str()), but it's harmless.
> - self.__serialize_string(six.text_type(obj))
> - elif isinstance(obj, dict):
> - self.stream.write(u"{")
> -
> - self.depth += 1
> - self.__indent_line()
> -
> - if self.sort_keys:
> - items = sorted(obj.items())
> - else:
> - items = six.iteritems(obj)
> - for i, (key, value) in enumerate(items):
> - if i > 0:
> - self.stream.write(u",")
> - self.__indent_line()
> - self.__serialize_string(six.text_type(key))
> - self.stream.write(u":")
> - if self.pretty:
> - self.stream.write(u' ')
> - self.serialize(value)
> -
> - self.stream.write(u"}")
> - self.depth -= 1
> - elif isinstance(obj, (list, tuple)):
> - self.stream.write(u"[")
> - self.depth += 1
> -
> - if obj:
> - self.__indent_line()
> -
> - for i, value in enumerate(obj):
> - if i > 0:
> - self.stream.write(u",")
> - self.__indent_line()
> - self.serialize(value)
> -
> - self.depth -= 1
> - self.stream.write(u"]")
> - else:
> - raise Exception("can't serialize %s as JSON" % obj)
> +dumper = functools.partial(json.dumps, separators=(",", ":"),
> + ensure_ascii=False)
>
>
> def to_stream(obj, stream, pretty=False, sort_keys=True):
> - _Serializer(stream, pretty, sort_keys).serialize(obj)
> + stream.write(dumper(obj, indent=SPACES_PER_LEVEL if pretty else None,
> + sort_keys=sort_keys))
>
>
> def to_file(obj, name, pretty=False, sort_keys=True):
> - stream = open(name, "w")
> - try:
> + with open(name, "w") as stream:
> to_stream(obj, stream, pretty, sort_keys)
> - finally:
> - stream.close()
>
>
> def to_string(obj, pretty=False, sort_keys=True):
> - output = six.StringIO()
> - to_stream(obj, output, pretty, sort_keys)
> - s = output.getvalue()
> - output.close()
> - return s
> + return dumper(obj, indent=SPACES_PER_LEVEL if pretty else None,
> + sort_keys=sort_keys)
>
>
> def from_stream(stream):
> diff --git a/tests/json.at b/tests/json.at
> index 32d7fff..ba7d4bb 100644
> --- a/tests/json.at
> +++ b/tests/json.at
> @@ -1,4 +1,4 @@
> -m4_define([JSON_CHECK_POSITIVE_C],
> +m4_define([JSON_CHECK_POSITIVE_C],
> [AT_SETUP([$1])
> AT_KEYWORDS([json positive])
> AT_CHECK([printf %s "AS_ESCAPE([$2])" > input])
> @@ -11,7 +11,7 @@ m4_define([JSON_CHECK_POSITIVE_C],
> # JSON_CHECK_POSITIVE_PY(TITLE, INPUT, OUTPUT, TEST-JSON-ARGS,
> # PYTHON-CHCEK, PYTHON-BIN)
> #
> -m4_define([JSON_CHECK_POSITIVE_PY],
> +m4_define([JSON_CHECK_POSITIVE_PY],
> [AT_SETUP([$1])
> AT_KEYWORDS([json positive Python])
> AT_SKIP_IF([test $5 = no])
> @@ -41,6 +41,12 @@ m4_define([JSON_CHECK_POSITIVE],
> JSON_CHECK_POSITIVE_PY([$1 - Python3], [$2], [$3], [$4],
> [$HAVE_PYTHON3], [$PYTHON3])])
>
> +m4_define([JSON_CHECK_POSITIVE_PY23],
> + [JSON_CHECK_POSITIVE_PY([$1 - Python2], [$2], [$3], [$4],
> + [$HAVE_PYTHON], [$PYTHON])
> + JSON_CHECK_POSITIVE_PY([$1 - Python3], [$2], [$3], [$4],
> + [$HAVE_PYTHON3], [$PYTHON3])])
> +
> m4_define([JSON_CHECK_NEGATIVE_C],
> [AT_SETUP([$1])
> AT_KEYWORDS([json negative])
> @@ -216,10 +222,14 @@ JSON_CHECK_POSITIVE(
> # It seems likely that the following test will fail on some system that
> # rounds slightly differently in arithmetic or in printf, but I'd like
> # to keep it this way until we run into such a system.
> -JSON_CHECK_POSITIVE(
> - [large integers that overflow to reals],
> +JSON_CHECK_POSITIVE_C(
> + [C - large integers that overflow to reals],
> [[[9223372036854775807000, -92233720368547758080000]]],
> [[[9.22337203685478e+21,-9.22337203685478e+22]]])
> +JSON_CHECK_POSITIVE_PY23(
> + [large integers that overflow to reals],
> + [[[9223372036854775807000, -92233720368547758080000]]],
> + [[[9.223372036854776e+21,-9.223372036854776e+22]]])
>
> JSON_CHECK_POSITIVE(
> [negative zero],
> @@ -237,10 +247,14 @@ JSON_CHECK_POSITIVE(
> # It seems likely that the following test will fail on some system that
> # rounds slightly differently in arithmetic or in printf, but I'd like
> # to keep it this way until we run into such a system.
> -JSON_CHECK_POSITIVE(
> - [+/- DBL_MAX],
> +JSON_CHECK_POSITIVE_C(
> + [C - +/- DBL_MAX],
> [[[1.7976931348623157e+308, -1.7976931348623157e+308]]],
> [[[1.79769313486232e+308,-1.79769313486232e+308]]])
> +JSON_CHECK_POSITIVE_PY23(
> + [+/- DBL_MAX],
> + [[[1.7976931348623157e+308, -1.7976931348623157e+308]]],
> + [[[1.7976931348623157e+308,-1.7976931348623157e+308]]])
>
> JSON_CHECK_POSITIVE(
> [negative reals],
> --
> 1.8.3.1
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
