From 1b9ab12f01e94697de02fbe534ab25a1bfd0b61f Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 12 Oct 2021 11:48:34 +0200 Subject: [PATCH 1/6] add which function to vsc.utils.py2vs3 --- lib/vsc/utils/py2vs3/py2.py | 25 +++++++++++++++++++++++++ lib/vsc/utils/py2vs3/py3.py | 1 + test/py2vs3.py | 31 ++++++++++++++++++++++++++++--- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/lib/vsc/utils/py2vs3/py2.py b/lib/vsc/utils/py2vs3/py2.py index 97060d34..cc36d8cf 100644 --- a/lib/vsc/utils/py2vs3/py2.py +++ b/lib/vsc/utils/py2vs3/py2.py @@ -133,3 +133,28 @@ def _rmtree(self, path): self._rmdir(path) except OSError: pass + + +def which(cmd, mode=os.F_OK | os.X_OK, path=None): + """ + Return the first absolute path that matches the given command. + + Searches via specified path, or $PATH as fallback. + + Returns absolute path to command if found, None if not. + """ + res = None + if os.path.isabs(cmd): + if os.access(cmd, cmd): + res = cmd + else: + if path is None: + path = os.environ.get('PATH', '') + + for cand_path in path.split(os.pathsep): + cmd_path = os.path.join(cand_path, cmd) + if os.access(cmd_path, mode): + res = cmd_path + break + + return res diff --git a/lib/vsc/utils/py2vs3/py3.py b/lib/vsc/utils/py2vs3/py3.py index 8609b334..ca1268b6 100644 --- a/lib/vsc/utils/py2vs3/py3.py +++ b/lib/vsc/utils/py2vs3/py3.py @@ -32,6 +32,7 @@ import pickle # noqa from io import StringIO # noqa from shlex import quote # noqa +from shutil import which # noqa from tempfile import TemporaryDirectory # noqa from urllib.parse import urlencode, unquote # noqa from urllib.request import HTTPError, HTTPSHandler, Request, build_opener, urlopen # noqa diff --git a/test/py2vs3.py b/test/py2vs3.py index b1bb0509..7238be78 100644 --- a/test/py2vs3.py +++ b/test/py2vs3.py @@ -30,9 +30,11 @@ @author: Kenneth Hoste (Ghent University) """ import os +import stat import sys -from vsc.utils.py2vs3 import ensure_ascii_string, is_py_ver, is_py2, is_py3, is_string, pickle, TemporaryDirectory +from vsc.utils.py2vs3 import ensure_ascii_string, is_py_ver, is_py2, is_py3, is_string, pickle, which +from vsc.utils.py2vs3 import TemporaryDirectory from vsc.install.testing import TestCase @@ -143,12 +145,11 @@ def test_ensure_ascii_string(self): def test_urllib_imports(self): """Test importing urllib* stuff from py2vs3.""" - from vsc.utils.py2vs3 import HTTPError, HTTPSHandler, Request, build_opener, unquote, urlencode, urlopen + from vsc.utils.py2vs3 import HTTPError, HTTPSHandler, Request, build_opener, unquote, urlencode, urlopen # noqa def test_temporary_directory(self): """Test the class TemporaryDirectory.""" with TemporaryDirectory() as temp_dir: - path = temp_dir self.assertTrue(os.path.exists(temp_dir), 'Directory created by TemporaryDirectory should work') self.assertTrue(os.path.isdir(temp_dir), 'Directory created by TemporaryDirectory should be a directory') self.assertFalse(os.path.exists(temp_dir), 'Directory created by TemporaryDirectory should cleanup automagically') @@ -164,3 +165,27 @@ def test_os_exceptions(self): f.write("abc") self.assertRaises(FileExistsErrorExc, lambda: os.open(afile, os.O_CREAT | os.O_WRONLY | os.O_EXCL)) + + def test_which(self): + """Test which function.""" + + self.assertTrue(which('ls')) + self.assertEqual(which('nosuchcommand'), None) + + # create test to test with + test_cmd = os.path.join(self.tmpdir, 'test123') + with open(test_cmd, 'w') as fp: + fp.write("echo 123") + + # missing exec permission + self.assertEqual(which('test123'), None) + self.assertEqual(which('test123', mode=os.R_OK, path=self.tmpdir), test_cmd) + + os.chmod(test_cmd, stat.S_IRUSR | stat.S_IXUSR) + + # not available via $PATH + self.assertEqual(which('test123'), None) + self.assertEqual(which('test123', path=self.tmpdir), test_cmd) + + os.environ['PATH'] = self.tmpdir + os.pathsep + os.getenv('PATH') + self.assertEqual(which('test123'), test_cmd) From d05d1ed1a0ddf9ffb4a8129aace3500780fd474e Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 12 Oct 2021 12:04:37 +0200 Subject: [PATCH 2/6] print warning and resolve to absolute path for commands specified with relative path --- lib/vsc/utils/run.py | 36 +++++++++++++++++++++++++++++-- test/run.py | 50 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/lib/vsc/utils/run.py b/lib/vsc/utils/run.py index eee8f716..c48e202f 100644 --- a/lib/vsc/utils/run.py +++ b/lib/vsc/utils/run.py @@ -74,7 +74,7 @@ import time from vsc.utils.fancylogger import getLogger -from vsc.utils.py2vs3 import ensure_ascii_string, is_py3, is_string +from vsc.utils.py2vs3 import ensure_ascii_string, is_py3, is_string, which PROCESS_MODULE_ASYNCPROCESS_PATH = 'vsc.utils.asyncprocess' PROCESS_MODULE_SUBPROCESS_PATH = 'subprocess' @@ -87,6 +87,38 @@ SHELL = BASH +def ensure_cmd_abs_path(cmd): + """Make sure that command is specified via an absolute path.""" + if not cmd: + raise ValueError("Empty command specified!") + if is_string(cmd): + cmd_path = cmd.split(' ')[0] + elif isinstance(cmd, (list, tuple,)): + cmd_path = cmd[0] + else: + raise ValueError("Unknown type of command: %s (type %s)" % (cmd, type(cmd))) + + if not os.path.isabs(cmd_path): + sys.stderr.write("WARNING: Command to run is specified via relative path: %s" % cmd_path) + + # resolve to absolute path via $PATH + cmd_abs_path = which(cmd_path) + if cmd_abs_path is None: + raise OSError("Command %s not found in $PATH!" % cmd_path) + + # re-assemble command with absolute path + if is_string(cmd): + cmd = ' '.join([cmd_abs_path] + cmd.split(' ')[1:]) + elif isinstance(cmd, list): + cmd[0] = cmd_abs_path + elif isinstance(cmd, tuple): + cmd = tuple([cmd_abs_path] + list(cmd[1:])) + else: + raise ValueError("Unknown type of command: %s (type %s)" % (cmd, type(cmd))) + + return cmd + + class CmdList(list): """Wrapper for 'list' type to be used for constructing a list of options & arguments for a command.""" @@ -172,7 +204,7 @@ def __init__(self, cmd=None, **kwargs): if not hasattr(self, 'log'): self.log = getLogger(self._get_log_name()) - self.cmd = cmd # actual command + self.cmd = ensure_cmd_abs_path(cmd) # actual command self._cwd_before_startpath = None diff --git a/test/run.py b/test/run.py index d0d344be..1fbe22d8 100644 --- a/test/run.py +++ b/test/run.py @@ -38,16 +38,16 @@ import shutil # Uncomment when debugging, cannot enable permanetnly, messes up tests that toggle debugging -#logging.basicConfig(level=logging.DEBUG) +# logging.basicConfig(level=logging.DEBUG) from vsc.utils.missing import shell_quote from vsc.utils.run import ( CmdList, run, run_simple, asyncloop, run_asyncloop, run_timeout, RunTimeout, RunQA, RunNoShellQA, - async_to_stdout, run_async_to_stdout, + async_to_stdout, ensure_cmd_abs_path, run_async_to_stdout, ) -from vsc.utils.py2vs3 import StringIO, is_py2, is_py3, is_string +from vsc.utils.py2vs3 import is_py2, is_py3, is_string from vsc.utils.run import RUNRUN_TIMEOUT_OUTPUT, RUNRUN_TIMEOUT_EXITCODE, RUNRUN_QA_MAX_MISS_EXITCODE from vsc.install.testing import TestCase @@ -58,14 +58,17 @@ SCRIPT_NESTED = os.path.join(SCRIPTS_DIR, 'run_nested.sh') -TEST_GLOB = ['ls','test/sandbox/testpkg/*'] +TEST_GLOB = ['ls', 'test/sandbox/testpkg/*'] + class RunQAShort(RunNoShellQA): LOOP_MAX_MISS_COUNT = 3 # approx 3 sec + class RunLegQAShort(RunQA): LOOP_MAX_MISS_COUNT = 3 # approx 3 sec + run_qas = RunQAShort.run run_legacy_qas = RunLegQAShort.run @@ -355,13 +358,14 @@ def test_qa_list_of_answers(self): qa_reg_dict = { "Enter a number \(.*\):": ['2', '3', '5', '0'] + ['100'] * 100, } - ec, output = run_qas([sys.executable, SCRIPT_QA, 'ask_number', '100'], qa_reg=qa_reg_dict) + cmd = [sys.executable, SCRIPT_QA, 'ask_number', '100'] + ec, output = run_qas(cmd, qa_reg=qa_reg_dict) self.assertEqual(ec, 0) answer_re = re.compile(".*Answer: 10$") self.assertTrue(answer_re.match(output), "'%s' matches pattern '%s'" % (output, answer_re.pattern)) # verify type checking on answers - self.assertErrorRegex(TypeError, "Invalid type for answer", run_qas, [], qa={'q': 1}) + self.assertErrorRegex(TypeError, "Invalid type for answer", run_qas, cmd, qa={'q': 1}) # test more questions than answers, both with and without cycling qa_reg_dict = { @@ -465,6 +469,38 @@ def test_cmdlist(self): self.assertErrorRegex(ValueError, "Found one or more spaces", CmdList, 'this has spaces', allow_spaces=False) def test_env(self): - ec, output = run(cmd="/usr/bin/env", env = {"MYENVVAR": "something"}) + ec, output = run(cmd="/usr/bin/env", env={"MYENVVAR": "something"}) self.assertEqual(ec, 0) self.assertTrue('myenvvar=something' in output.lower()) + + def test_ensure_cmd_abs_path(self): + """Test ensure_cmd_abs_path function.""" + ls = ensure_cmd_abs_path('ls') + self.assertTrue(os.path.isabs(ls) and os.path.exists(ls)) + + cmd = ensure_cmd_abs_path('ls foo') + self.assertTrue(cmd.endswith('/ls foo')) + cmd_path = cmd.split(' ')[0] + self.assertTrue(os.path.isabs(cmd_path) and os.path.exists(cmd_path)) + + for input_cmd in (['ls', 'foo'], ('ls', 'foo')): + cmd = ensure_cmd_abs_path(input_cmd) + self.assertTrue(isinstance(cmd, type(input_cmd))) + self.assertEqual(len(cmd), 2) + self.assertTrue(os.path.isabs(cmd[0]) and os.path.exists(cmd[0])) + self.assertEqual(cmd[1], 'foo') + + # command is left untouched if it already uses an absolute path + for cmd in ('/bin/nosuchcommand', ['/foo'], ('/bin/ls', 'foo')): + self.assertEqual(ensure_cmd_abs_path(cmd), cmd) + + # check handling of faulty input + error_msg = r"Command nosuchcommand not found in \$PATH!" + self.assertErrorRegex(OSError, error_msg, ensure_cmd_abs_path, 'nosuchcommand') + + error_msg = "Empty command specified!" + self.assertErrorRegex(ValueError, error_msg, ensure_cmd_abs_path, []) + self.assertErrorRegex(ValueError, error_msg, ensure_cmd_abs_path, ()) + + error_msg = "Unknown type of command: 1" + self.assertErrorRegex(ValueError, error_msg, ensure_cmd_abs_path, 1) From 32dbd349a84e11532b13e3bb814cd6f7f39ebb34 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 12 Oct 2021 12:10:14 +0200 Subject: [PATCH 3/6] also check for warning produced by ensure_cmd_abs_path --- test/run.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/test/run.py b/test/run.py index 1fbe22d8..e6ea04c6 100644 --- a/test/run.py +++ b/test/run.py @@ -475,16 +475,34 @@ def test_env(self): def test_ensure_cmd_abs_path(self): """Test ensure_cmd_abs_path function.""" - ls = ensure_cmd_abs_path('ls') + + def check_warning(cmd): + """Pass cmd to ensure_cmd_abs_path, and check for warning being printed.""" + if isinstance(cmd, (list, tuple)): + cmd_name = cmd[0] + else: + cmd_name = cmd.split(' ')[0] + + self.mock_stderr(True) + self.mock_stdout(True) + res = ensure_cmd_abs_path(cmd) + stderr, stdout = self.get_stderr(), self.get_stdout() + self.mock_stderr(False) + self.mock_stdout(False) + self.assertFalse(stdout) + self.assertEqual(stderr, "WARNING: Command to run is specified via relative path: %s" % cmd_name) + return res + + ls = check_warning('ls') self.assertTrue(os.path.isabs(ls) and os.path.exists(ls)) - cmd = ensure_cmd_abs_path('ls foo') + cmd = check_warning('ls foo') self.assertTrue(cmd.endswith('/ls foo')) cmd_path = cmd.split(' ')[0] self.assertTrue(os.path.isabs(cmd_path) and os.path.exists(cmd_path)) for input_cmd in (['ls', 'foo'], ('ls', 'foo')): - cmd = ensure_cmd_abs_path(input_cmd) + cmd = check_warning(input_cmd) self.assertTrue(isinstance(cmd, type(input_cmd))) self.assertEqual(len(cmd), 2) self.assertTrue(os.path.isabs(cmd[0]) and os.path.exists(cmd[0])) From 4b34d056eddf3864fd3d0ce2eb60b3de6c3f0229 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 12 Oct 2021 12:10:30 +0200 Subject: [PATCH 4/6] bump version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 18a1fcf7..6f4c842d 100755 --- a/setup.py +++ b/setup.py @@ -44,7 +44,7 @@ ] PACKAGE = { - 'version': '3.4.0', + 'version': '3.4.1', 'author': [sdw, jt, ag, kh], 'maintainer': [sdw, jt, ag, kh], # as long as 1.0.0 is not out, vsc-base should still provide vsc.fancylogger From 730192d9a414ca5dc6b35e264bdc6918259d6b09 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 12 Oct 2021 15:21:23 +0200 Subject: [PATCH 5/6] emit warning via logging.warning in ensure_cmd_abs_path --- lib/vsc/utils/run.py | 2 +- test/run.py | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/vsc/utils/run.py b/lib/vsc/utils/run.py index c48e202f..982cb380 100644 --- a/lib/vsc/utils/run.py +++ b/lib/vsc/utils/run.py @@ -99,7 +99,7 @@ def ensure_cmd_abs_path(cmd): raise ValueError("Unknown type of command: %s (type %s)" % (cmd, type(cmd))) if not os.path.isabs(cmd_path): - sys.stderr.write("WARNING: Command to run is specified via relative path: %s" % cmd_path) + logging.warning("Command to run is specified via relative path: %s" % cmd_path) # resolve to absolute path via $PATH cmd_abs_path = which(cmd_path) diff --git a/test/run.py b/test/run.py index e6ea04c6..83ba1069 100644 --- a/test/run.py +++ b/test/run.py @@ -30,6 +30,7 @@ @author: Stijn De Weirdt (Ghent University) @author: Kenneth Hoste (Ghent University) """ +import logging import os import re import sys @@ -476,21 +477,25 @@ def test_env(self): def test_ensure_cmd_abs_path(self): """Test ensure_cmd_abs_path function.""" + logger = logging.getLogger() + log_path = os.path.join(self.tmpdir, 'log.txt') + filelogger = logging.FileHandler(log_path) + logger.addHandler(filelogger) + def check_warning(cmd): - """Pass cmd to ensure_cmd_abs_path, and check for warning being printed.""" + """Pass cmd to ensure_cmd_abs_path, and check for warning being printed (through logger).""" if isinstance(cmd, (list, tuple)): cmd_name = cmd[0] else: cmd_name = cmd.split(' ')[0] - self.mock_stderr(True) - self.mock_stdout(True) res = ensure_cmd_abs_path(cmd) - stderr, stdout = self.get_stderr(), self.get_stdout() - self.mock_stderr(False) - self.mock_stdout(False) - self.assertFalse(stdout) - self.assertEqual(stderr, "WARNING: Command to run is specified via relative path: %s" % cmd_name) + + # check for emitted log message (last line of log file) + with open(log_path, 'r') as fh: + logline = fh.readlines()[-1].strip() + self.assertEqual(logline, "Command to run is specified via relative path: %s" % cmd_name) + return res ls = check_warning('ls') @@ -522,3 +527,6 @@ def check_warning(cmd): error_msg = "Unknown type of command: 1" self.assertErrorRegex(ValueError, error_msg, ensure_cmd_abs_path, 1) + + logger.removeHandler(filelogger) + filelogger.close() From 919b9ec0fa08350b53cbd026f73fbf67768ffb21 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Tue, 12 Oct 2021 15:35:25 +0200 Subject: [PATCH 6/6] add support for specifying custom search path for commands --- lib/vsc/utils/run.py | 15 +++++++++++---- test/run.py | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/vsc/utils/run.py b/lib/vsc/utils/run.py index 982cb380..1c8489eb 100644 --- a/lib/vsc/utils/run.py +++ b/lib/vsc/utils/run.py @@ -87,8 +87,12 @@ SHELL = BASH -def ensure_cmd_abs_path(cmd): - """Make sure that command is specified via an absolute path.""" +def ensure_cmd_abs_path(cmd, path=None): + """ + Make sure that command is specified via an absolute path. + + :param path: colon-separated string with list of paths to use for searching (as alternative to $PATH) + """ if not cmd: raise ValueError("Empty command specified!") if is_string(cmd): @@ -102,7 +106,7 @@ def ensure_cmd_abs_path(cmd): logging.warning("Command to run is specified via relative path: %s" % cmd_path) # resolve to absolute path via $PATH - cmd_abs_path = which(cmd_path) + cmd_abs_path = which(cmd_path, path=path) if cmd_abs_path is None: raise OSError("Command %s not found in $PATH!" % cmd_path) @@ -192,19 +196,22 @@ def __init__(self, cmd=None, **kwargs): @param use_shell: use the subshell @param shell: change the shell @param env: environment settings to pass on + @param command_path: colon-separated string of paths to use for searching for command + (only used if command name is specified a relative path) """ self.input = kwargs.pop('input', None) self.startpath = kwargs.pop('startpath', None) self.use_shell = kwargs.pop('use_shell', self.USE_SHELL) self.shell = kwargs.pop('shell', self.SHELL) self.env = kwargs.pop('env', None) + command_path = kwargs.pop('command_path', None) if kwargs.pop('disable_log', None): self.log = DummyFunction() # No logging if not hasattr(self, 'log'): self.log = getLogger(self._get_log_name()) - self.cmd = ensure_cmd_abs_path(cmd) # actual command + self.cmd = ensure_cmd_abs_path(cmd, path=command_path) # actual command self._cwd_before_startpath = None diff --git a/test/run.py b/test/run.py index 83ba1069..1119fc5d 100644 --- a/test/run.py +++ b/test/run.py @@ -33,6 +33,7 @@ import logging import os import re +import stat import sys import tempfile import time @@ -46,7 +47,7 @@ CmdList, run, run_simple, asyncloop, run_asyncloop, run_timeout, RunTimeout, RunQA, RunNoShellQA, - async_to_stdout, ensure_cmd_abs_path, run_async_to_stdout, + async_to_stdout, ensure_cmd_abs_path, run_async_to_stdout, run ) from vsc.utils.py2vs3 import is_py2, is_py3, is_string from vsc.utils.run import RUNRUN_TIMEOUT_OUTPUT, RUNRUN_TIMEOUT_EXITCODE, RUNRUN_QA_MAX_MISS_EXITCODE @@ -96,6 +97,20 @@ def test_simple(self): self.assertEqual(ec, 0) self.assertTrue('shortsleep' in output.lower()) + def test_run_path(self): + """Test use of run function when specifying custom path to use for resolving relative command name.""" + + test_cmd = os.path.join(self.tmpdir, 'test123') + with open(test_cmd, 'w') as fp: + fp.write("#!/bin/bash\necho 123\n") + os.chmod(test_cmd, stat.S_IRUSR | stat.S_IXUSR) + + self.assertErrorRegex(OSError, r"Command test123 not found in \$PATH!", run, 'test123') + + ec, out = run('test123', command_path=self.tmpdir + os.pathsep + '/usr/bin') + self.assertEqual(ec, 0) + self.assertEqual(out, '123\n') + def test_startpath(self): cwd = os.getcwd()