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

fix: set PYTHONNOUSERSITE #2576

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

froody
Copy link

@froody froody commented Jan 23, 2025

Set PYTHONNOUSERSITE to prevent adding user directories outside bazel. I found a py_test run with bazel test was importing modules from ~/.local/lib/python3.10/site-packages/ which allowed it to run successfully on my machine but fail on other hosts.

Before:

USER_SITE directory in sys.path when running python code via
bazel run or bazel test

After:

sys.path only contains paths relative to the bazel-out directory

Set PYTHONNOUSERSITE to prevent adding user directories outside bazel.
I found a py_test run with `bazel test` was importing modules from
~/.local/lib/python3.10/site-packages/ which allowed it to run
successfully on my machine but fail on other hosts.

Before:

USER_SITE directory in sys.path when running python code via
`bazel run` or `bazel test`

After:

sys.path only contains paths relative to the `bazel-out` directory
Copy link

google-cla bot commented Jan 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@rickeylev
Copy link
Collaborator

Hi @froody, thanks for your contribution!

I think this is a good idea, so +1 overall setting this. Changes needed to merge:

  • Pass -s to the interpreter instead of an environment variable. envvars contaminate subprocesses, which is undesirable.
  • Please update the bootstrap=script impl. i.e update python/private/stage1_bootstrap_template.sh to pass -s too
  • Please update CHANGELOG.md and add an entry under Changed.
  • We'll need tests. Have a look at tests/bootstrap_impls. I think you can copy/paste run_binary_zip_no_test, modify bin.py to print out sys.flags.no_user_site, and check the value in run_binary_zip_no_test.sh

I'm pretty certain this is gonna break something about some user's workflow, though I don't know what that would be.

@aignas
Copy link
Collaborator

aignas commented Jan 28, 2025

I would also like to see the behaviour switched based on a config flag so that users that need to opt out from the behaviour can do so.

Defaulting to always including -s should be good enough.

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

Successfully merging this pull request may close these issues.

4 participants