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

…nv (#281)

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:
@@ -73,33 +81,31 @@ def get_cset(self):
     def set_cset(self, cset_path):
         self._cset_path = cset_path
 
-    def has_denoise(self):
+    def ensure_denoise(self):
         if self._denoise_path is None:
-            self._denoise_path = self._absolute_path_for_command(
-                "rebench-denoise", ["--version"]
-            )
+            if os.access(denoise_py, os.X_OK):
+                self._denoise_path = denoise_py
+            elif not os.path.isfile(denoise_py):
+                raise UIError(
+                    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,
+                )
+            else:
+                raise UIError(
+                    f"{denoise_py} not marked executable. "
+                    f"Please run something similar to `chmod a+x {denoise_py}`. "
+                    "To use ReBench without denoise, use the --no-denoise option.\n",
+                    None,
+                )
 
         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",
-                None,
-            )
-
+        self.ensure_denoise()
         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 +359,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":

setup.py

@@ -19,14 +19,25 @@
 # 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 sys
+import os
+import stat
 
 from setuptools import setup, find_packages
+from setuptools.command.install import install
 from rebench import __version__ as rebench_version
 
 with open("README.md", "r") as fh:
     long_description = fh.read()
 
+class InstallAndSetDenoisePermissions(install):
+    def run(self):
+        install.run(self)
+        install_path = os.path.join(self.install_lib, "rebench")
+        denoise_path = os.path.join(install_path, "denoise.py")
+        if os.path.exists(denoise_path):
+            st = os.stat(denoise_path)
+            os.chmod(denoise_path, st.st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
+
 setup(name='ReBench',
       version=rebench_version,
       description='Execute and document benchmarks reproducibly.',
@@ -53,6 +64,8 @@
               'rebench-denoise = rebench.denoise:main_func'
           ]
       },
+      scripts=['rebench/denoise.py'],
+      cmdclass={'install': InstallAndSetDenoisePermissions},
       test_suite='rebench.tests',
       license='MIT'
 )