lingua-franca icon indicating copy to clipboard operation
lingua-franca copied to clipboard

Wrong Python version invoked

Open edwardalee opened this issue 1 year ago • 15 comments

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 avatar May 29 '24 07:05 edwardalee

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?

edwardalee avatar May 29 '24 08:05 edwardalee

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.

lhstrh avatar May 31 '24 18:05 lhstrh

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.

edwardalee avatar Jun 01 '24 06:06 edwardalee

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).

lhstrh avatar Jun 03 '24 06:06 lhstrh

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.

edwardalee avatar Jun 03 '24 06:06 edwardalee

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.

cmnrd avatar Jun 05 '24 20:06 cmnrd

Since I'm using python venv, it would be helpful to be able to specify which binary to use as well as the version.

like this:

target Python {
    python-path: ".venv/bin/python"
}

DNDEMoto avatar Jul 29 '24 14:07 DNDEMoto

I installed pyenv in my environment today. and, I specified local python version by using pyenv. and I installed some libraries in local by using venv.

Then, LF(or should I say cmame) interpreted correctlly python path.( to venv)! This may not be an LF issue, maybe cmake can't discover the correct path when using venv only.

DNDEMoto avatar Aug 05 '24 12:08 DNDEMoto

@DNDEMoto, thanks for the follow up. Are you saying that the python version in your virtual environment is or is not correctly detected by CMake when compiling with the latest LF in master?

lhstrh avatar Aug 05 '24 18:08 lhstrh

I'm not sure what the default value of Python_FIND_VIRTUALENV is, but if it is FIRST, then it should work out of the box... Based on earlier reports from @edwardalee, I suspect that it might be STANDARD, however.

lhstrh avatar Aug 05 '24 18:08 lhstrh

@DNDEMoto, thanks for the follow up. Are you saying that the python version in your virtual environment is or is not correctly detected by CMake when compiling with the latest LF in master?

  • When using venv + LF(0.8.1), it referred to the global Python. (not correct)
  • When using pyenv + venv + LF(0.8.2), it referred to Python in venv (correct)

I will try venv + LF0.8.2 and report back!

DNDEMoto avatar Aug 06 '24 06:08 DNDEMoto

I tried venv + LF0.8.2 and it correctly references local python!

My first comment may have been my mistake. I sorry for the confusion.

DNDEMoto avatar Aug 06 '24 08:08 DNDEMoto

Thanks, @DNDEMoto, but I'm now confused as to what the problem is, because this discussion started because it wasn't working for @edwardalee (on macOS). What operating system are you using, @DNDEMoto?

lhstrh avatar Aug 06 '24 15:08 lhstrh

  • venv + LF(0.8.1), it referred to the global Python. (not correct)
    • Ubuntu ??? on WSL2
    • cmake ???

-> I'm sorry, I cannot check, because I deleted this machine.

  • pyenv + venv + LF(0.8.2), it referred to Python in venv (correct)
    • Debian 12 on WSL2
    • Cmake 3.25.1

-> I installed pyenv because Debian's default python is 3.11 and noticed that this problem does not occur.

  • venv + LF0.8.2, it referred to Python in venv (correct)
    • Ubuntu 22.04 on WSL2
    • Cmake 3.30.2

-> This is the environment I tried today.

DNDEMoto avatar Aug 06 '24 16:08 DNDEMoto

~Ok, this suggests though that this is only fixed for you because of the Python version we war looking for, not because cmake prefers the venv.~

Ok, nevermind, it seems to work file actually (see https://github.com/lf-lang/lingua-franca/pull/2332#issuecomment-2277617812)

I'm not sure what the default value of Python_FIND_VIRTUALENV is, but if it is FIRST, then it should work out of the box... Based on earlier reports from @edwardalee, I suspect that it might be STANDARD, however.

As I understand, the default is STANDARD and we should set it to FIRST.

cmnrd avatar Aug 09 '24 09:08 cmnrd