Merge pull request #344 from rstudio/242-potentially-misleading-permissions-error-when-incorrectly-specifying-python-path

Adds a check to Python executable validation.

Author: tdstein

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - The `--cacert` option now supports certificate files encoded in the Distinguished Encoding Rules (DER) binary format. Certificate files with DER encoding must end in a `.cer` or `.der` suffix.
+- The `--python` option now provides additional user guidance when an invalid path is provided.
 
 ### Changed
 

rsconnect/bundle.py

@@ -1074,19 +1074,24 @@ def are_apis_supported_on_server(connect_details):
     return connect_details["python"]["api_enabled"]
 
 
-def which_python(python, env=os.environ):
-    """Determine which python binary should be used.
-
-    In priority order:
-    * --python specified on the command line
-    * the python binary running this script
-    """
-    if python:
-        if not (exists(python) and os.access(python, os.X_OK)):
-            raise RSConnectException('The file, "%s", does not exist or is not executable.' % python)
-        return python
-
-    return sys.executable
+def which_python(python: typing.Optional[str] = None):
+    """Determines which Python executable to use.
+
+    If the :param python: is provided, then validation is performed to check if the path is an executable file. If
+    None, the invoking system Python executable location is returned.
+
+    :param python: (Optional) path to a python executable.
+    :return: :param python: or `sys.executable`.
+    """
+    if python is None:
+        return sys.executable
+    if not exists(python):
+        raise RSConnectException(f"The path '{python}' does not exist. Expected a Python executable.")
+    if isdir(python):
+        raise RSConnectException(f"The path '{python}' is a directory. Expected a Python executable.")
+    if not os.access(python, os.X_OK):
+        raise RSConnectException(f"The path '{python}' is not executable. Expected a Python executable")
+    return python
 
 
 def inspect_environment(

tests/test_bundle.py

@@ -646,13 +646,6 @@ def test_validate_entry_point(self):
         finally:
             shutil.rmtree(directory)
 
-    def test_which_python(self):
-        with self.assertRaises(RSConnectException):
-            which_python("fake.file")
-
-        self.assertEqual(which_python(sys.executable), sys.executable)
-        self.assertEqual(which_python(None), sys.executable)
-
     def test_default_title(self):
         self.assertEqual(_default_title("testing.txt"), "testing")
         self.assertEqual(_default_title("this.is.a.test.ext"), "this.is.a.test")
@@ -781,3 +774,30 @@ def fake_inspect_environment(
 
         assert python == expected_python
         assert environment == expected_environment
+
+
+class WhichPythonTestCase(TestCase):
+    def test_default(self):
+        self.assertEqual(which_python(), sys.executable)
+
+    def test_none(self):
+        self.assertEqual(which_python(None), sys.executable)
+
+    def test_sys(self):
+        self.assertEqual(which_python(sys.executable), sys.executable)
+
+    def test_does_not_exist(self):
+        with tempfile.NamedTemporaryFile() as tmpfile:
+            name = tmpfile.name
+        with self.assertRaises(RSConnectException):
+            which_python(name)
+
+    def test_is_directory(self):
+        with tempfile.TemporaryDirectory() as tmpdir:
+            with self.assertRaises(RSConnectException):
+                which_python(tmpdir)
+
+    def test_is_not_executable(self):
+        with tempfile.NamedTemporaryFile() as tmpfile:
+            with self.assertRaises(RSConnectException):
+                which_python(tmpfile.name)