holoviz/panel · #8940

Exec the command in TerminalSubprocess instead of running subprocess.run in the fork

philippjfr · merged Nov 1, 20263 files · 138 + / 29 −
examples/reference/widgets/Terminal.ipynb3 + / 3 −
@@ -48,10 +48,10 @@     "\n",     "#### Parameters\n",     "\n",-    "- **``args``** (str, list):  The arguments used to run the subprocess. This may be a string or a list. The string cannot contain spaces. See [subprocess.run](https://docs.python.org/3/library/subprocess.html) for more details.\n",-    "- **``kwargs``** (dict): Any other arguments to run the subprocess. See [subprocess.run](https://docs.python.org/3/library/subprocess.html) for more details.\n",+    "- **``args``** (str, list):  The arguments used to run the subprocess. This may be a string or a list. The string cannot contain spaces.\n",+    "- **``kwargs``** (dict): Additional options for running the subprocess. Only `cwd`, `env`, `executable` and `shell` are supported, with the same meaning as in [subprocess.run](https://docs.python.org/3/library/subprocess.html). Other keys emit a deprecation warning and are ignored.\n",     "- **``running``** (boolean, readonly): Whether or not the subprocess is running. Defaults to False.\n",-    "- **``run``** (action): Executes `subprocess.run` in a child process using the args and kwargs parameters provided as arguments or as parameter values on the instance. The child process is running in a *pseudo terminal* ([pty](https://docs.python.org/3/library/pty.html)) which is then connected to the Terminal.\n",+    "- **``run``** (action): Executes the command with `os.execvp` in a child process using the args and kwargs parameters provided as arguments or as parameter values on the instance. The child process is running in a *pseudo terminal* ([pty](https://docs.python.org/3/library/pty.html)) which is then connected to the Terminal.\n",     "- **``kill``** (action): Kills the subprocess if it is running.\n",     "\n",     "\n",
panel/tests/widgets/test_terminal.py77 + / 0 −
@@ -75,6 +75,83 @@ async def test_run_list_args():     assert subprocess.running     subprocess.kill() +_SCRIPT = """+import atexit, os, sys, time, warnings+import panel as pn++marker = sys.argv[1]+atexit.register(lambda: open(marker, 'a').write(f'{os.getpid()}\\n'))++terminal = pn.widgets.Terminal()+sub = terminal.subprocess+with warnings.catch_warnings(record=True) as caught:+    warnings.simplefilter('always')+    sub.run(*eval(sys.argv[2]), **eval(sys.argv[3]))+print('WARNINGS', [str(w.message) for w in caught])+for _ in range(100):+    sub._forward_subprocess_output_to_terminal()+    if not sub.running:+        break+    time.sleep(0.05)+print('RUNNING' if sub.running else 'DONE', repr(terminal.output))+"""+++def _run_script(tmp_path, args, kwargs=None):+    import os+    import subprocess++    marker = tmp_path / 'marker'+    env = {**os.environ, 'PYTHONPATH': os.pathsep.join(sys.path)}+    proc = subprocess.run(+        [sys.executable, '-c', _SCRIPT, str(marker), repr(args), repr(kwargs or {})],+        capture_output=True, text=True, timeout=60, env=env+    )+    assert proc.returncode == 0, proc.stderr+    pids = marker.read_text().split() if marker.exists() else []+    return proc.stdout.strip(), pids+++@not_windows+@pytest.mark.subprocess+def test_subprocess_child_does_not_run_parent_atexit_handlers(tmp_path):+    out, pids = _run_script(tmp_path, [sys.executable, '-c', 'print("hello")'])+    assert "DONE" in out+    assert 'hello' in out+    assert len(pids) == 1+++@not_windows+@pytest.mark.subprocess+def test_subprocess_failed_exec_does_not_run_parent_code(tmp_path):+    out, pids = _run_script(tmp_path, ['this-command-does-not-exist'])+    assert "DONE" in out+    assert 'this-command-does-not-exist' in out or 'No such file' in out+    assert len(pids) == 1+++@not_windows+@pytest.mark.subprocess+def test_subprocess_cwd_env_and_shell(tmp_path):+    out, pids = _run_script(+        tmp_path, ['echo $PWD-$FOO'],+        {'shell': True, 'cwd': str(tmp_path), 'env': {'FOO': 'bar', 'PATH': '/bin:/usr/bin'}}+    )+    assert "DONE" in out+    assert f'{tmp_path.resolve()}-bar' in out.replace('\\r', '').replace(str(tmp_path), str(tmp_path.resolve()))+    assert len(pids) == 1+++@not_windows+@pytest.mark.subprocess+def test_subprocess_unsupported_kwargs_warn(tmp_path):+    out, pids = _run_script(tmp_path, [sys.executable, '-c', 'print("hello")'], {'timeout': 1})+    assert 'timeout' in out.splitlines()[0]+    assert 'DONE' in out+    assert 'hello' in out+    assert len(pids) == 1++ def test_cannot_assign_string_args_with_spaces():     terminal = pn.widgets.Terminal()     subprocess = terminal.subprocess
panel/widgets/terminal.py58 + / 26 −
@@ -10,7 +10,6 @@ import select import shlex import signal-import subprocess import sys import typing as t @@ -20,6 +19,7 @@  from ..io.callbacks import PeriodicCallback from ..util import edit_readonly, lazy_load+from ..util.warnings import deprecated from .base import Widget  if t.TYPE_CHECKING:@@ -39,14 +39,16 @@ class TerminalSubprocess(param.Parameterized):      args = param.ClassSelector(class_=(str, list), doc="""         The arguments used to run the subprocess. This may be a string-        or a list. The string cannot contain spaces. See subprocess.run-        docs for more details.""")+        or a list. The string cannot contain spaces.""")      kill = param.Action(doc="Kills the running process", constant=True)      kwargs = param.Dict(doc="""-        Any other arguments to run the subprocess. See subprocess.run-        docs for more details.""" )+        Additional options for running the subprocess. The command is+        executed in a pseudo terminal with os.execvp(e), so only+        'cwd', 'env', 'executable' and 'shell' are supported, with the+        same meaning as in subprocess.run. Other keys emit a+        deprecation warning and are ignored.""" )      running = param.Boolean(default=False, constant=True, doc="""         Whether or not the subprocess is running.""")@@ -55,6 +57,8 @@ class TerminalSubprocess(param.Parameterized):      _fd = param.Integer(default=0, doc="Child file descriptor.") +    _exec_kwargs = frozenset({'cwd', 'env', 'executable', 'shell'})+     _max_read_bytes = param.Integer(default=1024 * 20)      _periodic_callback = param.ClassSelector(class_=PeriodicCallback, doc="""@@ -106,21 +110,43 @@ def run(self, *args, **kwargs):         if self.kwargs:             kwargs = {**self.kwargs, **kwargs} +        for key in sorted(set(kwargs) - self._exec_kwargs):+            deprecated(+                '2.0', f'TerminalSubprocess kwarg {key}',+                extra=(+                    f"The command is now executed directly, so {key!r} is ignored. "+                    f"Only {sorted(self._exec_kwargs)} are supported."+                )+            )+            del kwargs[key]++        argv = [args] if isinstance(args, str) else list(args)+        if kwargs.get('shell'):+            argv = ['/bin/sh', '-c', *argv]+        executable = kwargs.get('executable') or argv[0]+         # A fork is an operation whereby a process creates a copy of itself         # The two processes will continue from here as a PARENT and a CHILD process         (child_pid, fd) = pty.fork()          if child_pid == 0:-            # This is the CHILD process fork.-            # Anything printed here will show up in the pty, including the output-            # of this subprocess-            # The process will end by printing 'CompletedProcess(...)' to signal to the parent-            # that it finished.+            # This is the CHILD process fork. The command replaces this process+            # so that none of the parent's atexit handlers or finalizers can run.+            # Anything printed here will show up in the pty.             try:-                result = subprocess.run(args, **kwargs)-                print(str(result))  # noqa: T201-            except FileNotFoundError as e:-                print(str(e) + "\nCompletedProcess('FileNotFoundError')")  # noqa: T201+                signal.signal(signal.SIGPIPE, signal.SIG_DFL)+                if kwargs.get('cwd') is not None:+                    os.chdir(kwargs['cwd'])+                if kwargs.get('env') is None:+                    os.execvp(executable, argv)+                else:+                    os.execvpe(executable, argv, kwargs['env'])+            except BaseException as e:+                try:+                    print(str(e))  # noqa: T201+                    sys.stdout.flush()+                finally:+                    os._exit(127)         else:             # this is the PARENT process fork.             self._child_pid = child_pid@@ -176,10 +202,6 @@ def _reset(self):         with param.edit_constant(self):             self.running = False -    @staticmethod-    def _remove_last_line_from_string(value):-        return value[: value.rfind("CompletedProcess")]-     def _decode_utf8_on_boundary(self, fd, max_read_bytes, max_extra_bytes=2):         "UTF-8 characters can be multi-byte so need to decode on correct boundary"         data = os.read(fd, max_read_bytes)@@ -194,16 +216,26 @@ def _forward_subprocess_output_to_terminal(self):         if not self._fd:             return         (data_ready, _, _) = select.select([self._fd], [], [], self._timeout_sec)-        if not data_ready:+        if data_ready:+            try:+                output = self._decode_utf8_on_boundary(self._fd, self._max_read_bytes)+            except OSError:+                # Linux raises EIO on the pty master once the child has closed it+                output = ''+            if output:+                self._terminal.write(output)+                return+            self._child_has_exited()+        elif not self._child_has_exited():             return+        self._reset() -        output = self._decode_utf8_on_boundary(self._fd, self._max_read_bytes)--        # If Child Process finished it will signal this by appending "CompletedProcess(...)"-        if "CompletedProcess" in output:-            self._reset()-            output = self._remove_last_line_from_string(output)-        self._terminal.write(output)+    def _child_has_exited(self):+        try:+            pid, _ = os.waitpid(self._child_pid, os.WNOHANG)+        except ChildProcessError:+            return True+        return pid != 0      def _forward_terminal_input_to_subprocess(self, *events):         if self._fd: