Enable denoise.py to work as script to avoid need for PYTHONPATH to b…

…e set in environment

- make denoise.py executable for all
- denoise.py does not support —version any longer, but that seems like an ok tradeoff

Signed-off-by: Stefan Marr <[email protected]>

rebench/denoise.py

@@ -1,3 +1,4 @@
+#!/usr/bin/env python3
 import json
 import os
 import sys
@@ -7,13 +8,21 @@
 from multiprocessing import Pool
 from subprocess import check_output, CalledProcessError, DEVNULL, STDOUT
 
-from .output import output_as_str, UIError
-from .subprocess_kill import kill_process
+denoise_py = os.path.abspath(__file__)
 
-try:
-    from . import __version__ as rebench_version
-except ValueError:
-    rebench_version = "unknown"
+if __name__ == "__main__":
+    # ensure that the rebench module is available
+    rebench_module = os.path.dirname(denoise_py)
+    sys.path.append(os.path.dirname(rebench_module))
+
+    # pylint: disable-next=import-error
+    from output import output_as_str, UIError  # type: ignore
+
+    # pylint: disable-next=import-error
+    from subprocess_kill import kill_process  # type: ignore
+else:
+    from .output import output_as_str, UIError
+    from .subprocess_kill import kill_process
 
 
 class CommandsPaths:
@@ -23,7 +32,6 @@ def __init__(self):
         self._cset_path = None
         self._denoise_path = None
         self._which_path = None
-        self._denoise_python_path = None
 
     def get_which(self):
         if not self._which_path:
@@ -75,31 +83,21 @@ def set_cset(self, cset_path):
 
     def has_denoise(self):
         if self._denoise_path is None:
-            self._denoise_path = self._absolute_path_for_command(
-                "rebench-denoise", ["--version"]
-            )
+            self._denoise_path = denoise_py
 
         return self._denoise_path is not None and self._denoise_path is not False
 
     def get_denoise(self):
         if not self.has_denoise():
             raise UIError(
-                "rebench-denoise not found. "
-                "Was ReBench installed so that `rebench` and `rebench-denoise` are on the PATH? "
-                "Python's bin directory for packages may need to be added to PATH manually.\n\n"
-                "To use ReBench without rebench-denoise, use the --no-denoise option.\n",
+                f"{denoise_py} not found. "
+                "Could it be that the user has no access to the file? "
+                "To use ReBench without denoise, use the --no-denoise option.\n",
                 None,
             )
 
         return self._denoise_path
 
-    def get_denoise_python_path(self):
-        if self._denoise_python_path is None:
-            current_file = os.path.abspath(__file__)
-            self._denoise_python_path = os.path.dirname(os.path.dirname(current_file))
-
-        return self._denoise_python_path
-
 
 paths = CommandsPaths()
 
@@ -353,9 +351,6 @@ def _test(num_cores):
 
 def _shell_options():
     parser = ArgumentParser()
-    parser.add_argument(
-        "--version", action="version", version="%(prog)s " + rebench_version
-    )
     parser.add_argument(
         "--json",
         action="store_true",

rebench/denoise_client.py

@@ -21,25 +21,6 @@ def get_number_of_cores():
     return _num_cpu_cores
 
 
-def _get_env_with_python_path_for_denoise():
-    return add_denoise_python_path_to_env(os.environ)
-
-
-def add_denoise_python_path_to_env(env):
-    path = paths.get_denoise_python_path()
-
-    # did not find it, just leave the env unmodified
-    if path is False:
-        return env
-
-    env = env.copy()
-    if "PYTHONPATH" in env and env["PYTHONPATH"]:
-        env["PYTHONPATH"] += os.pathsep + path
-    else:
-        env["PYTHONPATH"] = path
-    return env
-
-
 class DenoiseResult:
 
     def __init__(self, succeeded, warn_msg, use_nice, use_shielding, details):
@@ -55,8 +36,8 @@ def minimize_noise(show_warnings, ui, for_profiling):  # pylint: disable=too-man
 
     result = {}
 
-    env = _get_env_with_python_path_for_denoise()
-    cmd = ["sudo", "--preserve-env=PYTHONPATH", "-n", paths.get_denoise()]
+    env = os.environ
+    cmd = ["sudo", "-n", paths.get_denoise()]
     if for_profiling:
         cmd += ["--for-profiling"]
 
@@ -134,14 +115,14 @@ def minimize_noise(show_warnings, ui, for_profiling):  # pylint: disable=too-man
         if 'password is required' in output:
             msg += '{ind}Please make sure `sudo ' + paths.get_denoise() + '`' \
                    + ' can be used without password.\n'
-            msg += '{ind}To be able to run rebench-denoise without password,\n'
+            msg += '{ind}To be able to run denoise without password,\n'
             msg += '{ind}add the following to the end of your sudoers file (using visudo):\n'
             msg += '{ind}{ind}' + getpass.getuser() + ' ALL = (root) NOPASSWD:SETENV: '\
                    + paths.get_denoise() + '\n'
         elif 'command not found' in output:
-            msg += '{ind}Please make sure `rebench-denoise` is on the PATH\n'
+            msg += '{ind}Please make sure ' + paths.get_denoise() + ' is accessible.\n'
         elif "No such file or directory: 'sudo'" in output:
-            msg += "{ind}sudo is not available. Can't use rebench-denoise to manage the system.\n"
+            msg += "{ind}sudo is not available. Can't use denoise to manage the system.\n"
         else:
             msg += "{ind}Error: " + escape_braces(output)
 
@@ -158,14 +139,14 @@ def restore_noise(denoise_result, show_warning, ui):
 
     num_cores = get_number_of_cores()
 
-    env = _get_env_with_python_path_for_denoise()
+    env = os.environ
     values = set(denoise_result.details.values())
     if len(values) == 1 and "failed" in values:
         # everything failed, don't need to try to restore things
         pass
     else:
         try:
-            cmd = ["sudo", "--preserve-env=PYTHONPATH", "-n", paths.get_denoise(), "--json"]
+            cmd = ["sudo", "-n", paths.get_denoise(), "--json"]
             if not denoise_result.use_shielding:
                 cmd += ["--without-shielding"]
             elif paths.has_cset():
@@ -183,10 +164,10 @@ def restore_noise(denoise_result, show_warning, ui):
 
 
 def deliver_kill_signal(pid):
-    env = _get_env_with_python_path_for_denoise()
+    env = os.environ
 
     try:
-        cmd = ['sudo', '--preserve-env=PYTHONPATH',
+        cmd = ['sudo',
                '-n', paths.get_denoise(), '--json', 'kill', str(pid)]
         subprocess.check_output(cmd, stderr=subprocess.STDOUT, env=env)
     except (subprocess.CalledProcessError, FileNotFoundError):

rebench/executor.py

@@ -30,7 +30,7 @@
 
 from . import subprocess_with_timeout as subprocess_timeout
 from .denoise import paths as denoise_paths
-from .denoise_client import add_denoise_python_path_to_env, get_number_of_cores
+from .denoise_client import get_number_of_cores
 from .interop.adapter import ExecutionDeliveredNoResults, instantiate_adapter, OutputNotParseable, \
     ResultsIndicatedAsInvalid
 from .model.build_cmd import BuildCommand
@@ -346,7 +346,7 @@ def _create_scheduler(self, scheduler, print_execution_plan):
 
     def _construct_cmdline(self, run_id, gauge_adapter):
         num_cores = get_number_of_cores()
-        env = add_denoise_python_path_to_env(run_id.env)
+        env = run_id.env
         cmdline = ""
 
         if self.use_denoise:
@@ -534,7 +534,7 @@ def _generate_data_point(self, cmdline, gauge_adapter, run_id,
             location = run_id.location
             if location:
                 location = os.path.expanduser(location)
-            env = add_denoise_python_path_to_env(run_id.env)
+            env = run_id.env
 
             self.ui.debug_output_info("{ind}Starting run\n", run_id, cmdline, location, env)
 

rebench/tests/features/issue_42_test.py

@@ -138,8 +138,8 @@ def test_construct_cmdline_env_vars_set_as_expected(self):
 
         self.assertRegex(
             ex._construct_cmdline(runs[0], TimeManualAdapter(False, ex)),
-            r" --preserve-env=IMPORTANT_ENV_VARIABLE,ALSO_IMPORTANT,PYTHONPATH " +
-            r"[^\s]*rebench-denoise")
+            r" --preserve-env=IMPORTANT_ENV_VARIABLE,ALSO_IMPORTANT " +
+            r"[^\s]*denoise\.py")
 
     def test_construct_cmdline_build_with_env(self):
         cnf = Configurator(load_config(self._path + '/issue_42.conf'), DataStore(self.ui),
@@ -148,7 +148,7 @@ def test_construct_cmdline_build_with_env(self):
         ex = Executor(runs, True, self.ui, use_nice=True, use_shielding=True)
 
         self.assertRegex(ex._construct_cmdline(runs[0], TimeManualAdapter(False, ex)),
-                         r" --preserve-env=VAR1,VAR3,PYTHONPATH [^\s]*rebench-denoise")
+                         r" --preserve-env=VAR1,VAR3 [^\s]*denoise\.py")
 
     def _assert_empty_standard_env(self, log_remainder):
         env_parts = log_remainder.split(":")

rebench/tests/features/issue_42_vm.py

@@ -13,7 +13,7 @@
 
 known_envvars = ["PWD", "SHLVL", "VERSIONER_PYTHON_VERSION",
                  "_", "__CF_USER_TEXT_ENCODING", "LC_CTYPE",
-                 "CPATH", "LIBRARY_PATH", "MANPATH", "SDKROOT", "PYTHONPATH"]
+                 "CPATH", "LIBRARY_PATH", "MANPATH", "SDKROOT"]
 
 if test == "as-expected":
     if os.environ.get("IMPORTANT_ENV_VARIABLE", None) != "exists":