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-90355: Add isolated flag if currently isolated #92857

Merged
merged 7 commits into from
Jul 5, 2022

Conversation

kcdodd
Copy link
Contributor

@kcdodd kcdodd commented May 16, 2022

https://bugs.python.org/issue46197

Potential resolution of the issue introduced around Python 3.8.7 where ensurepip module would run the bootstrapped pip in a subprocess and no longer obeyed the isolated flag set when run by the venv module. This could cause pip to not install itself in the virtual environment if it found an installation in another path.

Approach taken is simple, to access the isolated flag of the current process, and, if it set, also add it to the sub-process. If the current process was not run as isolated the sub-process will not, restoring this particular case seen in Python <= 3.8.6.

This behavior was manually verified to work in the case that PYTHONPATH contains another installation of pip.

The test was attempted, but the test_venv test appears to fail for another un-related issue persisting from the last commit of the forked branch 3.10 (without the requested change). For clarity that the test does not indicate a problem with this request inspected traces of the failure is included below:

Traceback (most recent call last):
  File "<string>", line 6, in <module>
  File "/media/cdodd/box/projects/contrib/cpython/Lib/runpy.py", line 205, in run_module
    mod_name, mod_spec, code = _get_module_details(mod_name)
  File "/media/cdodd/box/projects/contrib/cpython/Lib/runpy.py", line 129, in _get_module_details
    spec = importlib.util.find_spec(mod_name)
  File "/media/cdodd/box/projects/contrib/cpython/Lib/importlib/util.py", line 103, in find_spec
    return _find_spec(fullname, parent_path)
  File "<frozen importlib._bootstrap>", line 945, in _find_spec
  File "/tmp/tmpr0dlj3q6/lib/python3.10/site-packages/_distutils_hack/__init__.py", line 79, in find_spec
    return method()
  File "/tmp/tmpr0dlj3q6/lib/python3.10/site-packages/_distutils_hack/__init__.py", line 100, in spec_for_pip
    if self.pip_imported_during_build():
  File "/tmp/tmpr0dlj3q6/lib/python3.10/site-packages/_distutils_hack/__init__.py", line 111, in pip_imported_during_build
    return any(
  File "/tmp/tmpr0dlj3q6/lib/python3.10/site-packages/_distutils_hack/__init__.py", line 112, in <genexpr>
    frame.f_globals['__file__'].endswith('setup.py')
KeyError: '__file__'
Traceback (most recent call last):
  File "/media/cdodd/box/projects/contrib/cpython/Lib/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/media/cdodd/box/projects/contrib/cpython/Lib/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/media/cdodd/box/projects/contrib/cpython/Lib/ensurepip/_uninstall.py", line 31, in <module>
    sys.exit(_main())
  File "/media/cdodd/box/projects/contrib/cpython/Lib/ensurepip/_uninstall.py", line 27, in _main
    return ensurepip._uninstall_helper(verbosity=args.verbosity)
  File "/media/cdodd/box/projects/contrib/cpython/Lib/ensurepip/__init__.py", line 231, in _uninstall_helper
    return _run_pip([*args, *reversed(_PACKAGE_NAMES)])
  File "/media/cdodd/box/projects/contrib/cpython/Lib/ensurepip/__init__.py", line 102, in _run_pip
    return subprocess.run( cmd, check = True ).returncode
  File "/media/cdodd/box/projects/contrib/cpython/Lib/subprocess.py", line 524, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/tmp/tmpr0dlj3q6/bin/python', '-I', '-W', 'ignore::DeprecationWarning', '-c', '\nimport runpy\nimport sys\nsys.path = [] + sys.path\nsys.argv[1:] = [\'uninstall\', \'-y\', \'--disable-pip-version-check\', \'pip\', \'setuptools\']\nrunpy.run_module("pip", run_name="__main__", alter_sys=True)\n']' returned non-zero exit status 1.

ERROR

======================================================================
ERROR: test_with_pip (test.test_venv.EnsurePipTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/media/cdodd/box/projects/contrib/cpython/Lib/test/test_venv.py", line 551, in test_with_pip
    self.do_test_with_pip(True)
  File "/media/cdodd/box/projects/contrib/cpython/Lib/test/test_venv.py", line 517, in do_test_with_pip
    out, err = check_output([envpy,
  File "/media/cdodd/box/projects/contrib/cpython/Lib/test/test_venv.py", line 46, in check_output
    raise subprocess.CalledProcessError(
subprocess.CalledProcessError: Command '['/tmp/tmpr0dlj3q6/bin/python', '-W', 'ignore::DeprecationWarning', '-W', 'ignore::ImportWarning', '-I', '-m', 'ensurepip._uninstall']' returned non-zero exit status 1.

https://bugs.python.org/issue46197

#90355
#30307

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented May 16, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@kcdodd kcdodd changed the title 3.11 bpo 46197 [3.10] bpo-46197: Add isolated flag if currently isolated May 16, 2022
@arhadthedev
Copy link
Member

arhadthedev commented May 16, 2022

To help automation bots, could you please rename the issue to gh-90355: Add isolated flag if currently isolated (since the original BPO issue was migrated to gh-90355)? The [3.10] is used for backporting only.

@kcdodd kcdodd changed the title [3.10] bpo-46197: Add isolated flag if currently isolated gh-90355: Add isolated flag if currently isolated May 16, 2022
@kcdodd
Copy link
Contributor Author

kcdodd commented May 16, 2022

I noticed the CLA bot is actually complaining about my work email, which I guess was set in one of the commits, instead of setting to the email I have set for github.

@ambv
Copy link
Contributor

ambv commented May 16, 2022

You can add the work email to your Github account, it doesn't need to be public. Just enough so that Github (and thus the bot) will know it's yours.

@ambv
Copy link
Contributor

ambv commented May 16, 2022

And if you have a separate Github user for that work email, log into that work account and click the CLA bot button to sign from that account.

@ambv
Copy link
Contributor

ambv commented May 16, 2022

Alright, CLA situation looks good now!

@ambv ambv added needs backport to 3.8 only security fixes needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Jul 5, 2022
@ambv ambv merged commit c8556bc into python:main Jul 5, 2022
@miss-islington
Copy link
Contributor

Thanks @kcdodd for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9, 3.10, 3.11.
🐍🍒⛏🤖

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 5, 2022
@bedevere-bot
Copy link

GH-94568 is a backport of this pull request to the 3.11 branch.

@miss-islington
Copy link
Contributor

Sorry, @kcdodd and @ambv, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker c8556bcf6c0b05ac46bd74880626a2853e7c99a1 3.9

@miss-islington
Copy link
Contributor

Sorry @kcdodd and @ambv, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker c8556bcf6c0b05ac46bd74880626a2853e7c99a1 3.8

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 5, 2022
@bedevere-bot
Copy link

GH-94569 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 5, 2022
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)

Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 5, 2022
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)

Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
ambv pushed a commit to ambv/cpython that referenced this pull request Jul 5, 2022
…GH-92857)

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)

Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
@bedevere-bot
Copy link

GH-94570 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Jul 5, 2022
@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label Jul 5, 2022
ambv pushed a commit to ambv/cpython that referenced this pull request Jul 5, 2022
…GH-92857)

Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)

Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
ambv pushed a commit that referenced this pull request Jul 5, 2022
Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)
ambv pushed a commit that referenced this pull request Jul 5, 2022
Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)
@merwok
Copy link
Member

merwok commented Jul 5, 2022

We should have used this for title: Add isolated flag to ensurepip if currently isolated
Otherwise, it’s not clear if this is about core interpreter or subprocess module for example.
Let’s keep in mind titles that give context for future PRs 🙂

ambv added a commit that referenced this pull request Jul 5, 2022
…H-94570)

Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)
ambv added a commit that referenced this pull request Jul 5, 2022
…H-94571)

Co-authored-by: Carter Dodd <carter.dodd@gmail.com>
Co-authored-by: Éric <merwok@netwok.org>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit c8556bc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants