bpo-31351: Set return code in ensurepip when pip fails (GH-3626) (GH-3683)

Previously ensurepip would always report success, even if the
pip installation failed.
(cherry picked from commit 9adda0cdf89432386b7a04444a6199b580d287a1)

* Update version changed notice for backport

diff --git a/Doc/library/ensurepip.rst b/Doc/library/ensurepip.rst
index c797f63326d1a26b690c6df3172bed649c1e70ed..652339e57f31aba9bfa6b405afe950a4a8ada026 100644 (file)
--- 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.6.3
+   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 9f5d15109a3c43bdb67738e39828e900f477111e..d69e09fab084e6d217439b0c7c7c7355a621631b 100644 (file)
--- 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 77527d7a3513596852d1eb0ff8d8935a98e0cc26..03eef0dd94d0b77764cbd737f69945d04e4b1bc6 100644 (file)
--- 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 750365ec4d04da980399c94c35668935482b9cce..b257904328d2f5dc3b5f07e7d6c883fb9e2055a9 100644 (file)
--- 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 9b04c18b0e296882487642744f54784946eb9607..89966893092851cc5296a741177dc5538d1ea8a2 100644 (file)
--- 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 (file)
index 0000000..20f2c1b
--- /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.