[TW#20820] CMake: menuconfig python executable should be used in cmake

[kylehendricks]

Environment

• IDF version (git rev-parse --short HEAD to get the commit id.):
  48c3ad3
• Development Env: CMake
• Operating System: Arch

Problem Description

When the python 2 executable isn't python, idf.py build fails even if the python executable was specified correctly in menuconfig.

Expected Behavior

idf.py build runs without error.

Actual Behavior

idf.py build errors.

[2/2] Generating partition-table.bin
FAILED: partition_table/partition-table.bin 
cd /home/khendricks/dev/esp32/hello_world/build/partition_table && python /home/khendricks/dev/esp32/tools/esp-idf-cmake/components/partition_table/gen_esp32part.py -q --disable-md5sum --flash-size 2MB /home/khendricks/dev/esp32/tools/esp-idf-cmake/components/partition_table/partitions_singleapp.csv partition-table.bin
Traceback (most recent call last):
  File "/home/khendricks/dev/esp32/tools/esp-idf-cmake/components/partition_table/gen_esp32part.py", line 423, in <module>
    main()
  File "/home/khendricks/dev/esp32/tools/esp-idf-cmake/components/partition_table/gen_esp32part.py", line 395, in main
    table_size = table.flash_size()
  File "/home/khendricks/dev/esp32/tools/esp-idf-cmake/components/partition_table/gen_esp32part.py", line 122, in flash_size
    last = sorted(self, reverse=True)[0]
TypeError: '<' not supported between instances of 'PartitionDefinition' and 'PartitionDefinition'
ninja: build stopped: subcommand failed.
ninja failed with exit code 1

Steps to reproduce

1. Be in an environment where python === python 3 and python2 === python 2.
2. Run idf.py menuconfig and set the python executable to python2
3. Run idf.py build and the above error is thrown.

If I change the two instances of ${PYTHON} in components/partition_table/CMakeLists.txt to ${CONFIG_PYTHON} everything works. I wasn't sure if this was the right fix.

[projectgus]

Thanks for reporting this @kylehendricks .

You are totally right that the CMake build system ignores any Python interpreter specified in menuconfig, and that this is a bug.

At the moment, idf.py assumes that whatever Python interpreter it runs with can be passed to the rest of the Python instances run by CMake, and CMake ignores the value in menuconfig. (This is clearly not great.)

An alternative workaround is to run python2 idf.py build or to change the shebang line at the top of idf.py to be #!/usr/bin/env/python2. No other changes should be required at that point.

What we will aim for instead of any of these workarounds is to make the final changes necessary for Python 2 & 3 support in IDF, so systems with default Python 3 can use it as-is without any problems.

(By the way, just to share for the record, I use Arch as well and I've worked around this since early ESP-IDF by creating a Python 2 virtualenv and then putting an .envrc file in my ~/esp directory which automatically activates it, so /usr/bin/env python resolves to the virtualenv's Python 2 interpreter. This is too fiddly to be a recommended solution, but it works OK for me.)

[kylehendricks]

If I'm not mistaken, the `PYTHON` cmake var is hardcoded to `python` using the set_default function. So I think that inheriting idf.py's python interpreter may also be broken. Edit: Here's where it hardcodes the python interpreter. https://github.com/espressif/esp-idf/blob/feature/cmake/tools/cmake/project.cmake#L29

…

On Mon, May 7, 2018, 05:41 Angus Gratton ***@***.***> wrote: Thanks for reporting this @kylehendricks <https://github.com/kylehendricks> . You are totally right that the CMake build system ignores any Python interpreter specified in menuconfig, and that this is a bug. At the moment, idf.py assumes that whatever Python interpreter it runs with can be passed to the rest of the Python instances run by CMake, and CMake ignores the value in menuconfig. (This is clearly not great.) An alternative workaround is to run python2 idf.py build or to change the shebang line at the top of idf.py to be #!/usr/bin/env/python2. No other changes should be required at that point. What we will aim for instead of any of these workarounds is to make the final changes necessary for Python 2 & 3 support in IDF, so systems with default Python 3 can use it as-is without any problems. (By the way, just to share for the record, I use Arch as well and I've worked around this for a while now by creating a Python 2 virtualenv <https://virtualenv.pypa.io/en/stable/userguide/> and then putting an .envrc file in my ~esp directory <https://direnv.net/> which automatically activates it, so /usr/bin/env python resolves to the virtualenv's Python 2 interpreter. This is too fiddly to be a recommended solution, but it works OK for me.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1924 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA77xhcyTncvXz_AyNlCF-_MXyZeRMVbks5twBbdgaJpZM4Tzc2f> .

[projectgus]

idf.py sets a PYTHON environment variable to its own Python interpreter path:
https://github.com/espressif/esp-idf/blob/feature/cmake/tools/idf.py#L45

and set_default() will use an environment variable of the same name if one is set:
https://github.com/espressif/esp-idf/blob/feature/cmake/tools/cmake/utilities.cmake#L3

So it's not hardcoded in that case. But this all still needs a rethink. :)

[projectgus]

Oh, set_default() appears to broken for environment variables. So you're right. Sorry!

[projectgus]

Hi @kylehendricks , this should be taken care of in the CMake branch now (the Python Interpreter option is removed from menuconfig - whichever interpreter you use to run idf.py is used, or PYTHON environment variable if not using idf.py.). Please let us know if that's not the case.

We are planning Python 2+3 support for IDF v4.0 as well, which will hopefully make this issue less commonly encountered (default Python should usually be a safe choice, then).