[scons] Python 3 support for SConscripts and related tools

[mhthies]

SCons 3.0.0 has been released a few days ago and it features support for Python 3.5+, without dropping Python 2.7 support. [1] Since the update, some Linux distributions (such as Arch Linux) default to the Python 3 interpreter to run scons. This breaks the xpcc build process, as the SConscript files and some other Python files, which are imported by them, are written in old Python 2 syntax. (See #293)

I tried to fix the scripts using current Python syntax and compatibility packages, that should be compatible with at least Python 2.7 and Python 3.6. Depending on the system environment, it might be necessary to install the configparser package for Python 2 and future package for the Python interpreter in use. The only minor drawback for Python 2-users might be a slightly poorer performance of some scripts, due the the use of dict.items() instead of dict.iteritems().

Though I also updated some seperate scripts, not required by the SConscripts, I didn't test them with both Python interpreters yet. Indeed, I did only test building the RCA's robot software; so there still might be problems with some scripts.

[1] http://scons.org/scons-300-is-available.html

[ekiwi]

scons 3 should also be provided by Fedora 27: https://bodhi.fedoraproject.org/updates/FEDORA-2017-0c9b229e88

[strongly-typed]

I already stumbled upon these problems. Could you please verify that all commands from the TravisCI (scons "check=examples examples=stm32f4_discovery") work correctly? I have problems with scons 3.0 in CircleCI just doing nothing. Working on it.

Yes, the weather already felt like November++ today ...

[mhthies]

Update: The examples start compiling now with SCons 3.0.0/Python 3.6, but there seem to be some more problems, including some Python errors, other build system problems:

Warn: 'driver/timer/atmega' implementation for target 'atmega328p' does not exist!

(and similar) and some C++ errors, I don't understand yet:

/home/michael/Documents/Freizeit/RCA/software/xpcc/src/xpcc/math/geometry/location_2d_impl.hpp:169:51: error: call of overloaded 'abs(float)' is ambiguous

I'll do more investigations in the next days.

[salkinium]

Thanks for fixing this!

Warn: 'driver/timer/atmega' implementation for target 'atmega328p' does not exist!

This just says that even though the device file specifies the atmega328p to have a timer, no such implementation was found. It was supposed to be a reminder to add that. However, the model has changed completely in MODM, so can just ignore (or remove) this warning.

[salkinium]

Related upstream issue for the SCons tools we use for MODM.

[mhthies]

On my machine all the xpcc examples and unittests are building now with SCons 2.5 and 3.0 without errors, except for the above mentioned warnings and c++ errors. However, the latter seems to be a problem of compiling with GCC 7.2 for AVR, which is unrelated to SCons.

[ekiwi]

I also tested it on my (Fedora 27) machine with scons 3. scons check mostly works.

One of the tricky things is that you need jinja2 installed for python2 as well as python3 because the communication code is still running under python2.

[strongly-typed]

With CircleCI the job scons "check=examples examples=stm32f4_discovery" is still empty.

#!/bin/bash -eo pipefail
scons "check=examples examples=stm32f4_discovery"
scons: Reading SConscript files ...

RESULTS:



OK!

Can anyone reproduce this?

[ekiwi]

With CircleCI the job scons "check=examples examples=stm32f4_discovery" is still empty.

Can you try running scons check=examples examples=stm32f4_discovery (i.e., without the quotation marks)

[strongly-typed]

At least, SCons now finds something to compile. I am not sure if the UnicodeDecodeError is related to CircleCI/Ubuntu/SCons or to the changes introduced here (decode() instead of maybe decode('utf-8')). So sorry for hijacking this PR with something maybe completely unrelated.

#!/bin/bash -eo pipefail
export SCONS_LIB_DIR=/usr/local/lib/python2.7/dist-packages/scons-2.5.1;
scons check=examples examples=stm32f4_discovery
scons: Reading SConscript files ...
scons: Entering directory `/root/project/examples/stm32f4_discovery/radio/nrf24-phy-test'
Template: '/root/project/src/xpcc/architecture/platform/platform.hpp.in' to '/root/project/build/stm32f4_discovery/radio/nrf24-phy-test/libxpcc/src/xpcc/architecture/platform.hpp'
Template: '/root/project/src/xpcc/architecture/platform/drivers.hpp.in' to '/root/project/build/stm32f4_discovery/radio/nrf24-phy-test/libxpcc/generated_platform/drivers.hpp'
scons: *** [/root/project/build/stm32f4_discovery/radio/nrf24-phy-test/main.o] UnicodeDecodeError : 'ascii' codec can't decode byte 0xe2 in position 2545: ordinal not in range(128)

[mhthies]

I am not sure if the UnicodeDecodeError is related to CircleCI/Ubuntu/SCons or to the changes introduced here (decode() instead of maybe decode('utf-8'))

I would bet, this error is related to my changes. (Though, I'm wondering, why it doesn't print a complete stacktrace of the Python Exception in the CircleCI output.)

I thought, it was a nice idea to update the scripts to use unicode strings instead of bytearrays, which is the native string representation in Python 3 but a bit hacky in Python 2. Later, I discovered that the decode() method defaults to decoding from ASCII in Python 2 and UTF-8 in Python 3. Actually the best choice would be using the individual system's default character encoding for subprocess calls. My first guess was, it could be fixed using universal_newlines=True, but this doesn't imply character decoding in Python 2. Moreover I missed a few decode()-occurrences. So. Next try: Using locale.getpreferredencoding() for subprocesses and UTF-8 for files.

[strongly-typed]

I just rebased and gave it another try on CircleCI but it failed :-(

[mhthies]

What is the locale in your CircleCI environment? Precisely, what is the value of the LANG environment variable?

I can reproduce the error when setting LANG="C" with both, SCons 2.5.1 and 3.0.0. From that I assume, g++ outputs some unicode characters, even if the system locale is set to "C", which – of course – cannot be decoded by Python from ASCII.

[strongly-typed]

I'll add an env to the CircleCI. Hold tight.

[strongly-typed]

Seems to me that LANG is not set at all.

[strongly-typed]

Sadly, setting the LANG to en_US.UTF-8 does not help..

[mhthies]

Just tested a bit on my machine (where I have generated en_US.utf8 but not en_GB.utf8):

$ LANG=C python -c "import locale; print(locale.getpreferredencoding())"
ANSI_X3.4-1968
$ LANG=en_US.UTF-8 python -c "import locale; print(locale.getpreferredencoding())"
UTF-8
$ LANG=en_GB.UTF-8 python -c "import locale; print(locale.getpreferredencoding())"
ANSI_X3.4-1968

Presumably the locale must be actually available for Python to detect the correct encoding.

[strongly-typed]

Trying to add locale and locale-gen to Docker container.

[mhthies]

If this turns out to be too instable or too much overhead, we can still step back to hard-coding UTF-8 decoding or even handling raw bytearrays (and not doing any decoding in python at all).

[strongly-typed]

It seems to run now. Let's see.It now works.

Status reporting of CircleCI workflow does not yet work, so it will be enabled later.

Great work!

[strongly-typed]

@mhthies Could you please rebase onto the latest develop branch and maybe cherry-pick cd8c851 to enable CircleCI for this PR?

@salkinium CircleCI then should run without a problem.

@salkinium Do you suggest any rework of these commits?

[salkinium]

I believe #68 needs to be reverted too.

[salkinium]

This is kinda ugly. Is there no execfile in Python 3?

[mhthies]

Nope, no execfile in Python 3. This is the official workaround, see https://docs.python.org/3.0/whatsnew/3.0.html#builtins

[mhthies]

One thing left to do would be changing the Python interpreter in the shebang lines of all scripts and some documentation strings from 'python2' to 'python', so each system's default Python interpreter is used and we do not depend on python2 being installed anymore.

Before doing this: Could anyone test all the utility scripts (that are not included or called in the build process) for Python 3 compatibility?

[ekiwi]

One thing left to do would be changing the Python interpreter in the shebang lines of all scripts and some documentation strings from 'python2' to 'python', so each system's default Python interpreter is used and we do not depend on python2 being installed anymore.

Can't we just change them to use python3? While scons 3 is still new and not (easily) available on every system, python3 has been out for > 10 years and thus we should be able to rely on it being available.

Not having to maintain backwards compatibility to python2 makes development and maintenance a lot easier.

Within a year or two, once scons 3 is available on Ubuntu, we can switch everything over to using python3.

[strongly-typed]

I'm working on it here. I added the scripts to the deploy stage in TravisCI and they now both work with Python2.7 and Python3.

I opt for deleting tools/bitmap/pbm2c.py. What about the tools/bootloader stuff?

I'll try to get in running in CircleCI as well.

[salkinium]

I opt for deleting tools/bitmap/pbm2c.py.

Isn't that used to convert bitmap formats?

What about the tools/bootloader stuff?

Removed in modm too. Kill it.

[strongly-typed]

I opt for deleting tools/bitmap/pbm2c.py

Isn't that used to convert bitmap formats?

I can't see when and where it might be called. I only found scons/site_tools/bitmap.py.

[strongly-typed]

I setup CircleCI and TravisCI to test with Python2 and Python3 here.

@mhthies Could you cherry-pick this to your branch and check if this runs smoothly in TravisCI and CircleCI?

I'm still struggling with the "🎉🎊" from the tools/authors.py to make it run on CircleCI in Python2 and Python3.

[salkinium]

Any update on this?

[mhthies]

Sorry, I've been into other projects for the last few weeks.

Now, this branch is rebased to the current develop and supplemented with the two comments by @strongly-typed. I already fixed the missing LANG environment of the python script tests.

As soon as all the ci tests are running successfully, I would change all the Shebangs and readme files from 'python2' to 'python3', so we can drop support for Python2 when necessary and not required by scons 2, as discussed by @ekiwi.

[ekiwi]

There are in general two classes of python files in xpcc:

1. files that are directly included in the scons scripts
2. files that are called as external tools from scons

The first category needs to work with python 2+3 and thus should probably be marked #!/usr/bin/env python. The second category is going to be python 3 only and thus #!/usr/bin/env python3.

Sorry for the build system being so confusing @mhthies.

[mhthies]

The first category needs to work with python 2+3 and thus should probably be marked #!/usr/bin/env python. The second category is going to be python 3 only and thus #!/usr/bin/env python3.

I considered differentiating between those two groups of scripts, but decided to change all of them to #!/usr/bin/env python3.

The Shebang is only useful for scripts that may be called directly from the commandline and has no effect when the file is included from another python script, e.g. a SConscript. Thus, for those Python scripts that are only included in the scons scripts, we can declare whatever interpreter we want, or even omit this line (which, however, could confuse file managers and editors about the file type). With the Shebang line beeing #!/usr/bin/env python, a shell, starting the script, would call the system's default Python interpreter, which currently varies between Python 2.7 and Python 3.x depending on the os and distribution – so we could not make any assumptions about the Python environment.

With this in mind, I decided for /usr/bin/env python3 for all scripts, as it proposes using the Python 3 interpreter (whenever calling the script directly) but has no effect on scons 2.x including the file from a Python 2 instance.

[salkinium]

I can't really judge the state of this feature, is it finished? If not, what's missing?

[mhthies]

From my side, it's finished. We should probably squash the commits and separate pure compatibility fixes, breaking changes and CI config things.

Breaking changes are (as far as I'm aware):

• Changing the default interpreter of the XPCC system builder to python3.
• Changing the string handling of the SConscripts and included scripts from raw bytes to Unicode-parsing. (This is, of course, also part of the Python 3 compatibility changes).

Maybe @ekiwi wants to comment on my thoughts about the Shebang above?

[salkinium]

Ok, then I'll merge this latest in the first week of January for the Q4 release, until then feel free to rebase your changes as you like. And poke whoever you need to review this.

[salkinium]

Thanks for doing this work!

[salkinium]

Is this supposed to be duplicated?

[mhthies]

Yep, as the commit message says, we want to test all scripts with Python 2 and Python 3.

[salkinium]

Ah, I didn’t see the difference there 🤦‍♂️