From 3de72309610418e2eebff6f9a7a217b9fd9aec2d Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Thu, 5 Oct 2023 11:13:51 +0100 Subject: [PATCH] BUG: Remove prepended entry from sys.path Python prepends the directory of the script being executed to `sys.path` and this can cause other packages to be shadowed, e.g. the builtin `statistics` module being shadowed by `asv.statistics` because the path of the `asv` package is prepended when the benchmark runner script is executed. There was a prior fix for this but it was removed as part of refactoring and splitting into `asv_runner`. Python 3.11+ has a mechanism to avoid this via use of `-P` on the command line or setting the `PYTHONSAFEPATH` environment variable. This doesn't really help though as support for older versions of Python is required. This patch does, however, check `sys.flags.safe_path` to avoid fixing up the path if this feature is being used. This further highlights that the original approach is not particularly robust as it removed the first item unconditionally and sometimes tried to add it back in. Instead this patch checks for and removes the path only if it matches the directory that the runner script resides in. Other alternatives could be to consider running with the `-I` command line flag to force isolated mode (Python 3.4+), although that might cause other unexpected breakage as it implies `-E` (which ignores all `PYTHON*` environment variables), etc. See the following links for details: - https://docs.python.org/3/library/sys.html#sys.path - https://docs.python.org/3/using/cmdline.html#envvar-PYTHONSAFEPATH - https://docs.python.org/3/using/cmdline.html#cmdoption-P - https://docs.python.org/3/using/cmdline.html#cmdoption-I Regression in 5ab2d29d039a080c3a3f48ecac65526842faa7f1. --- asv/benchmark.py | 12 ++++++++++++ test/test_runner.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/asv/benchmark.py b/asv/benchmark.py index 330d3f341..b0d8963a4 100644 --- a/asv/benchmark.py +++ b/asv/benchmark.py @@ -22,6 +22,7 @@ Run a Unix socket forkserver. """ +import os import sys from asv_runner.discovery import _discover @@ -49,6 +50,17 @@ def _help(args): def main(): + # Remove asv package directory from `sys.path`. This script file resides + # there although it's not part of the package, so Python prepends it to + # `sys.path` on start which can shadow other modules. On Python 3.11+ it is + # possible to use `PYTHONSAFEPATH` to prevent this, but the script needs to + # work for older versions of Python. + if ( + not getattr(sys.flags, 'safe_path', False) # Python 3.11+ only. + and sys.path[0] == os.path.dirname(os.path.abspath(__file__)) + ): + sys.path.pop(0) + if len(sys.argv) < 2: _help([]) sys.exit(1) diff --git a/test/test_runner.py b/test/test_runner.py index 6ba63b1c4..287d7e16e 100644 --- a/test/test_runner.py +++ b/test/test_runner.py @@ -450,3 +450,33 @@ def test_timeraw_benchmark(benchmarks_fixture): assert times['timeraw_examples.TimerawSuite.timeraw_setup'].result is not None assert 'timed out' in times['timeraw_examples.TimerawSuite.timeraw_timeout'].stderr assert '0' * 7 * 3 in times['timeraw_examples.TimerawSuite.timeraw_count'].stderr + + +def test_asv_package_not_on_sys_path(capsys, benchmarks_fixture): + # Check benchmark script directory is not prepended to sys.path + + conf, repo, envs, commit_hash = benchmarks_fixture + + with open(os.path.join('benchmark', 'sys_path.py'), 'w') as f: + f.write('import sys; print(repr(sys.path[0]), flush=True)') + + try: + benchmarks.Benchmarks.discover(conf, repo, envs, [commit_hash], check=True) + except util.UserError: + pass + + out, err = capsys.readouterr() + + assert repr(os.path.dirname(runner.BENCHMARK_RUN_SCRIPT)) not in out + + +def test_builtin_statistics_module_not_shadowed(benchmarks_fixture): + # Crashes with the following exception on stderr if builtin statistics module is shadowed: + # ImportError: cannot import name 'pstdev' from 'statistics' (.../asv/statistics.py) + + conf, repo, envs, commit_hash = benchmarks_fixture + + with open(os.path.join('benchmark', 'no_shadow.py'), 'w') as f: + f.write('from statistics import pstdev') + + benchmarks.Benchmarks.discover(conf, repo, envs, [commit_hash])