search

lf-lang / lingua-franca

Intuitive concurrent programming in any language
https://www.lf-lang.org
Other
220 stars 57 forks source link

Wrong Python version invoked #2298

Open edwardalee opened 1 month ago

edwardalee commented 1 month ago

Between lfc 0.7.0 and the current version (lfc 0.7.3-SNAPSHOT), something changed that results in the wrong Python being used. In version 0.7.0, lfc creates a script in bin that looks like this for me:

        /Users/edwardlee/.pyenv/shims/python3.10 /Users/edwardlee/git/playground-lingua-franca/examples/Python/src-gen/lib/Plotters/Plotters.py "$@"

This is the version of Python that is in my path:

~/git/playground-lingua-franca/examples/Python % which python
/Users/edwardlee/.pyenv/shims/python
~/git/playground-lingua-franca/examples/Python % python --version
Python 3.10.13

But in the current version, the script uses the built-in Mac version of Python:

        /Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/bin/python3.9 /Users/edwardlee/git/playground-lingua-franca/examples/Python/src-gen/lib/Plotters/Plotters.py "$@"

What changed and why? This breaks almost everything in the Python target.

edwardalee commented 1 month ago

The offending (merged) PR is this one #2292. It is a one-line change:

-            find_package(Python 3.10.0...<3.11.0 REQUIRED COMPONENTS Interpreter Development)
+           find_package(Python 3.9.0...<3.11.0 REQUIRED COMPONENTS Interpreter Development)

Unfortunately, this change causes CMake to reference the built-in Python on Mac, not the one in the path, which means virtual environments don't work anymore and you are stuck using the built-in Python. This change was put in to support the no-GIL Python, but I thought we had other reasons for requiring Python 3.10? @jackyk02 ? Suggestions?

lhstrh commented 4 weeks ago

I thought we had other reasons for requiring Python 3.10? @jackyk02 ? Suggestions?

Are these reasons documented anywhere? One thing we could do is add a workflow that runs the tests for Python 3.10 and see what happens. If the tests don't pass, we should revert https://github.com/lf-lang/lingua-franca/pull/2292. => https://github.com/lf-lang/lingua-franca/issues/2304

The problem you're running into is triggered by the change, but it's really because you apparently have Python 3.10 installed on your machine and CMake finds it, which is a different issue. Following your suggestion to not let CMake determine which version of Python to use, I created https://github.com/lf-lang/lingua-franca/issues/2299.

edwardalee commented 3 weeks ago

I thought we had other reasons for requiring Python 3.10? @jackyk02 ? Suggestions?

Are these reasons documented anywhere? One thing we could do is add a workflow that runs the tests for Python 3.10 and see what happens. If the tests don't pass, we should revert #2292. => #2304

Do you mean run the tests with Python 3.9?

The problem, however, is deeper. With CMake choosing the version of Python, it also bypasses any Python environments the user has set up. This means that Python programs will only work with globally installed packages, and lots of conflicts are likely to result.

The problem you're running into is triggered by the change, but it's really because you apparently have Python 3.10 installed on your machine and CMake finds it, which is a different issue. Following your suggestion to not let CMake determine which version of Python to use, I created #2299.

All MacOS systems come with a built-in Python. I'm not sure we should ask users to modify the MacOS system to be able to run LF's Python targets.

lhstrh commented 3 weeks ago

Are these reasons documented anywhere? One thing we could do is add a workflow that runs the tests for Python 3.10 and see what happens. If the tests don't pass, we should revert #2292. => #2304

Do you mean run the tests with Python 3.9?

Yes, that was a typo, but my question still stands.

The problem, however, is deeper. With CMake choosing the version of Python, it also bypasses any Python environments the user has set up. This means that Python programs will only work with globally installed packages, and lots of conflicts are likely to result.

AFAIK, the line that @jackyk02 changed tells CMake that it is free to choose any of the listed versions it can find. If that's not true or desired because some of them won't work, we need to either not list them of fix the associated problems. If, for whatever reason, we're not OK with just any supported version and need something more specific, then this mechanism generally isn't going to work (even though it may have worked by accident on your machine prior to Jacky's change).

edwardalee commented 3 weeks ago

I think that in the Python ecosystem, it is never ok to automatically just pick a Python and use that to execute a program. This bypasses virtual environments and makes it impossible to have a system with multiple versions of Python that are compatible with multiple Python applications. Which Python version you need to use to run an LF program will depend on the Python that you write in reaction bodies and on the modules that you depend on. So any strategy that just lets CMake pick whatever version of Python it wants is going to fail.

cmnrd commented 3 weeks ago

There is a plethora of ways to provide hints for cmake on where to look: https://cmake.org/cmake/help/latest/module/FindPython.html.

I would say the root cause of this issue is that we build outside of the Python ecosystem, so we have to guess the correct interpreter, no matter which build tool we use. Perhaps the solution would be to generate a Python package and use the python tools to invoke the Cmake build instructing it on the right Python version to use.