Avoid shell-escaping of env vars (#283)

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):

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

rebench/tests/bugs/env_quote_test.py

@@ -0,0 +1,46 @@
+# Copyright (c) 2009-2025 Stefan Marr <https://www.stefan-marr.de/>
+#
+# 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)

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)

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()