[execute_process] emulate_tty configurable and defaults to true (ros2…

…#265)

* [execute_process] emulate_tty now configurable and defaults to true

Signed-off-by: Daniel Stonier <[email protected]>

* [examples] do not emulate ttys

Signed-off-by: Daniel Stonier <[email protected]>

* trivial

Signed-off-by: Daniel Stonier <[email protected]>

* test for emulate_tty configuration

Signed-off-by: Daniel Stonier <[email protected]>

* copyright

Signed-off-by: Daniel Stonier <[email protected]>

* attempted flake8 fixes

Signed-off-by: Daniel Stonier <[email protected]>

* Fix flake8 errors.

Signed-off-by: Chris Lalancette <[email protected]>

* guard for windows (#1)

Signed-off-by: Daniel Stonier <[email protected]>

* linting, fix quotes on strings

Signed-off-by: Daniel Stonier <[email protected]>

* drop the warning and guard, lean on the graceful degradation as recommended in review

Signed-off-by: Daniel Stonier <[email protected]>

launch/examples/disable_emulate_tty_counters.py

@@ -0,0 +1,60 @@
+#!/usr/bin/env python3
+
+# Copyright 2019 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Script that demonstrates disabling tty emulation.
+
+This is most significant for python processes which, without tty
+emulation, will be buffered by default and have various other
+capabilities disabled."
+"""
+
+import os
+import sys
+from typing import cast
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))  # noqa
+
+import launch
+
+
+def generate_launch_description():
+    ld = launch.LaunchDescription()
+
+    # Disable tty emulation (on by default).
+    ld.add_action(launch.actions.SetLaunchConfiguration('emulate_tty', 'false'))
+
+    # Wire up stdout from processes
+    def on_output(event: launch.Event) -> None:
+        for line in event.text.decode().splitlines():
+            print('[{}] {}'.format(
+                cast(launch.events.process.ProcessIO, event).process_name, line))
+
+    ld.add_action(launch.actions.RegisterEventHandler(launch.event_handlers.OnProcessIO(
+        on_stdout=on_output,
+    )))
+
+    # Execute
+    ld.add_action(launch.actions.ExecuteProcess(
+        cmd=[sys.executable, './counter.py']
+    ))
+    return ld
+
+
+if __name__ == '__main__':
+    # ls = LaunchService(argv=argv, debug=True)  # Use this instead to get more debug messages.
+    ls = launch.LaunchService(argv=sys.argv[1:])
+    ls.include_launch_description(generate_launch_description())
+    sys.exit(ls.run())

launch/launch/actions/execute_process.py

@@ -38,6 +38,8 @@
 from osrf_pycommon.process_utils import async_execute_process
 from osrf_pycommon.process_utils import AsyncSubprocessProtocol
 
+import yaml
+
 from .emit_event import EmitEvent
 from .opaque_function import OpaqueFunction
 from .timer_action import TimerAction
@@ -95,6 +97,7 @@ def __init__(
             'sigterm_timeout', default=5),
         sigkill_timeout: SomeSubstitutionsType = LaunchConfiguration(
             'sigkill_timeout', default=5),
+        emulate_tty: bool = True,
         prefix: Optional[SomeSubstitutionsType] = None,
         output: Text = 'log',
         output_format: Text = '[{this.name}] {line}',
@@ -173,6 +176,8 @@ def __init__(
             as a string or a list of strings and Substitutions to be resolved
             at runtime, defaults to the LaunchConfiguration called
             'sigkill_timeout'
+        :param: emulate_tty emulate a tty (terminal), defaults to
+            the LaunchConfiguration called 'emulate_tty'
         :param: prefix a set of commands/arguments to preceed the cmd, used for
             things like gdb/valgrind and defaults to the LaunchConfiguration
             called 'launch-prefix'
@@ -211,6 +216,7 @@ def __init__(
         self.__shell = shell
         self.__sigterm_timeout = normalize_to_list_of_substitutions(sigterm_timeout)
         self.__sigkill_timeout = normalize_to_list_of_substitutions(sigkill_timeout)
+        self.__emulate_tty = emulate_tty
         self.__prefix = normalize_to_list_of_substitutions(
             LaunchConfiguration('launch-prefix', default='') if prefix is None else prefix
         )
@@ -577,6 +583,16 @@ async def __execute_process(self, context: LaunchContext) -> None:
             self.__logger.info("process details: cmd=[{}], cwd='{}', custom_env?={}".format(
                 ', '.join(cmd), cwd, 'True' if env is not None else 'False'
             ))
+        try:
+            emulate_tty = yaml.safe_load(
+                context.launch_configurations['emulate_tty']
+            )
+            if type(emulate_tty) is not bool:
+                raise TypeError('emulate_tty is not boolean [{}]'.format(
+                    type(emulate_tty)
+                ))
+        except KeyError:
+            emulate_tty = self.__emulate_tty
         try:
             transport, self._subprocess_protocol = await async_execute_process(
                 lambda **kwargs: self.__ProcessProtocol(
@@ -586,7 +602,7 @@ async def __execute_process(self, context: LaunchContext) -> None:
                 cwd=cwd,
                 env=env,
                 shell=self.__shell,
-                emulate_tty=False,
+                emulate_tty=emulate_tty,
                 stderr_to_stdout=False,
             )
         except Exception:

launch/test/launch/actions/test_emulate_tty.py

@@ -0,0 +1,70 @@
+# Copyright 2019 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Tests for emulate_tty configuration of ExecuteProcess actions."""
+
+import platform
+import sys
+
+import launch
+import pytest
+
+
+class OnExit(object):
+
+    def __init__(self):
+        self.returncode = None
+
+    def handle(self, event, context):
+        self.returncode = event.returncode
+
+
+def tty_expected_unless_windows():
+    return 1 if platform.system() != 'Windows' else 0
+
+
+@pytest.mark.parametrize('test_input,expected', [
+    # use the default defined by ExecuteProcess
+    (None, tty_expected_unless_windows()),
+    # redundantly override the default via LaunchConfiguration
+    ('true', tty_expected_unless_windows()),
+    # override the default via LaunchConfiguration
+    ('false', 0)
+])
+def test_emulate_tty(test_input, expected):
+    on_exit = OnExit()
+    ld = launch.LaunchDescription()
+    ld.add_action(launch.actions.ExecuteProcess(
+        cmd=[sys.executable,
+             '-c',
+             'import sys; sys.exit(sys.stdout.isatty())'
+             ]
+        )
+    )
+    if test_input is not None:
+        ld.add_action(
+            launch.actions.SetLaunchConfiguration(
+                'emulate_tty',
+                test_input
+            )
+        )
+    ld.add_action(
+        launch.actions.RegisterEventHandler(
+            launch.event_handlers.OnProcessExit(on_exit=on_exit.handle)
+        )
+    )
+    ls = launch.LaunchService()
+    ls.include_launch_description(ld)
+    ls.run()
+    assert on_exit.returncode == expected