bpo-37354: Make Powershell Activate.ps1 script static to allow for signing #14967

d3r3kk · 2019-07-26T18:52:26Z

Make Powershell Activation.ps1 script static and therefore sign-able. The script now determines the interpreter path based on where the Activate script is. The prompt is determined based on the directory name of the parent directory the script is located in, or can be overridden via an argument to the Activate script, or from within the pyvenv.cfg file.

Tested On:

OS	Powershell Version	Powershell Build Version
Windows 8.1	5.1.14409.1018	10.0.14409.1018
Windows 7 SP1	5.1.14409.1018	10.0.14409.1018
Windows Server 2012R2	4.0	6.3.9600.19170
Windows 10	5.1.18362.145	10.0.18362.145
Debian 10.0 (Buster)	6.2.2 (Core)	?

https://bugs.python.org/issue37354

- Remove use of replacement text in the script - Make use of the pyvenv.cfg file for prompt value. - Add parameters to allow more flexibility - Make use of the current path, and assumptions about where �env puts things, to compensate - Make the script a bit more 'idiomatic' Powershell - Add script documentation (Get-Help .\.venv\Scripts\Activate.ps1 shows PS help page now

- with prompt - with usage in Linux on PowerShell core

d3r3kk · 2019-07-26T19:03:12Z

Note, also fixes bpo-35667.

zooba · 2019-07-26T22:09:45Z

Lib/venv/scripts/common/Activate.ps1

+    Write-Verbose "Prompt not specified as argument to script, checking pyvenv.cfg value"
+    if ($pyvenvCfg -and $pyvenvCfg['prompt']) {
+        Write-Verbose "  Setting based on value in pyvenv.cfg='$($pyvenvCfg['prompt'])'"
+        $Prompt = $pyvenvCfg['prompt'];


This is the line that gave me the most nightmares when thinking about how to do this, because the prompt value in the config file is quoted and escaped (basically, it's repr(prompt)), so we need to do more processing here.

Thanks for the review @zooba!

Agreed that the escaped character problem is a bit tricky in the context of Powershell... \n is not really available in Powershell, for instance, but could be replaced with the back-tick-quote `. Finding and replacing all types of escaped characters (newlines, tabs, colors?) might be a bit of a deep dark hole (there be ANSI-escaped coloured dragons there)!

Would it be appropriate to add handling of escaped characters in a different issue/PR? I don't expect that the prior version of this Activate.ps1 script handled this problem particularly well either (a brief test with the latest 3.8 beta shows a few hiccups right away).
Of course, I would be most happy to tackle that problem next!

What sample input would show the shortcoming of this implementation, so that I can write some unit tests and fix it correctly?

Just from thinking about it briefly I can think up input that contains:

newlines (\n, \r\n)

tabs (\t)

colours? (\u001b[31m, ... , \u001b[0m)

Ideally I'd love to show something that would have the class of input we need to be concerned about working with, and have it handled at least as well as the prior implementation handled it.

A specific scenario that is trivial to setup and play with, for example the newline escaped character \n in the prompt:

PS /home/d3r3kk/dev/venvtest> ../cpython/python -m venv --prompt "\nd3r3kk\n" .newline_prompt PS /home/d3r3kk/dev/venvtest> cat ./.newline_prompt/pyvenv.cfg home = /home/d3r3kk/dev/cpython include-system-site-packages = false version = 3.9.0 prompt = '\\nd3r3kk\\n' PS /home/d3r3kk/dev/venvtest> ./.newline_prompt/bin/Activate.ps1 (\\nd3r3kk\\n) PS /home/d3r3kk/dev/venvtest> deactivate PS /home/d3r3kk/dev/venvtest> exit

...and then in bash...

d3r3kk@deb:~/dev/venvtest$ . .newline_prompt/bin/activate ( d3r3kk ) d3r3kk@deb:~/dev/venvtest$ deactivate d3r3kk@deb:~/dev/venvtest$

...as we can see, the \\n being stored in the pyvenv.cfg is written to the prompt verbatim in Powershell with this change (and is also in the latest 3.8 beta release).

Note: it is possible in Powershell to replace some escaped character's \\ with a ` and it will work. (But definitely not colours, those are weird special creatures!).

vsajip

I'm a PowerShell tyro, so I can't comment on some of the finer points. The handling of escapes seems orthogonal to the making of the script to be entirely static, but I concur with the comment on the issue made by @zooba : "get it out in 3.8.0b4 and see how it fares".

Lib/venv/scripts/common/Activate.ps1

d3r3kk · 2019-08-07T20:48:58Z

Thanks for the review @vsajip! (and for introducing me to the term tyro). I've updated the PR with your suggestions.

vsajip

Nothing more to add to my earlier review.

d3r3kk · 2019-08-12T18:56:57Z

@zooba anything else for me to address?

miss-islington · 2019-08-12T20:06:06Z

Thanks @d3r3kk for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

…gning (pythonGH-14967) - Remove use of replacement text in the script - Make use of the pyvenv.cfg file for prompt value. - Add parameters to allow more flexibility - Make use of the current path, and assumptions about where env puts things, to compensate - Make the script a bit more 'idiomatic' Powershell - Add script documentation (Get-Help .\.venv\Scripts\Activate.ps1 shows PS help page now (cherry picked from commit 732775d) Co-authored-by: Derek Keeler <d3r3kk@users.noreply.github.com>

bedevere-bot · 2019-08-12T20:06:23Z

GH-15233 is a backport of this pull request to the 3.8 branch.

…for signing (GH-14967) - Remove use of replacement text in the script - Make use of the pyvenv.cfg file for prompt value. - Add parameters to allow more flexibility - Make use of the current path, and assumptions about where env puts things, to compensate - Make the script a bit more 'idiomatic' Powershell - Add script documentation (Get-Help .\.venv\Scripts\Activate.ps1 shows PS help page now (cherry picked from commit 732775d) Co-authored-by: Derek Keeler <d3r3kk@users.noreply.github.com>

…gning (pythonGH-14967) - Remove use of replacement text in the script - Make use of the pyvenv.cfg file for prompt value. - Add parameters to allow more flexibility - Make use of the current path, and assumptions about where env puts things, to compensate - Make the script a bit more 'idiomatic' Powershell - Add script documentation (Get-Help .\.venv\Scripts\Activate.ps1 shows PS help page now

d3r3kk and others added 5 commits July 25, 2019 09:56

Add news entry

f15d490

Bug fixes

d7e442e

Promp(t) typo

1099c9b

Correct problems

481b7b7

- with prompt - with usage in Linux on PowerShell core

the-knights-who-say-ni added the CLA signed label Jul 26, 2019

bedevere-bot added the awaiting review label Jul 26, 2019

brettcannon added the OS-windows label Jul 26, 2019

brettcannon requested review from zware and zooba July 26, 2019 18:54

brettcannon added the type-feature A feature request or enhancement label Jul 26, 2019

zooba reviewed Jul 26, 2019

View reviewed changes

zooba requested a review from vsajip July 29, 2019 23:50

vsajip reviewed Aug 2, 2019

View reviewed changes

Lib/venv/scripts/common/Activate.ps1 Outdated Show resolved Hide resolved

Lib/venv/scripts/common/Activate.ps1 Outdated Show resolved Hide resolved

Lib/venv/scripts/common/Activate.ps1 Outdated Show resolved Hide resolved

Lib/venv/scripts/common/Activate.ps1 Show resolved Hide resolved

PR suggestions

5874721

d3r3kk mentioned this pull request Aug 9, 2019

Activating an environment fails because running powershell scripts is disabled by default on Windows microsoft/vscode-python#2559

Closed

brettcannon requested a review from vsajip August 9, 2019 21:33

vsajip approved these changes Aug 10, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Aug 10, 2019

zooba added the needs backport to 3.8 label Aug 12, 2019

zooba merged commit 732775d into python:master Aug 12, 2019

bedevere-bot removed the awaiting merge label Aug 12, 2019

bedevere-bot removed the needs backport to 3.8 label Aug 12, 2019

d3r3kk deleted the pwsh_activate branch August 12, 2019 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-37354: Make Powershell Activate.ps1 script static to allow for signing #14967

bpo-37354: Make Powershell Activate.ps1 script static to allow for signing #14967

d3r3kk commented Jul 26, 2019 •

edited by bedevere-bot

Loading

d3r3kk commented Jul 26, 2019 •

edited by bedevere-bot

Loading

zooba Jul 26, 2019

d3r3kk Jul 27, 2019

vsajip left a comment

d3r3kk commented Aug 7, 2019

vsajip left a comment

d3r3kk commented Aug 12, 2019

miss-islington commented Aug 12, 2019

bedevere-bot commented Aug 12, 2019

bpo-37354: Make Powershell Activate.ps1 script static to allow for signing #14967

bpo-37354: Make Powershell Activate.ps1 script static to allow for signing #14967

Conversation

d3r3kk commented Jul 26, 2019 • edited by bedevere-bot Loading

Tested On:

d3r3kk commented Jul 26, 2019 • edited by bedevere-bot Loading

zooba Jul 26, 2019

Choose a reason for hiding this comment

d3r3kk Jul 27, 2019

Choose a reason for hiding this comment

vsajip left a comment

Choose a reason for hiding this comment

d3r3kk commented Aug 7, 2019

vsajip left a comment

Choose a reason for hiding this comment

d3r3kk commented Aug 12, 2019

miss-islington commented Aug 12, 2019

bedevere-bot commented Aug 12, 2019

d3r3kk commented Jul 26, 2019 •

edited by bedevere-bot

Loading

d3r3kk commented Jul 26, 2019 •

edited by bedevere-bot

Loading