holoviz/panel · #8940
Exec the command in TerminalSubprocess instead of running subprocess.run in the fork
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.subprocesspanel/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: