Shim python interpreter for windows if interpreter uses reparse points #5874
Conversation
There was a problem hiding this comment.
LGTM otherwise.
One question I have is around our IsVirtualEnv and VirtualEnvCommand functions.
IsVirtualEnvchecks if there's apython.exebinary inside the specified virtual environment directory, via call toos.Statlooking forScripts/python.exeVirtualEnvCommandreturns anexec.Cmdfor the binary inScripts/<bin>.exe. This function is used to runScripts/python.exeinside the virtual environment.
Do these work on systems where Python is installed from the app store? (Basically, when we run pulumi-python-shim.cmd <resolved_execution_alias_python_binary> -m venv venv to create the virtual environment, in the resulting venv directory, what type of file is venv/Scripts/python.exe? Can we successfully os.Stat and exec.Cmd it, or is it one of these execution aliases that needs Lstat and shimming?)
A way to test it: With the changes in this PR, create a standard Python Pulumi project via pulumi new python and then run pulumi up. That should exercise these code paths.
| set "python_interpreter=%1" | ||
|
|
||
| SHIFT | ||
|
|
||
| set params=%1 | ||
| :loop | ||
| shift | ||
| if [%1]==[] goto afterloop | ||
| set params=%params% %1 | ||
| goto loop | ||
| :afterloop | ||
|
|
||
| %python_interpreter% %params% |
There was a problem hiding this comment.
Could all this be replaced with a simple two character one liner?
| set "python_interpreter=%1" | |
| SHIFT | |
| set params=%1 | |
| :loop | |
| shift | |
| if [%1]==[] goto afterloop | |
| set params=%params% %1 | |
| goto loop | |
| :afterloop | |
| %python_interpreter% %params% | |
| %* |
I think this will run the first arg as the command to run with the remaining as args to the command, but I am not a batch script expert and don't have my Windows WorkSpace booted up and handy, so haven't tried it.
There was a problem hiding this comment.
I followed your advice mostly but left the initial command hard coded.
| @@ -0,0 +1,8 @@ | |||
| //+build !windows | |||
| @@ -0,0 +1,19 @@ | |||
| package python | |||
Sorry - I got lazy updating the PR body last night. I did test with the venv setup - i.e. create a new project with python and make sure the python within the venv isn't affected. I was able to do a pulumi up just fine. What I have seen is that the venv's python interpreter is a regular file with a non-zero length so its not affected by the bug. |
| @@ -36,6 +36,7 @@ RunGoBuild "github.com/pulumi/pulumi/sdk/v2/go/pulumi-language-go" "sdk" "pulumi | |||
| CopyPackage "$Root\sdk\nodejs\bin" "pulumi" | |||
|
|
|||
| Copy-Item "$Root\sdk\nodejs\dist\pulumi-resource-pulumi-nodejs.cmd" "$PublishDir\bin" | |||
| Copy-Item "$Root\sdk\nodejs\dist\pulumi-python-shim.cmd" "$PublishDir\bin" | |||
There was a problem hiding this comment.
Note that we need the pulumi-python-shim.cmd packaged as part of the plugin and be installed in PATH. Based on what I saw, this should be sufficient but I am not familiar enough with the mechanics of the build/publish for windows to know for sure this works. My crappy windows VM can't handle the build for pulumi so I am leaving this to CI to build and will validate the published zip to check if the script makes it in. FYI @justinvp
|
Merging so we can test with dev build. |
Alternative implementation to #5775
Fixes #5388
Uses a shim script to workaround golang/go#42919