When creating the pyvenv.cfg file, the prompt setting should be stored there so that tools can introspect on it (e.g. VS Code could read the value to tell users the name of the venv they have selected in the status bar).

Adding my comment from the PR review:

Dealing with leading/trailing whitespace could get interesting here. All the other values can be trimmed at both ends, while this one (currently) must have one space removed from the start.

Perhaps we should insert the repr of the prompt instead? Or at least double-quote it and escape internal quotes and backslashes?

Currently, the prompt is enclosed in parentheses, both in the venv and when storing:

PS N:\projects\cpython> python -m venv testvenv --prompt=" prompt with spaces "
Running Release|Win32 interpreter...
PS N:\projects\cpython> .\testvenv\Scripts\activate
( prompt with spaces ) PS N:\projects\cpython>

Contents of pyvenv.cfg:
home = N:\projects\cpython\PCbuild\win32
include-system-site-packages = false
version = 3.8.0
prompt = ( prompt with spaces )

I also tested using quotes in the prompt, which lead to opening bpo-35667.

Here's a prompt that itself has parentheses:
PS N:\projects\cpython> python -m venv testvenv --prompt="(my prompt)"
Running Release|Win32 interpreter...
PS N:\projects\cpython> .\testvenv\Scripts\activate
((my prompt)) PS N:\projects\cpython>

It seemed to me that the parenthesis would allow the retrieval of both the original prompt definition and how it looked in display. Maybe my tests didn't demonstrate this clearly?

Your tests actually went the other way. This one puts a trailing space after the prompt and expected it to be there in the file, but also ensured that exactly one space was added at the start.

    self.assertEqual(context.prompt, '(My prompt) ')
    builder.create(self.env_dir)
    data = self.get_text_file_contents('pyvenv.cfg')
    self.assertIn('prompt = (My prompt) \n', data)

In the line "prompt = (My prompt) ", we need to define which of the spaces at either end of the value should be retained by tools trying to use the same prompt. The obvious assumption is to strip both ends, which would break your test. The next most obvious is to quote the string value (e.g. write repr(context.prompt) instead of the prompt directly).

Also, I'm pretty sure I saw some advice once to use multiple lines worth of prompt, which probably breaks many shells but presumably works okay in Bash. You'll need to handle that case as well - again, repr() will do it naturally by encoding newlines an "\n".

Anyone reading the string will have to do the equivalent of Python's literal_eval(), but that's pretty manageable in exchange for avoiding a whole lot of edge cases.

Steve, thanks for explaining that. I understand what you're saying now. It's funny but because the context prompt has the space added at the end when it's created (context.prompt = '(%s) ' % prompt), I added it to the test, but completely forgot about it. I was thinking about just getting back what was inside the parentheses without considering at all what the whole line looked like.

First, Cheryl, thanks for taking this on!

I think one way to potentially simplify this whole situation about the whitespace for the prompt is to actually store the raw value that gets passed into EnvBuilder instead of the prompt as formatted for the activation scripts. I personally only want that initial value anyway and not the formatted version for the prompt for us in VS Code. Plus if we document that the value that we save in the pyvenv.cfg will be stripped then that should help with this.

