From a7b4f797208de8425533aaf9cf8857d7ec5d50d4 Mon Sep 17 00:00:00 2001 From: David Mirabito Date: Fri, 21 Jul 2017 15:49:53 +1000 Subject: [PATCH 01/91] Fix for issue #398. Give up on trying to fancy import if there is no parent in the call stack. This seems to be the case when calling PyImport_ImportModule() from C code, and if sh has already added itself to sys.meta_path exceptions would be thrown on subsequent imports. --- sh.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sh.py b/sh.py index 5b250e12..2c2281e5 100644 --- a/sh.py +++ b/sh.py @@ -3430,6 +3430,13 @@ def find_module(self, mod_fullname, path=None): the rest of this function """ parent_frame = inspect.currentframe().f_back + + # Calling PyImport_ImportModule("some_module"); via the C API may not + # have a parent frame. Early-out to avoid in_importlib() trying to + # get f_code from None when looking for 'some_module' + if not parent_frame: + return None + while in_importlib(parent_frame): parent_frame = parent_frame.f_back From b7a2cf9d9dd1d550e62fd574c1e066651af9bc9c Mon Sep 17 00:00:00 2001 From: Ben Felder <20211140+pykong@users.noreply.github.com> Date: Sun, 4 Feb 2018 13:30:23 +0100 Subject: [PATCH 02/91] Stressing any Mirrors *not* in paragraph below. Makes it clearer that this package is not a mere collection of system calls wrapped into python. --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 73b68c44..ac3387a9 100644 --- a/README.rst +++ b/README.rst @@ -20,7 +20,7 @@ | sh is a full-fledged subprocess replacement for Python 2.6 - 3.6, PyPy and PyPy3 -that allows you to call any program as if it were a function: +that allows you to call *any* program as if it were a function: .. code:: python From 6ee2b75834e3469bc45637b40988f47061f9342a Mon Sep 17 00:00:00 2001 From: ATolkachev Date: Tue, 10 Jul 2018 15:39:29 +0300 Subject: [PATCH 03/91] Update sh.py Fix `process completed` output every wait() run(e.g. for stdout, stderr and exit_code). --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..6df4fc6e 100644 --- a/sh.py +++ b/sh.py @@ -798,7 +798,7 @@ def wait(self): if self.process._stdin_process: self.process._stdin_process.command.wait() - self.log.info("process completed") + self.log.info("process completed") return self From 0439fea50e9049d8d30f61fbf6e4c83c9d514334 Mon Sep 17 00:00:00 2001 From: Hongxin Liang Date: Mon, 13 Aug 2018 17:26:43 +0200 Subject: [PATCH 04/91] capture full command in TimeoutException --- sh.py | 5 +++-- test.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sh.py b/sh.py index 5b250e12..be3aa748 100644 --- a/sh.py +++ b/sh.py @@ -394,8 +394,9 @@ class SignalException(ErrorReturnCode): pass class TimeoutException(Exception): """ the exception thrown when a command is killed because a specified timeout (via _timeout) was hit """ - def __init__(self, exit_code): + def __init__(self, exit_code, full_cmd): self.exit_code = exit_code + self.full_cmd = full_cmd super(Exception, self).__init__() SIGNALS_THAT_SHOULD_THROW_EXCEPTION = set(( @@ -786,7 +787,7 @@ def wait(self): # if we timed out, our exit code represents a signal, which is # negative, so let's make it positive to store in our # TimeoutException - raise TimeoutException(-exit_code) + raise TimeoutException(-exit_code, self.ran) else: self.handle_command_exit_code(exit_code) diff --git a/test.py b/test.py index 68ef40c5..e5c9c77a 100644 --- a/test.py +++ b/test.py @@ -1876,8 +1876,8 @@ def test_timeout(self): started = time() try: sh.sleep(sleep_for, _timeout=timeout).wait() - except sh.TimeoutException: - pass + except sh.TimeoutException as e: + self.assertEqual(e.full_cmd, '/bin/sleep 3') else: self.fail("no timeout exception") elapsed = time() - started From 342939b48021a1645aa72511e7d0000e5d09dfa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kiss=20Gy=C3=B6rgy?= Date: Tue, 14 Aug 2018 18:40:00 +0200 Subject: [PATCH 05/91] Less verbose logging for process startup on INFO level It is super important to have on INFO level which command is currently running, but it's super annoying and it makes debugging harder if very long commands get logged three times (with almost the exact output). We use sh extensively for short running command invocations (like CLI git calls) and it's just makes the log unnecessarily very noisy. I lowered the level for the "starting process" and "process completed" messages, and kept "process started", because it has the most information about the process (it includes the PID). I understand that it can be handy for longer running processes, but in that case, DEBUG level still can be used. --- sh.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sh.py b/sh.py index 5b250e12..22bdd937 100644 --- a/sh.py +++ b/sh.py @@ -753,7 +753,7 @@ def __init__(self, cmd, call_args, stdin, stdout, stderr): logger_str = log_str_factory(self.ran, call_args) self.log = Logger("command", logger_str) - self.log.info("starting process") + self.log.debug("starting process") if should_wait: self._spawned_and_waited = True @@ -798,7 +798,7 @@ def wait(self): if self.process._stdin_process: self.process._stdin_process.command.wait() - self.log.info("process completed") + self.log.debug("process completed") return self From 0f2e1ce94d090aae89df1ed7b7b1155b79aecfc5 Mon Sep 17 00:00:00 2001 From: Lukasz Dabrowski Date: Wed, 3 Oct 2018 15:53:33 +0200 Subject: [PATCH 06/91] avoid crashing when sys.meta_path contains importer which is not a class instance --- sh.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..3eedc76a 100644 --- a/sh.py +++ b/sh.py @@ -3391,7 +3391,12 @@ def register_importer(): """ def test(importer): - return importer.__class__.__name__ == ModuleImporterFromVariables.__name__ + try: + return importer.__class__.__name__ == ModuleImporterFromVariables.__name__ + except AttributeError: + # ran into importer which is not a class instance + return False + already_registered = any([True for i in sys.meta_path if test(i)]) if not already_registered: From b4bf708128d439a9e4fdf7135e097bb7d12ece3a Mon Sep 17 00:00:00 2001 From: Carl George Date: Wed, 20 Jun 2018 11:02:01 -0500 Subject: [PATCH 07/91] always use fully versioned python command in tests --- test.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/test.py b/test.py index 68ef40c5..f688a6c6 100644 --- a/test.py +++ b/test.py @@ -93,22 +93,21 @@ def append_module_path(env, m): ioStringIO = StringIO from io import BytesIO as cStringIO iocStringIO = cStringIO - python = sh.Command(sh.which("python%d.%d" % sys.version_info[:2])) else: from StringIO import StringIO from cStringIO import StringIO as cStringIO from io import StringIO as ioStringIO from io import BytesIO as iocStringIO - python = sh.python THIS_DIR = dirname(os.path.abspath(__file__)) +system_python = sh.Command(sys.executable) # this is to ensure that our `python` helper here is able to import our local sh # module, and not the system one baked_env = os.environ.copy() append_module_path(baked_env, sh) -python = python.bake(_env=baked_env) +python = system_python.bake(_env=baked_env) if hasattr(logging, 'NullHandler'): @@ -396,7 +395,6 @@ def test_quote_escaping(self): self.assertEqual(out, "[\"one two's three\"]") def test_multiple_pipes(self): - from sh import tr, python import time py = create_tmp_test(""" @@ -1634,10 +1632,10 @@ def test_cmd_eq(self): def test_fg(self): py = create_tmp_test("exit(0)") - # notice we're using `sh.python`, and not `python`. this is because + # notice we're using `system_python`, and not `python`. this is because # `python` has an env baked into it, and we want `_env` to be None for # coverage - sh.python(py.name, _fg=True) + system_python(py.name, _fg=True) def test_fg_env(self): py = create_tmp_test(""" @@ -2058,8 +2056,10 @@ def test_signal_group(self): """) parent = create_tmp_test(""" +import sys import sh -p = sh.python("{child_file}", _bg=True, _new_session=False) +python = sh.Command(sys.executable) +p = python("{child_file}", _bg=True, _new_session=False) print(p.pid) print(p.process.pgid) p.wait() @@ -2525,7 +2525,8 @@ def handle_sighup(signum, frame): child_file = sys.argv[1] output_file = sys.argv[2] -os.spawnlp(os.P_NOWAIT, "python", "python", child_file, output_file) +python_name = os.path.basename(sys.executable) +os.spawnlp(os.P_NOWAIT, python_name, python_name, child_file, output_file) time.sleep(1) # give child a chance to set up """) @@ -2769,10 +2770,11 @@ def test_stdin_nohang(self): def test_unicode_path(self): from sh import Command - py = create_tmp_test("""#!/usr/bin/env python + python_name = os.path.basename(sys.executable) + py = create_tmp_test("""#!/usr/bin/env {0} # -*- coding: utf8 -*- print("字") -""", "字", delete=False) +""".format(python_name), prefix="字", delete=False) try: py.close() @@ -2875,11 +2877,11 @@ def test_threaded_with_contexts(self): def f1(): with p1: time.sleep(1) - results[0] = str(sh.python("one")) + results[0] = str(system_python("one")) def f2(): with p2: - results[1] = str(sh.python("two")) + results[1] = str(system_python("two")) t1 = threading.Thread(target=f1) t1.start() @@ -3007,7 +3009,7 @@ def test_importer_detects_module_name(self): import sh _sh = sh() omg = _sh - from omg import python + from omg import cat def test_importer_only_works_with_sh(self): def unallowed_import(): From 55d5fd1c47d54443370f02ba4b05570bfb2f9045 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sat, 2 Mar 2019 11:10:27 -0800 Subject: [PATCH 08/91] Raising exceptions should use native strings. In Python3, the str type is a text string. On Python2, the str type is a byte string. When we raise exceptions we should use native strings otherwise Python may end up truncating or omitting the exception message (on Python2) or mangling them (if a byte string were to be given on Python3). Fixes #463 --- sh.py | 6 ++++++ test.py | 7 +++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sh.py b/sh.py index 5b250e12..87cda630 100644 --- a/sh.py +++ b/sh.py @@ -387,6 +387,12 @@ def __init__(self, full_cmd, stdout, stderr, truncate=True): stderr=exc_stderr.decode(DEFAULT_ENCODING, "replace") ) + if not IS_PY3: + # Exception messages should be treated as an API which takes native str type on both + # Python2 and Python3. (Meaning, it's a byte string on Python2 and a text string on + # Python3) + msg = encode_to_py3bytes_or_py2str(msg) + super(ErrorReturnCode, self).__init__(msg) diff --git a/test.py b/test.py index 68ef40c5..38d8fa0c 100644 --- a/test.py +++ b/test.py @@ -245,16 +245,15 @@ def test_unicode_exception(self): py = create_tmp_test("exit(1)") arg = "漢字" + native_arg = arg if not IS_PY3: arg = arg.decode("utf8") + try: python(py.name, arg, _encoding="utf8") except ErrorReturnCode as e: - if IS_PY3: - self.assertTrue(arg in str(e)) - else: - self.assertTrue(arg in unicode(e)) + self.assertTrue(native_arg in str(e)) else: self.fail("exception wasn't raised") From a4beb39f5143beb6095226fb7f4fee5da98da4e4 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sat, 2 Mar 2019 09:31:23 -0800 Subject: [PATCH 09/91] Fix for execution contexts from the REPL On the REPL and from the command line, Python doesn't keep source code in the stack frame for the toplevel. Therefore we can't inspect it and look for the variable being assigned to in order to remove it from sys.modules. This is a cornercase but we can hack around it by trying to retrieve the variable name from the code object instead. This depends more heavily on Python internals but this was already code which dove into Python's internals a bit so perhaps this is okay. Fixes #422 --- sh.py | 26 ++++++++++++++++++++++---- test.py | 25 +++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/sh.py b/sh.py index 5b250e12..24583678 100644 --- a/sh.py +++ b/sh.py @@ -3364,12 +3364,30 @@ def __call__(self, **kwargs): # cached module from sys.modules. if we don't, it gets re-used, and any # old baked params get used, which is not what we want parent = inspect.stack()[1] - code = parent[4][0].strip() - parsed = ast.parse(code) - module_name = parsed.body[0].targets[0].id + try: + code = parent[4][0].strip() + except TypeError: + # On the REPL or from the commandline, we don't get the source code in the + # top stack frame + # Older versions of pypy don't set parent[1] the same way as CPython or newer versions + # of Pypy so we have to special case that too. + if parent[1] in ('', '') or ( + parent[1] == '' and platform.python_implementation().lower() == 'pypy'): + # This depends on things like Python's calling convention and the layout of stack + # frames but it's a fix for a bug in a very cornery cornercase so.... + module_name = parent[0].f_code.co_names[-1] + else: + parsed = ast.parse(code) + try: + module_name = parsed.body[0].targets[0].id + except Exception: + # Diagnose what went wrong + if not isinstance(parsed.body[0], ast.Assign): + raise RuntimeError("A new execution context must be assigned to a variable") + raise if module_name == __name__: - raise RuntimeError("Cannot use the name 'sh' as an execution context") + raise RuntimeError("Cannot use the name '%s' as an execution context" % __name__) sys.modules.pop(module_name, None) diff --git a/test.py b/test.py index 68ef40c5..e41dcf33 100644 --- a/test.py +++ b/test.py @@ -3015,6 +3015,31 @@ def unallowed_import(): from _os import path self.assertRaises(ImportError, unallowed_import) + def test_reimport_from_cli(self): + # The REPL and CLI both need special handling to create an execution context that is safe to + # reimport + if IS_PY3: + cmdstr = '; '.join(('import sh, io, sys', + 'out = io.StringIO()', + '_sh = sh(_out=out)', + 'import _sh', + '_sh.echo("-n", "TEST")', + 'sys.stderr.write(out.getvalue())', + )) + else: + cmdstr = '; '.join(('import sh, StringIO, sys', + 'out = StringIO.StringIO()', + '_sh = sh(_out=out)', + 'import _sh', + '_sh.echo("-n", "TEST")', + 'sys.stderr.write(out.getvalue())', + )) + + err = StringIO() + + python('-c', cmdstr, _err=err) + self.assertEqual('TEST', err.getvalue()) + if __name__ == "__main__": root = logging.getLogger() From 22f69c483836e2d1bd7dc08683fefa151359270d Mon Sep 17 00:00:00 2001 From: pteromys Date: Sat, 20 Apr 2019 05:42:53 -0500 Subject: [PATCH 10/91] Make ErrorReturnCode "RAN:" copy-pastable. Using shlex.quote. Before: sh.ErrorReturnCode_1: RAN: /usr/bin/sh -c echo -rf build && ls / && exit 1 After: sh.ErrorReturnCode_1: RAN: /usr/bin/sh -c 'echo -rf build && ls / && exit 1' --- sh.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..0cd0cd16 100644 --- a/sh.py +++ b/sh.py @@ -87,6 +87,11 @@ def callable(ob): from io import BytesIO as iocStringIO from Queue import Queue, Empty +try: + from shlex import quote as shlex_quote # here from 3.3 onward +except ImportError: + from pipes import quote as shlex_quote # undocumented before 2.7 + IS_OSX = platform.system() == "Darwin" THIS_DIR = os.path.dirname(os.path.realpath(__file__)) SH_LOGGER_NAME = __name__ @@ -692,7 +697,7 @@ def __init__(self, cmd, call_args, stdin, stdout, stderr): # arguments are the encoding we pass into _encoding, which falls back to # the system's encoding enc = call_args["encoding"] - self.ran = " ".join([arg.decode(enc, "ignore") for arg in cmd]) + self.ran = " ".join([shlex_quote(arg.decode(enc, "ignore")) for arg in cmd]) self.call_args = call_args self.cmd = cmd From 151ce82d49d5d44d2d5ad28b54d67e8889c7a3b1 Mon Sep 17 00:00:00 2001 From: Tim Hatch Date: Wed, 3 Jul 2019 15:07:43 -0700 Subject: [PATCH 11/91] Use raw string to avoid invalid escape deprecation warning. See https://docs.python.org/3/whatsnew/3.6.html#deprecated-python-behavior --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..df246e7b 100644 --- a/sh.py +++ b/sh.py @@ -421,7 +421,7 @@ class CommandNotFound(AttributeError): pass -rc_exc_regex = re.compile("(ErrorReturnCode|SignalException)_((\d+)|SIG[a-zA-Z]+)") +rc_exc_regex = re.compile(r"(ErrorReturnCode|SignalException)_((\d+)|SIG[a-zA-Z]+)") rc_exc_cache = {} SIGNAL_MAPPING = {} From 33a6362ec442080f020328a08484cb38f0289557 Mon Sep 17 00:00:00 2001 From: mohameda Date: Wed, 2 Oct 2019 09:25:35 +0200 Subject: [PATCH 12/91] prefix True valued arguments with the right prefix --- sh.py | 2 +- test.py | 48 +++++++++++++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/sh.py b/sh.py index 5b250e12..7ddcb6b6 100644 --- a/sh.py +++ b/sh.py @@ -1521,7 +1521,7 @@ def aggregate_keywords(keywords, sep, prefix, raw=False): k = k.replace("_", "-") if v is True: - processed.append(encode("--" + k)) + processed.append(encode(prefix + k)) elif v is False: pass elif sep is None or sep == " ": diff --git a/test.py b/test.py index 68ef40c5..128cd48c 100644 --- a/test.py +++ b/test.py @@ -295,7 +295,7 @@ def test_exit_code(self): def test_patched_glob(self): from glob import glob - + py = create_tmp_test(""" import sys print(sys.argv[1:]) @@ -515,17 +515,23 @@ def test_environment(self): "VERSIONER_PYTHON_VERSION", ] + linux_cruft = [ + "LC_CTYPE", + ] + + crufts_env_vars = osx_cruft + linux_cruft + # first we test that the environment exists in our child process as # we've set it py = create_tmp_test(""" import os -osx_cruft = %s -for key in osx_cruft: +cruft = %s +for key in cruft: try: del os.environ[key] except: pass print(os.environ["HERP"] + " " + str(len(os.environ))) -""" % osx_cruft) +""" % crufts_env_vars) out = python(py.name, _env=env).strip() self.assertEqual(out, "DERP 1") @@ -533,12 +539,12 @@ def test_environment(self): import os, sys sys.path.insert(0, os.getcwd()) import sh -osx_cruft = %s -for key in osx_cruft: +cruft = %s +for key in cruft: try: del os.environ[key] except: pass print(sh.HERP + " " + str(len(os.environ))) -""" % osx_cruft) +""" % crufts_env_vars) out = python(py.name, _env=env, _cwd=THIS_DIR).strip() self.assertEqual(out, "DERP 1") @@ -595,7 +601,7 @@ def test_piped_exception1(self): exit(2) """) - py2 = create_tmp_test("") + py2 = create_tmp_test("") def fn(): list(python(python(py.name, _piped=True), "-u", py2.name, _iter=True)) @@ -612,7 +618,7 @@ def test_piped_exception2(self): exit(2) """) - py2 = create_tmp_test("") + py2 = create_tmp_test("") def fn(): python(python(py.name, _piped=True), "-u", py2.name) @@ -827,11 +833,19 @@ def test_custom_long_prefix(self): _long_prefix="-custom-").strip() self.assertEqual(out, "-custom-long-option=underscore") + out = python(py.name, {"long-option": True}, + _long_prefix="-custom-").strip() + self.assertEqual(out, "-custom-long-option") + # test baking too out = python.bake(py.name, {"long-option": "underscore"}, _long_prefix="-baked-")().strip() self.assertEqual(out, "-baked-long-option=underscore") + out = python.bake(py.name, {"long-option": True}, + _long_prefix="-baked-")().strip() + self.assertEqual(out, "-baked-long-option") + def test_command_wrapper(self): from sh import Command, which @@ -1321,7 +1335,7 @@ def test_stdout_callback_terminate(self): import os import time -for i in range(5): +for i in range(5): print(i) time.sleep(.5) """) @@ -1357,7 +1371,7 @@ def test_stdout_callback_kill(self): import os import time -for i in range(5): +for i in range(5): print(i) time.sleep(.5) """) @@ -1396,7 +1410,7 @@ def test_general_signal(self): def sig_handler(sig, frame): print(10) exit(0) - + signal.signal(signal.SIGINT, sig_handler) for i in range(5): @@ -1426,7 +1440,7 @@ def test_iter_generator(self): import os import time -for i in range(42): +for i in range(42): print(i) sys.stdout.flush() """) @@ -1488,7 +1502,7 @@ def test_for_generator_to_err(self): import sys import os -for i in range(42): +for i in range(42): sys.stderr.write(str(i)+"\\n") """) @@ -1597,7 +1611,7 @@ def test_generator_and_callback(self): import os for i in range(42): - sys.stderr.write(str(i * 2)+"\\n") + sys.stderr.write(str(i * 2)+"\\n") print(i) """) @@ -2162,7 +2176,7 @@ def test_non_existant_cwd(self): from sh import ls # sanity check - non_exist_dir = join(tempdir, "aowjgoahewro") + non_exist_dir = join(tempdir, "aowjgoahewro") self.assertFalse(exists(non_exist_dir)) self.assertRaises(OSError, ls, _cwd=non_exist_dir) @@ -2667,7 +2681,7 @@ def test_no_fd_leak(self): import sh import os from itertools import product - + # options whose combinations can possibly cause fd leaks kwargs = { "_tty_out": (True, False), From b3828bcc391b90ed273457651054b070a2013960 Mon Sep 17 00:00:00 2001 From: mohameda Date: Wed, 2 Oct 2019 09:44:25 +0200 Subject: [PATCH 13/91] added .coverage file to ingored files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 78ec7b12..8dc93bc8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/ *.py[co] +.coverage From 9f12911bc853e45cad45f72ff5673e0b517accbb Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Mon, 25 Nov 2019 17:19:29 +0000 Subject: [PATCH 14/91] Display exception information on 255 error Fix issue where the origina exception information is lost when sh is unable to write it the designated file descriptor. This can happen with at least two identified reasons: - passed cwd does not exist - the output pipe is closed or invalid In both cases the library failed without giving any meaningful message. New behavior is to attempt to write exception information to sys.stderr, which at least in my cases worked fine. --- .gitignore | 1 + sh.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 78ec7b12..b543085b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/ *.py[co] +.tox diff --git a/sh.py b/sh.py index 5b250e12..c2c86510 100644 --- a/sh.py +++ b/sh.py @@ -1968,12 +1968,17 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # if your parent process experiences an exit code 255, it is most # likely that an exception occurred between the fork of the child # and the exec. this should be reported. - except: + except Exception as e: # some helpful debugging try: - tb = traceback.format_exc().encode("utf8", "ignore") + tb = traceback.format_exc() + tb = tb.encode("utf8", "ignore") os.write(exc_pipe_write, tb) - + except: + # dump to stderr if we cannot save it to exc_pipe_write + if not exc_pipe_write or not hasattr(exc_pipe_write, 'write'): + sys.stderr.write( + "\nFATAL: %s\n%s" % (e, str(tb))) finally: os._exit(255) From 29b2cf4da4b10dd0d6f4fbc98169efd001fe02d3 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Mon, 25 Nov 2019 17:39:13 +0000 Subject: [PATCH 15/91] Updated supported versions of Python - dropped obsolete versions like py26, py33, py34 - added newer releases py37, py38 - fixed broken test on py37+ --- .gitignore | 1 + .travis.yml | 9 ++++----- setup.py | 7 ++----- test.py | 1 + tox.ini | 12 ++++++++++++ 5 files changed, 20 insertions(+), 10 deletions(-) create mode 100644 tox.ini diff --git a/.gitignore b/.gitignore index 78ec7b12..b543085b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/ *.py[co] +.tox diff --git a/.travis.yml b/.travis.yml index 881c82e1..1cedd435 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,14 +4,13 @@ os: language: python python: - - 2.6 - 2.7 - - pypy - - 3.3 - - 3.4 - 3.5 - 3.6 - - pypy-5.3.1 + - 3.7 + - 3.8 + - pypy + - pypy3.5 before_script: - pip install -r requirements-dev.txt diff --git a/setup.py b/setup.py index 7c3d1c1b..75d887f4 100644 --- a/setup.py +++ b/setup.py @@ -39,15 +39,12 @@ def read(*parts): "License :: OSI Approved :: MIT License", "Programming Language :: Python", "Programming Language :: Python :: 2", - "Programming Language :: Python :: 2.6", "Programming Language :: Python :: 2.7", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.1", - "Programming Language :: Python :: 3.2", - "Programming Language :: Python :: 3.3", - "Programming Language :: Python :: 3.4", "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", + "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", "Programming Language :: Python :: Implementation :: CPython", "Programming Language :: Python :: Implementation :: PyPy", "Topic :: Software Development :: Build Tools", diff --git a/test.py b/test.py index 68ef40c5..f9640a0d 100644 --- a/test.py +++ b/test.py @@ -511,6 +511,7 @@ def test_environment(self): osx_cruft = [ "__CF_USER_TEXT_ENCODING", "__PYVENV_LAUNCHER__", + "LC_CTYPE", "VERSIONER_PYTHON_PREFER_32_BIT", "VERSIONER_PYTHON_VERSION", ] diff --git a/tox.ini b/tox.ini new file mode 100644 index 00000000..05f95706 --- /dev/null +++ b/tox.ini @@ -0,0 +1,12 @@ +[tox] +envlist = py{27,35,36,37,38},docs + +[testenv] +deps = -r requirements-dev.txt +commands = + python sh.py travis + +[testenv:docs] +basepython = python3 +commands = + python setup.py check --restructuredtext --metadata --strict From b616a19514a3cd49e68cf7d9e363d75235f73483 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 13:35:22 -0800 Subject: [PATCH 16/91] changelog update --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ff3904b..c4619a96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## X.XX.XX - XX/XX/XX +* minor Travis CI fixes [#492](https://github.com/amoffat/sh/pull/492) + ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) * allow passing raw integer file descriptors for `_out` and `_err` handlers From a42d466539f893358f4f52723a5ba06fc22aec76 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 13:38:55 -0800 Subject: [PATCH 17/91] Jinja upgrade to 2.10.3 to fix security issues https://github.com/advisories/GHSA-462w-v97r-4m45 https://github.com/advisories/GHSA-hj2j-77xm-mc5v --- requirements-docs.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-docs.txt b/requirements-docs.txt index dbaddca3..da363530 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -2,7 +2,7 @@ alabaster==0.7.9 Babel==2.3.4 docutils==0.12 imagesize==0.7.1 -Jinja2==2.8 +Jinja2==2.10.3 MarkupSafe==0.23 Pygments==2.1.3 pytz==2016.7 From 8607fc72a9fe9131fd5fed60cb7e629a11235f0e Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 19:25:26 -0800 Subject: [PATCH 18/91] docker test suite --- docker_test_suite/Dockerfile | 46 ++++++++++++++++++++++++++ docker_test_suite/build.sh | 4 +++ docker_test_suite/requirements-dev.txt | 6 ++++ docker_test_suite/run.sh | 3 ++ sh.py | 11 +++--- tox.ini | 5 +-- 6 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 docker_test_suite/Dockerfile create mode 100755 docker_test_suite/build.sh create mode 100644 docker_test_suite/requirements-dev.txt create mode 100755 docker_test_suite/run.sh diff --git a/docker_test_suite/Dockerfile b/docker_test_suite/Dockerfile new file mode 100644 index 00000000..e6e7c6b0 --- /dev/null +++ b/docker_test_suite/Dockerfile @@ -0,0 +1,46 @@ +FROM ubuntu:bionic + +ARG cache_bust +RUN apt-get update +RUN apt-get -y install locales + +RUN locale-gen en_US.UTF-8 +ENV LANG en_US.UTF-8 +ENV LANGUAGE en_US:en +ENV LC_ALL en_US.UTF-8 + +RUN apt-get -y install\ + software-properties-common\ + curl\ + sudo\ + python + +RUN add-apt-repository ppa:deadsnakes/ppa +RUN apt-get update +RUN apt-get -y install\ + python2.6\ + python2.7\ + python3.1\ + python3.2\ + python3.3\ + python3.4\ + python3.5\ + python3.6\ + python3.7\ + python3.8 + +RUN apt-get -y install python3-distutils\ + && curl https://bootstrap.pypa.io/get-pip.py | python - + +ARG uid=1000 +RUN groupadd -g $uid shtest\ + && useradd -m -u $uid -g $uid shtest\ + && gpasswd -a shtest sudo\ + && echo "shtest:shtest" | chpasswd + +COPY requirements-dev.txt /tmp/ +RUN pip install -r /tmp/requirements-dev.txt + +USER shtest +WORKDIR /home/shtest/sh +ENTRYPOINT python sh.py test diff --git a/docker_test_suite/build.sh b/docker_test_suite/build.sh new file mode 100755 index 00000000..07a2c5ad --- /dev/null +++ b/docker_test_suite/build.sh @@ -0,0 +1,4 @@ +#!/bin/bash +set -ex +cp ../requirements-dev.txt . +docker build -t amoffat/shtest . diff --git a/docker_test_suite/requirements-dev.txt b/docker_test_suite/requirements-dev.txt new file mode 100644 index 00000000..5277a92c --- /dev/null +++ b/docker_test_suite/requirements-dev.txt @@ -0,0 +1,6 @@ +Pygments==2.1.3 +coverage==4.2 +coveralls==1.1 +docopt==0.6.2 +docutils==0.12 +requests==2.12.1 diff --git a/docker_test_suite/run.sh b/docker_test_suite/run.sh new file mode 100755 index 00000000..7cf9c98b --- /dev/null +++ b/docker_test_suite/run.sh @@ -0,0 +1,3 @@ +#!/bin/bash +set -ex +docker run -it --rm -v $(pwd)/../:/home/shtest/sh amoffat/shtest diff --git a/sh.py b/sh.py index 33193a01..ea1aed94 100644 --- a/sh.py +++ b/sh.py @@ -3517,7 +3517,7 @@ def parse_args(): if args: action = args[0] - if action in ("test", "travis"): + if action in ("test", "travis", "tox"): import test coverage = None if test.HAS_UNICODE_LITERAL: @@ -3530,12 +3530,11 @@ def parse_args(): # if we're testing locally, run all versions of python on the system if action == "test": - all_versions = ("2.6", "2.7", "3.1", "3.2", "3.3", "3.4", "3.5", "3.6") + all_versions = ("2.6", "2.7", "3.1", "3.2", "3.3", "3.4", "3.5", "3.6", "3.7", "3.8") - # if we're testing on travis, just use the system's default python, - # since travis will spawn a vm per python version in our .travis.yml - # file - elif action == "travis": + # if we're testing on travis or tox, just use the system's default python, since travis will spawn a vm per + # python version in our .travis.yml file, and tox will run its matrix via tox.ini + elif action in ("travis", "tox"): v = sys.version_info sys_ver = "%d.%d" % (v[0], v[1]) all_versions = (sys_ver,) diff --git a/tox.ini b/tox.ini index 05f95706..1f986c70 100644 --- a/tox.ini +++ b/tox.ini @@ -1,10 +1,11 @@ [tox] -envlist = py{27,35,36,37,38},docs +# virtualenv for py26 is broken, so don't put it here +envlist = py{27,31,32,33,34,35,36,37,38},docs [testenv] deps = -r requirements-dev.txt commands = - python sh.py travis + python sh.py tox [testenv:docs] basepython = python3 From c060f761d229d40692a9aacfbb26faf374eab977 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 19:29:20 -0800 Subject: [PATCH 19/91] allow passing a specific test to the docker container --- docker_test_suite/Dockerfile | 2 +- docker_test_suite/run.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docker_test_suite/Dockerfile b/docker_test_suite/Dockerfile index e6e7c6b0..0bb762eb 100644 --- a/docker_test_suite/Dockerfile +++ b/docker_test_suite/Dockerfile @@ -43,4 +43,4 @@ RUN pip install -r /tmp/requirements-dev.txt USER shtest WORKDIR /home/shtest/sh -ENTRYPOINT python sh.py test +ENTRYPOINT ["python", "sh.py", "test"] diff --git a/docker_test_suite/run.sh b/docker_test_suite/run.sh index 7cf9c98b..bd92b851 100755 --- a/docker_test_suite/run.sh +++ b/docker_test_suite/run.sh @@ -1,3 +1,3 @@ #!/bin/bash set -ex -docker run -it --rm -v $(pwd)/../:/home/shtest/sh amoffat/shtest +docker run -it --rm -v $(pwd)/../:/home/shtest/sh amoffat/shtest $@ From a41e39bcee9423a21e6fd8fbe77f194a735e558e Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 19:33:23 -0800 Subject: [PATCH 20/91] update readme for docker test suite --- README.rst | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/README.rst b/README.rst index 73b68c44..b4ad1740 100644 --- a/README.rst +++ b/README.rst @@ -49,21 +49,24 @@ Developers Testing ------- -First install the development requirements:: +I've included a Docker test suite in the `docker_test_suit/` folder. To build the image, `cd` into that directory and +run:: - $> pip install -r requirements-dev.txt + $> ./build.sh -The run the tests for all Python versions on your system:: +This will install ubuntu 18.04 LTS and all python versions from 2.6-3.8. Once it's done, stay in that directory and +run:: - $> python sh.py test + $> ./run.sh -To run a single test for all environments:: +This will mount your local code directory into the container and start the test suite, which will take a long time to +run. If you wish to run a single test, you may pass that test to `./run.sh`:: - $> python sh.py test FunctionalTests.test_unicode_arg + $> ./run.sh FunctionalTests.test_unicode_arg To run a single test for a single environment:: - $> python sh.py test -e 3.4 FunctionalTests.test_unicode_arg + $> ./run.sh -e 3.4 FunctionalTests.test_unicode_arg Coverage -------- From 992d936af1e5655cd1d62bc3b70cd6fe946b46aa Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 19:37:21 -0800 Subject: [PATCH 21/91] actually print out the python versions we tested --- sh.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sh.py b/sh.py index ea1aed94..f3b59a54 100644 --- a/sh.py +++ b/sh.py @@ -3545,17 +3545,21 @@ def parse_args(): all_locales = ("en_US.UTF-8", "C") i = 0 + ran_versions = set() for locale in all_locales: + # make sure this locale is allowed if constrain_locales and locale not in constrain_locales: continue for version in all_versions: + # make sure this version is allowed if constrain_versions and version not in constrain_versions: continue for force_select in all_force_select: env_copy = env.copy() + ran_versions.add(version) exit_code = run_tests(env_copy, locale, args, version, force_select, SH_TEST_RUN_IDX=i) @@ -3568,8 +3572,7 @@ def parse_args(): i += 1 - ran_versions = ",".join(all_versions) - print("Tested Python versions: %s" % ran_versions) + print("Tested Python versions: %s" % ",".join(sorted(list(ran_versions)))) else: env = Environment(globals()) From c114d83171f60ab13937fd46ad3973e95258d855 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Mon, 25 Nov 2019 21:04:54 -0800 Subject: [PATCH 22/91] don't commit this --- docker_test_suite/requirements-dev.txt | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 docker_test_suite/requirements-dev.txt diff --git a/docker_test_suite/requirements-dev.txt b/docker_test_suite/requirements-dev.txt deleted file mode 100644 index 5277a92c..00000000 --- a/docker_test_suite/requirements-dev.txt +++ /dev/null @@ -1,6 +0,0 @@ -Pygments==2.1.3 -coverage==4.2 -coveralls==1.1 -docopt==0.6.2 -docutils==0.12 -requests==2.12.1 From cf4a0a9ab8fc61583e62a7e040adead49be34df0 Mon Sep 17 00:00:00 2001 From: Cheaterman Date: Fri, 29 Nov 2019 16:04:59 +0100 Subject: [PATCH 23/91] Fix for the fix for issue amoffat#398. @davidm-mm proposed a fix for this issue in https://github.com/amoffat/sh/issues/398. However it didn't work for me when using gevent inside docker (I suspect gevent is the issue, but have no evidence for/against it). This seems to work for me. --- sh.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sh.py b/sh.py index 2c2281e5..8e349e28 100644 --- a/sh.py +++ b/sh.py @@ -3431,15 +3431,16 @@ def find_module(self, mod_fullname, path=None): parent_frame = inspect.currentframe().f_back + while parent_frame and in_importlib(parent_frame): + parent_frame = parent_frame.f_back + # Calling PyImport_ImportModule("some_module"); via the C API may not # have a parent frame. Early-out to avoid in_importlib() trying to - # get f_code from None when looking for 'some_module' + # get f_code from None when looking for 'some_module'. + # This also happens when using gevent apparently. if not parent_frame: return None - while in_importlib(parent_frame): - parent_frame = parent_frame.f_back - # this line is saying "hey, does mod_fullname exist as a name we've # defind previously?" the purpose of this is to ensure that # mod_fullname is really a thing we've defined. if we haven't defined From 5b274778fb4be647bbb95f9bec14153f186bcc1f Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 13:08:36 +0100 Subject: [PATCH 24/91] Add a validator for env. Fixes #460 --- sh.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/sh.py b/sh.py index 5b250e12..67937b30 100644 --- a/sh.py +++ b/sh.py @@ -1051,6 +1051,28 @@ def bufsize_validator(kwargs): return invalid +def env_validator(kwargs): + """ a validator to check that env is a dictionary and that all environment variable + keys and values are strings. Otherwise, we would exit with a confusing exit code 255. """ + invalid = [] + + env = kwargs.get("env", None) + if env is None: + return invalid + + if not isinstance(env, dict): + invalid.append((("env"), "env must be a dict. Got {!r}".format(env))) + return invalid + + for k, v in kwargs["env"].items(): + if not isinstance(k, str): + invalid.append((("env"), "env key {!r} must be a str".format(k))) + if not isinstance(v, str): + invalid.append((("env"), "value {!r} of env key {!r} must be a str".format(v, k))) + + return invalid + + class Command(object): """ represents an un-run system program, like "ls" or "cd". because it represents the program itself (and not a running instance of it), it should @@ -1182,6 +1204,7 @@ class Command(object): output"), tty_in_validator, bufsize_validator, + env_validator, ) From 668b043ceefa57685129b03a7fb1bb0eb62d19ae Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 13:18:25 +0100 Subject: [PATCH 25/91] Add test --- test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test.py b/test.py index 68ef40c5..755bc2b7 100644 --- a/test.py +++ b/test.py @@ -576,6 +576,16 @@ def test_incompatible_special_args(self): self.assertRaises(TypeError, ls, _iter=True, _piped=True) + def test_invalid_env(self): + from sh import ls + + with self.assertRaises(TypeError): + ls(_env="XXX") + with self.assertRaises(TypeError): + ls(_env={"foo": 123}) + with self.assertRaises(TypeError): + ls(_env={123: "bar"}) + def test_exception(self): from sh import ErrorReturnCode_2 From db0f0f245aa3a2737169799d4e7c3da91d760da7 Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 13:21:30 +0100 Subject: [PATCH 26/91] Remove deprecated and add new Python versions --- .travis.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 881c82e1..55c0da5b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,14 +4,11 @@ os: language: python python: - - 2.6 - - 2.7 - pypy - - 3.3 - - 3.4 - 3.5 - 3.6 - - pypy-5.3.1 + - 3.7 + - 3.8 before_script: - pip install -r requirements-dev.txt From 5ecd95ab45360357dd8c320e01f1c28631b52e2a Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 13:51:21 +0100 Subject: [PATCH 27/91] Test failures on 3.7 and 3.8. Remove for now. --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 55c0da5b..1fe23837 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,8 +7,6 @@ python: - pypy - 3.5 - 3.6 - - 3.7 - - 3.8 before_script: - pip install -r requirements-dev.txt From 202c28fb6b0c1c956ee200fd0b8b88f1dbbce6a7 Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 14:24:35 +0100 Subject: [PATCH 28/91] Mor robust cleaning of environment This fixes Travis tests for Python 3.7 and 3.8 --- test.py | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/test.py b/test.py index 68ef40c5..5c7882e5 100644 --- a/test.py +++ b/test.py @@ -506,41 +506,30 @@ def test_environment(self): # this is the environment we'll pass into our commands env = {"HERP": "DERP"} - # python on osx will bizarrely add some extra environment variables that - # i didn't ask for. for this test, we prune those out if they exist - osx_cruft = [ - "__CF_USER_TEXT_ENCODING", - "__PYVENV_LAUNCHER__", - "VERSIONER_PYTHON_PREFER_32_BIT", - "VERSIONER_PYTHON_VERSION", - ] - # first we test that the environment exists in our child process as # we've set it py = create_tmp_test(""" import os -osx_cruft = %s -for key in osx_cruft: - try: del os.environ[key] - except: pass -print(os.environ["HERP"] + " " + str(len(os.environ))) -""" % osx_cruft) +for key in list(os.environ.keys()): + if key != "HERP": + del os.environ[key] +print(dict(os.environ)) +""") out = python(py.name, _env=env).strip() - self.assertEqual(out, "DERP 1") + self.assertEqual(out, "{'HERP': 'DERP'}") py = create_tmp_test(""" import os, sys sys.path.insert(0, os.getcwd()) import sh -osx_cruft = %s -for key in osx_cruft: - try: del os.environ[key] - except: pass -print(sh.HERP + " " + str(len(os.environ))) -""" % osx_cruft) +for key in list(os.environ.keys()): + if key != "HERP": + del os.environ[key] +print(dict(HERP=sh.HERP)) +""") out = python(py.name, _env=env, _cwd=THIS_DIR).strip() - self.assertEqual(out, "DERP 1") + self.assertEqual(out, "{'HERP': 'DERP'}") def test_which(self): From d6874e9f3f1a963e3b11dc7522f8ac121882859c Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 14:26:33 +0100 Subject: [PATCH 29/91] Let Travis test only supported Python versions --- .travis.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 881c82e1..55c0da5b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,14 +4,11 @@ os: language: python python: - - 2.6 - - 2.7 - pypy - - 3.3 - - 3.4 - 3.5 - 3.6 - - pypy-5.3.1 + - 3.7 + - 3.8 before_script: - pip install -r requirements-dev.txt From 0f2f4f42a8348c0bda74670e468251b88696d8ea Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Fri, 10 Jan 2020 14:30:02 +0100 Subject: [PATCH 30/91] Announce support for Python 3.7 and 3.8 --- setup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/setup.py b/setup.py index 7c3d1c1b..3d1e256e 100644 --- a/setup.py +++ b/setup.py @@ -48,6 +48,8 @@ def read(*parts): "Programming Language :: Python :: 3.4", "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", + "Programming Language :: Python :: 3.7", + "Programming Language :: Python :: 3.8", "Programming Language :: Python :: Implementation :: CPython", "Programming Language :: Python :: Implementation :: PyPy", "Topic :: Software Development :: Build Tools", From b66a624baadf872693c3995fcbf27fcb95c5d416 Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 5 Feb 2020 05:44:49 +1100 Subject: [PATCH 31/91] Fix simple typo: refects -> reflects Closes #502 --- sh.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sh.py b/sh.py index 5b250e12..50bb5584 100644 --- a/sh.py +++ b/sh.py @@ -2250,13 +2250,13 @@ def stderr(self): def get_pgid(self): """ return the CURRENT group id of the process. this differs from - self.pgid in that this refects the current state of the process, where + self.pgid in that this reflects the current state of the process, where self.pgid is the group id at launch """ return os.getpgid(self.pid) def get_sid(self): """ return the CURRENT session id of the process. this differs from - self.sid in that this refects the current state of the process, where + self.sid in that this reflects the current state of the process, where self.sid is the session id at launch """ return os.getsid(self.pid) From b781bd048f7caa9398dbacfb816e4c0a8cf26dde Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 5 Feb 2020 05:46:56 +1100 Subject: [PATCH 32/91] Fix simple typo: synchronzing -> synchronizing Closes #504 --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..7f3a6aed 100644 --- a/sh.py +++ b/sh.py @@ -1847,7 +1847,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, session_pipe_read, session_pipe_write = os.pipe() exc_pipe_read, exc_pipe_write = os.pipe() - # this pipe is for synchronzing with the child that the parent has + # this pipe is for synchronizing with the child that the parent has # closed its in/out/err fds. this is a bug on OSX (but not linux), # where we can lose output sometimes, due to a race, if we do # os.close(self._stdout_write_fd) in the parent after the child starts From 9662f156ac51862d8b41c110d6cba664e1252629 Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 5 Feb 2020 05:48:28 +1100 Subject: [PATCH 33/91] Fix simple typo: throug -> through Closes #506 --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..900e6618 100644 --- a/sh.py +++ b/sh.py @@ -3426,7 +3426,7 @@ def find_module(self, mod_fullname, path=None): derp = sh() from derp import ls - here, mod_fullname will be "derp". keep that in mind as we go throug + here, mod_fullname will be "derp". keep that in mind as we go through the rest of this function """ parent_frame = inspect.currentframe().f_back From 4703211a6f56c51c404079f7ad08a585403eef39 Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 5 Feb 2020 05:49:40 +1100 Subject: [PATCH 34/91] Fix simple typo: verions -> versions Closes #508 --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..932988bf 100644 --- a/sh.py +++ b/sh.py @@ -3457,7 +3457,7 @@ def load_module(self, mod_fullname): module = fetch_module_from_frame(mod_fullname, parent_frame) # we HAVE to include the module in sys.modules, per the import PEP. - # older verions of python were more lenient about this being set, but + # older versions of python were more lenient about this being set, but # not in >= python3.3, unfortunately. this requirement necessitates the # ugly code in SelfWrapper.__call__ sys.modules[mod_fullname] = module From e11de069a0c0e53b089e43cbd6a32af30a03dfee Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Thu, 23 Apr 2020 14:48:04 +0200 Subject: [PATCH 35/91] Add new versions for local testing --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 5b250e12..f2182986 100644 --- a/sh.py +++ b/sh.py @@ -3530,7 +3530,7 @@ def parse_args(): # if we're testing locally, run all versions of python on the system if action == "test": - all_versions = ("2.6", "2.7", "3.1", "3.2", "3.3", "3.4", "3.5", "3.6") + all_versions = ("2.6", "2.7", "3.1", "3.2", "3.3", "3.4", "3.5", "3.6", "3.7", "3.8") # if we're testing on travis, just use the system's default python, # since travis will spawn a vm per python version in our .travis.yml From 1bc8128956fb01a0b9df8f2c2bf92f5f941b0ae6 Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Thu, 23 Apr 2020 14:51:10 +0200 Subject: [PATCH 36/91] Re-add older versions We want to keep testing on old versions until we intentionally break support for these versions. --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 55c0da5b..f738b440 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,11 @@ os: language: python python: + - 2.6 + - 2.7 - pypy + - 3.3 + - 3.4 - 3.5 - 3.6 - 3.7 From 2b154b8f30249f63cdf72222d03ce63e79ec2742 Mon Sep 17 00:00:00 2001 From: Erik Cederstrand Date: Thu, 23 Apr 2020 15:08:35 +0200 Subject: [PATCH 37/91] Add note on removal of 2.6 and 3.3 --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f738b440..e165ab11 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,10 +4,10 @@ os: language: python python: - - 2.6 + # - 2.6 No longer supported on Travis - 2.7 - pypy - - 3.3 + # - 3.3 No longer supported on Travis - 3.4 - 3.5 - 3.6 From 456c210f9752a5104fd235ad9f6457e71c40ebb2 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 19:23:20 -0700 Subject: [PATCH 38/91] tests were missing lsof command --- docker_test_suite/Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker_test_suite/Dockerfile b/docker_test_suite/Dockerfile index 0bb762eb..d169ecf1 100644 --- a/docker_test_suite/Dockerfile +++ b/docker_test_suite/Dockerfile @@ -13,7 +13,8 @@ RUN apt-get -y install\ software-properties-common\ curl\ sudo\ - python + python\ + lsof RUN add-apt-repository ppa:deadsnakes/ppa RUN apt-get update From 7cb636b85ae9c1b494a7eff6ac5c2fb770060ac6 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 19:23:34 -0700 Subject: [PATCH 39/91] allow cache busting through --build-arg --- docker_test_suite/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker_test_suite/build.sh b/docker_test_suite/build.sh index 07a2c5ad..6ed7ec41 100755 --- a/docker_test_suite/build.sh +++ b/docker_test_suite/build.sh @@ -1,4 +1,4 @@ #!/bin/bash set -ex cp ../requirements-dev.txt . -docker build -t amoffat/shtest . +docker build -t amoffat/shtest $@ . From a9a6b9ebb31ab39661e3da763f239c66a14065b8 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 19:38:28 -0700 Subject: [PATCH 40/91] a little more clear about what's running --- sh.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sh.py b/sh.py index f3b59a54..684dec68 100644 --- a/sh.py +++ b/sh.py @@ -3488,6 +3488,7 @@ def run_tests(env, locale, args, version, force_select, **extra_env): # pragma: env[k] = str(v) cmd = [py_bin, "-W", "ignore", os.path.join(THIS_DIR, "test.py")] + args[1:] + print("Running %r" % cmd) launch = lambda: os.spawnve(os.P_WAIT, cmd[0], cmd, env) return_code = launch() From 180f97b20ade6cd8e28a033ea716901e064d70ce Mon Sep 17 00:00:00 2001 From: Viacheslav Biriukov Date: Wed, 4 Mar 2020 15:12:09 +0000 Subject: [PATCH 41/91] Fixing os.closerange bug: 406 --- sh.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sh.py b/sh.py index 684dec68..a90346ab 100644 --- a/sh.py +++ b/sh.py @@ -1952,8 +1952,11 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # don't inherit file descriptors - max_fd = resource.getrlimit(resource.RLIMIT_NOFILE)[0] - os.closerange(3, max_fd) + for fd in filter(lambda x: x not in (0, 1, 2), (int(fd) for fd in os.listdir("/dev/fd"))): + try: + os.close(fd) + except OSError: + pass # actually execute the process if ca["env"] is None: From 58308b46fb675e33bcae3dd39b0b1fb1cb8a599e Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 22:15:47 -0700 Subject: [PATCH 42/91] pass_fds, close_fds, and efficient closing of inherited fds --- CHANGELOG.md | 2 ++ sh.py | 37 ++++++++++++++++-------- test.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 703de9de..a7130fc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ * minor Travis CI fixes [#492](https://github.com/amoffat/sh/pull/492) * bugfix for boolean long options not respecting `_long_prefix` [#488](https://github.com/amoffat/sh/pull/488) * fix deprecation warning on Python 3.6 regexes [#482](https://github.com/amoffat/sh/pull/482) +* `_pass_fds` and `close_fds` special kwargs for controlling file descriptor inheritance in child. +* more efficiently closing inherited fds [#406](https://github.com/amoffat/sh/issues/406) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index a90346ab..b9a639e7 100644 --- a/sh.py +++ b/sh.py @@ -1167,6 +1167,13 @@ class Command(object): # a callable that produces a log message from an argument tuple of the # command and the args "log_msg": None, + + # whether or not to close all inherited fds. typically, this should be True, as inheriting fds can be a security + # vulnerability + "close_fds": True, + + # a whitelist of the integer fds to pass through to the child process. setting this forces close_fds to be True + "pass_fds": set(), } # this is a collection of validators to make sure the special kwargs make @@ -1176,10 +1183,9 @@ class Command(object): (("fg", "err_to_out"), "Can't redirect STDERR in foreground mode"), (("err", "err_to_out"), "Stderr is already being redirected"), (("piped", "iter"), "You cannot iterate when this command is being piped"), - (("piped", "no_pipe"), "Using a pipe doesn't make sense if you've \ -disabled the pipe"), - (("no_out", "iter"), "You cannot iterate over output if there is no \ -output"), + (("piped", "no_pipe"), "Using a pipe doesn't make sense if you've disabled the pipe"), + (("no_out", "iter"), "You cannot iterate over output if there is no output"), + (("close_fds", "pass_fds"), "Passing `pass_fds` forces `close_fds` to be True"), tty_in_validator, bufsize_validator, ) @@ -1950,13 +1956,22 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, if callable(preexec_fn): preexec_fn() - - # don't inherit file descriptors - for fd in filter(lambda x: x not in (0, 1, 2), (int(fd) for fd in os.listdir("/dev/fd"))): - try: - os.close(fd) - except OSError: - pass + close_fds = ca["close_fds"] + if ca["pass_fds"]: + close_fds = True + + if close_fds: + pass_fds = set((0, 1, 2)) + pass_fds.update(ca["pass_fds"]) + + # don't inherit file descriptors + inherited_fds = os.listdir("/dev/fd") + inherited_fds = set(int(fd) for fd in inherited_fds) - pass_fds + for fd in inherited_fds: + try: + os.close(fd) + except OSError: + pass # actually execute the process if ca["env"] is None: diff --git a/test.py b/test.py index d54e60cf..c2a9072f 100644 --- a/test.py +++ b/test.py @@ -50,6 +50,7 @@ import signal import errno import stat +import fcntl import platform from functools import wraps import time @@ -552,6 +553,84 @@ def test_which_paths(self): found_path = which(test_name, [test_path]) self.assertEqual(found_path, py.name) + def test_no_close_fds(self): + import sh + + # guarantee some extra fds in our parent process that don't close on exec. we have to explicitly do this + # because at some point (I believe python 3.4), python started being more stringent with closing fds to prevent + # security vulnerabilities. python 2.7, for example, doesn't set CLOEXEC on tempfile.TemporaryFile()s + # + # https://www.python.org/dev/peps/pep-0446/ + tmp = [tempfile.TemporaryFile() for i in range(10)] + for t in tmp: + flags = fcntl.fcntl(t.fileno(), fcntl.F_GETFD) + flags &= ~fcntl.FD_CLOEXEC + fcntl.fcntl(t.fileno(), fcntl.F_SETFD, flags) + first_fd = tmp[0].fileno() + + py = create_tmp_test(""" +import os +print(len(os.listdir("/dev/fd"))) +""") + out = python(py.name, _close_fds=False).strip() + # pick some number greater than 4, since it's hard to know exactly how many fds will be open/inherted in the + # child + self.assertTrue(int(out) > 7) + + for t in tmp: + t.close() + + def test_close_fds(self): + import sh + + # guarantee some extra fds in our parent process that don't close on exec. we have to explicitly do this + # because at some point (I believe python 3.4), python started being more stringent with closing fds to prevent + # security vulnerabilities. python 2.7, for example, doesn't set CLOEXEC on tempfile.TemporaryFile()s + # + # https://www.python.org/dev/peps/pep-0446/ + tmp = [tempfile.TemporaryFile() for i in range(10)] + for t in tmp: + flags = fcntl.fcntl(t.fileno(), fcntl.F_GETFD) + flags &= ~fcntl.FD_CLOEXEC + fcntl.fcntl(t.fileno(), fcntl.F_SETFD, flags) + + py = create_tmp_test(""" +import os +print(os.listdir("/dev/fd")) +""") + out = python(py.name).strip() + self.assertEqual(out, "['0', '1', '2', '3']") + + for t in tmp: + t.close() + + + def test_pass_fds(self): + import sh + + # guarantee some extra fds in our parent process that don't close on exec. we have to explicitly do this + # because at some point (I believe python 3.4), python started being more stringent with closing fds to prevent + # security vulnerabilities. python 2.7, for example, doesn't set CLOEXEC on tempfile.TemporaryFile()s + # + # https://www.python.org/dev/peps/pep-0446/ + tmp = [tempfile.TemporaryFile() for i in range(10)] + for t in tmp: + flags = fcntl.fcntl(t.fileno(), fcntl.F_GETFD) + flags &= ~fcntl.FD_CLOEXEC + fcntl.fcntl(t.fileno(), fcntl.F_SETFD, flags) + last_fd = tmp[-1].fileno() + + py = create_tmp_test(""" +import os +print(os.listdir("/dev/fd")) +""") + out = python(py.name, _pass_fds=[last_fd]).strip() + inherited = [0, 1, 2, 3, last_fd] + inherited_str = [str(i) for i in inherited] + self.assertEqual(out, str(inherited_str)) + + for t in tmp: + t.close() def test_no_arg(self): import pwd @@ -2654,7 +2733,6 @@ def test_percent_doesnt_fail_logging(self): out = python(py.name, "%%") out = python(py.name, "%%%") - # TODO # for some reason, i can't get a good stable baseline measured in this test # on osx. so skip it for now if osx From 421b262a054ab4882189ece01043d4d6abfe39af Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 22:16:00 -0700 Subject: [PATCH 43/91] don't fail tests if can't import coverage --- test.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/test.py b/test.py index c2a9072f..9872eacc 100644 --- a/test.py +++ b/test.py @@ -15,20 +15,23 @@ run_idx = int(os.environ.pop("SH_TEST_RUN_IDX", "0")) first_run = run_idx == 0 - import coverage - - # for some reason, we can't run auto_data on the first run, or the coverage - # numbers get really screwed up - auto_data = True - if first_run: - auto_data = False - - cov = coverage.Coverage(auto_data=auto_data) - - if first_run: - cov.erase() - - cov.start() + try: + import coverage + except ImportError: + pass + else: + # for some reason, we can't run auto_data on the first run, or the coverage + # numbers get really screwed up + auto_data = True + if first_run: + auto_data = False + + cov = coverage.Coverage(auto_data=auto_data) + + if first_run: + cov.erase() + + cov.start() from os.path import exists, join, realpath, dirname, split From 342f246dec4f4341cc65513286d3ecdcec152e5b Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 23:36:16 -0700 Subject: [PATCH 44/91] don't close child exception pipe --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 3878f0e1..b14de5c3 100644 --- a/sh.py +++ b/sh.py @@ -1984,7 +1984,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, close_fds = True if close_fds: - pass_fds = set((0, 1, 2)) + pass_fds = set((0, 1, 2, exc_pipe_write)) pass_fds.update(ca["pass_fds"]) # don't inherit file descriptors From 5ef82c52ded37c8893fa855c21c686c500f5d252 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 23:43:02 -0700 Subject: [PATCH 45/91] changelog update --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7130fc2..f468c27f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ * minor Travis CI fixes [#492](https://github.com/amoffat/sh/pull/492) * bugfix for boolean long options not respecting `_long_prefix` [#488](https://github.com/amoffat/sh/pull/488) * fix deprecation warning on Python 3.6 regexes [#482](https://github.com/amoffat/sh/pull/482) -* `_pass_fds` and `close_fds` special kwargs for controlling file descriptor inheritance in child. +* `_pass_fds` and `_close_fds` special kwargs for controlling file descriptor inheritance in child. * more efficiently closing inherited fds [#406](https://github.com/amoffat/sh/issues/406) +* bugfix where passing invalid dictionary to `_env` will cause a mysterious child 255 exit code. [#497](https://github.com/amoffat/sh/pull/497) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) From b125452361a8857318465f3fe477feabe8ec67b9 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 23:52:24 -0700 Subject: [PATCH 46/91] cruft cleanup --- test.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test.py b/test.py index 95d4934c..23874b3c 100644 --- a/test.py +++ b/test.py @@ -557,8 +557,6 @@ def test_which_paths(self): self.assertEqual(found_path, py.name) def test_no_close_fds(self): - import sh - # guarantee some extra fds in our parent process that don't close on exec. we have to explicitly do this # because at some point (I believe python 3.4), python started being more stringent with closing fds to prevent # security vulnerabilities. python 2.7, for example, doesn't set CLOEXEC on tempfile.TemporaryFile()s @@ -584,8 +582,6 @@ def test_no_close_fds(self): t.close() def test_close_fds(self): - import sh - # guarantee some extra fds in our parent process that don't close on exec. we have to explicitly do this # because at some point (I believe python 3.4), python started being more stringent with closing fds to prevent # security vulnerabilities. python 2.7, for example, doesn't set CLOEXEC on tempfile.TemporaryFile()s @@ -609,8 +605,6 @@ def test_close_fds(self): def test_pass_fds(self): - import sh - # guarantee some extra fds in our parent process that don't close on exec. we have to explicitly do this # because at some point (I believe python 3.4), python started being more stringent with closing fds to prevent # security vulnerabilities. python 2.7, for example, doesn't set CLOEXEC on tempfile.TemporaryFile()s From 1e151901c5f7fcec63b9449f1f24456a943aac4e Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Thu, 23 Apr 2020 23:57:52 -0700 Subject: [PATCH 47/91] bugfix where accidentally using fd 0 (stdin) as falsy --- CHANGELOG.md | 1 + sh.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f468c27f..22eca813 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * `_pass_fds` and `_close_fds` special kwargs for controlling file descriptor inheritance in child. * more efficiently closing inherited fds [#406](https://github.com/amoffat/sh/issues/406) * bugfix where passing invalid dictionary to `_env` will cause a mysterious child 255 exit code. [#497](https://github.com/amoffat/sh/pull/497) +* bugfix where `_in` using 0 or `sys.stdin` wasn't behaving like a TTY, if it was in fact a TTY. [#514](https://github.com/amoffat/sh/issues/514) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index 795105cb..bebbca32 100644 --- a/sh.py +++ b/sh.py @@ -1003,7 +1003,7 @@ def ob_is_tty(ob): """ checks if an object (like a file-like object) is a tty. """ fileno = get_fileno(ob) is_tty = False - if fileno: + if fileno is not None: is_tty = os.isatty(fileno) return is_tty From 1a56211350ad309df9123dd53e40150d5a395888 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 00:22:47 -0700 Subject: [PATCH 48/91] allow help(sh) --- CHANGELOG.md | 1 + sh.py | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22eca813..cb8e9a6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * more efficiently closing inherited fds [#406](https://github.com/amoffat/sh/issues/406) * bugfix where passing invalid dictionary to `_env` will cause a mysterious child 255 exit code. [#497](https://github.com/amoffat/sh/pull/497) * bugfix where `_in` using 0 or `sys.stdin` wasn't behaving like a TTY, if it was in fact a TTY. [#514](https://github.com/amoffat/sh/issues/514) +* bugfix where `help(sh)` raised an exception [#455](https://github.com/amoffat/sh/issues/455) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index bebbca32..ee98fd5f 100644 --- a/sh.py +++ b/sh.py @@ -50,6 +50,7 @@ from functools import partial import inspect import tempfile +import warnings import stat import glob as glob_module import ast @@ -3229,9 +3230,8 @@ def __getitem__(self, k): # somebody tried to be funny and do "from sh import *" if k == "__all__": - raise RuntimeError("Cannot import * from sh. \ -Please import sh or import programs individually.") - + warnings.warn("Cannot import * from sh. Please import sh or import programs individually.") + return [] # check if we're naming a dynamically generated ReturnCode exception exc = get_exc_from_name(k) From a4e2ef603b496e1c51f640ea55e9d76cf4a4b1ba Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 00:55:24 -0700 Subject: [PATCH 49/91] regression on hanging tests --- sh.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sh.py b/sh.py index ee98fd5f..b5cee37b 100644 --- a/sh.py +++ b/sh.py @@ -1902,6 +1902,19 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, os.close(close_pipe_read) os.close(close_pipe_write) + # this is critical + # our exc_pipe_write must have CLOEXEC enabled. the reason for this is tricky: + # if our child (the block we're in now), has an exception, we need to be able to write to exc_pipe_write, so + # that when the parent does os.read(exc_pipe_read), it gets our traceback. however, os.read(exc_pipe_read) + # in the parent blocks, so if our child *doesn't* have an exception, and doesn't close the writing end, it + # hangs forever. not good! but obviously the child can't close the writing end until it knows it's not + # going to have an exception, which is impossible to know because but what if os.execv has an exception? so + # the answer is CLOEXEC, so that the writing end of the pipe gets closed upon successful exec, and the + # parent reading the read end won't block (close breaks the block). + flags = fcntl.fcntl(exc_pipe_write, fcntl.F_GETFD) + flags |= fcntl.FD_CLOEXEC + fcntl.fcntl(exc_pipe_write, fcntl.F_SETFD, flags) + try: # ignoring SIGHUP lets us persist even after the parent process # exits. only ignore if we're backgrounded From 959c854242dd68b40bc7c8828bf3c4a1fd83983c Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 00:56:56 -0700 Subject: [PATCH 50/91] not a useful test --- test.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test.py b/test.py index 23874b3c..d756ce80 100644 --- a/test.py +++ b/test.py @@ -2724,13 +2724,6 @@ def test_fd_over_1024(self): def test_args_deprecated(self): self.assertRaises(DeprecationWarning, sh.args, _env={}) - def test_cant_import_all(self): - def go(): - # we have to use exec, because in py3, this syntax raises a - # SyntaxError upon compilation - exec("from sh import *") - self.assertRaises(RuntimeError, go) - def test_percent_doesnt_fail_logging(self): """ test that a command name doesn't interfere with string formatting in the internal loggers """ From 2dba4ca4df013d9d5f02f31ee13e619d2332cf20 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 01:41:07 -0700 Subject: [PATCH 51/91] make child exception fallback more useful --- sh.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sh.py b/sh.py index 10814ddf..520470b9 100644 --- a/sh.py +++ b/sh.py @@ -2028,17 +2028,17 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # if your parent process experiences an exit code 255, it is most # likely that an exception occurred between the fork of the child # and the exec. this should be reported. - except Exception as e: + except: # some helpful debugging + tb = traceback.format_exc().encode("utf8", "ignore") + try: - tb = traceback.format_exc() - tb = tb.encode("utf8", "ignore") os.write(exc_pipe_write, tb) - except: + + except Exception as e: # dump to stderr if we cannot save it to exc_pipe_write - if not exc_pipe_write or not hasattr(exc_pipe_write, 'write'): - sys.stderr.write( - "\nFATAL: %s\n%s" % (e, str(tb))) + sys.stderr.write("\nFATAL SH ERROR: %s\n" % e) + finally: os._exit(255) From 8eb730e7329b59bedf9150603210a6b581650593 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 17:32:33 -0700 Subject: [PATCH 52/91] better comments --- sh.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/sh.py b/sh.py index a89463ef..67a49064 100644 --- a/sh.py +++ b/sh.py @@ -1787,9 +1787,22 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # TTY. this is the only way some secure programs like ssh will # output correctly (is if stdout and stdin are both the same TTY) if single_tty: + # master_fd, slave_fd = pty.openpty() + # + # Anything that is written on the master end is provided to the process on the slave end as though it was + # input typed on a terminal. -"man 7 pty" + # + # later, in the child process, we're going to do this, so keep it in mind: + # + # os.dup2(self._stdin_write_fd, 0) + # os.dup2(self._stdout_write_fd, 1) + # os.dup2(self._stderr_write_fd, 2) self._stdin_read_fd, self._stdin_write_fd = pty.openpty() self._stdout_read_fd = os.dup(self._stdin_read_fd) + + # this line is what makes stdout and stdin attached to the same pty. in other words the process will write + # to the same underlying fd as stdout as it uses to read from for stdin. self._stdout_write_fd = os.dup(self._stdin_write_fd) self._stderr_read_fd = os.dup(self._stdin_read_fd) From 7c0fc800d121992fd16a23ef33ca17f8f8a3fcdc Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 17:41:46 -0700 Subject: [PATCH 53/91] rename fds to be something more intuitive --- sh.py | 113 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/sh.py b/sh.py index 67a49064..0209b0a1 100644 --- a/sh.py +++ b/sh.py @@ -1794,19 +1794,19 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # # later, in the child process, we're going to do this, so keep it in mind: # - # os.dup2(self._stdin_write_fd, 0) - # os.dup2(self._stdout_write_fd, 1) - # os.dup2(self._stderr_write_fd, 2) - self._stdin_read_fd, self._stdin_write_fd = pty.openpty() + # os.dup2(self._stdin_child_fd, 0) + # os.dup2(self._stdout_child_fd, 1) + # os.dup2(self._stderr_child_fd, 2) + self._stdin_parent_fd, self._stdin_child_fd = pty.openpty() - self._stdout_read_fd = os.dup(self._stdin_read_fd) + self._stdout_parent_fd = os.dup(self._stdin_parent_fd) # this line is what makes stdout and stdin attached to the same pty. in other words the process will write # to the same underlying fd as stdout as it uses to read from for stdin. - self._stdout_write_fd = os.dup(self._stdin_write_fd) + self._stdout_child_fd = os.dup(self._stdin_child_fd) - self._stderr_read_fd = os.dup(self._stdin_read_fd) - self._stderr_write_fd = os.dup(self._stdin_write_fd) + self._stderr_parent_fd = os.dup(self._stdin_parent_fd) + self._stderr_child_fd = os.dup(self._stdin_child_fd) # do not consolidate stdin and stdout. this is the most common use- # case @@ -1814,32 +1814,32 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # this check here is because we may be doing piping and so our stdin # might be an instance of OProc if isinstance(stdin, OProc) and stdin.call_args["piped"]: - self._stdin_write_fd = stdin._pipe_fd - self._stdin_read_fd = None + self._stdin_child_fd = stdin._pipe_fd + self._stdin_parent_fd = None self._stdin_process = stdin elif stdin_is_tty_or_pipe: - self._stdin_write_fd = os.dup(get_fileno(stdin)) - self._stdin_read_fd = None + self._stdin_child_fd = os.dup(get_fileno(stdin)) + self._stdin_parent_fd = None elif ca["tty_in"]: - self._stdin_read_fd, self._stdin_write_fd = pty.openpty() + self._stdin_parent_fd, self._stdin_child_fd = pty.openpty() # tty_in=False is the default else: - self._stdin_write_fd, self._stdin_read_fd = os.pipe() + self._stdin_child_fd, self._stdin_parent_fd = os.pipe() if stdout_is_tty_or_pipe and not tee_out: - self._stdout_write_fd = os.dup(get_fileno(stdout)) - self._stdout_read_fd = None + self._stdout_child_fd = os.dup(get_fileno(stdout)) + self._stdout_parent_fd = None # tty_out=True is the default elif ca["tty_out"]: - self._stdout_read_fd, self._stdout_write_fd = pty.openpty() + self._stdout_parent_fd, self._stdout_child_fd = pty.openpty() else: - self._stdout_read_fd, self._stdout_write_fd = os.pipe() + self._stdout_parent_fd, self._stdout_child_fd = os.pipe() # unless STDERR is going to STDOUT, it ALWAYS needs to be a pipe, # and never a PTY. the reason for this is not totally clear to me, @@ -1853,26 +1853,26 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # directly to the stdout fd (no pipe), and so stderr won't have # a slave end of a pipe either to dup if stdout_is_tty_or_pipe and not tee_out: - self._stderr_read_fd = None + self._stderr_parent_fd = None else: - self._stderr_read_fd = os.dup(self._stdout_read_fd) - self._stderr_write_fd = os.dup(self._stdout_write_fd) + self._stderr_parent_fd = os.dup(self._stdout_parent_fd) + self._stderr_child_fd = os.dup(self._stdout_child_fd) elif stderr_is_tty_or_pipe and not tee_err: - self._stderr_write_fd = os.dup(get_fileno(stderr)) - self._stderr_read_fd = None + self._stderr_child_fd = os.dup(get_fileno(stderr)) + self._stderr_parent_fd = None else: - self._stderr_read_fd, self._stderr_write_fd = os.pipe() + self._stderr_parent_fd, self._stderr_child_fd = os.pipe() piped = ca["piped"] self._pipe_fd = None if piped: - fd_to_use = self._stdout_read_fd + fd_to_use = self._stdout_parent_fd if piped == "err": - fd_to_use = self._stderr_read_fd + fd_to_use = self._stderr_parent_fd self._pipe_fd = os.dup(fd_to_use) @@ -1881,7 +1881,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, self.ctty = None if needs_ctty: - self.ctty = os.ttyname(self._stdin_write_fd) + self.ctty = os.ttyname(self._stdin_child_fd) # this is a hack, but what we're doing here is intentionally throwing an # OSError exception if our child processes's directory doesn't exist, @@ -1905,7 +1905,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # this pipe is for synchronizing with the child that the parent has # closed its in/out/err fds. this is a bug on OSX (but not linux), # where we can lose output sometimes, due to a race, if we do - # os.close(self._stdout_write_fd) in the parent after the child starts + # os.close(self._stdout_child_fd) in the parent after the child starts # writing. if IS_OSX: close_pipe_read, close_pipe_write = os.pipe() @@ -1976,19 +1976,19 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # we HAVE to do this here, and not in the parent process, # because we have to guarantee that this is set before the # child process is run, and we can't do it twice. - tty.setraw(self._stdout_write_fd) + tty.setraw(self._stdout_child_fd) # if the parent-side fd for stdin exists, close it. the case # where it may not exist is if we're using piping - if self._stdin_read_fd: - os.close(self._stdin_read_fd) + if self._stdin_parent_fd: + os.close(self._stdin_parent_fd) - if self._stdout_read_fd: - os.close(self._stdout_read_fd) + if self._stdout_parent_fd: + os.close(self._stdout_parent_fd) - if self._stderr_read_fd: - os.close(self._stderr_read_fd) + if self._stderr_parent_fd: + os.close(self._stderr_parent_fd) os.close(session_pipe_read) os.close(exc_pipe_read) @@ -1996,9 +1996,9 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, if cwd: os.chdir(cwd) - os.dup2(self._stdin_write_fd, 0) - os.dup2(self._stdout_write_fd, 1) - os.dup2(self._stderr_write_fd, 2) + os.dup2(self._stdin_child_fd, 0) + os.dup2(self._stdout_child_fd, 1) + os.dup2(self._stderr_child_fd, 2) # set our controlling terminal, but only if we're using a tty @@ -2067,9 +2067,9 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, if gc_enabled: gc.enable() - os.close(self._stdin_write_fd) - os.close(self._stdout_write_fd) - os.close(self._stderr_write_fd) + os.close(self._stdin_child_fd) + os.close(self._stdout_child_fd) + os.close(self._stderr_child_fd) # tell our child process that we've closed our write_fds, so it is # ok to proceed towards exec. see the comment where this pipe is @@ -2124,7 +2124,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, self._stderr = deque(maxlen=ca["internal_bufsize"]) if ca["tty_in"] and not stdin_is_tty_or_pipe: - setwinsize(self._stdin_read_fd, ca["tty_size"]) + setwinsize(self._stdin_parent_fd, ca["tty_size"]) self.log = parent_log.get_child("process", repr(self)) @@ -2134,9 +2134,9 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # disable echoing, but only if it's a tty that we created ourselves if ca["tty_in"] and not stdin_is_tty_or_pipe: - attr = termios.tcgetattr(self._stdin_read_fd) + attr = termios.tcgetattr(self._stdin_parent_fd) attr[3] &= ~termios.ECHO - termios.tcsetattr(self._stdin_read_fd, termios.TCSANOW, attr) + termios.tcsetattr(self._stdin_parent_fd, termios.TCSANOW, attr) # we're only going to create a stdin thread iff we have potential # for stdin to come in. this would be through a stdout callback or @@ -2146,9 +2146,9 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # this represents the connection from a Queue object (or whatever # we're using to feed STDIN) to the process's STDIN fd self._stdin_stream = None - if self._stdin_read_fd and potentially_has_input: + if self._stdin_parent_fd and potentially_has_input: log = self.log.get_child("streamwriter", "stdin") - self._stdin_stream = StreamWriter(log, self._stdin_read_fd, + self._stdin_stream = StreamWriter(log, self._stdin_parent_fd, self.stdin, ca["in_bufsize"], ca["encoding"], ca["tty_in"]) @@ -2173,20 +2173,19 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # already hooked up this processes's stdout fd to the other # processes's stdin fd self._stdout_stream = None - if not pipe_out and self._stdout_read_fd: + if not pipe_out and self._stdout_parent_fd: if callable(stdout): stdout = construct_streamreader_callback(self, stdout) self._stdout_stream = \ StreamReader( self.log.get_child("streamreader", "stdout"), - self._stdout_read_fd, stdout, self._stdout, + self._stdout_parent_fd, stdout, self._stdout, ca["out_bufsize"], ca["encoding"], ca["decode_errors"], stdout_pipe, save_data=save_stdout) - elif self._stdout_read_fd: - os.close(self._stdout_read_fd) - + elif self._stdout_parent_fd: + os.close(self._stdout_parent_fd) # if stderr is going to one place (because it's grouped with stdout, # or we're dealing with a single tty), then we don't actually need a @@ -2194,7 +2193,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # stdout above self._stderr_stream = None if stderr is not OProc.STDOUT and not single_tty and not pipe_err \ - and self._stderr_read_fd: + and self._stderr_parent_fd: stderr_pipe = None if pipe is OProc.STDERR and not ca["no_pipe"]: @@ -2207,12 +2206,12 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, stderr = construct_streamreader_callback(self, stderr) self._stderr_stream = StreamReader(Logger("streamreader"), - self._stderr_read_fd, stderr, self._stderr, + self._stderr_parent_fd, stderr, self._stderr, ca["err_bufsize"], ca["encoding"], ca["decode_errors"], stderr_pipe, save_data=save_stderr) - elif self._stderr_read_fd: - os.close(self._stderr_read_fd) + elif self._stderr_parent_fd: + os.close(self._stderr_parent_fd) def timeout_fn(): @@ -2424,8 +2423,8 @@ def _process_just_ended(self): # the CTTY, and closing it prematurely will send a SIGHUP. we also # don't want to close it if there's a self._stdin_stream, because that # is in charge of closing it also - if self._stdin_read_fd and not self._stdin_stream: - os.close(self._stdin_read_fd) + if self._stdin_parent_fd and not self._stdin_stream: + os.close(self._stdin_parent_fd) def wait(self): From ca7980a62014fddadb9f85398194f02ff50631bb Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 17:59:59 -0700 Subject: [PATCH 54/91] more fd comments --- sh.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 0209b0a1..76cfddac 100644 --- a/sh.py +++ b/sh.py @@ -1799,10 +1799,12 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # os.dup2(self._stderr_child_fd, 2) self._stdin_parent_fd, self._stdin_child_fd = pty.openpty() + # this makes our parent fds behave like a terminal. it says that the very same fd that we "type" to (for + # stdin) is the same one that we see output printed to (for stdout) self._stdout_parent_fd = os.dup(self._stdin_parent_fd) # this line is what makes stdout and stdin attached to the same pty. in other words the process will write - # to the same underlying fd as stdout as it uses to read from for stdin. + # to the same underlying fd as stdout as it uses to read from for stdin. this makes programs like ssh happy self._stdout_child_fd = os.dup(self._stdin_child_fd) self._stderr_parent_fd = os.dup(self._stdin_parent_fd) From 96c8db4ecc06406514d15a646712f31f8bf6baa1 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 20:52:06 -0700 Subject: [PATCH 55/91] fixing interactive ssh snippet * updating kwarg validators to be given passed_kwargs and merged_kwargs * introduce `_unify_ttys` which stitches together ttys IFF it is True --- CHANGELOG.md | 3 +++ sh.py | 51 +++++++++++++++++++++++++++------------------------ 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb8e9a6c..5956a270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ * bugfix where passing invalid dictionary to `_env` will cause a mysterious child 255 exit code. [#497](https://github.com/amoffat/sh/pull/497) * bugfix where `_in` using 0 or `sys.stdin` wasn't behaving like a TTY, if it was in fact a TTY. [#514](https://github.com/amoffat/sh/issues/514) * bugfix where `help(sh)` raised an exception [#455](https://github.com/amoffat/sh/issues/455) +* bugfix fixing broken interactive ssh tutorial from docs +* change to automatic tty merging into a single pty if `_tty_in=True` and `_tty_out=True` +* introducing `_unify_ttys`, default False, which allows explicit tty merging into single pty ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index 76cfddac..0665af81 100644 --- a/sh.py +++ b/sh.py @@ -968,15 +968,15 @@ def get_prepend_stack(): return tl._prepend_stack -def special_kwarg_validator(kwargs, invalid_list): - s1 = set(kwargs.keys()) +def special_kwarg_validator(passed_kwargs, merged_kwargs, invalid_list): + s1 = set(passed_kwargs.keys()) invalid_args = [] for args in invalid_list: if callable(args): fn = args - ret = fn(kwargs) + ret = fn(passed_kwargs, merged_kwargs) invalid_args.extend(ret) else: @@ -1024,19 +1024,26 @@ def ob_is_pipe(ob): return is_pipe -def tty_in_validator(kwargs): +def tty_in_validator(passed_kwargs, merged_kwargs): + # here we'll validate that people aren't randomly shotgun-debugging different tty options and hoping that they'll + # work, without understanding what they do pairs = (("tty_in", "in"), ("tty_out", "out")) invalid = [] for tty, std in pairs: - if tty in kwargs and ob_is_tty(kwargs.get(std, None)): + if tty in passed_kwargs and ob_is_tty(passed_kwargs.get(std, None)): args = (tty, std) - error = "`_%s` is a TTY already, so so it doesn't make sense \ -to set up a TTY with `_%s`" % (std, tty) + error = "`_%s` is a TTY already, so so it doesn't make sense to set up a TTY with `_%s`" % (std, tty) invalid.append((args, error)) + # if unify_ttys is set, then both tty_in and tty_out must both be True + if merged_kwargs["unify_ttys"] and not (merged_kwargs["tty_in"] and merged_kwargs["tty_out"]): + invalid.append((("unify_ttys", "tty_in", "tty_out"), + "`_tty_in` and `_tty_out` must both be True if `_unify_ttys` is True")) + return invalid -def bufsize_validator(kwargs): + +def bufsize_validator(passed_kwargs, merged_kwargs): """ a validator to prevent a user from saying that they want custom buffering when they're using an in/out object that will be os.dup'd to the process, and has its own buffering. an example is a pipe or a tty. it @@ -1044,11 +1051,11 @@ def bufsize_validator(kwargs): controls this. """ invalid = [] - in_ob = kwargs.get("in", None) - out_ob = kwargs.get("out", None) + in_ob = passed_kwargs.get("in", None) + out_ob = passed_kwargs.get("out", None) - in_buf = kwargs.get("in_bufsize", None) - out_buf = kwargs.get("out_bufsize", None) + in_buf = passed_kwargs.get("in_bufsize", None) + out_buf = passed_kwargs.get("out_bufsize", None) in_no_buf = ob_is_tty(in_ob) or ob_is_pipe(in_ob) out_no_buf = ob_is_tty(out_ob) or ob_is_pipe(out_ob) @@ -1064,12 +1071,12 @@ def bufsize_validator(kwargs): return invalid -def env_validator(kwargs): +def env_validator(passed_kwargs, merged_kwargs): """ a validator to check that env is a dictionary and that all environment variable keys and values are strings. Otherwise, we would exit with a confusing exit code 255. """ invalid = [] - env = kwargs.get("env", None) + env = passed_kwargs.get("env", None) if env is None: return invalid @@ -1077,7 +1084,7 @@ def env_validator(kwargs): invalid.append((("env"), "env must be a dict. Got {!r}".format(env))) return invalid - for k, v in kwargs["env"].items(): + for k, v in passed_kwargs["env"].items(): if not isinstance(k, str): invalid.append((("env"), "env key {!r} must be a str".format(k))) if not isinstance(v, str): @@ -1152,6 +1159,7 @@ class Command(object): # ssh is one of those programs "tty_in": False, "tty_out": True, + "unify_ttys": False, "encoding": DEFAULT_ENCODING, "decode_errors": "strict", @@ -1290,8 +1298,9 @@ def _extract_call_args(kwargs): call_args[parg] = kwargs[key] del kwargs[key] - invalid_kwargs = special_kwarg_validator(call_args, - Command._kwarg_validators) + merged_args = Command._call_args.copy() + merged_args.update(call_args) + invalid_kwargs = special_kwarg_validator(call_args, merged_args, Command._kwarg_validators) if invalid_kwargs: exc_msg = [] @@ -1775,12 +1784,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, tee_out = ca["tee"] in (True, "out") tee_err = ca["tee"] == "err" - # if we're passing in a custom stdout/out/err value, we obviously have - # to force not using single_tty - custom_in_out_err = stdin or stdout or stderr - - single_tty = (ca["tty_in"] and ca["tty_out"])\ - and not custom_in_out_err + single_tty = ca["tty_in"] and ca["tty_out"] and ca["unify_ttys"] # this logic is a little convoluted, but basically this top-level # if/else is for consolidating input and output TTYs into a single @@ -1831,7 +1835,6 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, else: self._stdin_child_fd, self._stdin_parent_fd = os.pipe() - if stdout_is_tty_or_pipe and not tee_out: self._stdout_child_fd = os.dup(get_fileno(stdout)) self._stdout_parent_fd = None From fb047dd6781a4e39920e8ea48f5dbdd7625e3745 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 22:50:26 -0700 Subject: [PATCH 56/91] ssh contrib command --- CHANGELOG.md | 1 + sh.py | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5956a270..9d790b72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * bugfix fixing broken interactive ssh tutorial from docs * change to automatic tty merging into a single pty if `_tty_in=True` and `_tty_out=True` * introducing `_unify_ttys`, default False, which allows explicit tty merging into single pty +* contrib command for `ssh` connections requiring passwords ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index 0665af81..2496b5d3 100644 --- a/sh.py +++ b/sh.py @@ -52,6 +52,7 @@ import tempfile import warnings import stat +from collections import deque import glob as glob_module import ast from contextlib import contextmanager @@ -3361,6 +3362,7 @@ def git(orig): # pragma: no cover cmd = orig.bake(_tty_out=False) return cmd + @contrib("sudo") def sudo(orig): # pragma: no cover """ a nicer version of sudo that uses getpass to ask for a password, or @@ -3388,6 +3390,85 @@ def process(args, kwargs): return cmd +@contrib("ssh") +def ssh(orig): # pragma: no cover + """ An ssh command for automatic password login """ + + class SessionContent(object): + def __init__(self): + self.chars = deque(maxlen=50000) + self.lines = deque(maxlen=5000) + self.line_chars = [] + self.last_line = "" + self.cur_char = "" + + def append_char(self, char): + if char == "\n": + line = self.cur_line + self.last_line = line + self.lines.append(line) + self.line_chars = [] + else: + self.line_chars.append(char) + + self.chars.append(char) + self.cur_char = char + + @property + def cur_line(self): + line = "".join(self.line_chars) + return line + + class SSHInteract(object): + def __init__(self, prompt_match, pass_getter, out_handler, login_success): + self.prompt_match = prompt_match + self.pass_getter = pass_getter + self.out_handler = out_handler + self.login_success = login_success + self.content = SessionContent() + + # some basic state + self.pw_entered = False + self.success = False + + def __call__(self, char, stdin): + self.content.append_char(char) + + if self.pw_entered and not self.success: + self.success = self.login_success(self.content) + + if self.success: + return self.out_handler(self.content, stdin) + + if self.prompt_match(self.content): + password = self.pass_getter() + stdin.put(password + "\n") + self.pw_entered = True + + + def process(args, kwargs): + real_out_handler = kwargs.pop("interact") + password = kwargs.pop("password", None) + login_success = kwargs.pop("login_success", None) + prompt_match = kwargs.pop("prompt", None) + prompt = "Please enter SSH password: " + + if prompt_match is None: + prompt_match = lambda content: content.cur_line.endswith("password: ") + + if password is None: + pass_getter = lambda: getpass.getpass(prompt=prompt) + else: + pass_getter = lambda: password.rstrip("\n") + + if login_success is None: + login_success = lambda content: True + + kwargs["_out"] = SSHInteract(prompt_match, pass_getter, real_out_handler, login_success) + return args, kwargs + + cmd = orig.bake(_out_bufsize=0, _tty_in=True, _unify_ttys=True, _arg_preprocess=process) + return cmd def run_repl(env): # pragma: no cover From 558a7ba7b5f19b525eb4b831846074b32483a74b Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Fri, 24 Apr 2020 22:50:31 -0700 Subject: [PATCH 57/91] updating copyright --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 2496b5d3..ba8babd6 100644 --- a/sh.py +++ b/sh.py @@ -2,7 +2,7 @@ http://amoffat.github.io/sh/ """ #=============================================================================== -# Copyright (C) 2011-2017 by Andrew Moffat +# Copyright (C) 2011-2020 by Andrew Moffat # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal From af8147172ba99a88ec6c667a2a154d37dc3a3443 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 01:50:23 -0700 Subject: [PATCH 58/91] version update --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d790b72..02b31fff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## X.XX.XX - XX/XX/XX +## 1.13.0 - XX/XX/XX * minor Travis CI fixes [#492](https://github.com/amoffat/sh/pull/492) * bugfix for boolean long options not respecting `_long_prefix` [#488](https://github.com/amoffat/sh/pull/488) * fix deprecation warning on Python 3.6 regexes [#482](https://github.com/amoffat/sh/pull/482) From a3e8773f50492dd162cd1a3c815d109fd4a75f26 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 02:07:56 -0700 Subject: [PATCH 59/91] introduce `_iter_poll_time` and improve polling performance we were polling the output queue extremely frequently by using an obscenely short timeout. here we extend the timeout and make the timeout configurable. closes #462 --- CHANGELOG.md | 1 + sh.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b31fff..9b85a58e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * change to automatic tty merging into a single pty if `_tty_in=True` and `_tty_out=True` * introducing `_unify_ttys`, default False, which allows explicit tty merging into single pty * contrib command for `ssh` connections requiring passwords +* performance fix for polling output too fast when using `_iter` [462](https://github.com/amoffat/sh/issues/462) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index ba8babd6..dc1c11c4 100644 --- a/sh.py +++ b/sh.py @@ -868,7 +868,7 @@ def next(self): # so the slight timeout allows for that. while True: try: - chunk = self.process._pipe_queue.get(True, 0.001) + chunk = self.process._pipe_queue.get(True, self.call_args["iter_poll_time"]) except Empty: if self.call_args["iter_noblock"]: return errno.EWOULDBLOCK @@ -1144,6 +1144,8 @@ class Command(object): "piped": None, "iter": None, "iter_noblock": None, + # the amount of time to sleep between polling for the iter output queue + "iter_poll_time": 0.1, "ok_code": 0, "cwd": None, From 404b50603fb91df1b83a00f8bf6fa060cfd0c0f6 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 05:00:03 -0700 Subject: [PATCH 60/91] a little less brittle --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index bce31788..1b3a041e 100644 --- a/sh.py +++ b/sh.py @@ -3590,7 +3590,7 @@ def test(importer): if not already_registered: importer = ModuleImporterFromVariables( - restrict_to=["SelfWrapper"], + restrict_to=[SelfWrapper.__name__], ) sys.meta_path.insert(0, importer) From 804ff23954551fa6703cce34f31d707e3dedfde5 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 05:34:39 -0700 Subject: [PATCH 61/91] changelog update --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b85a58e..a46991e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,8 @@ * change to automatic tty merging into a single pty if `_tty_in=True` and `_tty_out=True` * introducing `_unify_ttys`, default False, which allows explicit tty merging into single pty * contrib command for `ssh` connections requiring passwords -* performance fix for polling output too fast when using `_iter` [462](https://github.com/amoffat/sh/issues/462) +* performance fix for polling output too fast when using `_iter` [#462](https://github.com/amoffat/sh/issues/462) +* execution contexts can now be used in python shell [#466](https://github.com/amoffat/sh/pull/466) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) From d463c91d217f3afb3c55fa33a3d98a11ca91f2bd Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 05:42:51 -0700 Subject: [PATCH 62/91] canonicalize search paths. closes #426 --- sh.py | 6 ++++-- test.py | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/sh.py b/sh.py index 90d01009..bd5208b9 100644 --- a/sh.py +++ b/sh.py @@ -531,6 +531,8 @@ def glob(path, *args, **kwargs): glob_module.glob = glob +def canonicalize(path): + return os.path.abspath(os.path.expanduser(path)) def which(program, paths=None): @@ -550,7 +552,7 @@ def is_exe(fpath): # if there's a path component, then we've specified a path to the program, # and we should just test if that program is executable. if it is, return if fpath: - program = os.path.abspath(os.path.expanduser(program)) + program = canonicalize(program) if is_exe(program): found_path = program @@ -566,7 +568,7 @@ def is_exe(fpath): paths_to_search.extend(env_paths) for path in paths_to_search: - exe_file = os.path.join(path, program) + exe_file = os.path.join(canonicalize(path), program) if is_exe(exe_file): found_path = exe_file break diff --git a/test.py b/test.py index 79ca4b15..9e1d9534 100644 --- a/test.py +++ b/test.py @@ -715,6 +715,7 @@ def test_command_wrapper_equivalence(self): self.assertEqual(Command(which("ls")), ls) + def test_doesnt_execute_directories(self): save_path = os.environ['PATH'] bin_dir1 = tempfile.mkdtemp() From e3c49895cca67435e7d5370f36d5e7fd16f9313d Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 06:32:23 -0700 Subject: [PATCH 63/91] allow pickling of exceptions, closes #439 --- CHANGELOG.md | 1 + sh.py | 4 ++++ test.py | 21 +++++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a46991e6..7b4409a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * contrib command for `ssh` connections requiring passwords * performance fix for polling output too fast when using `_iter` [#462](https://github.com/amoffat/sh/issues/462) * execution contexts can now be used in python shell [#466](https://github.com/amoffat/sh/pull/466) +* bugfix `ErrorReturnCode` instances can now be pickled ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index bd5208b9..2f298597 100644 --- a/sh.py +++ b/sh.py @@ -367,10 +367,14 @@ class ErrorReturnCode(Exception): truncate_cap = 750 + def __reduce__(self): + return (self.__class__, (self.full_cmd, self.stdout, self.stderr, self.truncate)) + def __init__(self, full_cmd, stdout, stderr, truncate=True): self.full_cmd = full_cmd self.stdout = stdout self.stderr = stderr + self.truncate = truncate exc_stdout = self.stdout if truncate: diff --git a/test.py b/test.py index 9e1d9534..17eeec15 100644 --- a/test.py +++ b/test.py @@ -2703,6 +2703,27 @@ def test(cmd): class MiscTests(BaseTests): + def test_pickling(self): + import pickle + + py = create_tmp_test(""" +import sys +sys.stdout.write("some output") +sys.stderr.write("some error") +exit(1) +""") + + try: + python(py.name) + except sh.ErrorReturnCode as e: + restored = pickle.loads(pickle.dumps(e)) + self.assertEqual(restored.stdout, b"some output") + self.assertEqual(restored.stderr, b"some error") + self.assertEqual(restored.exit_code, 1) + else: + self.fail("Didn't get an exception") + + @requires_poller("poll") def test_fd_over_1024(self): py = create_tmp_test("""print("hi world")""") From 781c27a5ec3cb71188d056e3c45ab6b20f483bd2 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 06:35:04 -0700 Subject: [PATCH 64/91] test assert cleanup, closes #424 --- test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test.py b/test.py index 17eeec15..06146d55 100644 --- a/test.py +++ b/test.py @@ -212,8 +212,8 @@ def assert_deprecated(self, fn, *args, **kwargs): with warnings.catch_warnings(record=True) as w: fn(*args, **kwargs) - assert len(w) == 1 - assert issubclass(w[-1].category, DeprecationWarning) + self.assertEqual(len(w), 1) + self.assertTrue(issubclass(w[-1].category, DeprecationWarning)) @requires_posix From 0764716be7d4164665f47138a8a7bc8490bb6563 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 08:09:11 -0700 Subject: [PATCH 65/91] support empty _in param. closes #427 --- sh.py | 15 ++++++++------- test.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/sh.py b/sh.py index 2f298597..dd1203f9 100644 --- a/sh.py +++ b/sh.py @@ -2119,7 +2119,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # to prevent race conditions self.exit_code = None - self.stdin = stdin or Queue() + self.stdin = stdin # _pipe_queue is used internally to hand off stdout from one process # to another. by default, all stdout from a process gets dumped @@ -2152,15 +2152,10 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, attr[3] &= ~termios.ECHO termios.tcsetattr(self._stdin_parent_fd, termios.TCSANOW, attr) - # we're only going to create a stdin thread iff we have potential - # for stdin to come in. this would be through a stdout callback or - # through an object we've passed in for stdin - potentially_has_input = callable(stdout) or stdin - # this represents the connection from a Queue object (or whatever # we're using to feed STDIN) to the process's STDIN fd self._stdin_stream = None - if self._stdin_parent_fd and potentially_has_input: + if self._stdin_parent_fd: log = self.log.get_child("streamwriter", "stdin") self._stdin_stream = StreamWriter(log, self._stdin_parent_fd, self.stdin, ca["in_bufsize"], ca["encoding"], @@ -2651,6 +2646,12 @@ def determine_how_to_read_input(input_obj): log_msg = "generator" get_chunk = get_iter_chunk_reader(iter(input_obj)) + elif input_obj is None: + log_msg = "None" + def raise_(): + raise DoneReadingForever + get_chunk = raise_ + else: try: it = iter(input_obj) diff --git a/test.py b/test.py index 06146d55..b5588b09 100644 --- a/test.py +++ b/test.py @@ -287,6 +287,17 @@ def test_number_arg(self): out = python(py.name, 3).strip() self.assertEqual(out, "3") + def test_empty_stdin_no_hang(self): + py = create_tmp_test(""" +import sys +data = sys.stdin.read() +sys.stdout.write("no hang") +""") + out = python(py.name, _in="", _timeout=2) + self.assertEqual(out, "no hang") + + out = python(py.name, _in=None, _timeout=2) + self.assertEqual(out, "no hang") def test_exit_code(self): from sh import ErrorReturnCode From e6bec57aa5caa9b2cd689cce7339b9d91a848267 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 08:10:23 -0700 Subject: [PATCH 66/91] changelog update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b4409a5..4fa2a794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * performance fix for polling output too fast when using `_iter` [#462](https://github.com/amoffat/sh/issues/462) * execution contexts can now be used in python shell [#466](https://github.com/amoffat/sh/pull/466) * bugfix `ErrorReturnCode` instances can now be pickled +* bugfix passing empty string or `None` for `_in` hanged [#427](https://github.com/amoffat/sh/pull/427) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) From 23addc01cf1c3fc178f9801fd8ab4a4ee302e58c Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 08:17:49 -0700 Subject: [PATCH 67/91] fixing tests --- sh.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sh.py b/sh.py index dd1203f9..41dd9e21 100644 --- a/sh.py +++ b/sh.py @@ -2121,6 +2121,11 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, self.stdin = stdin + # this accounts for when _out is a callable that is passed stdin. in that case, if stdin is unspecified, we + # must set it to a queue, so callbacks can put things on it + if callable(ca["out"]) and self.stdin is None: + self.stdin = Queue() + # _pipe_queue is used internally to hand off stdout from one process # to another. by default, all stdout from a process gets dumped # into this pipe queue, to be consumed in real time (hence the From 81ede3bcf337b8060554ea3ea870e2307162fd94 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 08:20:59 -0700 Subject: [PATCH 68/91] inconsistent docstring fix. closes #429 --- sh.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/sh.py b/sh.py index 41dd9e21..ec3ab168 100644 --- a/sh.py +++ b/sh.py @@ -699,8 +699,7 @@ class RunningCommand(object): def __init__(self, cmd, call_args, stdin, stdout, stderr): """ - cmd is an array, where each element is encoded as bytes (PY3) or str - (PY2) + cmd is a list, where each element is encoded as bytes (PY3) or str (PY2) """ # self.ran is used for auditing what actually ran. for example, in @@ -1750,14 +1749,11 @@ class OProc(object): def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, call_args, pipe, process_assign_lock): """ - cmd is the full string that will be exec'd. it includes the program - name and all its arguments + cmd is the full list of arguments that will be exec'd. it includes the program name and all its arguments. - stdin, stdout, stderr are what the child will use for standard - input/output/err + stdin, stdout, stderr are what the child will use for standard input/output/err. - call_args is a mapping of all the special keyword arguments to apply - to the child process + call_args is a mapping of all the special keyword arguments to apply to the child process. """ self.command = command self.call_args = call_args From 608f4c3bf5ad75ad40035d03a9c5ffcce0898f07 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 08:32:00 -0700 Subject: [PATCH 69/91] allow is_alive() on RunningCommand. closes #405 --- sh.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sh.py b/sh.py index ec3ab168..9009ed4d 100644 --- a/sh.py +++ b/sh.py @@ -680,7 +680,7 @@ class RunningCommand(object): finishes, RunningCommand is smart enough to translate exit codes to exceptions. """ - # these are attributes that we allow to passthrough to OProc for + # these are attributes that we allow to passthrough to OProc _OProc_attr_whitelist = set(( "signal", "terminate", @@ -821,6 +821,11 @@ def wait(self): return self + def is_alive(self): + """ returns whether or not we're still alive. this call has side-effects on OProc """ + return self.process.is_alive()[0] + + def handle_command_exit_code(self, code): """ here we determine if we had an exception, or an error code that we weren't expecting to see. if we did, we create and raise an exception From 753e4ec4f59bbbd58e75cde45c37d07a4037baf9 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sat, 25 Apr 2020 08:39:27 -0700 Subject: [PATCH 70/91] re-raise an exception if we were unable to handle it If we get TypeError from finding the module_name but it's not in the situation that we know what to do, we should re-raise the exception so that it's easier to track down where the error is coming from --- sh.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sh.py b/sh.py index ec3ab168..30d16e2b 100644 --- a/sh.py +++ b/sh.py @@ -3550,11 +3550,13 @@ def __call__(self, **kwargs): # top stack frame # Older versions of pypy don't set parent[1] the same way as CPython or newer versions # of Pypy so we have to special case that too. - if parent[1] in ('', '') or ( + if (parent[1] in ('', '') or parent[1] == '' and platform.python_implementation().lower() == 'pypy'): # This depends on things like Python's calling convention and the layout of stack # frames but it's a fix for a bug in a very cornery cornercase so.... module_name = parent[0].f_code.co_names[-1] + else: + raise else: parsed = ast.parse(code) try: From 563b69ea8d59ce643117213a761a4bfbc6116ac9 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 11:28:17 -0700 Subject: [PATCH 71/91] macos != osx --- sh.py | 14 +++++++------- test.py | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sh.py b/sh.py index 9009ed4d..d2d16040 100644 --- a/sh.py +++ b/sh.py @@ -94,7 +94,7 @@ def callable(ob): except ImportError: from pipes import quote as shlex_quote # undocumented before 2.7 -IS_OSX = platform.system() == "Darwin" +IS_MACOS = platform.system() == "Darwin" THIS_DIR = os.path.dirname(os.path.realpath(__file__)) SH_LOGGER_NAME = __name__ @@ -1092,14 +1092,14 @@ def env_validator(passed_kwargs, merged_kwargs): return invalid if not isinstance(env, dict): - invalid.append((("env"), "env must be a dict. Got {!r}".format(env))) + invalid.append(("env", "env must be a dict. Got {!r}".format(env))) return invalid for k, v in passed_kwargs["env"].items(): if not isinstance(k, str): - invalid.append((("env"), "env key {!r} must be a str".format(k))) + invalid.append(("env", "env key {!r} must be a str".format(k))) if not isinstance(v, str): - invalid.append((("env"), "value {!r} of env key {!r} must be a str".format(v, k))) + invalid.append(("env", "value {!r} of env key {!r} must be a str".format(v, k))) return invalid @@ -1922,7 +1922,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # where we can lose output sometimes, due to a race, if we do # os.close(self._stdout_child_fd) in the parent after the child starts # writing. - if IS_OSX: + if IS_MACOS: close_pipe_read, close_pipe_write = os.pipe() # session id, group id, process id @@ -1932,7 +1932,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # child if self.pid == 0: # pragma: no cover - if IS_OSX: + if IS_MACOS: os.read(close_pipe_read, 1) os.close(close_pipe_read) os.close(close_pipe_write) @@ -2089,7 +2089,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # tell our child process that we've closed our write_fds, so it is # ok to proceed towards exec. see the comment where this pipe is # opened, for why this is necessary - if IS_OSX: + if IS_MACOS: os.close(close_pipe_read) os.write(close_pipe_write, str(1).encode(DEFAULT_ENCODING)) os.close(close_pipe_write) diff --git a/test.py b/test.py index b5588b09..cdd23183 100644 --- a/test.py +++ b/test.py @@ -62,7 +62,7 @@ # we have to use the real path because on osx, /tmp is a symlink to # /private/tmp, and so assertions that gettempdir() == sh.pwd() will fail tempdir = realpath(tempfile.gettempdir()) -IS_OSX = platform.system() == "Darwin" +IS_MACOS = platform.system() == "Darwin" # these 3 functions are helpers for modifying PYTHONPATH with a module's main @@ -157,7 +157,7 @@ def requires_progs(*progs): requires_posix = skipUnless(os.name == "posix", "Requires POSIX") requires_utf8 = skipUnless(sh.DEFAULT_ENCODING == "UTF-8", "System encoding must be UTF-8") -not_osx = skipUnless(not IS_OSX, "Doesn't work on OSX") +not_macos = skipUnless(not IS_MACOS, "Doesn't work on MacOS") requires_py3 = skipUnless(IS_PY3, "Test only works on Python 3") requires_py35 = skipUnless(IS_PY3 and MINOR_VER >= 5, "Test only works on Python 3.5 or higher") @@ -370,7 +370,7 @@ def test_ok_code(self): exc_to_test = ErrorReturnCode_2 code_to_pass = 2 - if IS_OSX: + if IS_MACOS: exc_to_test = ErrorReturnCode_1 code_to_pass = 1 self.assertRaises(exc_to_test, ls, "/aofwje/garogjao4a/eoan3on") @@ -959,7 +959,7 @@ def test_background_exception(self): p = ls("/ofawjeofj", _bg=True) # should not raise exc_to_test = ErrorReturnCode_2 - if IS_OSX: exc_to_test = ErrorReturnCode_1 + if IS_MACOS: exc_to_test = ErrorReturnCode_1 self.assertRaises(exc_to_test, p.wait) # should raise @@ -2770,7 +2770,7 @@ def test_percent_doesnt_fail_logging(self): # TODO # for some reason, i can't get a good stable baseline measured in this test # on osx. so skip it for now if osx - @not_osx + @not_macos @requires_progs("lsof") def test_no_fd_leak(self): import sh From 30dfe61734d0f60de86656237ddd6f9ac9a2a575 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 11:38:06 -0700 Subject: [PATCH 72/91] fg validator that links users to docs. closes #395 --- sh.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/sh.py b/sh.py index d2d16040..4acabd23 100644 --- a/sh.py +++ b/sh.py @@ -1053,6 +1053,18 @@ def tty_in_validator(passed_kwargs, merged_kwargs): return invalid +def fg_validator(passed_kwargs, merged_kwargs): + """ fg is not valid with basically every other option """ + + invalid = [] + msg = """\ +_fg is invalid with nearly every other option, see warning and workaround here: + + https://amoffat.github.io/sh/sections/special_arguments.html#fg""" + if "fg" in passed_kwargs and len(passed_kwargs) > 1: + invalid.append(("fg", msg)) + return invalid + def bufsize_validator(passed_kwargs, merged_kwargs): """ a validator to prevent a user from saying that they want custom @@ -1235,8 +1247,6 @@ class Command(object): # this is a collection of validators to make sure the special kwargs make # sense _kwarg_validators = ( - (("fg", "bg"), "Command can't be run in the foreground and background"), - (("fg", "err_to_out"), "Can't redirect STDERR in foreground mode"), (("err", "err_to_out"), "Stderr is already being redirected"), (("piped", "iter"), "You cannot iterate when this command is being piped"), (("piped", "no_pipe"), "Using a pipe doesn't make sense if you've disabled the pipe"), @@ -1245,6 +1255,7 @@ class Command(object): tty_in_validator, bufsize_validator, env_validator, + fg_validator, ) From d261a8b8287e8f57f483c2463578c888292e9b8b Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 11:46:15 -0700 Subject: [PATCH 73/91] _fg is invalid with *almost* every other kwarg... --- sh.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 4acabd23..54f9ea73 100644 --- a/sh.py +++ b/sh.py @@ -1061,7 +1061,10 @@ def fg_validator(passed_kwargs, merged_kwargs): _fg is invalid with nearly every other option, see warning and workaround here: https://amoffat.github.io/sh/sections/special_arguments.html#fg""" - if "fg" in passed_kwargs and len(passed_kwargs) > 1: + whitelist = set(("env", "fg")) + offending = set(passed_kwargs.keys()) - whitelist + + if "fg" in passed_kwargs and offending: invalid.append(("fg", msg)) return invalid From 56e18cff5a82c7a3253a0dfe8792a44ae7050aae Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 16:15:55 -0700 Subject: [PATCH 74/91] fd-based test for os.dup2 is better than tty/pipe test. closes #449 --- CHANGELOG.md | 1 + sh.py | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fa2a794..27dffded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ * execution contexts can now be used in python shell [#466](https://github.com/amoffat/sh/pull/466) * bugfix `ErrorReturnCode` instances can now be pickled * bugfix passing empty string or `None` for `_in` hanged [#427](https://github.com/amoffat/sh/pull/427) +* bugfix where passing a filename or file-like object to `_out` wasn't using os.dup2 [#449](https://github.com/amoffat/sh/issues/449) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index 54f9ea73..f60710fe 100644 --- a/sh.py +++ b/sh.py @@ -1017,6 +1017,8 @@ def get_fileno(ob): return fileno +def ob_is_fd_based(ob): + return get_fileno(ob) is not None def ob_is_tty(ob): """ checks if an object (like a file-like object) is a tty. """ @@ -1083,8 +1085,8 @@ def bufsize_validator(passed_kwargs, merged_kwargs): in_buf = passed_kwargs.get("in_bufsize", None) out_buf = passed_kwargs.get("out_bufsize", None) - in_no_buf = ob_is_tty(in_ob) or ob_is_pipe(in_ob) - out_no_buf = ob_is_tty(out_ob) or ob_is_pipe(out_ob) + in_no_buf = ob_is_fd_based(in_ob) + out_no_buf = ob_is_fd_based(out_ob) err = "Can't specify an {target} bufsize if the {target} target is a pipe or TTY" @@ -1801,9 +1803,9 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # file-like object that is a tty, for example `sys.stdin`, then, later # on in this constructor, we're going to skip out on setting up pipes # and pseudoterminals for those endpoints - stdin_is_tty_or_pipe = ob_is_tty(stdin) or ob_is_pipe(stdin) - stdout_is_tty_or_pipe = ob_is_tty(stdout) or ob_is_pipe(stdout) - stderr_is_tty_or_pipe = ob_is_tty(stderr) or ob_is_pipe(stderr) + stdin_is_fd_based = ob_is_fd_based(stdin) + stdout_is_fd_based = ob_is_fd_based(stdout) + stderr_is_fd_based = ob_is_fd_based(stderr) tee_out = ca["tee"] in (True, "out") tee_err = ca["tee"] == "err" @@ -1848,7 +1850,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, self._stdin_parent_fd = None self._stdin_process = stdin - elif stdin_is_tty_or_pipe: + elif stdin_is_fd_based: self._stdin_child_fd = os.dup(get_fileno(stdin)) self._stdin_parent_fd = None @@ -1859,7 +1861,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, else: self._stdin_child_fd, self._stdin_parent_fd = os.pipe() - if stdout_is_tty_or_pipe and not tee_out: + if stdout_is_fd_based and not tee_out: self._stdout_child_fd = os.dup(get_fileno(stdout)) self._stdout_parent_fd = None @@ -1881,14 +1883,14 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, # we should not specify a read_fd, because stdout is dup'd # directly to the stdout fd (no pipe), and so stderr won't have # a slave end of a pipe either to dup - if stdout_is_tty_or_pipe and not tee_out: + if stdout_is_fd_based and not tee_out: self._stderr_parent_fd = None else: self._stderr_parent_fd = os.dup(self._stdout_parent_fd) self._stderr_child_fd = os.dup(self._stdout_child_fd) - elif stderr_is_tty_or_pipe and not tee_err: + elif stderr_is_fd_based and not tee_err: self._stderr_child_fd = os.dup(get_fileno(stderr)) self._stderr_parent_fd = None @@ -1997,7 +1999,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, payload = ("%d,%d" % (sid, pgid)).encode(DEFAULT_ENCODING) os.write(session_pipe_write, payload) - if ca["tty_out"] and not stdout_is_tty_or_pipe and not single_tty: + if ca["tty_out"] and not stdout_is_fd_based and not single_tty: # set raw mode, so there isn't any weird translation of # newlines to \r\n and other oddities. we're not outputting # to a terminal anyways @@ -2036,7 +2038,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, tmp_fd = os.open(os.ttyname(0), os.O_RDWR) os.close(tmp_fd) - if ca["tty_out"] and not stdout_is_tty_or_pipe: + if ca["tty_out"] and not stdout_is_fd_based: setwinsize(1, ca["tty_size"]) if ca["uid"] is not None: @@ -2157,7 +2159,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, self._stdout = deque(maxlen=ca["internal_bufsize"]) self._stderr = deque(maxlen=ca["internal_bufsize"]) - if ca["tty_in"] and not stdin_is_tty_or_pipe: + if ca["tty_in"] and not stdin_is_fd_based: setwinsize(self._stdin_parent_fd, ca["tty_size"]) @@ -2167,7 +2169,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, self.log.debug("started process") # disable echoing, but only if it's a tty that we created ourselves - if ca["tty_in"] and not stdin_is_tty_or_pipe: + if ca["tty_in"] and not stdin_is_fd_based: attr = termios.tcgetattr(self._stdin_parent_fd) attr[3] &= ~termios.ECHO termios.tcsetattr(self._stdin_parent_fd, termios.TCSANOW, attr) From 3e86bb06be578001ebe92734f561052b5c91bbbc Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 16:49:44 -0700 Subject: [PATCH 75/91] easy docker test suite shell --- docker_test_suite/shell.sh | 3 +++ 1 file changed, 3 insertions(+) create mode 100755 docker_test_suite/shell.sh diff --git a/docker_test_suite/shell.sh b/docker_test_suite/shell.sh new file mode 100755 index 00000000..234e8e0a --- /dev/null +++ b/docker_test_suite/shell.sh @@ -0,0 +1,3 @@ +#!/bin/bash +set -ex +docker run -it --rm -v $(pwd)/../:/home/shtest/sh --entrypoint=/bin/bash amoffat/shtest $@ From 99bd1bd2ca3fa30e015b34321652d9c272f2c80f Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 20:15:47 -0700 Subject: [PATCH 76/91] make _fg work with _cwd. closes #330 --- CHANGELOG.md | 2 ++ sh.py | 20 +++++++------------- test.py | 21 +++++++++++++++++++-- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27dffded..0ac38eb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ * bugfix `ErrorReturnCode` instances can now be pickled * bugfix passing empty string or `None` for `_in` hanged [#427](https://github.com/amoffat/sh/pull/427) * bugfix where passing a filename or file-like object to `_out` wasn't using os.dup2 [#449](https://github.com/amoffat/sh/issues/449) +* regression make `_fg` work with `_cwd` again [#330](https://github.com/amoffat/sh/issues/330) +* an invalid `_cwd` now raises a `ForkException` not an `OSError`. ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index f60710fe..e4a9641f 100644 --- a/sh.py +++ b/sh.py @@ -1063,7 +1063,7 @@ def fg_validator(passed_kwargs, merged_kwargs): _fg is invalid with nearly every other option, see warning and workaround here: https://amoffat.github.io/sh/sections/special_arguments.html#fg""" - whitelist = set(("env", "fg")) + whitelist = set(("env", "fg", "cwd")) offending = set(passed_kwargs.keys()) - whitelist if "fg" in passed_kwargs and offending: @@ -1476,12 +1476,16 @@ def __call__(self, *args, **kwargs): # if we're running in foreground mode, we need to completely bypass # launching a RunningCommand and OProc and just do a spawn if call_args["fg"]: + if call_args["env"] is None: launch = lambda: os.spawnv(os.P_WAIT, cmd[0], cmd) else: launch = lambda: os.spawnve(os.P_WAIT, cmd[0], cmd, call_args["env"]) - exit_code = launch() + cwd = call_args["cwd"] or os.getcwd() + with pushd(cwd): + exit_code = launch() + exc_class = get_exc_exit_code_would_raise(exit_code, call_args["ok_code"], call_args["piped"]) if exc_class: @@ -1914,17 +1918,6 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, if needs_ctty: self.ctty = os.ttyname(self._stdin_child_fd) - # this is a hack, but what we're doing here is intentionally throwing an - # OSError exception if our child processes's directory doesn't exist, - # but we're doing it BEFORE we fork. the reason for before the fork is - # error handling. i'm currently too lazy to implement what - # subprocess.py did and set up a error pipe to handle exceptions that - # happen in the child between fork and exec. it has only been seen in - # the wild for a missing cwd, so we'll handle it here. - cwd = ca["cwd"] - if cwd is not None and not os.path.exists(cwd): - os.chdir(cwd) - gc_enabled = gc.isenabled() if gc_enabled: gc.disable() @@ -2024,6 +2017,7 @@ def __init__(self, command, parent_log, cmd, stdin, stdout, stderr, os.close(session_pipe_read) os.close(exc_pipe_read) + cwd = ca["cwd"] if cwd: os.chdir(cwd) diff --git a/test.py b/test.py index cdd23183..aa6dfb17 100644 --- a/test.py +++ b/test.py @@ -1775,6 +1775,24 @@ def test_cwd(self): self.assertEqual(str(pwd(_cwd="/etc")), realpath("/etc") + "\n") + def test_cwd_fg(self): + td = tempfile.mkdtemp() + py = create_tmp_test(""" +import sh +import os +orig = os.getcwd() +print(orig) +sh.pwd(_cwd="{newdir}", _fg=True) +print(os.getcwd()) +""".format(newdir=td)) + + orig, newdir, restored = python(py.name).strip().split("\n") + self.assertEqual(newdir, td) + self.assertEqual(orig, restored) + self.assertNotEqual(orig, newdir) + os.rmdir(td) + + def test_huge_piped_data(self): from sh import tr @@ -2259,8 +2277,7 @@ def test_non_existant_cwd(self): # sanity check non_exist_dir = join(tempdir, "aowjgoahewro") self.assertFalse(exists(non_exist_dir)) - - self.assertRaises(OSError, ls, _cwd=non_exist_dir) + self.assertRaises(sh.ForkException, ls, _cwd=non_exist_dir) # https://github.com/amoffat/sh/issues/176 From 192d21311d15ed9ffe3556837520b0134b79625f Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sat, 25 Apr 2020 20:28:25 -0700 Subject: [PATCH 77/91] very basic AIX support. closes #477 --- CHANGELOG.md | 1 + sh.py | 2 +- test.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac38eb0..f7760317 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ * bugfix where passing a filename or file-like object to `_out` wasn't using os.dup2 [#449](https://github.com/amoffat/sh/issues/449) * regression make `_fg` work with `_cwd` again [#330](https://github.com/amoffat/sh/issues/330) * an invalid `_cwd` now raises a `ForkException` not an `OSError`. +* AIX support [#477](https://github.com/amoffat/sh/issues/477) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index e4a9641f..ee2ca736 100644 --- a/sh.py +++ b/sh.py @@ -94,7 +94,7 @@ def callable(ob): except ImportError: from pipes import quote as shlex_quote # undocumented before 2.7 -IS_MACOS = platform.system() == "Darwin" +IS_MACOS = platform.system() in ("AIX", "Darwin") THIS_DIR = os.path.dirname(os.path.realpath(__file__)) SH_LOGGER_NAME = __name__ diff --git a/test.py b/test.py index aa6dfb17..40bebffc 100644 --- a/test.py +++ b/test.py @@ -62,7 +62,7 @@ # we have to use the real path because on osx, /tmp is a symlink to # /private/tmp, and so assertions that gettempdir() == sh.pwd() will fail tempdir = realpath(tempfile.gettempdir()) -IS_MACOS = platform.system() == "Darwin" +IS_MACOS = platform.system() in ("AIX", "Darwin") # these 3 functions are helpers for modifying PYTHONPATH with a module's main From 11c6857cd6a42a908ddccf7a24f4159509cf94f3 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sun, 26 Apr 2020 06:24:25 -0700 Subject: [PATCH 78/91] Oops, fix logic mistake --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 30d16e2b..b6dff8c3 100644 --- a/sh.py +++ b/sh.py @@ -3550,7 +3550,7 @@ def __call__(self, **kwargs): # top stack frame # Older versions of pypy don't set parent[1] the same way as CPython or newer versions # of Pypy so we have to special case that too. - if (parent[1] in ('', '') or + if parent[1] in ('', '') or ( parent[1] == '' and platform.python_implementation().lower() == 'pypy'): # This depends on things like Python's calling convention and the layout of stack # frames but it's a fix for a bug in a very cornery cornercase so.... From 1141e59453f8c03e7b37e6ca3c0f84d6b0924409 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 12:11:18 -0700 Subject: [PATCH 79/91] added timeout=None param to RunningCommand.wait(), closes #515 this optional arg can also cause a `TimeoutException` to be raised on timeout. --- CHANGELOG.md | 1 + sh.py | 41 ++++++++++++++++++++++++++++++++++++++--- test.py | 18 ++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7760317..8361b42e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ * regression make `_fg` work with `_cwd` again [#330](https://github.com/amoffat/sh/issues/330) * an invalid `_cwd` now raises a `ForkException` not an `OSError`. * AIX support [#477](https://github.com/amoffat/sh/issues/477) +* added a `timeout=None` param to `RunningCommand.wait()` [#515](https://github.com/amoffat/sh/issues/515) ## 1.12.14 - 6/6/17 * bugfix for poor sleep performance [#378](https://github.com/amoffat/sh/issues/378) diff --git a/sh.py b/sh.py index 1e3d111e..e3b7a3a6 100644 --- a/sh.py +++ b/sh.py @@ -410,7 +410,7 @@ def __init__(self, full_cmd, stdout, stderr, truncate=True): class SignalException(ErrorReturnCode): pass class TimeoutException(Exception): """ the exception thrown when a command is killed because a specified - timeout (via _timeout) was hit """ + timeout (via _timeout or .wait(timeout)) was hit """ def __init__(self, exit_code, full_cmd): self.exit_code = exit_code self.full_cmd = full_cmd @@ -793,14 +793,49 @@ def __init__(self, cmd, call_args, stdin, stdout, stderr): self.wait() - def wait(self): + def wait(self, timeout=None): """ waits for the running command to finish. this is called on all running commands, eventually, except for ones that run in the background + + if timeout is a number, it is the number of seconds to wait for the process to resolve. otherwise block on wait. + + this function can raise a TimeoutException, either because of a `_timeout` on the command itself as it was + launched, or because of a timeout passed into this method. """ if not self._process_completed: self._process_completed = True - exit_code = self.process.wait() + # if we've been given a timeout, we need to poll is_alive() + if timeout is not None: + waited_for = 0 + sleep_amt = 0.1 + if timeout < 0: + raise RuntimeError("timeout cannot be negative") + + # while we still have time to wait, run this loop + # notice that alive and exit_code are only defined in this loop, but the loop is also guaranteed to run, + # defining them, given the constraints that timeout is non-negative + while waited_for <= timeout: + alive, exit_code = self.process.is_alive() + + # if we're alive, we need to wait some more, but let's sleep before we poll again + if alive: + time.sleep(sleep_amt) + waited_for += sleep_amt + + # but if we're not alive, we're done waiting + else: + break + + # if we've made it this far, and we're still alive, then it means we timed out waiting + if alive: + raise TimeoutException(None, self.ran) + + # if we didn't time out, we fall through and let the rest of the code handle exit_code + + else: + exit_code = self.process.wait() + if self.process.timed_out: # if we timed out, our exit code represents a signal, which is # negative, so let's make it positive to store in our diff --git a/test.py b/test.py index 40bebffc..0da79d4b 100644 --- a/test.py +++ b/test.py @@ -2002,6 +2002,24 @@ def test_timeout_overstep(self): self.assertTrue(abs(elapsed - 1) < 0.5) + def test_timeout_wait(self): + started = time.time() + p = sh.sleep(3, _bg=True) + self.assertRaises(sh.TimeoutException, p.wait, timeout=1) + + + def test_timeout_wait_overstep(self): + started = time.time() + p = sh.sleep(1, _bg=True) + p.wait(timeout=5) + + + def test_timeout_wait_negative(self): + started = time.time() + p = sh.sleep(3, _bg=True) + self.assertRaises(RuntimeError, p.wait, timeout=-3) + + def test_binary_pipe(self): binary = b'\xec;\xedr\xdbF' From 97f4f02228aae3fb7a9bcf0125d8c372402fc9aa Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 12:21:27 -0700 Subject: [PATCH 80/91] allow re-waiting when using .wait(timeout) this allow allows exceptions to propagate up if, for example, we wait with a timeout, catch the TimeoutException, kill(), then wait() without a timeout --- sh.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sh.py b/sh.py index e3b7a3a6..135c656d 100644 --- a/sh.py +++ b/sh.py @@ -717,7 +717,7 @@ def __init__(self, cmd, call_args, stdin, stdout, stderr): self.cmd = cmd self.process = None - self._process_completed = False + self._waited_until_completion = False should_wait = True spawn_process = True @@ -802,8 +802,7 @@ def wait(self, timeout=None): this function can raise a TimeoutException, either because of a `_timeout` on the command itself as it was launched, or because of a timeout passed into this method. """ - if not self._process_completed: - self._process_completed = True + if not self._waited_until_completion: # if we've been given a timeout, we need to poll is_alive() if timeout is not None: @@ -831,9 +830,13 @@ def wait(self, timeout=None): if alive: raise TimeoutException(None, self.ran) - # if we didn't time out, we fall through and let the rest of the code handle exit_code + # if we didn't time out, we fall through and let the rest of the code handle exit_code. + # notice that we set _waited_until_completion here, only if we didn't time out. this allows us to + # re-wait again on timeout, if we catch the TimeoutException in the parent frame + self._waited_until_completion = True else: + self._waited_until_completion = True exit_code = self.process.wait() if self.process.timed_out: From 57308ed76197d80ee32b455dd18ce4862d57009b Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 14:40:34 -0700 Subject: [PATCH 81/91] allow re-waiting after KeyboardInterrupting a .wait(). closes #407 --- sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sh.py b/sh.py index 135c656d..95b87f9e 100644 --- a/sh.py +++ b/sh.py @@ -836,8 +836,8 @@ def wait(self, timeout=None): self._waited_until_completion = True else: - self._waited_until_completion = True exit_code = self.process.wait() + self._waited_until_completion = True if self.process.timed_out: # if we timed out, our exit code represents a signal, which is From 79284d1589b3b52810e99e6f92f478ce6ea9bf32 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 15:12:44 -0700 Subject: [PATCH 82/91] quieter test --- test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.py b/test.py index 0da79d4b..bab4290a 100644 --- a/test.py +++ b/test.py @@ -956,7 +956,7 @@ def test_background(self): def test_background_exception(self): from sh import ls, ErrorReturnCode_1, ErrorReturnCode_2 - p = ls("/ofawjeofj", _bg=True) # should not raise + p = ls("/ofawjeofj", _bg=True, _bg_exc=False) # should not raise exc_to_test = ErrorReturnCode_2 if IS_MACOS: exc_to_test = ErrorReturnCode_1 From 6ecaba92640541c2ed56c1fed727374ad838770b Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 15:13:07 -0700 Subject: [PATCH 83/91] must canonicalize path on macos for tempfiles --- test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test.py b/test.py index bab4290a..c790409d 100644 --- a/test.py +++ b/test.py @@ -1776,17 +1776,19 @@ def test_cwd(self): def test_cwd_fg(self): - td = tempfile.mkdtemp() + td = realpath(tempfile.mkdtemp()) py = create_tmp_test(""" import sh import os -orig = os.getcwd() +from os.path import realpath +orig = realpath(os.getcwd()) print(orig) sh.pwd(_cwd="{newdir}", _fg=True) -print(os.getcwd()) +print(realpath(os.getcwd())) """.format(newdir=td)) orig, newdir, restored = python(py.name).strip().split("\n") + newdir = realpath(newdir) self.assertEqual(newdir, td) self.assertEqual(orig, restored) self.assertNotEqual(orig, newdir) From 59636d430550a13f61884e5a786bcbac52cf31a3 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 21:37:00 -0700 Subject: [PATCH 84/91] version bump to 1.13.0 --- CHANGELOG.md | 2 +- sh.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8361b42e..bdb86616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 1.13.0 - XX/XX/XX +## 1.13.0 - 4/27/20 * minor Travis CI fixes [#492](https://github.com/amoffat/sh/pull/492) * bugfix for boolean long options not respecting `_long_prefix` [#488](https://github.com/amoffat/sh/pull/488) * fix deprecation warning on Python 3.6 regexes [#482](https://github.com/amoffat/sh/pull/482) diff --git a/sh.py b/sh.py index 95b87f9e..a3758c1a 100644 --- a/sh.py +++ b/sh.py @@ -24,7 +24,7 @@ #=============================================================================== -__version__ = "1.12.14" +__version__ = "1.13.0" __project_url__ = "https://github.com/amoffat/sh" From 1affe0ce4a5121f00f18aa39f78122554aca51c2 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 21:53:03 -0700 Subject: [PATCH 85/91] github security warning. no need to pin this anyways --- requirements-dev.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 5277a92c..909e3e43 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,4 +3,3 @@ coverage==4.2 coveralls==1.1 docopt==0.6.2 docutils==0.12 -requests==2.12.1 From aff8b59c1083aa88ab0ce38eeb6ee200113a2da3 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 21:56:15 -0700 Subject: [PATCH 86/91] add monthly downloads badge --- README.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.rst b/README.rst index 1172b265..894e9b90 100644 --- a/README.rst +++ b/README.rst @@ -16,6 +16,9 @@ .. image:: https://img.shields.io/coveralls/amoffat/sh.svg?style=flat-square :target: https://coveralls.io/r/amoffat/sh?branch=master :alt: Coverage Status +.. image:: https://img.shields.io/pypi/dm/amoffat/sh.svg?style=flat-square + :target: https://pypi.python.org/pypi/sh + :alt: Downloads Status | From 71cdd931462e02231a5f5d79a95306958d0f420e Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 21:56:42 -0700 Subject: [PATCH 87/91] updating version range in readme --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 894e9b90..54918c6e 100644 --- a/README.rst +++ b/README.rst @@ -22,7 +22,7 @@ | -sh is a full-fledged subprocess replacement for Python 2.6 - 3.6, PyPy and PyPy3 +sh is a full-fledged subprocess replacement for Python 2.6 - 3.8, PyPy and PyPy3 that allows you to call *any* program as if it were a function: .. code:: python From 8101c53dede557fb5c3cb36f02a2da3474aa4dd3 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 22:01:38 -0700 Subject: [PATCH 88/91] updating setup.py --- setup.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/setup.py b/setup.py index 75d887f4..53d37a1d 100644 --- a/setup.py +++ b/setup.py @@ -23,6 +23,7 @@ def read(*parts): version=sh.__version__, description="Python subprocess replacement", long_description=read("README.rst"), + long_description_content_type="text/x-rst", author=author, author_email=author_email, maintainer=author, @@ -39,8 +40,13 @@ def read(*parts): "License :: OSI Approved :: MIT License", "Programming Language :: Python", "Programming Language :: Python :: 2", + "Programming Language :: Python :: 2.6", "Programming Language :: Python :: 2.7", "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.1", + "Programming Language :: Python :: 3.2", + "Programming Language :: Python :: 3.3", + "Programming Language :: Python :: 3.4", "Programming Language :: Python :: 3.5", "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", From 42444d090c0b405a08e3a82e97d9824c9274631d Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 22:04:57 -0700 Subject: [PATCH 89/91] updating email --- setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 53d37a1d..97cba90f 100644 --- a/setup.py +++ b/setup.py @@ -10,7 +10,7 @@ HERE = dirname(abspath(__file__)) author = "Andrew Moffat" -author_email = "andrew.robert.moffat@gmail.com" +author_email = "arwmoffat@gmail.com" keywords = ["subprocess", "process", "shell", "launch", "program"] @@ -23,7 +23,6 @@ def read(*parts): version=sh.__version__, description="Python subprocess replacement", long_description=read("README.rst"), - long_description_content_type="text/x-rst", author=author, author_email=author_email, maintainer=author, From 48554ced93a40cd130ee6f36f8b95104aaa01003 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 22:07:25 -0700 Subject: [PATCH 90/91] fixing badge --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 54918c6e..d199f1b2 100644 --- a/README.rst +++ b/README.rst @@ -16,7 +16,7 @@ .. image:: https://img.shields.io/coveralls/amoffat/sh.svg?style=flat-square :target: https://coveralls.io/r/amoffat/sh?branch=master :alt: Coverage Status -.. image:: https://img.shields.io/pypi/dm/amoffat/sh.svg?style=flat-square +.. image:: https://img.shields.io/pypi/dm/sh.svg?style=flat-square :target: https://pypi.python.org/pypi/sh :alt: Downloads Status From 00873f8858cce385be067df3b7d73e3e787a49e1 Mon Sep 17 00:00:00 2001 From: Andrew Moffat Date: Sun, 26 Apr 2020 22:08:16 -0700 Subject: [PATCH 91/91] vainly shuffling around badges --- README.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.rst b/README.rst index d199f1b2..e583178b 100644 --- a/README.rst +++ b/README.rst @@ -7,6 +7,9 @@ .. image:: https://img.shields.io/pypi/v/sh.svg?style=flat-square :target: https://pypi.python.org/pypi/sh :alt: Version +.. image:: https://img.shields.io/pypi/dm/sh.svg?style=flat-square + :target: https://pypi.python.org/pypi/sh + :alt: Downloads Status .. image:: https://img.shields.io/pypi/pyversions/sh.svg?style=flat-square :target: https://pypi.python.org/pypi/sh :alt: Python Versions @@ -16,9 +19,6 @@ .. image:: https://img.shields.io/coveralls/amoffat/sh.svg?style=flat-square :target: https://coveralls.io/r/amoffat/sh?branch=master :alt: Coverage Status -.. image:: https://img.shields.io/pypi/dm/sh.svg?style=flat-square - :target: https://pypi.python.org/pypi/sh - :alt: Downloads Status |