Skip to content

refactor: create full venv for bootstrap=system_python #3473

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.

Sign up for GitHub

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

Draft
rickeylev wants to merge 9 commits into
base: main
Choose a base branch
from
+182 −90
Draft

refactor: create full venv for bootstrap=system_python #3473

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c5fd17c
create full venv for system_python
rickeylev Dec 19, 2025
b0164d5
force rules_pkg 1.2
rickeylev Dec 20, 2025
a419cf5
remove defunct code paths
rickeylev Dec 20, 2025
0850b9b
add debugging
rickeylev Dec 21, 2025
536672f
add more debugging
rickeylev Dec 21, 2025
ade95c0
seems like venv isnt supported?
rickeylev Dec 21, 2025
36b1f94
implemented venv creation at runtime
rickeylev Dec 22, 2025
44ce289
use rules_pkg 1.2 via bazel_dep
rickeylev Dec 22, 2025
0357dad
cleanup
rickeylev Dec 22, 2025
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 MODULE.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,14 @@ bazel_dep(name = "another_module", version = "0", dev_dependency = True)
# We use `WORKSPACE.bzlmod` because it is impossible to have dev-only local overrides.
bazel_dep(name = "rules_go", version = "0.41.0", dev_dependency = True, repo_name = "io_bazel_rules_go")

archive_override(
module_name = "rules_pkg",
urls = [
"https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/1.2.0/rules_pkg-1.2.0.tar.gz",
"https://github.com/bazelbuild/rules_pkg/releases/download/1.2.0/rules_pkg-1.2.0.tar.gz",
],
)

internal_dev_deps = use_extension(
"//python/private:internal_dev_deps.bzl",
"internal_dev_deps",
Expand Down
77 changes: 36 additions & 41 deletions python/private/py_executable.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,23 +513,21 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
# * https://github.com/python/cpython/blob/main/Modules/getpath.py
# * https://github.com/python/cpython/blob/main/Lib/site.py
def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root_to_sys_path):
create_full_venv = BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SCRIPT
venv = "_{}.venv".format(output_prefix.lstrip("_"))

if create_full_venv:
# The pyvenv.cfg file must be present to trigger the venv site hooks.
# Because it's paths are expected to be absolute paths, we can't reliably
# put much in it. See https://github.com/python/cpython/issues/83650
pyvenv_cfg = ctx.actions.declare_file("{}/pyvenv.cfg".format(venv))
ctx.actions.write(pyvenv_cfg, "")
else:
pyvenv_cfg = None
# The pyvenv.cfg file must be present to trigger the venv site hooks.
# Because it's paths are expected to be absolute paths, we can't reliably
# put much in it. See https://github.com/python/cpython/issues/83650
pyvenv_cfg = ctx.actions.declare_file("{}/pyvenv.cfg".format(venv))
ctx.actions.write(pyvenv_cfg, "")

runtime = runtime_details.effective_runtime

venvs_use_declare_symlink_enabled = (
VenvsUseDeclareSymlinkFlag.get_value(ctx) == VenvsUseDeclareSymlinkFlag.YES
)

# todo: default this to True for bootstrap=system_python ?
recreate_venv_at_runtime = False

if runtime.interpreter:
Expand All @@ -539,40 +537,37 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root

bin_dir = "{}/bin".format(venv)

if create_full_venv:
# Some wrappers around the interpreter (e.g. pyenv) use the program
# name to decide what to do, so preserve the name.
py_exe_basename = paths.basename(interpreter_actual_path)

if not venvs_use_declare_symlink_enabled or not runtime.supports_build_time_venv:
recreate_venv_at_runtime = True

# When the venv symlinks are disabled, the $venv/bin/python3 file isn't
# needed or used at runtime. However, the zip code uses the interpreter
# File object to figure out some paths.
interpreter = ctx.actions.declare_file("{}/{}".format(bin_dir, py_exe_basename))
ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path))

elif runtime.interpreter:
# Even though ctx.actions.symlink() is used, using
# declare_symlink() is required to ensure that the resulting file
# in runfiles is always a symlink. An RBE implementation, for example,
# may choose to write what symlink() points to instead.
interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename))

rel_path = relative_path(
# dirname is necessary because a relative symlink is relative to
# the directory the symlink resides within.
from_ = paths.dirname(runfiles_root_path(ctx, interpreter.short_path)),
to = interpreter_actual_path,
)
# Some wrappers around the interpreter (e.g. pyenv) use the program
# name to decide what to do, so preserve the name.
py_exe_basename = paths.basename(interpreter_actual_path)

if not venvs_use_declare_symlink_enabled or not runtime.supports_build_time_venv:
recreate_venv_at_runtime = True

# When the venv symlinks are disabled, the $venv/bin/python3 file isn't
# needed or used at runtime. However, the zip code uses the interpreter
# File object to figure out some paths.
interpreter = ctx.actions.declare_file("{}/{}".format(bin_dir, py_exe_basename))
ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path))

elif runtime.interpreter:
# Even though ctx.actions.symlink() is used, using
# declare_symlink() is required to ensure that the resulting file
# in runfiles is always a symlink. An RBE implementation, for example,
# may choose to write what symlink() points to instead.
interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename))

rel_path = relative_path(
# dirname is necessary because a relative symlink is relative to
# the directory the symlink resides within.
from_ = paths.dirname(runfiles_root_path(ctx, interpreter.short_path)),
to = interpreter_actual_path,
)

ctx.actions.symlink(output = interpreter, target_path = rel_path)
else:
interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename))
ctx.actions.symlink(output = interpreter, target_path = runtime.interpreter_path)
ctx.actions.symlink(output = interpreter, target_path = rel_path)
else:
interpreter = None
interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename))
ctx.actions.symlink(output = interpreter, target_path = runtime.interpreter_path)
Comment on lines 542 to 569
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The elif and else blocks here have some repeated logic for creating the interpreter symlink. You can refactor this to reduce duplication by declaring the symlink first and then conditionally determining the target path. This would make the code a bit cleaner and easier to maintain.

    if not venvs_use_declare_symlink_enabled or not runtime.supports_build_time_venv:
        recreate_venv_at_runtime = True

        # When the venv symlinks are disabled, the $venv/bin/python3 file isn't
        # needed or used at runtime. However, the zip code uses the interpreter
        # File object to figure out some paths.
        interpreter = ctx.actions.declare_file("{}/{}".format(bin_dir, py_exe_basename))
        ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path))
    else:
        # Even though ctx.actions.symlink() is used, using
        # declare_symlink() is required to ensure that the resulting file
        # in runfiles is always a symlink. An RBE implementation, for example,
        # may choose to write what symlink() points to instead.
        interpreter = ctx.actions.declare_symlink("{}/{}".format(bin_dir, py_exe_basename))

        if runtime.interpreter:
            target_path = relative_path(
                # dirname is necessary because a relative symlink is relative to
                # the directory the symlink resides within.
                from_ = paths.dirname(runfiles_root_path(ctx, interpreter.short_path)),
                to = interpreter_actual_path,
            )
        else:
            target_path = runtime.interpreter_path

        ctx.actions.symlink(output = interpreter, target_path = target_path)


if runtime.interpreter_version_info:
version = "{}.{}".format(
Expand Down