Repository: smarr/ReBench

Issue:
The ~ in a path should be replaced by the home directory
Currently, I think we can't use `~` in a path, as one normally can in a shell.
This means many paths need to be fully qualified, which binds a conf file to a very specific setup.

The problem is, we don't want expand the `~` too early and in the wrong place either, because then we end up with `RunId` objects not being the same anymore for the same path.
So, we could not compare across different machines. Generally, comparing between machines is done very rarely because it's not generally useful, but sometimes, we are indeed interested in the machines.

So, a good point for the expansion might be here:

https://github.com/smarr/ReBench/blob/1de0e22fb3e9578b3aeb7a994481ead300fa977f/rebench/model/run_id.py#L279

This bit was added allow iterations to be part of the command line, also with the goal to avoid breaking the `RunId` identity: https://github.com/smarr/ReBench/issues/216

Discussion:
@smarr: Resolved with #240.

Solution patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f75c19b..4959dd9 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -8,9 +8,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.7", "3.12", "pypy3.10"]
+        python-version: ["3.8", "3.12", "pypy3.10"]
         include:
-          - python-version: 3.7
+          - python-version: 3.8
             coverage: "--cov=rebench"
     name: "Ubuntu-latest: Python ${{ matrix.python-version }}"
     steps:
diff --git a/rebench/executor.py b/rebench/executor.py
index bd8461b..3b58d5a 100644
--- a/rebench/executor.py
+++ b/rebench/executor.py
@@ -523,8 +523,12 @@ class Executor(object):
                 self.ui.warning(
                     "Keep alive, current job runs for %dmin\n" % (seconds / 60), run_id, cmdline)
 
+            location = run_id.location
+            if location:
+                location = os.path.expanduser(location)
+
             (return_code, output, _) = subprocess_timeout.run(
-                cmdline, env=run_id.env, cwd=run_id.location, stdout=subprocess.PIPE,
+                cmdline, env=run_id.env, cwd=location, stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT, shell=True, verbose=self.debug,
                 timeout=run_id.max_invocation_time,
                 keep_alive_output=_keep_alive,
diff --git a/rebench/model/benchmark_suite.py b/rebench/model/benchmark_suite.py
index 081274f..939606a 100644
--- a/rebench/model/benchmark_suite.py
+++ b/rebench/model/benchmark_suite.py
@@ -32,7 +32,7 @@ class BenchmarkSuite(object):
         command = suite.get('command')
 
         location = suite.get('location', executor.path)
-        if location:
+        if location and not location.startswith('~'):
             location = os.path.abspath(location)
         build = BuildCommand.create_commands(suite.get('build'), build_commands, location)
         benchmarks_config = suite.get('benchmarks')
diff --git a/rebench/model/executor.py b/rebench/model/executor.py
index e2d9c80..f03794e 100644
--- a/rebench/model/executor.py
+++ b/rebench/model/executor.py
@@ -31,7 +31,7 @@ class Executor(object):
     @classmethod
     def compile(cls, executor_name, executor, run_details, variables, build_commands, action):
         path = executor.get('path')
-        if path:
+        if path and not path.startswith('~'):
             path = os.path.abspath(path)
         executable = executor.get('executable')
         args = executor.get('args')
diff --git a/rebench/model/run_id.py b/rebench/model/run_id.py
index b880f14..726d63a 100644
--- a/rebench/model/run_id.py
+++ b/rebench/model/run_id.py
@@ -17,7 +17,9 @@
 # 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.
+import os
 import re
+import shlex
 
 from .benchmark import Benchmark
 from .termination_check import TerminationCheck
@@ -278,7 +280,13 @@ class RunId(object):
 
     def cmdline_for_next_invocation(self):
         """Replace the invocation number in the command line"""
-        return self.cmdline() % {'invocation': self.completed_invocations + 1}
+        cmdline = self.cmdline() % {'invocation': self.completed_invocations + 1}
+        # split will change the type of quotes, which may cause issues with shell variables
+        parts = shlex.split(cmdline)
+        for i, part in enumerate(parts):
+            parts[i] = os.path.expanduser(part)
+        cmdline = shlex.join(parts)
+        return cmdline
 
     def _construct_cmdline(self):
         cmdline = ""


Test patch:
diff --git a/rebench/tests/model/runs_config_test.py b/rebench/tests/model/runs_config_test.py
index 68a405e..1765af4 100644
--- a/rebench/tests/model/runs_config_test.py
+++ b/rebench/tests/model/runs_config_test.py
@@ -17,6 +17,7 @@
 # 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 os.path import expanduser
 from ...model.data_point import DataPoint
 from ...model.measurement import Measurement
 from ...model.termination_check import TerminationCheck
@@ -79,3 +80,17 @@ class RunsConfigTestCase(ReBenchTestCase):
 
         check.indicate_failed_execution()
         self.assertTrue(check.should_terminate(0, None))
+
+    def test_command_user_path_expansion(self):
+        self._cnf = Configurator(
+            load_config(self._path + '/small_expand_user.conf'), DataStore(self.ui),
+            self.ui, None, data_file=self._tmp_file)
+        runs = self._cnf.get_runs()
+        sorted_runs = sorted(list(runs), key=lambda r: r.cmdline())
+        self._run = sorted_runs[0]
+
+        expected_cmd = '~/tests/vm1.py 1 Bench ~/suiteFolder/Bench1'.replace('~', expanduser('~'))
+        self.assertNotIn("~", expected_cmd)
+        expected_cmd += " -message 'a string with a ~'"
+        next_invocation_cmdline = self._run.cmdline_for_next_invocation()
+        self.assertEqual(expected_cmd, next_invocation_cmdline)
diff --git a/rebench/tests/small_expand_user.conf b/rebench/tests/small_expand_user.conf
new file mode 100644
index 0000000..50ba72f
--- /dev/null
+++ b/rebench/tests/small_expand_user.conf
@@ -0,0 +1,32 @@
+# Config file for ReBench
+# Config format is YAML (see http://yaml.org/ for detailed spec)
+
+# this run definition will be chosen if no parameters are given to rebench.py
+default_experiment: Test
+default_data_file:  'small.data'
+
+# general configuration for runs
+runs:
+    invocations:  10
+    retries_after_failure: 3
+
+benchmark_suites:
+    Suite:
+        gauge_adapter: TestExecutor
+        command: Bench ~/suiteFolder/%(benchmark)s -message "a string with a ~"
+        benchmarks:
+            - Bench1
+            - Bench2
+
+executors:
+    TestRunner1:
+        path: ~/tests
+        executable: vm1.py %(cores)s
+        cores: [1]
+
+experiments:
+    Test:
+        suites:
+            - Suite
+        executions:
+            - TestRunner1