From 30ef0ed9fef5f0f2c3831aaf5bac84cfa316b3ea Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sat, 22 Jun 2019 03:12:54 -0400 Subject: [PATCH 01/10] [execute_process] emulate_tty now configurable and defaults to true Signed-off-by: Daniel Stonier --- launch/launch/actions/execute_process.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 46af6cb0e..2fe2962d7 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -22,6 +22,7 @@ import signal import threading import traceback +import yaml from typing import Any # noqa: F401 from typing import Callable from typing import cast @@ -95,6 +96,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 +175,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 +215,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 +582,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 +601,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: From 9e2b25c76bb3419522193e94171f1ed1ac885cba Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sat, 22 Jun 2019 17:56:32 -0400 Subject: [PATCH 02/10] [examples] do not emulate ttys Signed-off-by: Daniel Stonier --- .../examples/disable_emulate_tty_counters.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100755 launch/examples/disable_emulate_tty_counters.py diff --git a/launch/examples/disable_emulate_tty_counters.py b/launch/examples/disable_emulate_tty_counters.py new file mode 100755 index 000000000..3f3f0f966 --- /dev/null +++ b/launch/examples/disable_emulate_tty_counters.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 + +# Copyright 2015 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()) From d7aa0748085b64f91f13acc57112784eb16e8d8a Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sat, 22 Jun 2019 17:58:31 -0400 Subject: [PATCH 03/10] trivial Signed-off-by: Daniel Stonier --- launch/examples/disable_emulate_tty_counters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/examples/disable_emulate_tty_counters.py b/launch/examples/disable_emulate_tty_counters.py index 3f3f0f966..bf5ca01b1 100755 --- a/launch/examples/disable_emulate_tty_counters.py +++ b/launch/examples/disable_emulate_tty_counters.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -# Copyright 2015 Open Source Robotics Foundation, Inc. +# 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. From 7fd3e656a3bff6e71c3d086f20d82de141392afd Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sun, 23 Jun 2019 00:02:55 -0400 Subject: [PATCH 04/10] test for emulate_tty configuration Signed-off-by: Daniel Stonier --- launch/test/launch/test_emulate_tty.py | 52 ++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 launch/test/launch/test_emulate_tty.py diff --git a/launch/test/launch/test_emulate_tty.py b/launch/test/launch/test_emulate_tty.py new file mode 100644 index 000000000..5d66a0ee8 --- /dev/null +++ b/launch/test/launch/test_emulate_tty.py @@ -0,0 +1,52 @@ +# Copyright 2018 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 sys + +import launch +import pytest + + +class OnExit(object): + def __init__(self): + self.returncode = None + + def handle(self, event, context): + self.returncode = event.returncode + + +@pytest.mark.parametrize('test_input,expected', [ + (None, 1), # use the default defined by ExecuteProcess + ('true', 1), # redundantly override the default via LaunchConfiguration + ('false', 0) # override the default via LaunchConfiguration +]) +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 From bb5ac23eeb2ae307ff2798089ea132431414a9a9 Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sun, 23 Jun 2019 00:05:46 -0400 Subject: [PATCH 05/10] copyright Signed-off-by: Daniel Stonier --- launch/test/launch/test_emulate_tty.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/test/launch/test_emulate_tty.py b/launch/test/launch/test_emulate_tty.py index 5d66a0ee8..0cb53c332 100644 --- a/launch/test/launch/test_emulate_tty.py +++ b/launch/test/launch/test_emulate_tty.py @@ -1,4 +1,4 @@ -# Copyright 2018 Open Source Robotics Foundation, Inc. +# 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. From 27fada4ec05b3bf14b94d65a3bca1d4fff328caf Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Mon, 24 Jun 2019 23:07:54 -0400 Subject: [PATCH 06/10] attempted flake8 fixes Signed-off-by: Daniel Stonier --- launch/launch/actions/execute_process.py | 9 +++++---- launch/test/launch/test_emulate_tty.py | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 2fe2962d7..499e0c200 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -22,7 +22,6 @@ import signal import threading import traceback -import yaml from typing import Any # noqa: F401 from typing import Callable from typing import cast @@ -34,6 +33,8 @@ from typing import Tuple # noqa: F401 from typing import Union +import yaml + import launch.logging from osrf_pycommon.process_utils import async_execute_process @@ -587,9 +588,9 @@ async def __execute_process(self, context: LaunchContext) -> None: context.launch_configurations['emulate_tty'] ) if type(emulate_tty) is not bool: - raise TypeError( - "emulate_tty is not boolean [{}]".format(type(emulate_tty)) - ) + raise TypeError("emulate_tty is not boolean [{}]".format( + type(emulate_tty) + )) except KeyError: emulate_tty = self.__emulate_tty try: diff --git a/launch/test/launch/test_emulate_tty.py b/launch/test/launch/test_emulate_tty.py index 0cb53c332..f531622e9 100644 --- a/launch/test/launch/test_emulate_tty.py +++ b/launch/test/launch/test_emulate_tty.py @@ -21,6 +21,7 @@ class OnExit(object): + def __init__(self): self.returncode = None From f636053ad41dfb00f6bbc85a64ee338344d48833 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 25 Jun 2019 12:58:08 +0000 Subject: [PATCH 07/10] Fix flake8 errors. Signed-off-by: Chris Lalancette --- launch/launch/actions/execute_process.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 499e0c200..a1bf9a67e 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -33,13 +33,13 @@ from typing import Tuple # noqa: F401 from typing import Union -import yaml - import launch.logging 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 @@ -588,7 +588,7 @@ async def __execute_process(self, context: LaunchContext) -> None: context.launch_configurations['emulate_tty'] ) if type(emulate_tty) is not bool: - raise TypeError("emulate_tty is not boolean [{}]".format( + raise TypeError('emulate_tty is not boolean [{}]'.format( type(emulate_tty) )) except KeyError: From 9565430e87266a4989d8cd51b8e3c4348fb94e01 Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sat, 29 Jun 2019 11:35:47 -0400 Subject: [PATCH 08/10] guard for windows (#1) Signed-off-by: Daniel Stonier --- launch/launch/actions/execute_process.py | 7 +++++ .../launch/{ => actions}/test_emulate_tty.py | 27 +++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) rename launch/test/launch/{ => actions}/test_emulate_tty.py (67%) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index a1bf9a67e..f380b737c 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -593,6 +593,13 @@ async def __execute_process(self, context: LaunchContext) -> None: )) except KeyError: emulate_tty = self.__emulate_tty + # guard against windows - tty emulation is not yet working + # https://github.com/ros2/launch/issues/268 + if emulate_tty and platform.system() == 'Windows': + self.__logger.warning( + "tty emulation not yet supported on windows, disabling" + ) + emulate_tty = False try: transport, self._subprocess_protocol = await async_execute_process( lambda **kwargs: self.__ProcessProtocol( diff --git a/launch/test/launch/test_emulate_tty.py b/launch/test/launch/actions/test_emulate_tty.py similarity index 67% rename from launch/test/launch/test_emulate_tty.py rename to launch/test/launch/actions/test_emulate_tty.py index f531622e9..45b49b64e 100644 --- a/launch/test/launch/test_emulate_tty.py +++ b/launch/test/launch/actions/test_emulate_tty.py @@ -14,6 +14,7 @@ """Tests for emulate_tty configuration of ExecuteProcess actions.""" +import platform import sys import launch @@ -29,19 +30,35 @@ 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', [ - (None, 1), # use the default defined by ExecuteProcess - ('true', 1), # redundantly override the default via LaunchConfiguration - ('false', 0) # override the default via LaunchConfiguration + # 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())']) + 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.SetLaunchConfiguration( + 'emulate_tty', + test_input + ) + ) ld.add_action( launch.actions.RegisterEventHandler( launch.event_handlers.OnProcessExit(on_exit=on_exit.handle) From 32c44f744d9ff0cbc440a21de936060e489a74d6 Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Sun, 7 Jul 2019 11:02:37 -0400 Subject: [PATCH 09/10] linting, fix quotes on strings Signed-off-by: Daniel Stonier --- launch/launch/actions/execute_process.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index f380b737c..7705c0139 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -597,7 +597,7 @@ async def __execute_process(self, context: LaunchContext) -> None: # https://github.com/ros2/launch/issues/268 if emulate_tty and platform.system() == 'Windows': self.__logger.warning( - "tty emulation not yet supported on windows, disabling" + 'tty emulation not yet supported on windows, disabling' ) emulate_tty = False try: From 71d805c7ed220d0331871bd444b83f219b6a841a Mon Sep 17 00:00:00 2001 From: Daniel Stonier Date: Tue, 16 Jul 2019 18:07:51 -0400 Subject: [PATCH 10/10] drop the warning and guard, lean on the graceful degradation as recommended in review Signed-off-by: Daniel Stonier --- launch/launch/actions/execute_process.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 7705c0139..a1bf9a67e 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -593,13 +593,6 @@ async def __execute_process(self, context: LaunchContext) -> None: )) except KeyError: emulate_tty = self.__emulate_tty - # guard against windows - tty emulation is not yet working - # https://github.com/ros2/launch/issues/268 - if emulate_tty and platform.system() == 'Windows': - self.__logger.warning( - 'tty emulation not yet supported on windows, disabling' - ) - emulate_tty = False try: transport, self._subprocess_protocol = await async_execute_process( lambda **kwargs: self.__ProcessProtocol(