Otherwise I say go with the repr as Steve suggested, but I would still like to have access to the unformatted value (and probably not bother setting it if a custom value isn't provided to facilitate relocating venvs).

One other aspect of this may be the confusion that ensues when changing the setting doesn't change the prompt when you activate it.

It would be possible (though not necessarily trivial) to update the activate scripts to read the prompt from the file, though I don't think it's necessary. But we'll get a bug report sooner or later anyway.

Maybe if the setting is called "provided-custom-prompt" that will imply enough that it's a optional record of the prompt rather than an active configuration setting?

I've made the changes suggested by Brett to store the raw value instead of the formatted prompt.

I also have been looking into issues Steve raised about using the prompt value from pyvenv.cfg instead of having the prompt hard-coded into the activate scripts. Since these scripts are .bat, bash, Powershell, etc scripts, I didn't know if there was anything special that needed to be taken into account (such as encoding) to use the values from the config file. As Steve said, I could see this being opened as a bug in the future, so I don't mind working on it now in this issue or a separate one, whichever is the better option. I just don't have much experience with those types of scripts, so I'm sure any changes I make will require some time and a lot of code review. Plus, those changes add complexity to this change that Brett said he doesn't really need for his initial request. Because of the possibility of changing those scripts soon, I've left the key name as prompt for now. Fixing the activate scripts seems to be the right way to go instead of changing the name so it's not misused.

One thing that I did find which may be a current bug is:

PS N:\projects\cpython> python -m venv myvenv --prompt='this is my prompt'
Running Release|Win32 interpreter...
PS N:\projects\cpython> .\myvenv\Scripts\activate
(this is my prompt) PS N:\projects\cpython> deactivate
PS N:\projects\cpython> python -m venv myvenv --upgrade --prompt='new prompt'
Running Release|Win32 interpreter...
PS N:\projects\cpython> .\myvenv\Scripts\activate
(this is my prompt) PS N:\projects\cpython>

--upgrade changes the prompt on pyvenv.cfg, but doesn't update activate scripts, so the prompt doesn't really change.

And for reference, there is another bug issue for storing the --prompt value in an environment variable (bpo-35328).

I think upgrading the scripts to read from pyvenv.cfg is a separate issue per activation script (e.g. an issue for PowerShell, an issue for fish, etc.) as this could be done piecemeal.

One benefit to doing this is it would help move towards the activation scripts becoming static and thus skipping the string substitution when writing the files to disk.

New changeset d5a70c6 by Cheryl Sabella in branch 'master':
bpo-35661: Store the venv prompt in pyvenv.cfg (GH-11440)
d5a70c6

This issue broke x86 Gentoo Installed with X 3.x buildbot:

https://buildbot.python.org/all/#/builders/103/builds/2208

ERROR: test_prompt (test.test_venv.BasicTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.8/test/test_venv.py", line 123, in test_prompt
    builder.create(self.env_dir)
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.8/venv/__init__.py", line 70, in create
    self.setup_scripts(context)
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.8/venv/__init__.py", line 278, in setup_scripts
    self.install_scripts(context, path)
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.8/venv/__init__.py", line 354, in install_scripts
    with open(dstfile, 'wb') as f:
PermissionError: [Errno 13] Permission denied: '/buildbot/tmp/tmpdir/tmph7_oob5e/bin/Activate.ps1'

Please either fix or revert the change.

Presumably the issue is in the test and not the main change itself.

Is this another case where Linux can't run venv tests inside a venv?

Sorry about that. I'll work on a fix and test it on the buildbots first.

There are some tests with pyvenv.cfg that are skipped if it happens in a venv (@skipInVenv decorator), but the test right above this one (test_defaults) does not use the decorator and it checks pyvenv.cfg, so I think test_prompt might just need to do some cleanup first.

Maybe you can just call builder.create_configuration() from the test and pass in enough context that it doesn't have to create a real venv? That would also make the test faster.

It may also be that the directory exists already - the previous test explicitly removes it before calling create().

Steve, thanks for the suggestions.

Looking at the other tests, test_prompt was the only one (prior to the original change) that didn't create a venv, so maybe actually creating the venv instead of just instantiating the class were testing that everything worked together? For example, see test_isolation and test_symlinking.

I've added the code to explicitly remove the directory and I also slightly reorganized the create. Not all the tests use the run_with_capture, but at the same time, none of the ones that use it do anything with the output, so I thought it wouldn't hurt to add it.

I tried to submit the tests just to the buildbots based on the devguide, but they were failing for other reasons, so I made the PR. I don't think we'll know if this fixes the bots until the PR is merged, but if one of you could take a look and approve it, I'd appreciate it. And then I'll keep my fingers crossed. :-)

The PR looks good to me.

Are you able to test this test running from in a venv? I'm not at all clear why it would work there when the others fail (there's another issue looking at this, since it's only Linux that doesn't support venv-in-venv, but the tests are mostly all skipped anyway)

Thanks Steve.

The venv tests run for me from inside a venv on Windows 10 from a command prompt. From Powershell, all the tests fail with the following when run from inside a venv:
FileNotFoundError: [Errno 2] No such file or directory: 'N:\\projects\\cpython\\lib\\venv\\scripts\\nt\\python.exe'

I haven't tried on linux.

New changeset 839b925 by Cheryl Sabella in branch 'master':
bpo-35661: Fix failing test on buildbot (GH-12297)
839b925

Yeah, that failure on Windows is due to not correctly detecting a venv made from a build tree, rather than a venv from a proper install. I have a fix for that in the other bug. If it got that far, it means it used the correct prefix path, so it'll be fine.

Looks like the buildbot is happy now. Thank you everyone!