Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stage-2 bootstrap script incorrectly removes inherited PYTHONPATH entries on Python 3.11 and above #2318

Open
chowder opened this issue Oct 20, 2024 · 2 comments · May be fixed by #2418
Open

Comments

@chowder
Copy link
Contributor

chowder commented Oct 20, 2024

🐞 bug report

Affected Rule

The issue is caused by the rule: py_binary / py_test

Is this a regression?

Yes.

Description

The stage-1 bootstrap script runs python with PYTHONSAFEPATH=1, which means that no unsafe paths are preprended to the runtime environment of stage-2.

However, the stage-2 script unconditionally deletes sys.path[0]. This is harmless most of the time, since the search path that is deleted is /usr/lib/python311.zip (or similar).

But if the target caller attempts to propagate PYTHONPATH to the application, the first entry in PYTHONPATH is incorrectly removed.

🔬 Minimal Reproduction

script.py:

import sys
print("\n".join(sys.path))

BUILD.bazel:

py_binary(
    name = "script",
    srcs = [":script.py"],
)

Build and run the target with PYTHONPATH=foo:bar, observe that foo is missing from sys.path.

🌍 Your Environment

Operating System:

Linux AMD64 - CentOS 7, and Debian 11

Output of bazel version:

Bazel 7.2.1

Rules_python version:

0.36.0 (but present in 0.37.0 too)

@chowder
Copy link
Contributor Author

chowder commented Oct 20, 2024

I suppose something is to be said about the smell that is attempting to set PYTHONPATH for a Bazel target.

Where I run into this problem is with VSCode/PyCharm debuggers which run their own bootstrap code via PYTHONPATH propagation, before running your actual target.

In any case, a simple fix for this is to replace this line, with:

if not getattr(sys.flags, "safe_path", False):
    del sys.path[0]

Happy to raise a PR if agreed.

@rickeylev
Copy link
Contributor

simple fix is to replace the del sys.path[0] line with guard

Yes. A PR for that would be approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants