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: Stop using PYTHONPATH to set up sys.path #1780

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions python/private/common/attributes.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ Specifies additional environment variables to set when the target is executed by
# The value is required, but varies by rule and/or rule type. Use
# create_stamp_attr to create one.
"stamp": None,
"_entrypoint_py_template": attr.label(
default = "@rules_python//python/private/common:entrypoint.tmpl.py",
allow_single_file = True,
),
"_runfiles": attr.label(
default = "@rules_python//python/runfiles",
providers = [PyInfo],
),
},
allow_none = True,
)
Expand Down
8 changes: 7 additions & 1 deletion python/private/common/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,13 @@ def filter_to_py_srcs(srcs):
return [f for f in srcs if f.extension == "py"]

def collect_imports(ctx, semantics):
return depset(direct = semantics.get_imports(ctx), transitive = [
if hasattr(ctx.attr, "_runfiles"):
# Executable rules have a private _runfiles attribute so they can pull
# in the runfiles library.
base = [ctx.attr._runfiles[PyInfo].imports]
else:
base = []
return depset(direct = semantics.get_imports(ctx), transitive = base + [
dep[PyInfo].imports
for dep in ctx.attr.deps
if PyInfo in dep
Expand Down
49 changes: 49 additions & 0 deletions python/private/common/entrypoint.tmpl.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import runpy
import sys
from pathlib import Path

# TODO(phil): Migrate runfiles detection from
# python/private/python_bootstrap_template.txt into here.
from python.runfiles import runfiles

RUNFILES = runfiles.Create()
RUNFILES_DIR = Path(RUNFILES.EnvVars()["RUNFILES_DIR"])
MAIN = RUNFILES_DIR / "%MAIN_REPO%/%MAIN_SHORT_PATH%"

IMPORTS = %IMPORTS%

# We add the root of each repository with external Python dependencies to the
# import search path.
# Can we get rid of this now that bzlmod mangles the repository names?
MODULE_IMPORTS = [
Path(path).parts[0]
for path in IMPORTS
]

def Deduplicate(items):
"""Efficiently filter out duplicates, keeping the first element only."""
seen = set()
for it in items:
if it not in seen:
seen.add(it)
yield it

# Work around sys.path[0] escaping the sandbox by deleting it.
# See https://github.com/bazelbuild/rules_python/issues/382 for more info.
if getattr(sys.flags, "safe_path", False):
# We are running on Python 3.11 and we don't need this workaround
pass
elif ".runfiles" not in sys.path[0]:
sys.path = sys.path[1:]

sys.path[0:0] = list(Deduplicate([
# The top of the runfiles tree always should be first in the search path.
# Can we get rid of this now that bzlmod mangles the repository names?
str(RUNFILES_DIR),
] + MODULE_IMPORTS + [
# Inject the import paths for all the bazel-managed packages so that they are
# searched before standard library paths.
str(RUNFILES_DIR / path) for path in IMPORTS
]))

runpy.run_path(str(MAIN), run_name="__main__")
49 changes: 48 additions & 1 deletion python/private/common/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,27 @@ _CC_TOOLCHAINS = [config_common.toolchain_type(
mandatory = False,
)] if hasattr(config_common, "toolchain_type") else []

_feature_flags = {
"incompatible_generate_entrypoint_shim": attr.bool(
default = False,
doc = """\
Generate a small shim that executes before the "main" script.
The shim is responsible for performing some hermeticity fixes inside the running
Python process.

For example, it will provide PYTHONSAFEPATH support for Python versions prior to
3.11.
""",
),
}

# Non-Google-specific attributes for executables
# These attributes are for rules that accept Python sources.
EXECUTABLE_ATTRS = union_attrs(
COMMON_ATTRS,
AGNOSTIC_EXECUTABLE_ATTRS,
PY_SRCS_ATTRS,
_feature_flags,
{
# TODO(b/203567235): In the Java impl, any file is allowed. While marked
# label, it is more treated as a string, and doesn't have to refer to
Expand Down Expand Up @@ -125,11 +140,26 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
_validate_executable(ctx)

main_py = determine_main(ctx)

if ctx.attr.incompatible_generate_entrypoint_shim:
entrypoint_py = ctx.actions.declare_file(ctx.label.name + "_entrypoint.py")

direct_sources = filter_to_py_srcs(ctx.files.srcs)
output_sources = semantics.maybe_precompile(ctx, direct_sources)
if ctx.attr.incompatible_generate_entrypoint_shim:
output_sources.append(entrypoint_py)
imports = collect_imports(ctx, semantics)
executable, files_to_build = _compute_outputs(ctx, output_sources)

if ctx.attr.incompatible_generate_entrypoint_shim:
_generate_entrypoint_py(
ctx,
imports = imports,
main_py = main_py,
entrypoint_py = entrypoint_py,
)
main_py = entrypoint_py

runtime_details = _get_runtime_details(ctx, semantics)
if ctx.configuration.coverage_enabled:
extra_deps = semantics.get_coverage_deps(ctx, runtime_details)
Expand All @@ -141,6 +171,11 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
if not _is_tool_config(ctx):
extra_deps.extend(semantics.get_debugger_deps(ctx, runtime_details))

if ctx.attr.incompatible_generate_entrypoint_shim:
runfiles_library_dep = [ctx.attr._runfiles[DefaultInfo].default_runfiles]
else:
runfiles_library_dep = []

cc_details = semantics.get_cc_details_for_binary(ctx, extra_deps = extra_deps)
native_deps_details = _get_native_deps_details(
ctx,
Expand All @@ -158,7 +193,7 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
cc_details.extra_runfiles,
native_deps_details.runfiles,
semantics.get_extra_common_runfiles_for_binary(ctx),
],
] + runfiles_library_dep,
semantics = semantics,
)
exec_result = semantics.create_executable(
Expand Down Expand Up @@ -201,6 +236,18 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
providers = modern_providers,
)

def _generate_entrypoint_py(ctx, imports, main_py, entrypoint_py):
ctx.actions.expand_template(
template = ctx.file._entrypoint_py_template,
output = entrypoint_py,
substitutions = {
"%IMPORTS%": json.encode_indent(imports.to_list()),
# Is there a better way to retrieve the name of the main repo?
"%MAIN_REPO%": ctx.label.workspace_name or ctx.workspace_name,
"%MAIN_SHORT_PATH%": main_py.short_path,
},
)

def _get_build_info(ctx, cc_toolchain):
build_info_files = py_internal.cc_toolchain_build_info_files(cc_toolchain)
if cc_helper.is_stamping_enabled(ctx):
Expand Down
1 change: 1 addition & 0 deletions python/private/common/py_executable_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ def _expand_bootstrap_template(
"%coverage_tool%": coverage_tool_runfiles_path,
"%import_all%": "True" if ctx.fragments.bazel_py.python_import_all_repositories else "False",
"%imports%": ":".join(imports.to_list()),
"%incompatible_generate_entrypoint_shim%": str(ctx.attr.incompatible_generate_entrypoint_shim),
"%is_zipfile%": "True" if is_for_zip else "False",
"%main%": "{}/{}".format(
ctx.workspace_name,
Expand Down
7 changes: 6 additions & 1 deletion python/private/python_bootstrap_template.txt
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,12 @@ def Main():
module_space = FindModuleSpace(main_rel_path)
delete_module_space = False

python_imports = '%imports%'
incompatible_generate_entrypoint_shim = %incompatible_generate_entrypoint_shim%
if incompatible_generate_entrypoint_shim:
python_imports = ''
else:
python_imports = '%imports%'

python_path_entries = CreatePythonPathEntries(python_imports, module_space)
python_path_entries += GetRepositoriesImports(module_space, %import_all%)
# Remove duplicates to avoid overly long PYTHONPATH (#10977). Preserve order,
Expand Down
21 changes: 21 additions & 0 deletions tests/pythonpath/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Copyright 2024 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

load("//python:py_test.bzl", "py_test")

py_test(
name = "pythonpath_test",
srcs = ["pythonpath_test.py"],
incompatible_generate_entrypoint_shim = True,
)
11 changes: 11 additions & 0 deletions tests/pythonpath/pythonpath_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import os
import unittest

class PythonPathTest(unittest.TestCase):

def test_environment(self):
"""Validates that PYTHONPATH is empty."""
self.assertFalse(os.environ["PYTHONPATH"])

if __name__ == "__main__":
unittest.main()
Loading