diff --git a/rebench/model/run_id.py b/rebench/model/run_id.py index 12d996f5..afd1549a 100644 --- a/rebench/model/run_id.py +++ b/rebench/model/run_id.py @@ -35,6 +35,28 @@ from ..statistics import WithSamples +def expand_user(possible_path, shell_escape): + something_changed = False + + # split will change the type of quotes, which may cause issues with shell variables + parts = shlex.split(possible_path) + for i, part in enumerate(parts): + expanded = os.path.expanduser(part) + if "~" in expanded and ":" in expanded: + path_list = expanded.split(":") + expanded = ":".join([os.path.expanduser(p) for p in path_list]) + if parts[i] != expanded: + something_changed = True + parts[i] = expanded + + if something_changed: + if shell_escape: + return shlex.join(parts) + return ' '.join(parts) + + return possible_path + + class RunId(object): """ A RunId is a concrete instantiation of the possible combinations of @@ -111,7 +133,7 @@ def env(self): self._expandend_env = self.benchmark.run_details.env for key, value in self._expandend_env.items(): - self._expandend_env[key] = self._expand_user(value) + self._expandend_env[key] = expand_user(value, False) return self._expandend_env @property @@ -312,21 +334,10 @@ def cmdline(self): return self._cmdline return self._construct_cmdline() - def _expand_user(self, possible_path): - # split will change the type of quotes, which may cause issues with shell variables - parts = shlex.split(possible_path) - for i, part in enumerate(parts): - expanded = os.path.expanduser(part) - if "~" in expanded and ":" in expanded: - path_list = expanded.split(":") - expanded = ":".join([os.path.expanduser(p) for p in path_list]) - parts[i] = expanded - return shlex.join(parts) - def cmdline_for_next_invocation(self): """Replace the invocation number in the command line""" cmdline = self.cmdline() % {"invocation": self.completed_invocations + 1} - cmdline = self._expand_user(cmdline) + cmdline = expand_user(cmdline, True) return cmdline def _construct_cmdline(self): diff --git a/rebench/tests/bugs/env_quote.conf b/rebench/tests/bugs/env_quote.conf new file mode 100644 index 00000000..fad780f1 --- /dev/null +++ b/rebench/tests/bugs/env_quote.conf @@ -0,0 +1,22 @@ +default_experiment: Test + +benchmark_suites: + Suite: + gauge_adapter: Time + command: TestBenchMarks %(benchmark)s %(warmup)s + benchmarks: + - Bench1 + env: + LUA_PATH: "?.lua;../../awfy/Lua/?.lua" + +executors: + TestRunner1: + path: . + executable: env_quote_vm.py + +experiments: + Test: + suites: + - Suite + executions: + - TestRunner1 diff --git a/rebench/tests/bugs/env_quote_test.py b/rebench/tests/bugs/env_quote_test.py new file mode 100644 index 00000000..f633bc7d --- /dev/null +++ b/rebench/tests/bugs/env_quote_test.py @@ -0,0 +1,46 @@ +# Copyright (c) 2009-2025 Stefan Marr +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. +from ...configurator import Configurator, load_config +from ...executor import Executor +from ...persistence import DataStore + +from ..rebench_test_case import ReBenchTestCase + + +class EnvQuoteTest(ReBenchTestCase): + + def setUp(self): + super(EnvQuoteTest, self).setUp() + self._set_path(__file__) + + def test_execution_should_recognize_invalid_run_and_continue_normally(self): + cnf = Configurator( + load_config(self._path + "/env_quote.conf"), + DataStore(self.ui), + self.ui, + data_file=self._tmp_file, + ) + runs = list(cnf.get_runs()) + self.assertEqual(runs[0].get_number_of_data_points(), 0) + + ex = Executor([runs[0]], False, self.ui) + ex.execute() + + self.assertEqual(runs[0].get_number_of_data_points(), 1) diff --git a/rebench/tests/bugs/env_quote_vm.py b/rebench/tests/bugs/env_quote_vm.py new file mode 100755 index 00000000..53f60e4a --- /dev/null +++ b/rebench/tests/bugs/env_quote_vm.py @@ -0,0 +1,13 @@ +#!/usr/bin/env python3 +import os +import sys + +# get the environemnt variable LUA_PATH +lua_path = os.environ.get("LUA_PATH", "") +if lua_path == "?.lua;../../awfy/Lua/?.lua": + print("Correct") + sys.exit(0) +else: + print("Error: LUA_PATH has unexpected value: " + lua_path) + print("Previously we has stray single quotes around the value.") + sys.exit(1) diff --git a/rebench/tests/model/run_id_test.py b/rebench/tests/model/run_id_test.py index e205c102..f582561a 100644 --- a/rebench/tests/model/run_id_test.py +++ b/rebench/tests/model/run_id_test.py @@ -1,8 +1,42 @@ +from os.path import expanduser +import pytest + from ...configurator import Configurator, load_config +from ...model.run_id import expand_user from ...persistence import DataStore from ..rebench_test_case import ReBenchTestCase +def _simple_expand(path): + return path.replace("~", expanduser("~")) + + +def _expand(paths): + return [(p, _simple_expand(p)) for p in paths] + + +@pytest.mark.parametrize( + "possible_path, after_expansion", + _expand( + ["~/foo/bar", "~/foo ~/bar -cp ~/here:~/there", "?.lua;../../awfy/Lua/?.lua"] + ), +) +def test_expand_user_no_escape(possible_path, after_expansion): + expanded = expand_user(possible_path, False) + assert expanded == after_expansion + + +@pytest.mark.parametrize( + "possible_path, after_expansion", + _expand( + ["~/foo/bar", "~/foo ~/bar -cp ~/here:~/there", "'?.lua;../../awfy/Lua/?.lua'"] + ), +) +def test_expand_user_with_escape(possible_path, after_expansion): + expanded = expand_user(possible_path, True) + assert expanded == after_expansion + + class RunIdTest(ReBenchTestCase): def setUp(self): super(RunIdTest, self).setUp()