Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong Python version invoked #2298

Open
edwardalee opened this issue May 29, 2024 · 15 comments
Open

Wrong Python version invoked #2298

edwardalee opened this issue May 29, 2024 · 15 comments
Assignees
Milestone

Comments

@edwardalee
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Member

lhstrh commented May 31, 2024

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

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.

@edwardalee
Copy link
Collaborator Author

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
Copy link
Member

lhstrh commented Jun 3, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

cmnrd commented Jun 5, 2024

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.

@DNDEMoto
Copy link

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
Copy link

DNDEMoto commented Aug 5, 2024

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.

@lhstrh
Copy link
Member

lhstrh commented Aug 5, 2024

@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
Copy link
Member

lhstrh commented Aug 5, 2024

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.

@DNDEMoto
Copy link

DNDEMoto commented Aug 6, 2024

@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
Copy link

DNDEMoto commented Aug 6, 2024

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.

@lhstrh
Copy link
Member

lhstrh commented Aug 6, 2024

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?

@DNDEMoto
Copy link

DNDEMoto commented Aug 6, 2024

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

@cmnrd
Copy link
Collaborator

cmnrd commented Aug 9, 2024

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 #2332 (comment))

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants