Skip to content

Commit

Permalink
fix(pypi): use python -B for repo-phase invocations (#2641)
Browse files Browse the repository at this point in the history
Before this change we would just invoke the Python interpreter. This
means that in the `rules_python` directory there would be `__pycache__`
folders created in the source tree and the same `__pycache__` folders
would be created in the python interpreter repository rules if the
directories were writable.

This change ensures that we are executing `python` with `-B` in those
contexts and reduces any likelihood of us doing the wrong thing.

Work towards #1169.

---------

Co-authored-by: Richard Levasseur <[email protected]>
  • Loading branch information
aignas and rickeylev authored Mar 7, 2025
1 parent 0fa6667 commit 4910961
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 25 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ Unreleased changes template.
([#1169](https://github.com/bazelbuild/rules_python/issues/1169)).
* (gazelle) Don't collapse depsets to a list or into args when generating the modules mapping file.
Support spilling modules mapping args into a params file.
* (pypi) From now on `python` invocations in repository and module extension
evaluation contexts will invoke Python interpreter with `-B` to avoid
creating `.pyc` files.

{#v0-0-0-added}
### Added
Expand Down
10 changes: 5 additions & 5 deletions python/private/pypi/evaluate_markers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ def evaluate_markers(mrctx, *, requirements, python_interpreter, python_interpre
pypi_repo_utils.execute_checked(
mrctx,
op = "ResolveRequirementEnvMarkers({})".format(in_file),
python = pypi_repo_utils.resolve_python_interpreter(
mrctx,
python_interpreter = python_interpreter,
python_interpreter_target = python_interpreter_target,
),
arguments = [
pypi_repo_utils.resolve_python_interpreter(
mrctx,
python_interpreter = python_interpreter,
python_interpreter_target = python_interpreter_target,
),
"-m",
"python.private.pypi.requirements_parser.resolve_target_platforms",
in_file,
Expand Down
10 changes: 7 additions & 3 deletions python/private/pypi/patch_whl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ other patches ensures that the users have overview on exactly what has changed
within the wheel.
"""

load("//python/private:repo_utils.bzl", "repo_utils")
load(":parse_whl_name.bzl", "parse_whl_name")
load(":pypi_repo_utils.bzl", "pypi_repo_utils")

_rules_python_root = Label("//:BUILD.bazel")

Expand Down Expand Up @@ -102,10 +102,14 @@ def patch_whl(rctx, *, python_interpreter, whl_path, patches, **kwargs):
record_patch = rctx.path("RECORD.patch")
whl_patched = patched_whl_name(whl_input.basename)

repo_utils.execute_checked(
pypi_repo_utils.execute_checked(
rctx,
python = python_interpreter,
srcs = [
Label("//python/private/pypi:repack_whl.py"),
Label("//tools:wheelmaker.py"),
],
arguments = [
python_interpreter,
"-m",
"python.private.pypi.repack_whl",
"--record-patch",
Expand Down
55 changes: 41 additions & 14 deletions python/private/pypi/pypi_repo_utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,30 @@ def _construct_pypath(mrctx, *, entries):
])
return pypath

def _execute_checked(mrctx, *, srcs, **kwargs):
def _execute_prep(mrctx, *, python, srcs, **kwargs):
for src in srcs:
# This will ensure that we will re-evaluate the bzlmod extension or
# refetch the repository_rule when the srcs change. This should work on
# Bazel versions without `mrctx.watch` as well.
repo_utils.watch(mrctx, mrctx.path(src))

environment = kwargs.pop("environment", {})
pythonpath = environment.get("PYTHONPATH", "")
if pythonpath and not types.is_string(pythonpath):
environment["PYTHONPATH"] = _construct_pypath(mrctx, entries = pythonpath)
kwargs["environment"] = environment

# -B is added to prevent the repo-phase invocation from creating timestamp
# based pyc files, which contributes to race conditions and non-determinism
kwargs["arguments"] = [python, "-B"] + kwargs.get("arguments", [])
return kwargs

def _execute_checked(mrctx, *, python, srcs, **kwargs):
"""Helper function to run a python script and modify the PYTHONPATH to include external deps.
Args:
mrctx: Handle to the module_ctx or repository_ctx.
python: The python interpreter to use.
srcs: The src files that the script depends on. This is important to
ensure that the Bazel repository cache or the bzlmod lock file gets
invalidated when any one file changes. It is advisable to use
Expand All @@ -118,26 +137,34 @@ def _execute_checked(mrctx, *, srcs, **kwargs):
the `environment` has a value `PYTHONPATH` and it is a list, then
it will be passed to `construct_pythonpath` function.
"""
return repo_utils.execute_checked(
mrctx,
**_execute_prep(mrctx, python = python, srcs = srcs, **kwargs)
)

for src in srcs:
# This will ensure that we will re-evaluate the bzlmod extension or
# refetch the repository_rule when the srcs change. This should work on
# Bazel versions without `mrctx.watch` as well.
repo_utils.watch(mrctx, mrctx.path(src))

env = kwargs.pop("environment", {})
pythonpath = env.get("PYTHONPATH", "")
if pythonpath and not types.is_string(pythonpath):
env["PYTHONPATH"] = _construct_pypath(mrctx, entries = pythonpath)
def _execute_checked_stdout(mrctx, *, python, srcs, **kwargs):
"""Helper function to run a python script and modify the PYTHONPATH to include external deps.
return repo_utils.execute_checked(
Args:
mrctx: Handle to the module_ctx or repository_ctx.
python: The python interpreter to use.
srcs: The src files that the script depends on. This is important to
ensure that the Bazel repository cache or the bzlmod lock file gets
invalidated when any one file changes. It is advisable to use
`RECORD` files for external deps and the list of srcs from the
rules_python repo for any scripts.
**kwargs: Arguments forwarded to `repo_utils.execute_checked`. If
the `environment` has a value `PYTHONPATH` and it is a list, then
it will be passed to `construct_pythonpath` function.
"""
return repo_utils.execute_checked_stdout(
mrctx,
environment = env,
**kwargs
**_execute_prep(mrctx, python = python, srcs = srcs, **kwargs)
)

pypi_repo_utils = struct(
construct_pythonpath = _construct_pypath,
execute_checked = _execute_checked,
execute_checked_stdout = _execute_checked_stdout,
resolve_python_interpreter = _resolve_python_interpreter,
)
8 changes: 5 additions & 3 deletions python/private/pypi/whl_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ def _get_toolchain_unix_cflags(rctx, python_interpreter, logger = None):
if not is_standalone_interpreter(rctx, python_interpreter, logger = logger):
return []

stdout = repo_utils.execute_checked_stdout(
stdout = pypi_repo_utils.execute_checked_stdout(
rctx,
op = "GetPythonVersionForUnixCflags",
python = python_interpreter,
arguments = [
python_interpreter,
"-c",
"import sys; print(f'{sys.version_info[0]}.{sys.version_info[1]}', end='')",
],
srcs = [],
)
_python_version = stdout
include_path = "{}/include/python{}".format(
Expand Down Expand Up @@ -181,7 +182,6 @@ def _whl_library_impl(rctx):
python_interpreter_target = rctx.attr.python_interpreter_target,
)
args = [
python_interpreter,
"-m",
"python.private.pypi.whl_installer.wheel_installer",
"--requirement",
Expand Down Expand Up @@ -247,6 +247,7 @@ def _whl_library_impl(rctx):
# truncate the requirement value when logging it / reporting
# progress since it may contain several ' --hash=sha256:...
# --hash=sha256:...' substrings that fill up the console
python = python_interpreter,
op = op_tmpl.format(name = rctx.attr.name, requirement = rctx.attr.requirement.split(" ", 1)[0]),
arguments = args,
environment = environment,
Expand Down Expand Up @@ -295,6 +296,7 @@ def _whl_library_impl(rctx):
pypi_repo_utils.execute_checked(
rctx,
op = "whl_library.ExtractWheel({}, {})".format(rctx.attr.name, whl_path),
python = python_interpreter,
arguments = args + [
"--whl-file",
whl_path,
Expand Down

0 comments on commit 4910961

Please sign in to comment.