-
Notifications
You must be signed in to change notification settings - Fork 887
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
Tutorial instructions for windows may not work correctly in Powershell #3260
Comments
Please provide evidence to support this claim. Confirmation from Microsoft would be good. There are instances throughout the documentation that would need to be updated to support this third option. |
@stevepiercy Said:
Found a couple of references.. |
@vagnerr thank you! Looks like it is the default now in Windows 10. I'll look through the docs for all the instances of Windows prompts and supplement them with the third option, Powershell. We'll leave both Windows prompts in place for a while. Would you be willing to do a code review once I have it in place? |
@stevepiercy I'm happy to review the changes. Incidentally is there a reason why the instructions explicitly access the environment scripts after the virtual environment has been created. eg
would it not be better to activate the virtual environment and then allow the implicit usage of the environment through the built in path change ie
That may help make the later instructions more consistent between platforms, as there are activate scripts for bash, CMD and Powershell in the scripts directory. |
We have an explanation for our choice of being explicit with the path versus activate in the blue note box here: https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/install.html#venv-bin-pip-vs-source-bin-activate |
@stevepiercy Ah, I had not seen that note. I withdraw my activate suggestion ;-) |
I suggest using the terms "Windows PowerShell" and "Windows Command Prompt (cmd.exe)" to distinguish the two flavors. (Apologies for the non-authoritative Wikipedia link, but Microsoft appears to have erased most instances of "cmd.exe" from its site. Let me know if are any objections or concerns with those names. |
@vagnerr would you check whether the environment variable
|
I've started a PR #3261, and noted a couple of problems. |
@stevepiercy The naming looks good. As for the %APPDATA% reference. No that should not be changed. While the access to environment variables has been changed in Powershell to be under the $Env object, the GUI configs still use the traditional %VARIABLE% format. |
@vagnerr for setting an environment variable, can you confirm that the value must be a quoted string? In other words, not this: set $env:VENV=c:\env ...but this: set $env:VENV="c:\env" I ran into a parsing issue with powershell as the code-block language, and a SO post indicated that quoting the string was necessary. |
@stevepiercy Yes that's correct the quoting is mandatory. If you don't quote it it will actually try and execute the right hand side. Sorry I should have been more explicit on that with my example. Also you should not use "set" as that is an alias for the Set-Variable it doesn't seem to do what you would expect. Just do the assigment. $env:VENV="c:\env" [EDIT1] $VENV="c:\env"
python -m venv $VENV
cd $VENV That would save some typing. [EDIT2] I have done some additional testing and while either of these suggestions is works for this code example. The later ones where the variable is used in the path do not. In order to use it you need to type out...
...which then substitutes the full value of $ENV. This obviously is not going to be a useful, especially when copy-pasting into the terminal window |
Can you clarify what is the correct process, assuming that the current session is all that is needed, not a script? Here's where we are now: cd \
$env:VENV="c:\env"
python -m venv $env:VENV
cd $env:VENV The above works for me, so ¯_(ツ)_/¯ |
PR #3266 is ready for review. |
@stevepiercy Sorry, took a while to find a clean solution to the $env expansion problem. The solution is to use Invoke-Expression (iex) You don't need to do it where the variable is in the arguments, so... cd \
$env:VENV="c:\env"
python -m venv $env:VENV
cd $env:VENV ...is fine. However when $env:VENV is part of the command path you need to call with iex and quote the entire command if it has any arguments eg.... iex "$env:VENV\Scripts\pip install --upgrade pip setuptools"
iex "$env:VENV\Scripts\pip install -e '.[testing]'"
iex "$env:VENV\Scripts\py.test -q"
iex "$env:VENV\Scripts\py.test --cov --cov-report=term-missing"
iex "$env:VENV\Scripts\initialize_tutorial_db development.ini"
iex "$env:VENV\Scripts\pserve development.ini --reload" Note how I needed to flip the quotes in the pip install -e ".[testing]" to single quotes. |
Obviously it's not going to be ok for us to recommend that... it's just distracting for the user to look at all the syntax and they won't learn the actual command we're trying to teach. Do we need to re-evaluate things such that our docs have just an "environment setup" section that is unix/windows/mac and then just drop the boilerplate from the commands such that we just show simply "pip install ..." and assume the user knows what to do? I'm sure I argued against this at one point but I also do not like having platform-specific instructions for every single command we want to show the user in the docs. |
Why does PowerShell have to make this so much more difficult than necessary? |
@vagnerr are you talking about using PowerShell as a shell session interface or as a scripting interface? I think you're talking about the latter, while I'm talking about the former. Please clarify. I'm with @mmerickel if you mean the latter. |
@mmerickel @berkerpeksag @stevepiercy
|
No, no it doesn't. :-) |
This statement says a lot about PowerShell:
I assumed it would be simple and make things better, but after trying it myself in a Win10 VM, I have come to the conclusion that PowerShell syntax would be too onerous to support for too little user benefit, given that
We do that in a couple of places now. https://docs.pylonsproject.org/projects/pyramid/en/latest/quick_tour.html#installation
https://docs.pylonsproject.org/projects/pyramid/en/latest/quick_tutorial/requirements.html
I think it is helpful to retain the current information for setting up environments on all platforms. I think it is helpful to retain and improve the statements for using UNIX syntax only, where present. I am uncertain whether we should remove existing Windows commands from all other places. They've been around since 2011, if not earlier. As docs maintainer, I love the idea of fewer things to support. As a Windows user, I might feel unwelcome. It doesn't hurt to leave them in for now, but if they ever require updating then I would give serious consideration to removing them. I would like to avoid the slog of changing docs from setuptools to pip again, but I can smell it coming with pipenv. At that time, if there are differences in platforms, I would much prefer dropping Windows-specific commands, except for install and setup of an environment. |
I'm confused by this statement.
works perfectly fine in my experience. (I use Powershell as my standard shell, and have for many years). In what way do you think that it fails? (Having said this, I don't have an opinion on whether Pyramid should switch to describing Powershell usage in their docs - I'm not a Pyramid user, just an interested Windows and Python user). |
@pfmoore fascinating! What version of PowerShell and Windows? How do you set the value of This is Windows 10 in a VM: PS C:\Users\Steve Piercy> cd \
PS C:\> $env:VENV="c:\env"
PS C:\> $env:VENV\Scripts\pip install --upgrade pip setuptools
At line:1 char:10
+ $env:VENV\Scripts\pip install --upgrade pip setuptools
+ ~~~~~~~~~~~~
Unexpected token '\Scripts\pip' in expression or statement.
+ CategoryInfo : ParserError: (:) [], ParentContainsErrorRecordException
+ FullyQualifiedErrorId : UnexpectedToken |
Ah, sorry. I'm so used to hitting
And yes, that's a bit ugly to be dumping on users who aren't experienced with Powershell. Apologies for the noise. I should have tested more carefully. |
@vagnerr I don't see a reason to keep this issue open, given that PowerShell syntax is far more complex to implement than we expected. The discussion was informative and helped steer us toward a desire for command simplification to one platform in the future. Feel free to reopen if you uncover something new that can keep it simple. Thank you! |
The current windows instructions seem to be orientated towards using the older Windows Command Prompt (CMD), rather than the more modern Powershell which is likely to become the default.
They are reasonably compatible with each other but there are some differences which may cause issues, such as Environment variables...
For example in https://docs.pylonsproject.org/projects/pyramid/en/latest/tutorials/wiki2/installation.html
Will not function as expected instead you will get a
%VENV%
directory in the current one. Instead you need a powershell alternative such asI think we still need to retain the CMD versions but it should be clear there that CMD and Powershell need different approaches.
The text was updated successfully, but these errors were encountered: