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

gh-98251: Allow venv to pass along PYTHON* variables to pip when they do not impact path resolution #98259

Merged
merged 2 commits into from
Oct 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Lib/test/test_venv.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def test_upgrade_dependencies(self):
if sys.platform == 'win32':
expect_exe = os.path.normcase(os.path.realpath(expect_exe))

def pip_cmd_checker(cmd):
def pip_cmd_checker(cmd, **kwargs):
cmd[0] = os.path.normcase(cmd[0])
self.assertEqual(
cmd,
Expand All @@ -232,7 +232,7 @@ def pip_cmd_checker(cmd):
)

fake_context = builder.ensure_directories(fake_env_dir)
with patch('venv.subprocess.check_call', pip_cmd_checker):
with patch('venv.subprocess.check_output', pip_cmd_checker):
builder.upgrade_dependencies(fake_context)

@requireVenvCreate
Expand Down Expand Up @@ -659,8 +659,8 @@ def nicer_error(self):
try:
yield
except subprocess.CalledProcessError as exc:
out = exc.output.decode(errors="replace")
err = exc.stderr.decode(errors="replace")
out = (exc.output or b'').decode(errors="replace")
err = (exc.stderr or b'').decode(errors="replace")
self.fail(
f"{exc}\n\n"
f"**Subprocess Output**\n{out}\n\n"
Expand Down
28 changes: 19 additions & 9 deletions Lib/venv/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,14 +339,25 @@ def setup_python(self, context):
shutil.copyfile(src, dst)
break

def _call_new_python(self, context, *py_args, **kwargs):
"""Executes the newly created Python using safe-ish options"""
# gh-98251: We do not want to just use '-I' because that masks
# legitimate user preferences (such as not writing bytecode). All we
# really need is to ensure that the path variables do not overrule
# normal venv handling.
args = [context.env_exec_cmd, *py_args]
kwargs['env'] = env = os.environ.copy()
env['VIRTUAL_ENV'] = context.env_dir
env.pop('PYTHONHOME', None)
env.pop('PYTHONPATH', None)
kwargs['cwd'] = context.env_dir
kwargs['executable'] = context.env_exec_cmd
subprocess.check_output(args, **kwargs)

def _setup_pip(self, context):
"""Installs or upgrades pip in a virtual environment"""
# We run ensurepip in isolated mode to avoid side effects from
# environment vars, the current directory and anything else
# intended for the global Python environment
cmd = [context.env_exec_cmd, '-Im', 'ensurepip', '--upgrade',
'--default-pip']
subprocess.check_output(cmd, stderr=subprocess.STDOUT)
self._call_new_python(context, '-m', 'ensurepip', '--upgrade',
'--default-pip', stderr=subprocess.STDOUT)

def setup_scripts(self, context):
"""
Expand Down Expand Up @@ -445,9 +456,8 @@ def upgrade_dependencies(self, context):
logger.debug(
f'Upgrading {CORE_VENV_DEPS} packages in {context.bin_path}'
)
cmd = [context.env_exec_cmd, '-m', 'pip', 'install', '--upgrade']
cmd.extend(CORE_VENV_DEPS)
subprocess.check_call(cmd)
self._call_new_python(context, '-m', 'pip', 'install', '--upgrade',
*CORE_VENV_DEPS)


def create(env_dir, system_site_packages=False, clear=False,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Allow :mod:`venv` to pass along :envvar:`PYTHON*` variables to ``ensurepip`` and ``pip`` when
they do not impact path resolution