Merge PR #174: Environment variable handling

.github/workflows/ci.yml

@@ -60,4 +60,4 @@ jobs:
         run: coveralls
         env:
           COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
-        if: matrix.coverage
+        if: ${{ matrix.coverage && env.COVERALLS_REPO_TOKEN != '' }}

docs/config.md

@@ -478,6 +478,9 @@ same build command without executing it multiple times.
 For this purpose, build commands are considered the same when they have the
 same command and location (based on simple string comparisons).
 
+Commands are executed with an empty environment, i.e., without any environment
+variables. All configuration is intended to be explicit to simplify reproduction.
+
 Example:
 
 ```yaml
@@ -538,6 +541,7 @@ Thus, one can use:
 - `parallel_interference_factor`
 - `execute_exclusively`
 - `retries_after_failure`
+- `env`
 
 As well as:
 

rebench/executor.py

@@ -301,7 +301,10 @@ def _construct_cmdline(self, run_id, gauge_adapter):
         cmdline = ""
 
         if self.use_denoise:
-            cmdline += "sudo rebench-denoise "
+            cmdline += "sudo "
+            if run_id.env:
+                cmdline += "--preserve-env=" + ','.join(run_id.keys()) + " "
+            cmdline += "rebench-denoise "
             if not self._use_nice:
                 cmdline += "--without-nice "
             if not self._use_shielding:
@@ -348,11 +351,11 @@ def _execute_build_cmd(self, build_command, name, run_id):
 
         def _keep_alive(seconds):
             self.ui.warning(
-                "Keep alive. current job runs since %dmin\n" % (seconds / 60), run_id, script, path)
+                "Keep alive, current job runs for %dmin\n" % (seconds / 60), run_id, script, path)
 
         try:
             return_code, stdout_result, stderr_result = subprocess_timeout.run(
-                '/bin/sh', path, False, True,
+                '/bin/sh', {}, path, False, True,
                 stdin_input=str.encode(script),
                 keep_alive_output=_keep_alive)
         except OSError as err:
@@ -467,10 +470,10 @@ def _generate_data_point(self, cmdline, gauge_adapter, run_id,
 
             def _keep_alive(seconds):
                 self.ui.warning(
-                    "Keep alive. current job runs since %dmin\n" % (seconds / 60), run_id, cmdline)
+                    "Keep alive, current job runs for %dmin\n" % (seconds / 60), run_id, cmdline)
 
             (return_code, output, _) = subprocess_timeout.run(
-                cmdline, cwd=run_id.location, stdout=subprocess.PIPE,
+                cmdline, env=run_id.env, cwd=run_id.location, stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT, shell=True, verbose=self.debug,
                 timeout=run_id.max_invocation_time,
                 keep_alive_output=_keep_alive)

rebench/model/__init__.py

@@ -65,6 +65,12 @@ def none_or_bool(value):
     return value
 
 
+def none_or_dict(value):
+    if value:
+        assert isinstance(value, dict)
+    return value
+
+
 def value_with_optional_details(value, default_details=None):
     if isinstance(value, dict):
         assert len(value) == 1

rebench/model/executor.py

@@ -40,6 +40,7 @@ def compile(cls, executor_name, executor, run_details, variables, build_commands
 
         description = executor.get('description')
         desc = executor.get('desc')
+        env = executor.get('env')
 
         profiler = Profiler.compile(executor.get('profiler'))
 
@@ -51,10 +52,10 @@ def compile(cls, executor_name, executor, run_details, variables, build_commands
                                      + "but no profiler details are given.")
 
         return Executor(executor_name, path, executable, args, build, description or desc,
-                        profiler, run_details, variables, action)
+                        profiler, run_details, variables, action, env)
 
     def __init__(self, name, path, executable, args, build, description,
-                 profiler, run_details, variables, action):
+                 profiler, run_details, variables, action, env):
         """Specializing the executor details in the run definitions with the settings from
            the executor definitions
         """
@@ -70,6 +71,7 @@ def __init__(self, name, path, executable, args, build, description,
 
         self.run_details = run_details
         self.variables = variables
+        self.env = env
 
         self.action = action
 

rebench/model/exp_run_details.py

@@ -17,7 +17,8 @@
 # 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 . import none_or_int, none_or_float, none_or_bool, remove_important, prefer_important
+from . import none_or_int, none_or_float, none_or_bool, none_or_dict, \
+    remove_important, prefer_important
 
 
 class ExpRunDetails(object):
@@ -42,24 +43,25 @@ def compile(cls, config, defaults):
 
         retries_after_failure = none_or_int(config.get('retries_after_failure',
                                                        defaults.retries_after_failure))
+        env = none_or_dict(config.get('env', defaults.env))
 
         return ExpRunDetails(invocations, iterations, warmup, min_iteration_time,
                              max_invocation_time, ignore_timeouts, parallel_interference_factor,
-                             execute_exclusively, retries_after_failure,
+                             execute_exclusively, retries_after_failure, env,
                              defaults.invocations_override, defaults.iterations_override)
 
     @classmethod
     def empty(cls):
-        return ExpRunDetails(None, None, None, None, None, None, None, None, None, None, None)
+        return ExpRunDetails(None, None, None, None, None, None, None, None, None, None, None, None)
 
     @classmethod
     def default(cls, invocations_override, iterations_override):
-        return ExpRunDetails(1, 1, None, 50, -1, None, None, True, 0,
+        return ExpRunDetails(1, 1, None, 50, -1, None, None, True, 0, {},
                              invocations_override, iterations_override)
 
     def __init__(self, invocations, iterations, warmup, min_iteration_time,
                  max_invocation_time, ignore_timeouts, parallel_interference_factor,
-                 execute_exclusively, retries_after_failure,
+                 execute_exclusively, retries_after_failure, env,
                  invocations_override, iterations_override):
         self.invocations = invocations
         self.iterations = iterations
@@ -71,6 +73,7 @@ def __init__(self, invocations, iterations, warmup, min_iteration_time,
         self.parallel_interference_factor = parallel_interference_factor
         self.execute_exclusively = execute_exclusively
         self.retries_after_failure = retries_after_failure
+        self.env = env
 
         self.invocations_override = invocations_override
         self.iterations_override = iterations_override

rebench/model/experiment.py

@@ -46,17 +46,20 @@ def compile(cls, name, exp, configurator):
         executions = exp.get('executions')
         suites = exp.get('suites')
 
-        return Experiment(name, description or desc, action, data_file, reporting,
+        env = exp.get('env')
+
+        return Experiment(name, description or desc, action, env, data_file, reporting,
                           run_details, variables, configurator, executions, suites)
 
-    def __init__(self, name, description, action, data_file, reporting, run_details,
+    def __init__(self, name, description, action, env, data_file, reporting, run_details,
                  variables, configurator, executions, suites):
         self.name = name
         self._description = description
         self._action = action
 
         self._run_details = run_details
         self._variables = variables
+        self._env = env
         self._reporting = reporting
 
         self._data_store = configurator.data_store

rebench/model/profiler.py

@@ -41,7 +41,7 @@ def _construct_report_cmdline(self, executor):
 
     def process_profile(self, run_id, executor):
         cmdline = self._construct_report_cmdline(executor)
-        (return_code, output, _) = run(cmdline, cwd=run_id.location, shell=True,
+        (return_code, output, _) = run(cmdline, run_id.env, cwd=run_id.location, shell=True,
                                        verbose=executor.debug)
 
         if return_code != 0:

rebench/model/run_id.py

@@ -76,6 +76,10 @@ def iterations(self):
     def invocations(self):
         return self.benchmark.run_details.invocations
 
+    @property
+    def env(self):
+        return self.benchmark.run_details.env
+
     @property
     def completed_invocations(self):
         return self._max_invocation

rebench/persistence.py

@@ -100,7 +100,7 @@ def get_config(self, name, executor_name, suite_name, extra_args):
 
         if key not in self._bench_cfgs:
             raise ValueError("Requested configuration is not available: " +
-                             key.__str__())
+                             str(key))
 
         return self._bench_cfgs[key]
 

rebench/rebench-schema.yml

@@ -64,6 +64,15 @@ schema;runs_type:
         Some experiments may fail non-deterministically. For these, it may be
         convenient to simply retry them a few times.
         This value indicates how often execution should be retried on failure.
+    env:
+      # default: an empty environment. Executors are start without anything
+      # in the environment to increase predictability and reproducibility.
+      type: map
+      mapping:
+        regex;(.+):
+          type: str
+      desc: |
+        Environment variables to be set when starting the executor.
 
 
 schema;reporting_type:
@@ -166,7 +175,7 @@ schema;build_type:
   desc: |
     A list of commands/strings to be executed by the system's shell.
     They are intended to set up the system for benchmarking,
-    typically to build binaries, compiled archieves, etc.
+    typically to build binaries, create archives, etc.
     Each command is executed once before any benchmark or executor that depend on it
     is executed. If the `location` or `path` of a suite/executor is set, it is used as
     working directory. Otherwise, it is the current working directory of ReBench.

rebench/subprocess_with_timeout.py

@@ -36,7 +36,8 @@ def output_as_str(string_like):
 
 class _SubprocessThread(Thread):
 
-    def __init__(self, executable_name, args, shell, cwd, verbose, stdout, stderr, stdin_input):
+    def __init__(self, executable_name, args, env,
+                 shell, cwd, verbose, stdout, stderr, stdin_input):
         Thread.__init__(self, name="Subprocess %s" % executable_name)
         self._args = args
         self._shell = shell
@@ -45,6 +46,7 @@ def __init__(self, executable_name, args, shell, cwd, verbose, stdout, stderr, s
         self._stdout = stdout
         self._stderr = stderr
         self._stdin_input = stdin_input
+        self._env = env
 
         self._pid = None
         self._started_cv = Condition()
@@ -62,9 +64,10 @@ def run(self):
         try:
             self._started_cv.acquire()
             stdin = PIPE if self._stdin_input else None
+
             # pylint: disable-next=consider-using-with
             proc = Popen(self._args, shell=self._shell, cwd=self._cwd,
-                         stdin=stdin, stdout=self._stdout, stderr=self._stderr)
+                         stdin=stdin, stdout=self._stdout, stderr=self._stderr, env=self._env)
             self._pid = proc.pid
             self._started_cv.notify()
             self._started_cv.release()
@@ -118,7 +121,7 @@ def _print_keep_alive(seconds_since_start):
     print("Keep alive, current job runs for %dmin\n" % (seconds_since_start / 60))
 
 
-def run(args, cwd=None, shell=False, kill_tree=True, timeout=-1,
+def run(args, env, cwd=None, shell=False, kill_tree=True, timeout=-1,
         verbose=False, stdout=PIPE, stderr=PIPE, stdin_input=None,
         keep_alive_output=_print_keep_alive):
     """
@@ -127,7 +130,7 @@ def run(args, cwd=None, shell=False, kill_tree=True, timeout=-1,
     """
     executable_name = args.split(' ', 1)[0]
 
-    thread = _SubprocessThread(executable_name, args, shell, cwd, verbose, stdout,
+    thread = _SubprocessThread(executable_name, args, env, shell, cwd, verbose, stdout,
                                stderr, stdin_input)
     thread.start()
 

rebench/tests/bugs/issue_4_run_equality_and_params_test.py

@@ -36,7 +36,7 @@ def setUp(self):
     @staticmethod
     def _create_template_run_id():
         executor = Executor('MyVM', 'foo_bar_path', 'foo_bar_bin',
-                            None, None, None, None, None, None, "benchmark")
+                            None, None, None, None, None, None, "benchmark", {})
         suite = BenchmarkSuite("MySuite", executor, '', '%(benchmark)s %(cores)s %(input)s',
                                None, None, [], None, None, None)
         benchmark = Benchmark("TestBench", "TestBench", None, suite, None,
@@ -46,7 +46,7 @@ def _create_template_run_id():
     @staticmethod
     def _create_hardcoded_run_id():
         executor = Executor('MyVM', 'foo_bar_path', 'foo_bar_bin',
-                            None, None, None, None, None, None, "benchmark")
+                            None, None, None, None, None, None, "benchmark", {})
         suite = BenchmarkSuite('MySuite', executor, '', '%(benchmark)s %(cores)s 2 3',
                                None, None, [], None, None, None)
         benchmark = Benchmark("TestBench", "TestBench", None, suite,

rebench/tests/features/issue_42.conf

@@ -0,0 +1,74 @@
+default_experiment: Test
+
+build_log: build.log
+
+runs:
+    invocations: 1
+    min_iteration_time: 0
+
+benchmark_suites:
+    as-expected:
+        gauge_adapter: TestExecutor
+        command: TestBenchMarks %(benchmark)s
+        benchmarks:
+            - as-expected
+    no-env:
+        gauge_adapter: TestExecutor
+        command: TestBenchMarks %(benchmark)s
+        benchmarks:
+            - no-env
+    env-value-expansion:
+        gauge_adapter: TestExecutor
+        command: TestBenchMarks %(benchmark)s
+        benchmarks:
+            - value-expansion
+        input_sizes: [2, 10]
+
+executors:
+    test-set-as-expected:
+        path: .
+        executable: issue_42_vm.py
+        env:
+            IMPORTANT_ENV_VARIABLE: "exists"
+            ALSO_IMPORTANT: "3"
+    test-env-with-value-expansion:
+        path: .
+        executable: issue_42_vm.py
+        env:
+            MY_VAR: "%(input)s"
+    test-no-env:
+        path: .
+        executable: issue_42_vm.py
+
+    exe-with-build-but-not-env:
+        path: .
+        executable: issue_42_vm.py
+        build:
+            - |
+                env
+    exe-with-build-and-env:
+        path: .
+        executable: issue_42_vm.py
+        env:
+            VAR1: test
+            VAR3: another test
+        build:
+            - |
+                env
+
+experiments:
+    test-set-as-expected:
+        suites: [as-expected]
+        executions: [test-set-as-expected]
+    test-no-env:
+        suites: [no-env]
+        executions: [test-no-env]
+    test-value-expansion:
+        suites: [env-value-expansion]
+        executions: [test-env-with-value-expansion]
+    build-with-env:
+        suites: [as-expected]
+        executions: [exe-with-build-and-env]
+    build-without-env:
+        suites: [as-expected]
+        executions: [exe-with-build-but-not-env]