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

tools/makemanifest.py: Use sys.executable when invoking Python scripts. #5311

Closed
wants to merge 1 commit into from

Conversation

Jongy
Copy link
Contributor

@Jongy Jongy commented Nov 9, 2019

Clean Ubuntu 18.04 doesn't even have a "python" program preinstalled, only "python3". So even if this script is py2 compatible, it won't run without additional installations.

Since everything in MicroPython is guaranteed to be py3 compatible, I think it's okay to explicitly state it whenever possible.

@dpgeorge
Copy link
Member

This script was originally invoked via the makefile like $(PYTHON) make-frozen.py, so the Python version could be set globally using the PYTHON variable. But now this script is invoked via makemanifest.py. We could make makemanifest.py use the value of PYTHON to run these scripts ... but that's a makefile variable and doesn't seem available to makemanifest.py. Instead could use the Python interpreter that makemanifest.py itself was invoked with. That way the user can still use Python 2.

@Jongy
Copy link
Contributor Author

Jongy commented Nov 11, 2019

Instead could use the Python interpreter that makemanifest.py itself was invoked with. That way the user can still use Python 2

Yeah I guess sys.executable should be used. It's just too much of a diff so I was trying to avoid it :)

The errors I get for a missing python binary are quite unclear: error freezing strings []: b'/usr/bin/env: \xe2\x80\x98python\xe2\x80\x99: No such file or directory\n' so I'd like to solve that.

@Jongy
Copy link
Contributor Author

Jongy commented Nov 11, 2019

It's just too much of a diff

Well, I dunno why I thought this way.

@Jongy Jongy changed the title tools/make-frozen.py: Run with python3 explicitly. tools/makemanifest.py: Use sys.executable when invoking Python scripts. Nov 11, 2019
@dpgeorge
Copy link
Member

Great! Merged in b2dd443

@dpgeorge dpgeorge closed this Nov 13, 2019
@Jongy Jongy deleted the tools-makefrozen-py3 branch November 29, 2019 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants