Merge pull request #888 from python-cmd2/pyscript_fixes

pyscript fixes

Author: kmvanbrunt

CHANGELOG.md

@@ -3,8 +3,17 @@
     * Corrected issue where the actual new value was not always being printed in do_set. This occurred in cases where
       the typed value differed from what the setter had converted it to.
     * Fixed bug where ANSI style sequences were not correctly handled in `utils.truncate_line()`. 
+    * Fixed bug where pyscripts could edit `cmd2.Cmd.py_locals` dictionary.
+    * Fixed bug where cmd2 set sys.path[0] for a pyscript to cmd2's working directory instead of the 
+    script file's directory.
+    * Fixed bug where sys.path was not being restored after a pyscript ran.
 * Enhancements
     * Renamed set command's `-l/--long` flag to `-v/--verbose` for consistency with help and history commands.
+    * Setting the following pyscript variables:
+        * `__name__`: __main__
+        * `__file__`: script path (as typed, ~ will be expanded)
+* Other
+    * Removed undocumented `py run` command since it was replaced by `run_pyscript` a while ago
 
 ## 0.10.0 (February 7, 2020)
 * Enhancements

cmd2/cmd2.py

@@ -3093,8 +3093,7 @@ def _restore_cmd2_env(self, cmd2_env: _SavedCmd2Env) -> None:
 
     # This is a hidden flag for telling do_py to run a pyscript. It is intended only to be used by run_pyscript
     # after it sets up sys.argv for the script being run. When this flag is present, it takes precedence over all
-    # other arguments. run_pyscript uses this method instead of "py run('file')" because file names with
-    # 2 or more consecutive spaces cause issues with our parser, which isn't meant to parse Python statements.
+    # other arguments.
     py_parser.add_argument('--pyscript', help=argparse.SUPPRESS)
 
     # Preserve quotes since we are passing these strings to Python
@@ -3104,65 +3103,68 @@ def do_py(self, args: argparse.Namespace) -> Optional[bool]:
         Enter an interactive Python shell
         :return: True if running of commands should stop
         """
+        def py_quit():
+            """Function callable from the interactive Python console to exit that environment"""
+            raise EmbeddedConsoleExit
+
         from .py_bridge import PyBridge
+        py_bridge = PyBridge(self)
+        saved_sys_path = None
+
         if self.in_pyscript():
             err = "Recursively entering interactive Python consoles is not allowed."
             self.perror(err)
             return
 
-        py_bridge = PyBridge(self)
-        py_code_to_run = ''
-
-        # Handle case where we were called by run_pyscript
-        if args.pyscript:
-            args.pyscript = utils.strip_quotes(args.pyscript)
-
-            # Run the script - use repr formatting to escape things which
-            # need to be escaped to prevent issues on Windows
-            py_code_to_run = 'run({!r})'.format(args.pyscript)
-
-        elif args.command:
-            py_code_to_run = args.command
-            if args.remainder:
-                py_code_to_run += ' ' + ' '.join(args.remainder)
-
-            # Set cmd_echo to True so PyBridge statements like: py app('help')
-            # run at the command line will print their output.
-            py_bridge.cmd_echo = True
-
         try:
             self._in_py = True
+            py_code_to_run = ''
 
-            def py_run(filename: str):
-                """Run a Python script file in the interactive console.
-                :param filename: filename of script file to run
-                """
-                expanded_filename = os.path.expanduser(filename)
+            # Use self.py_locals as the locals() dictionary in the Python environment we are creating, but make
+            # a copy to prevent pyscripts from editing it. (e.g. locals().clear()). Only make a shallow copy since
+            # it's OK for py_locals to contain objects which are editable in a pyscript.
+            localvars = dict(self.py_locals)
+            localvars[self.py_bridge_name] = py_bridge
+            localvars['quit'] = py_quit
+            localvars['exit'] = py_quit
+
+            if self.self_in_py:
+                localvars['self'] = self
+
+            # Handle case where we were called by run_pyscript
+            if args.pyscript:
+                # Read the script file
+                expanded_filename = os.path.expanduser(utils.strip_quotes(args.pyscript))
 
                 try:
                     with open(expanded_filename) as f:
-                        interp.runcode(f.read())
+                        py_code_to_run = f.read()
                 except OSError as ex:
                     self.pexcept("Error reading script file '{}': {}".format(expanded_filename, ex))
+                    return
 
-            def py_quit():
-                """Function callable from the interactive Python console to exit that environment"""
-                raise EmbeddedConsoleExit
+                localvars['__name__'] = '__main__'
+                localvars['__file__'] = expanded_filename
 
-            # Set up Python environment
-            self.py_locals[self.py_bridge_name] = py_bridge
-            self.py_locals['run'] = py_run
-            self.py_locals['quit'] = py_quit
-            self.py_locals['exit'] = py_quit
+                # Place the script's directory at sys.path[0] just as Python does when executing a script
+                saved_sys_path = list(sys.path)
+                sys.path.insert(0, os.path.dirname(os.path.abspath(expanded_filename)))
 
-            if self.self_in_py:
-                self.py_locals['self'] = self
-            elif 'self' in self.py_locals:
-                del self.py_locals['self']
+            else:
+                # This is the default name chosen by InteractiveConsole when no locals are passed in
+                localvars['__name__'] = '__console__'
+
+                if args.command:
+                    py_code_to_run = args.command
+                    if args.remainder:
+                        py_code_to_run += ' ' + ' '.join(args.remainder)
 
-            localvars = self.py_locals
+                    # Set cmd_echo to True so PyBridge statements like: py app('help')
+                    # run at the command line will print their output.
+                    py_bridge.cmd_echo = True
+
+            # Create the Python interpreter
             interp = InteractiveConsole(locals=localvars)
-            interp.runcode('import sys, os;sys.path.insert(0, os.getcwd())')
 
             # Check if we are running Python code
             if py_code_to_run:
@@ -3177,8 +3179,7 @@ def py_quit():
             else:
                 cprt = 'Type "help", "copyright", "credits" or "license" for more information.'
                 instructions = ('End with `Ctrl-D` (Unix) / `Ctrl-Z` (Windows), `quit()`, `exit()`.\n'
-                                'Non-Python commands can be issued with: {}("your command")\n'
-                                'Run Python code from external script files with: run("script.py")'
+                                'Non-Python commands can be issued with: {}("your command")'
                                 .format(self.py_bridge_name))
 
                 saved_cmd2_env = None
@@ -3205,7 +3206,10 @@ def py_quit():
             pass
 
         finally:
-            self._in_py = False
+            with self.sigint_protection:
+                if saved_sys_path is not None:
+                    sys.path = saved_sys_path
+                self._in_py = False
 
         return py_bridge.stop
 

tests/pyscript/environment.py

@@ -0,0 +1,20 @@
+# flake8: noqa F821
+# Tests that cmd2 populates __name__, __file__, and sets sys.path[0] to our directory
+import os
+import sys
+app.cmd_echo = True
+
+if __name__ != '__main__':
+    print("Error: __name__ is: {}".format(__name__))
+    quit()
+
+if __file__ != sys.argv[0]:
+    print("Error: __file__ is: {}".format(__file__))
+    quit()
+
+our_dir = os.path.dirname(os.path.abspath(__file__))
+if our_dir != sys.path[0]:
+    print("Error: our_dir is: {}".format(our_dir))
+    quit()
+
+print("PASSED")

tests/pyscript/recursive.py

@@ -5,6 +5,7 @@
 Example demonstrating that calling run_pyscript recursively inside another Python script isn't allowed
 """
 import os
+import sys
 
 app.cmd_echo = True
 my_dir = (os.path.dirname(os.path.realpath(sys.argv[0])))

tests/pyscript/run.py

@@ -205,14 +205,24 @@ def test_base_shell(base_app, monkeypatch):
 
 
 def test_base_py(base_app):
-    # Create a variable and make sure we can see it
-    out, err = run_cmd(base_app, 'py qqq=3')
-    assert not out
+    # Make sure py can't edit Cmd.py_locals. It used to be that cmd2 was passing its py_locals
+    # dictionary to the py environment instead of a shallow copy.
+    base_app.py_locals['test_var'] = 5
+    out, err = run_cmd(base_app, 'py del[locals()["test_var"]]')
+    assert not out and not err
+    assert base_app.py_locals['test_var'] == 5
 
-    out, err = run_cmd(base_app, 'py print(qqq)')
-    assert out[0].rstrip() == '3'
+    out, err = run_cmd(base_app, 'py print(test_var)')
+    assert out[0].rstrip() == '5'
+
+    # Place an editable object in py_locals. Since we make a shallow copy of py_locals,
+    # this object should be editable from the py environment.
+    base_app.py_locals['my_list'] = []
+    out, err = run_cmd(base_app, 'py my_list.append(2)')
+    assert not out and not err
+    assert base_app.py_locals['my_list'][0] == 2
 
-    # Add a more complex statement
+    # Try a print statement
     out, err = run_cmd(base_app, 'py print("spaces" + " in this " + "command")')
     assert out[0].rstrip() == 'spaces in this command'
 

tests/pyscript/to_run.py

@@ -117,10 +117,9 @@ def test_run_pyscript_stop(base_app, request):
     stop = base_app.onecmd_plus_hooks('run_pyscript {}'.format(python_script))
     assert stop
 
-def test_run_pyscript_run(base_app, request):
+def test_run_pyscript_environment(base_app, request):
     test_dir = os.path.dirname(request.module.__file__)
-    python_script = os.path.join(test_dir, 'pyscript', 'run.py')
-    expected = 'I have been run'
+    python_script = os.path.join(test_dir, 'pyscript', 'environment.py')
+    out, err = run_cmd(base_app, 'run_pyscript {}'.format(python_script))
 
-    out, err = run_cmd(base_app, "run_pyscript {}".format(python_script))
-    assert expected in out
+    assert out[0] == "PASSED"