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]



Hi Lumir,

On 7 August 2018 at 13:33, Lumir Balhar <lbalhar@xxxxxxxxxx> wrote:
>
> Hello.
>
>
>
> On 08/07/2018 07:07 PM, Simon Glass wrote:
>>
>> Hi Lumir,
>>
>> On 6 August 2018 at 00:24, Lumir Balhar <lbalhar@xxxxxxxxxx> wrote:
>>>
>>> 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.
>>>
>> It should be possible to build the modules one after the other,
>> though, which is what we are asking.
>>
>> In other words, can it handle both Python 2 and 3?
>
> I think that yes. We can change the move command on line 17 of Makefile.pylibfdt from
>
>     mv _libfdt.*so $@
>
> to
>
>     mv _libfdt.*so $(PYLIBFDT_objdir)/
>
> which causes that Python 2 and Python 3 build results will have a different names. For example
>
> $ ls -1 pylibfdt/*.so
> pylibfdt/_libfdt.cpython-36m-x86_64-linux-gnu.so
> pylibfdt/_libfdt.so
>
> Which is completely correct and it works for both Pythons but I was afraid that the module name is for some reason important for you and I cannot change it.
>
> When the modules have different names, we can automatically use the proper Python version for tests and if both modules will be available, we can run tests twice.
>
> Is that the correct description of what you want to implement?

I think that is correct. The libfdt.py name is important, but I
presume that the .so will be found automatically regardless of its
name?

David, any more thoughts on this one?

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