diff --git a/.github/config/localhost-config.yaml b/.github/config/localhost-config.yaml index e6a7912c85..ef0ca22365 100644 --- a/.github/config/localhost-config.yaml +++ b/.github/config/localhost-config.yaml @@ -1,2 +1,3 @@ --- +use_login_shell: true safe_interval: 0 diff --git a/aiida/transports/plugins/local.py b/aiida/transports/plugins/local.py index efc3628fdb..1a563e1f21 100644 --- a/aiida/transports/plugins/local.py +++ b/aiida/transports/plugins/local.py @@ -42,7 +42,6 @@ class LocalTransport(Transport): ``unset PYTHONPATH`` if you plan on running calculations that use Python. """ - # There are no valid parameters for the local transport _valid_auth_options = [] # There is no real limit on how fast you can safely connect to a localhost, unlike often the case with SSH transport @@ -744,7 +743,9 @@ def _exec_command_internal(self, command, **kwargs): # pylint: disable=unused-a # Note: The outer shell will eat one level of escaping, while # 'bash -l -c ...' will eat another. Thus, we need to escape again. - command = 'bash -l -c ' + escape_for_bash(command) + bash_commmand = self._bash_command_str + '-c ' + + command = bash_commmand + escape_for_bash(command) proc = subprocess.Popen( command, @@ -803,11 +804,8 @@ def gotocomputer_command(self, remotedir): :param str remotedir: the full path of the remote directory """ - script = ' ; '.join([ - 'if [ -d {escaped_remotedir} ]', 'then cd {escaped_remotedir}', 'bash', "else echo ' ** The directory'", - "echo ' ** {remotedir}'", "echo ' ** seems to have been deleted, I logout...'", 'fi' - ]).format(escaped_remotedir="'{}'".format(remotedir), remotedir=remotedir) - cmd = 'bash -c "{}"'.format(script) + connect_string = self._gotocomputer_string(remotedir) + cmd = 'bash -c {}'.format(connect_string) return cmd def rename(self, oldpath, newpath): diff --git a/aiida/transports/plugins/ssh.py b/aiida/transports/plugins/ssh.py index 7070fb5428..1482d49b60 100644 --- a/aiida/transports/plugins/ssh.py +++ b/aiida/transports/plugins/ssh.py @@ -1232,7 +1232,9 @@ def _exec_command_internal(self, command, combine_stderr=False, bufsize=-1): # # Note: The default shell will eat one level of escaping, while # 'bash -l -c ...' will eat another. Thus, we need to escape again. - channel.exec_command('bash -l -c ' + escape_for_bash(command_to_execute)) + bash_commmand = self._bash_command_str + '-c ' + + channel.exec_command(bash_commmand + escape_for_bash(command_to_execute)) stdin = channel.makefile('wb', bufsize) stdout = channel.makefile('rb', bufsize) @@ -1298,21 +1300,14 @@ def gotocomputer_command(self, remotedir): further_params.append('-i {}'.format(escape_for_bash(self._connect_args['key_filename']))) further_params_str = ' '.join(further_params) - # I use triple strings because I both have single and double quotes, but I still want everything in - # a single line - connect_string = ( - """ssh -t {machine} {further_params} "if [ -d {escaped_remotedir} ] ;""" - """ then cd {escaped_remotedir} ; bash -l ; else echo ' ** The directory' ; """ - """echo ' ** {remotedir}' ; echo ' ** seems to have been deleted, I logout...' ; fi" """.format( - further_params=further_params_str, - machine=self._machine, - escaped_remotedir="'{}'".format(remotedir), - remotedir=remotedir - ) - ) - # print connect_string - return connect_string + connect_string = self._gotocomputer_string(remotedir) + cmd = 'ssh -t {machine} {further_params} {connect_string}'.format( + further_params=further_params_str, + machine=self._machine, + connect_string=connect_string, + ) + return cmd def symlink(self, remotesource, remotedestination): """ diff --git a/aiida/transports/transport.py b/aiida/transports/transport.py index d0281f46e8..c51c577a42 100644 --- a/aiida/transports/transport.py +++ b/aiida/transports/transport.py @@ -47,21 +47,50 @@ class Transport(abc.ABC): _valid_auth_params = None _MAGIC_CHECK = re.compile('[*?[]') _valid_auth_options = [] - _common_auth_options = [( - 'safe_interval', { - 'type': float, - 'prompt': 'Connection cooldown time (s)', - 'help': 'Minimum time interval in seconds between opening new connections.', - 'callback': validate_positive_number - } - )] + _common_auth_options = [ + ( + 'use_login_shell', { + 'default': + True, + 'switch': + True, + 'prompt': + 'Use login shell when executing command', + 'help': + ' Not using a login shell can help suppress potential' + ' spurious text output that can prevent AiiDA from parsing the output of commands,' + ' but may result in some startup files (.profile) not being sourced.', + 'non_interactive_default': + True + } + ), + ( + 'safe_interval', { + 'type': float, + 'prompt': 'Connection cooldown time (s)', + 'help': 'Minimum time interval in seconds between opening new connections.', + 'callback': validate_positive_number + } + ), + ] def __init__(self, *args, **kwargs): # pylint: disable=unused-argument """ __init__ method of the Transport base class. + + :param safe_interval: (optional, default self._DEFAULT_SAFE_OPEN_INTERVAL) + Minimum time interval in seconds between opening new connections. + :param use_login_shell: (optional, default True) + if False, do not use a login shell when executing command """ from aiida.common import AIIDA_LOGGER self._safe_open_interval = kwargs.pop('safe_interval', self._DEFAULT_SAFE_OPEN_INTERVAL) + self._use_login_shell = kwargs.pop('use_login_shell', True) + if self._use_login_shell: + self._bash_command_str = 'bash -l ' + else: + self._bash_command_str = 'bash ' + self._logger = AIIDA_LOGGER.getChild('transport').getChild(self.__class__.__name__) self._logger_extra = None self._is_open = False @@ -193,6 +222,13 @@ def _get_safe_interval_suggestion_string(cls, computer): # pylint: disable=unus """ return cls._DEFAULT_SAFE_OPEN_INTERVAL + @classmethod + def _get_use_login_shell_suggestion_string(cls, computer): # pylint: disable=unused-argument + """ + Return a suggestion for the specific field. + """ + return 'True' + @property def logger(self): """ @@ -756,6 +792,18 @@ def glob0(self, dirname, basename): def has_magic(self, string): return self._MAGIC_CHECK.search(string) is not None + def _gotocomputer_string(self, remotedir): + """command executed when goto computer.""" + connect_string = ( + """ "if [ -d {escaped_remotedir} ] ;""" + """ then cd {escaped_remotedir} ; {bash_command} ; else echo ' ** The directory' ; """ + """echo ' ** {remotedir}' ; echo ' ** seems to have been deleted, I logout...' ; fi" """.format( + bash_command=self._bash_command_str, escaped_remotedir="'{}'".format(remotedir), remotedir=remotedir + ) + ) + + return connect_string + class TransportInternalError(InternalError): """ diff --git a/docs/source/intro/troubleshooting.rst b/docs/source/intro/troubleshooting.rst index ecc93ad576..c4307c20fd 100644 --- a/docs/source/intro/troubleshooting.rst +++ b/docs/source/intro/troubleshooting.rst @@ -269,6 +269,13 @@ To test if a the computer does not produce spurious output, run (after configuri which checks and, in case of problems, suggests how to solve the problem. +.. note:: + + If the methods explained above do not work, you can configure AiiDA to not use a login shell when connecting to your computer, which may prevent the spurious output from being printed: + During ``verdi computer configure``, set ``-no-use-login-shell`` or when asked to use a login shell, set it to ``False``. + Note, however, that this may result in a slightly different environment, since `certain startup files are only sourced for login shells `_. + + .. _StackExchange thread: https://apple.stackexchange.com/questions/51036/what-is-the-difference-between-bash-profile-and-bashrc diff --git a/tests/cmdline/commands/test_computer.py b/tests/cmdline/commands/test_computer.py index 668728c21f..b9a5467e79 100644 --- a/tests/cmdline/commands/test_computer.py +++ b/tests/cmdline/commands/test_computer.py @@ -370,9 +370,16 @@ def test_local_interactive(self): comp = self.comp_builder.new() comp.store() - result = self.cli_runner.invoke(computer_configure, ['local', comp.label], input='\n', catch_exceptions=False) + command_input = ('{use_login_shell}\n{safe_interval}\n').format(use_login_shell='False', safe_interval='1.0') + result = self.cli_runner.invoke( + computer_configure, ['local', comp.label], input=command_input, catch_exceptions=False + ) self.assertTrue(comp.is_user_configured(self.user), msg=result.output) + new_auth_params = comp.get_authinfo(self.user).get_auth_params() + self.assertEqual(new_auth_params['use_login_shell'], False) + self.assertEqual(new_auth_params['safe_interval'], 1.0) + def test_ssh_interactive(self): """ Check that the interactive prompt is accepting the correct values. @@ -411,6 +418,7 @@ def test_ssh_interactive(self): self.assertEqual(new_auth_params['port'], port) self.assertEqual(new_auth_params['look_for_keys'], look_for_keys) self.assertEqual(new_auth_params['key_filename'], key_filename) + self.assertEqual(new_auth_params['use_login_shell'], True) def test_local_from_config(self): """Test configuring a computer from a config file""" diff --git a/tests/transports/test_local.py b/tests/transports/test_local.py index 35b3e247f1..fa88e00468 100644 --- a/tests/transports/test_local.py +++ b/tests/transports/test_local.py @@ -48,3 +48,16 @@ def test_basic(): """Test constructor.""" with LocalTransport(): pass + + +def test_gotocomputer(): + """Test gotocomputer""" + with LocalTransport() as transport: + cmd_str = transport.gotocomputer_command('/remote_dir/') + + expected_str = ( + """bash -c "if [ -d '/remote_dir/' ] ;""" + """ then cd '/remote_dir/' ; bash -l ; else echo ' ** The directory' ; """ + """echo ' ** /remote_dir/' ; echo ' ** seems to have been deleted, I logout...' ; fi" """ + ) + assert cmd_str == expected_str diff --git a/tests/transports/test_ssh.py b/tests/transports/test_ssh.py index 2b1e083cf3..6c8c713622 100644 --- a/tests/transports/test_ssh.py +++ b/tests/transports/test_ssh.py @@ -55,3 +55,16 @@ def test_no_host_key(self): # Reset logging level logging.disable(logging.NOTSET) + + +def test_gotocomputer(): + """Test gotocomputer""" + with SshTransport(machine='localhost', timeout=30, use_login_shell=False, key_policy='AutoAddPolicy') as transport: + cmd_str = transport.gotocomputer_command('/remote_dir/') + + expected_str = ( + """ssh -t localhost "if [ -d '/remote_dir/' ] ;""" + """ then cd '/remote_dir/' ; bash ; else echo ' ** The directory' ; """ + """echo ' ** /remote_dir/' ; echo ' ** seems to have been deleted, I logout...' ; fi" """ + ) + assert cmd_str == expected_str