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

Launcher for Windows #103

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

BitRolher
Copy link

Discovers the path of pico-sdk-tools
Runs pico-env.cmd to prepare the environment
Uses py.exe to find the path to Python
Runs pico_project with gui

Discovers the path of pico-sdk-tools
Runs pico-env.cmd to prepare the environment
Uses py.exe to find the path to Python
Runs pico_project with gui
@lurch
Copy link
Contributor

lurch commented Dec 10, 2023

Does this "fix" #88 ?
ping also @ndabas

@BitRolher
Copy link
Author

Remark: It uses "cmd" and not Powershell as in #88, but the issues and workarounds are the same. So:
-It "fixes" the "Python was not found" error which occurs when using "py" to launch
-It "fixes" the "tkinter module not found" error when using "python" to launch
-No fix: "CMake Error", but the environment is correctly set and same as in "Pico - Developer Command Prompt" (double checked with "Process Hacker 2"


rem https://stackoverflow.com/questions/22352793/reading-a-registry-value-to-a-batch-variable-handling-spaces-in-value
for /f "usebackq tokens=2,*" %%h in (
`"reg query "HKLM\SOFTWARE\Kitware\CMake\Packages\pico-sdk-tools" 2>NUL | find /i "SDK""`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest that you check the uninstall entry to detect the path of pico-setup-windows, e.g. HKLM\Software\Microsoft\Windows\CurrentVersion\Uninstall\Raspberry Pi Pico SDK v*, value InstallPath.

I guess it'll be harder to enumerate keys to find the latest version this way, but maybe consider a PowerShell snippet here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I have changed the script accordingly. I can share if it is of interest.
PowerShell would be nice. I will consider changing the complete script to PowerShell instead of using just a snippet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are interested in the PowerShell version of the launcher: It is available here:
https://github.com/BitRolher/pico-project-generator/tree/BitRolher-WinLauncherPSversion

Comment on lines 19 to 23
REM https://blog.finxter.com/how-to-find-path-where-python-is-installed-on-windows/
py.exe -c "import os, sys; print(os.path.dirname(sys.executable))" > lspver.tmp
set /p ls-pypath= < lspver.tmp
del lspver.tmp
call "%ls-pypath%\python" pico_project.py --gui
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just py.exe pico_project.py --gui?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just using py.exe pico_project.py --gui shows "Python was not found".
At least if I don´t edit the "Manage app execution aliases"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the Microsoft docs, when called with arguments, the app execution aliases will return the error about Python not being found. Which means that py.exe -c "..." should also return an error unless a Python installer has added the 'real' py/python executables -- so I'm not sure if this workaround to discover the path will work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, but I can confirm that it is working on Windows 10 22H2. And so does the similar variant in the enhanced script. The new version does not need the lspver.tmp file.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they fixed the Python for Windows installer at some point to set the 'real' python executables as higher priority than the Windows store aliases.

So my own understanding is:

  • If you have a Python distribution installed, what should happen is that python.exe and py.exe point to that distribution.
  • However, on some older versions (or some specific configurations/versions of Windows), this doesn't happen -- but py.exe is still able to find the actual distribution and run it, but only if run without arguments.
  • So, I believe, any workaround that tries to extract the path by running a script with py.exe will not work, because if py.exe works to execute a script, it would have worked to directly launch our application as well, and does not handle the case where py.exe is the alias rather than the real one. (Or, run it without arguments, but pipe the script in instead of providing it as an argument.)

I've also noticed that there is no alias for pythonw.exe -- so maybe the solution is to use that instead? We'll still need to find the pythonw.exe from a full installation of Python rather than the embedded one in pico-setup-windows though, or cache the path to it before setting up the pico environment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLDR: Shall we use an own implementation of python path discovery or use py?

Important note: "Add python.exe to PATH" is not default and I did not use this option before.
So, following is happening on my Windows 10 system with Python 3.124.1 (64-bit) and Python Launcher installed:
py.exe pico_project.py --gui
Message: Python was not found

py.exe -c "import os, sys; print('Python Path ' + os.path.dirname(sys.executable))"
Message: Python Path C:\Program Files\Python312
(This is the full installation of Python and not the embedded one. So this detection is done correctly)

But it seems that you are expecting to use the non-default option "Add python.exe to PATH".
Normally, I keep everything as close to standard as possible. But in this case it cannot get much worse compared to the strange Windows store alias thing.

So, I did:
-Uninstall Python
-Reinstall python-3.12.1-amd64.exe with following options (I will only note what´s worth mentioning):
-Add python.exe to PATH
-Use admin privileges when installing py.exe
-Install Python 3.12 for all users
-Associate files with Python
-Disable path length limit

Afterwards checked:
-Run the current versions of .cmd and .ps1 launcher scripts: Still working
-Inside "Pico - Developer Command Prompt", but in directory of pico-project-generator:
-python pico_project.py --gui --> tkinter module not found --> Not working, but that was expected
-pythonw pico_project.py --gui --> It seems that nothing happens. Not any message or new window
-py pico_project.py --gui --> Python cannot be found
-where pythonw
C:\Program Files\Raspberry Pi\Pico SDK v1.5.1\python\pythonw.exe
C:\Program Files\Python312\pythonw.exe
-where python
C:\Program Files\Raspberry Pi\Pico SDK v1.5.1\python\python.exe
C:\Program Files\Python312\python.exe
C:\Users\Hero\AppData\Local\Microsoft\WindowsApps\python.exe
-py.exe -c "import os, sys; print('Python Path ' + os.path.dirname(sys.executable))"
--> Message: Python Path C:\Program Files\Python312 --> Wow, this method is really reliable

The reason, why the py.exe executed script detection is so good:
Most likely, it uses the official registry key for discovery and detection.
From https://peps.python.org/pep-0514/:
Tools that aim to select a single installed environment from all registered environments based on the Company-Tag pair, such as the py.exe launcher, should always select the environment registered in HKEY_CURRENT_USER...

Because of that, it finds the full installation of Python rather than the embedded one in pico-setup-windows.

It doesn´t matter if python or pythonw, the path to the full installation can be got from:
-Registry, e.g. HKEY_LOCAL_MACHINE\SOFTWARE\Python\PythonCore\ as mentioned in pep-0514
On my system, there is: Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Python\PythonCore\3.12\InstallPath
-Via py.exe -c "import os, sys; print(os.path.dirname(sys.executable))"

I have to admit that py.exe may not be present on every system, but on the other hand, it does a great job.
What do you think? Shall we use an own implementation of python path discovery or use py?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the late response. On further testing I see that this approach works for Python distributions installed using the downloaded installer; but does not work for distributions from the Microsoft Store.

On a computer with that configuration:

PS C:\Users\Nikhil>  py.exe -c "import os, sys; print(os.path.dirname(sys.executable))"
Can't find a default Python.

Running the same command with python.exe instead of py.exe works, but uses the wrong Python when run from the Pico command prompt.

Rolling our own Python path discovery isn't something I would want to add to this project. However we could figure out a way for the project generator to discover installed Pico SDK versions somehow, so if a user launches the project generator themselves, they would not be stuck with the embedded Python distribution with the Pico SDK.

@ndabas
Copy link

ndabas commented Dec 11, 2023

Thanks for putting this together, just added a couple of comments on the details -- let me know your thoughts.

@BitRolher
Copy link
Author

I have pushed the script changes to my fork

@BitRolher
Copy link
Author

I am sorry. It seems that it would be better if I created another branch for the "Use Ninja" changes.

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

Successfully merging this pull request may close these issues.

3 participants