Re: [PATCH 1/5] pylibfdt: Allow switch to Python 3 via environment variable PYTHON

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hello.

On 07/16/2018 01:33 AM, Simon Glass wrote:
Hi,

On 12 July 2018 at 18:54, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
On Thu, Jul 12, 2018 at 04:10:41PM +0200, frenzy@xxxxxxxxx wrote:
From: Lumir Balhar <lbalhar@xxxxxxxxxx>

Python 2 is still the default but it can be changed by
setting environment variable PYTHON before build/test.

Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx>
As I noted on an earlier version, I'd like to get Simon Glass's review
before merging these, since the Python bindings are his work.
However, a few preliminary comments from me.

---
  Makefile                   | 3 ++-
  pylibfdt/Makefile.pylibfdt | 4 ++--
  tests/Makefile.tests       | 6 +++---
  tests/run_tests.sh         | 2 +-
  4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 6d55e13..c84eb66 100644
--- a/Makefile
+++ b/Makefile
@@ -24,6 +24,7 @@ BISON = bison
  LEX = flex
  SWIG = swig
  PKG_CONFIG ?= pkg-config
+PYTHON ?= python2

  INSTALL = /usr/bin/install
  INSTALL_PROGRAM = $(INSTALL)
@@ -133,7 +134,7 @@ all: $(BIN) libfdt
  # We need both Python and swig to build/install pylibfdt.
  # This builds the given make ${target} if those deps are found.
  check_python_deps = \
-     if $(PKG_CONFIG) --cflags python2 >/dev/null 2>&1; then \
+     if $(PKG_CONFIG) --cflags $(PYTHON) >/dev/null 2>&1; then \
               if which swig >/dev/null 2>&1; then \
                       can_build=yes; \
               fi; \
diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 9507d3d..a05984c 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -8,13 +8,13 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
  define run_setup
       SOURCES="$(1)" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)"
       VERSION="$(dtc_version)"
-     $(PYLIBFDT_objdir)/setup.py --quiet $(2)
+     $(PYTHON) $(PYLIBFDT_objdir)/setup.py --quiet $(2)
  endef
Using the environment variable to select the default python for
install is fine, but..

  $(PYMODULE): $(PYLIBFDT_srcs)
       @$(VECHO) PYMOD $@
       $(call run_setup, $^, build_ext --inplace)
-     mv _libfdt.so $@
+     mv _libfdt.*so $@

  install_pylibfdt: $(PYMODULE)
       $(VECHO) INSTALL-PYLIB; \
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 2c2c4fd..554c840 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -76,13 +76,13 @@ tests_clean:
       rm -f $(TESTS_CLEANFILES)

  check:       tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
-     cd $(TESTS_PREFIX); ./run_tests.sh
+     cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh

  checkm: tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
-     cd $(TESTS_PREFIX); ./run_tests.sh -m 2>&1 | tee vglog.$$$$
+     cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh -m 2>&1 | tee vglog.$$$$
..for testing, I'd be much happied if we ran the pylibfdt tests under
*both* Python2 and Python3 if available.  Makes it much likelier that
we'll catch breakages for one or the other early.
I definitely agree with that, it is pretty important.
I am not sure how this can be done without breaking backward compatibility. I have to say that I am not dtc nor libfdt expert so I might be missing something.

AFAIK it's not possible to build the extension module for both Pythons in the same time. In Makefile.pylibfdt on line 17, there is a `mv` command which renames built extension to standard name pylibfdt/_libfdt.so. When we build it again with a different Python version, the previous one will be replaced here.

I can try to prepare some code which will handle whether use Python 2 or Python 3 (or both) for testing based on extension file name but it will mean that we cannot rename built extension file.

Other than that it looks fine.

Regards,
Simon

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux