From 9adda0cdf89432386b7a04444a6199b580d287a1 Mon Sep 17 00:00:00 2001 From: Igor Filatov Date: Thu, 21 Sep 2017 13:07:45 +0300 Subject: [PATCH] bpo-31351: Set return code in ensurepip when pip fails (GH-3626) Previously ensurepip would always report success, even if the pip installation failed. --- Doc/library/ensurepip.rst | 3 +++ Lib/ensurepip/__init__.py | 23 +++++++++++++++---- Lib/ensurepip/__main__.py | 3 ++- Lib/ensurepip/_uninstall.py | 5 ++-- Lib/test/test_ensurepip.py | 19 +++++++++++++-- .../2017-09-17-15-24-25.bpo-31351.yQdKv-.rst | 2 ++ 6 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-09-17-15-24-25.bpo-31351.yQdKv-.rst diff --git a/Doc/library/ensurepip.rst b/Doc/library/ensurepip.rst index c797f63326d..ed22180dc3f 100644 --- a/Doc/library/ensurepip.rst +++ b/Doc/library/ensurepip.rst @@ -78,6 +78,9 @@ options: Providing both of the script selection options will trigger an exception. +.. versionchanged:: 3.7.0 + The exit status is non-zero if the command fails. + Module API ---------- diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index 9f5d15109a3..d69e09fab08 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -25,7 +25,7 @@ def _run_pip(args, additional_paths=None): # Install the bundled software import pip - pip.main(args) + return pip.main(args) def version(): @@ -53,6 +53,21 @@ def bootstrap(*, root=None, upgrade=False, user=False, Bootstrap pip into the current Python installation (or the given root directory). + Note that calling this function will alter both sys.path and os.environ. + """ + # Discard the return value + _bootstrap(root=root, upgrade=upgrade, user=user, + altinstall=altinstall, default_pip=default_pip, + verbosity=verbosity) + + +def _bootstrap(*, root=None, upgrade=False, user=False, + altinstall=False, default_pip=False, + verbosity=0): + """ + Bootstrap pip into the current Python installation (or the given root + directory). Returns pip command status code. + Note that calling this function will alter both sys.path and os.environ. """ if altinstall and default_pip: @@ -99,7 +114,7 @@ def bootstrap(*, root=None, upgrade=False, user=False, if verbosity: args += ["-" + "v" * verbosity] - _run_pip(args + [p[0] for p in _PROJECTS], additional_paths) + return _run_pip(args + [p[0] for p in _PROJECTS], additional_paths) def _uninstall_helper(*, verbosity=0): """Helper to support a clean default uninstall process on Windows @@ -126,7 +141,7 @@ def _uninstall_helper(*, verbosity=0): if verbosity: args += ["-" + "v" * verbosity] - _run_pip(args + [p[0] for p in reversed(_PROJECTS)]) + return _run_pip(args + [p[0] for p in reversed(_PROJECTS)]) def _main(argv=None): @@ -180,7 +195,7 @@ def _main(argv=None): args = parser.parse_args(argv) - bootstrap( + return _bootstrap( root=args.root, upgrade=args.upgrade, user=args.user, diff --git a/Lib/ensurepip/__main__.py b/Lib/ensurepip/__main__.py index 77527d7a351..03eef0dd94d 100644 --- a/Lib/ensurepip/__main__.py +++ b/Lib/ensurepip/__main__.py @@ -1,4 +1,5 @@ import ensurepip +import sys if __name__ == "__main__": - ensurepip._main() + sys.exit(ensurepip._main()) diff --git a/Lib/ensurepip/_uninstall.py b/Lib/ensurepip/_uninstall.py index 750365ec4d0..b257904328d 100644 --- a/Lib/ensurepip/_uninstall.py +++ b/Lib/ensurepip/_uninstall.py @@ -2,6 +2,7 @@ import argparse import ensurepip +import sys def _main(argv=None): @@ -23,8 +24,8 @@ def _main(argv=None): args = parser.parse_args(argv) - ensurepip._uninstall_helper(verbosity=args.verbosity) + return ensurepip._uninstall_helper(verbosity=args.verbosity) if __name__ == "__main__": - _main() + sys.exit(_main()) diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index 9b04c18b0e2..89966893092 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -20,6 +20,7 @@ class EnsurepipMixin: def setUp(self): run_pip_patch = unittest.mock.patch("ensurepip._run_pip") self.run_pip = run_pip_patch.start() + self.run_pip.return_value = 0 self.addCleanup(run_pip_patch.stop) # Avoid side effects on the actual os module @@ -255,7 +256,7 @@ class TestBootstrappingMainFunction(EnsurepipMixin, unittest.TestCase): self.assertFalse(self.run_pip.called) def test_basic_bootstrapping(self): - ensurepip._main([]) + exit_code = ensurepip._main([]) self.run_pip.assert_called_once_with( [ @@ -267,6 +268,13 @@ class TestBootstrappingMainFunction(EnsurepipMixin, unittest.TestCase): additional_paths = self.run_pip.call_args[0][1] self.assertEqual(len(additional_paths), 2) + self.assertEqual(exit_code, 0) + + def test_bootstrapping_error_code(self): + self.run_pip.return_value = 2 + exit_code = ensurepip._main([]) + self.assertEqual(exit_code, 2) + class TestUninstallationMainFunction(EnsurepipMixin, unittest.TestCase): @@ -280,7 +288,7 @@ class TestUninstallationMainFunction(EnsurepipMixin, unittest.TestCase): def test_basic_uninstall(self): with fake_pip(): - ensurepip._uninstall._main([]) + exit_code = ensurepip._uninstall._main([]) self.run_pip.assert_called_once_with( [ @@ -289,6 +297,13 @@ class TestUninstallationMainFunction(EnsurepipMixin, unittest.TestCase): ] ) + self.assertEqual(exit_code, 0) + + def test_uninstall_error_code(self): + with fake_pip(): + self.run_pip.return_value = 2 + exit_code = ensurepip._uninstall._main([]) + self.assertEqual(exit_code, 2) if __name__ == "__main__": diff --git a/Misc/NEWS.d/next/Library/2017-09-17-15-24-25.bpo-31351.yQdKv-.rst b/Misc/NEWS.d/next/Library/2017-09-17-15-24-25.bpo-31351.yQdKv-.rst new file mode 100644 index 00000000000..20f2c1bdc11 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-09-17-15-24-25.bpo-31351.yQdKv-.rst @@ -0,0 +1,2 @@ +python -m ensurepip now exits with non-zero exit code if pip bootstrapping +has failed.