Skip to content

Commit

Permalink
BUG: Remove prepended entry from sys.path
Browse files Browse the repository at this point in the history
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 5ab2d29.
  • Loading branch information
ngnpope committed Oct 5, 2023
1 parent b8cbf81 commit 3de7230
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
12 changes: 12 additions & 0 deletions asv/benchmark.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
Run a Unix socket forkserver.
"""

import os
import sys

from asv_runner.discovery import _discover
Expand Down Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions test/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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])

0 comments on commit 3de7230

Please sign in to comment.