GNOME Bugzilla – Bug 691082
Support py2 and py3 loader in the build
Last modified: 2013-01-05 17:49:31 UTC
We need to build and install both loaders since some apps will use one and some the other
Created attachment 232653 [details] [review] patch
*** Bug 690350 has been marked as a duplicate of this bug. ***
Note: I have not tested yet, but I was happy enough to see the loader built and installed
Created attachment 232658 [details] [review] patch Updated patch, now make check passes. Note: I had to rename the testcase plugin files to "python3"... Maybe we can instead copy them and keep both py2 and py3 test? That would mean duplicate files, but I guess for tests it would be ok...
Created attachment 232659 [details] [review] patch Improved patch, it now generates both py2 and py3 test with the same trick of gjs/seed
Review of attachment 232659 [details] [review]: ::: configure.ac @@ +325,3 @@ + [enable_python2=auto]) + +found_python2=no should keep the old message "no (disabled, use ...)" @@ +331,3 @@ + AC_CHECK_PROGS(PYTHON2, [python2 python2.7 python2.6 python2.5]) + if test -z "${PYTHON2}"; then +found_python2=no should not error if enable_python2=auto @@ +336,3 @@ + AC_PATH_TOOL(PYTHON2_CONFIG, "python2-config") + if test -z "${PYTHON2_CONFIG}"; then + dnl NOTE: we do not use AM_PATH_PYTHON, since it does not allow same @@ +362,3 @@ + AC_PATH_TOOL(PYTHON3_CONFIG, "python3-config") + if test -z "${PYTHON3_CONFIG}"; then +found_python3=no should not error if enable_python3=auto ::: loaders/python/Makefile.am @@ +11,3 @@ + $(PYGOBJECT_CFLAGS) \ + $(PYTHON2_CFLAGS) \ + -DPEAS_PYEXECDIR=\""$(PYTHON_PYEXECDIR)"\" \ should be PYTHON2_PYEXECDIR ::: tests/libpeas/Makefile.am @@ +43,2 @@ endif Would be nice to have python 2 tests as well ::: tests/libpeas/extension-python.c @@ +119,2 @@ EXTENSION_TEST (python, "instance-refcount", instance_refcount); EXTENSION_TEST (python, "activatable-subject-refcount", activatable_subject_refcount); these should also be changed to python3 ::: tests/libpeas/plugins/extension-python/Makefile.am @@ +1,3 @@ include $(top_srcdir)/tests/Makefile.plugin +noinst_DATA = \ changing from noinst_PLUGIN to noinst_DATA will not work. noinst_PLUGIN is special see the included makefile @@ +25,3 @@ +EXTRA_DIST = \ + extension-py.gschema.xml \ + $(SED) -i -e 's%PY_LOADER%$(@:extension-%.gschema.xml=%)%g' $@ should not be .js ::: tests/libpeas/plugins/extension-python3-nonexistent.plugin @@ +1,2 @@ +[Plugin] +Module=extension-python-nonexistent python3 @@ +2,3 @@ +Module=extension-python-nonexistent +Loader=python3 +Name=Extension Python Nonexistent also python3
Created attachment 232670 [details] [review] patch Ok, now it should be ready. With respect to the previous version: - fix the py.js typo in the dist files - fix tests so that both py2 and py3 unit tests are executed
Created attachment 232671 [details] [review] patch This addresses all the issues pointed out by Garrett
*** Bug 691128 has been marked as a duplicate of this bug. ***
Created attachment 232795 [details] [review] prelim patch This preliminary patch prevents creating pyc files during tests, and makes our life easier
Created attachment 232796 [details] [review] patch Updated patch: it now passes distcheck
Created attachment 232797 [details] [review] prelim patch Preliminary patch that disables generating pyc
Created attachment 232798 [details] [review] patch Patch to support py2 and py3. Passes distcheck
Review of attachment 232798 [details] [review]: ::: .gitignore @@ +92,3 @@ +/tests/libpeas/plugins/extension-python/extension-python3.gschema.xml +/tests/libpeas/plugins/extension-python/extension-python3.py +/tests/libpeas/plugins/extension-python/extension-python3.plugin sort .plugin before .py ::: configure.ac @@ +357,3 @@ + ]) + ], [ + PYTHON2_CFLAGS=`${PYTHON2_CONFIG} --includes` should be config not found @@ +360,3 @@ + ]) + else + pyexecdir=`$PYTHON2 -c "import sys; from distutils import sysconfig; print sysconfig.get_python_lib(True, prefix='')"` should be version too old @@ +366,3 @@ +if test "x$enable_python2" = "xyes" -a "x$found_python2" != "xyes"; then + AC_MSG_ERROR([You need to have Python 2 and PyGobject installed to build libpeas]) PyGObject @@ +408,2 @@ +if test "x$enable_python3" = "xyes" -a "x$found_python3" != "xyes"; then + AC_MSG_ERROR([You need to have Python 3 and PyGobject installed to build libpeas]) PyGObject ::: loaders/python3/Makefile.am @@ +1,1 @@ +# Python plugin loader Python 3 plugin loader @@ +31,3 @@ + $(PYTHON3_LIBS) + +gcov_sources = $(libpython3loader_la_SOURCES) weird spacing after = ::: tests/libpeas/Makefile.am @@ +47,3 @@ +extension_python3_SOURCES = extension-python3.c +extension_python3_CFLAGS = $(PYGOBJECT_CFLAGS) $(PYTHON3_CFLAGS) + needs extra space after variable for correct alignment ::: tests/libpeas/plugins/extension-python/Makefile.am @@ +31,2 @@ +clean-local: + @rm -f gschemas.compiled; If we are going to remove gschemas.compiled like this then we should remove it from Makefile.plugin and add the same rule to extension-{c,js}/Makefile
Review of attachment 232797 [details] [review]: looks good
Review of attachment 232798 [details] [review]: ::: tests/libpeas/plugins/extension-python/Makefile.am @@ +31,2 @@ +clean-local: + @rm -f gschemas.compiled; I will just go back to pulling in Makefile.plugin even if I do not use noinst_PLUGIN. We can decide if we want to keep Makefile.plugin or not in an unrelated patch
Created attachment 232816 [details] [review] patch Fix the issues pointed out in the review
Created attachment 232830 [details] [review] patch Fixed a problem spotted by Garrett
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.