fix: add flag to use runtime venv creation when using bootstrap=script

CHANGELOG.md

@@ -77,6 +77,10 @@ Unreleased changes template.
   The related issue is [#908](https://github.com/bazelbuild/rules_python/issue/908).
 * (sphinxdocs) Do not crash when `tag_class` does not have a populated `doc` value.
   Fixes ([#2579](https://github.com/bazelbuild/rules_python/issues/2579)).
+* (binaries/tests) Fix packaging when using `--bootstrap_impl=script`: set
+  {obj}`--relative_venv_symlinks=no` to have it avoid creating symlinks at
+  build time.
+  Fixes ([#2489](https://github.com/bazelbuild/rules_python/issues/2489)
 
 {#v0-0-0-added}
 ### Added

MODULE.bazel

@@ -84,6 +84,7 @@ bazel_dep(name = "rules_testing", version = "0.6.0", dev_dependency = True)
 bazel_dep(name = "rules_shell", version = "0.3.0", dev_dependency = True)
 bazel_dep(name = "rules_multirun", version = "0.9.0", dev_dependency = True)
 bazel_dep(name = "bazel_ci_rules", version = "1.0.0", dev_dependency = True)
+bazel_dep(name = "rules_pkg", version = "1.0.1", dev_dependency = True)
 
 # Extra gazelle plugin deps so that WORKSPACE.bzlmod can continue including it for e2e tests.
 # We use `WORKSPACE.bzlmod` because it is impossible to have dev-only local overrides.

docs/api/rules_python/python/config_settings/index.md

@@ -212,6 +212,28 @@ Values:
 :::
 ::::
 
+::::{bzl:flag} relative_venv_symlinks
+
+Determines if relative symlinks are created using `declare_symlink()` at build
+time.
+
+This is only intended to work around
+[#2489](https://github.com/bazelbuild/rules_python/issues/2489), where some
+packaging rules don't support `declare_symlink()` artifacts.
+
+Values:
+* `yes`: Use `declare_symlink()` and create relative symlinks at build time.
+* `no`: Do not use `declare_symlink()`. Instead, the venv will be created at
+  runtime.
+
+:::{seealso}
+{envvar}`RULES_PYTHON_VENVS_ROOT` for customizing where the runtime venv
+is created.
+:::
+
+:::{versionadded} VERSION_NEXT_PATCH
+:::
+
 ::::{bzl:flag} bootstrap_impl
 Determine how programs implement their startup process.
 

docs/environment-variables.md

@@ -68,3 +68,16 @@ When `1`, debug information about coverage behavior is printed to stderr.
 
 When `1`, debug information from gazelle is printed to stderr.
 :::
+
+:::{envvar} RULES_PYTHON_VENVS_ROOT
+
+Directory to use as the root for creating venvs for binaries. Only applicable
+when {obj}`--relative_venvs_symlinks=no` is used. A binary will attempt to
+find a unique, reusable, location for itself within this directory. When set,
+the created venv is not deleted upon program exit; it is the responsibility of
+the caller to manage cleanup.
+
+If not set, then a temporary directory will be created and deleted upon program
+exit.
+
+:::

python/config_settings/BUILD.bazel

@@ -9,6 +9,7 @@ load(
     "LibcFlag",
     "PrecompileFlag",
     "PrecompileSourceRetentionFlag",
+    "RelativeVenvSymlinksFlag",
 )
 load(
     "//python/private/pypi:flags.bzl",
@@ -121,6 +122,13 @@ config_setting(
     visibility = ["//visibility:public"],
 )
 
+string_flag(
+    name = "relative_venv_symlinks",
+    build_setting_default = RelativeVenvSymlinksFlag.YES,
+    values = RelativeVenvSymlinksFlag.flag_values(),
+    visibility = ["//visibility:public"],
+)
+
 # pip.parse related flags
 
 string_flag(

python/private/flags.bzl

@@ -123,6 +123,21 @@ PrecompileSourceRetentionFlag = enum(
     get_effective_value = _precompile_source_retention_flag_get_effective_value,
 )
 
+def _relative_venv_symlinks_flag_get_value(ctx):
+    return ctx.attr._relative_venv_symlinks_flag[BuildSettingInfo].value
+
+# Decides if the venv created by bootstrap=script uses declare_file() to
+# create relative symlinks. Workaround for #2489 (packaging rules not supporting
+# declare_link() files).
+# buildifier: disable=name-conventions
+RelativeVenvSymlinksFlag = FlagEnum(
+    # Use declare_file() and relative symlinks in the venv
+    YES = "yes",
+    # Do not use declare_file() and relative symlinks in the venv
+    NO = "no",
+    get_value = _relative_venv_symlinks_flag_get_value,
+)
+
 # Used for matching freethreaded toolchains and would have to be used in wheels
 # as well.
 # buildifier: disable=name-conventions

python/private/py_executable.bzl

@@ -51,7 +51,7 @@ load(
     "target_platform_has_any_constraint",
     "union_attrs",
 )
-load(":flags.bzl", "BootstrapImplFlag")
+load(":flags.bzl", "BootstrapImplFlag", "RelativeVenvSymlinksFlag")
 load(":precompile.bzl", "maybe_precompile")
 load(":py_cc_link_params_info.bzl", "PyCcLinkParamsInfo")
 load(":py_executable_info.bzl", "PyExecutableInfo")
@@ -195,6 +195,10 @@ accepting arbitrary Python versions.
         "_python_version_flag": attr.label(
             default = "//python/config_settings:python_version",
         ),
+        "_relative_venv_symlinks_flag": attr.label(
+            default = "//python/config_settings:relative_venv_symlinks",
+            providers = [BuildSettingInfo],
+        ),
         "_windows_constraints": attr.label_list(
             default = [
                 "@platforms//os:windows",
@@ -512,7 +516,25 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
     ctx.actions.write(pyvenv_cfg, "")
 
     runtime = runtime_details.effective_runtime
-    if runtime.interpreter:
+    relative_venv_symlinks_enabled = (
+        RelativeVenvSymlinksFlag.get_value(ctx) == RelativeVenvSymlinksFlag.YES
+    )
+
+    if not relative_venv_symlinks_enabled:
+        if runtime.interpreter:
+            interpreter_actual_path = _runfiles_root_path(ctx, runtime.interpreter.short_path)
+        else:
+            interpreter_actual_path = runtime.interpreter_path
+
+        py_exe_basename = paths.basename(interpreter_actual_path)
+
+        # 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("{}/bin/{}".format(venv, py_exe_basename))
+        ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path))
+
+    elif runtime.interpreter:
         py_exe_basename = paths.basename(runtime.interpreter.short_path)
 
         # Even though ctx.actions.symlink() is used, using
@@ -571,6 +593,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
 
     return struct(
         interpreter = interpreter,
+        recreate_venv_at_runtime = not relative_venv_symlinks_enabled,
         # Runfiles root relative path or absolute path
         interpreter_actual_path = interpreter_actual_path,
         files_without_interpreter = [pyvenv_cfg, pth, site_init],
@@ -657,15 +680,13 @@ def _create_stage1_bootstrap(
     else:
         python_binary_path = runtime_details.executable_interpreter_path
 
-    if is_for_zip and venv:
-        python_binary_actual = venv.interpreter_actual_path
-    else:
-        python_binary_actual = ""
+    python_binary_actual = venv.interpreter_actual_path if venv else ""
 
     subs = {
         "%is_zipfile%": "1" if is_for_zip else "0",
         "%python_binary%": python_binary_path,
         "%python_binary_actual%": python_binary_actual,
+        "%recreate_venv_at_runtime%": str(int(venv.recreate_venv_at_runtime)) if venv else "0",
         "%target%": str(ctx.label),
         "%workspace_name%": ctx.workspace_name,
     }

python/private/stage1_bootstrap_template.sh

@@ -9,15 +9,17 @@ fi
 # runfiles-relative path
 STAGE2_BOOTSTRAP="%stage2_bootstrap%"
 
-# runfiles-relative path
+# runfiles-relative path to python interpreter to use
 PYTHON_BINARY='%python_binary%'
 # The path that PYTHON_BINARY should symlink to.
 # runfiles-relative path, absolute path, or single word.
-# Only applicable for zip files.
+# Only applicable for zip files or when venv is recreated at runtime.
 PYTHON_BINARY_ACTUAL="%python_binary_actual%"
 
 # 0 or 1
 IS_ZIPFILE="%is_zipfile%"
+# 0 or 1
+RECREATE_VENV_AT_RUNTIME="%recreate_venv_at_runtime%"
 
 if [[ "$IS_ZIPFILE" == "1" ]]; then
   # NOTE: Macs have an old version of mktemp, so we must use only the
@@ -104,6 +106,7 @@ python_exe=$(find_python_interpreter $RUNFILES_DIR $PYTHON_BINARY)
 # Zip files have to re-create the venv bin/python3 symlink because they
 # don't contain it already.
 if [[ "$IS_ZIPFILE" == "1" ]]; then
+  use_exec=0
   # It should always be under runfiles, but double check this. We don't
   # want to accidentally create symlinks elsewhere.
   if [[ "$python_exe" != $RUNFILES_DIR/* ]]; then
@@ -121,13 +124,60 @@ if [[ "$IS_ZIPFILE" == "1" ]]; then
     symlink_to=$(which $PYTHON_BINARY_ACTUAL)
     # Guard against trying to symlink to an empty value
     if [[ $? -ne 0 ]]; then
-      echo >&2 "ERROR: Python to use found on PATH: $PYTHON_BINARY_ACTUAL"
+      echo >&2 "ERROR: Python to use not found on PATH: $PYTHON_BINARY_ACTUAL"
       exit 1
     fi
   fi
   # The bin/ directory may not exist if it is empty.
   mkdir -p "$(dirname $python_exe)"
   ln -s "$symlink_to" "$python_exe"
+elif [[ "$RECREATE_VENV_AT_RUNTIME" == "1" ]]; then
+  runfiles_venv="$RUNFILES_DIR/$(dirname $(dirname $PYTHON_BINARY))"
+  if [[ -n "$RULES_PYTHON_VENVS_ROOT" ]]; then
+    use_exec=1
+    # Use our runfiles path as a unique, reusable, location for the
+    # binary-specific venv being created.
+    venv="$RULES_PYTHON_VENVS_ROOT/$(dirname $(dirname $PYTHON_BINARY))"
+    mkdir -p $RULES_PYTHON_VENVS_ROOT
+  else
+    # Re-exec'ing can't be used because we have to clean up the temporary
+    # venv directory that is created.
+    use_exec=0
+    venv=$(mktemp -d)
+    if [[ -n "$venv" && -z "${RULES_PYTHON_BOOTSTRAP_VERBOSE:-}" ]]; then
+      trap 'rm -fr "$venv"' EXIT
+    fi
+  fi
+
+  if [[ "$PYTHON_BINARY_ACTUAL" == /* ]]; then
+    # An absolute path, i.e. platform runtime, e.g. /usr/bin/python3
+    symlink_to=$PYTHON_BINARY_ACTUAL
+  elif [[ "$PYTHON_BINARY_ACTUAL" == */* ]]; then
+    # A runfiles-relative path
+    symlink_to="$RUNFILES_DIR/$PYTHON_BINARY_ACTUAL"
+  else
+    # A plain word, e.g. "python3". Symlink to where PATH leads
+    symlink_to=$(which $PYTHON_BINARY_ACTUAL)
+    # Guard against trying to symlink to an empty value
+    if [[ $? -ne 0 ]]; then
+      echo >&2 "ERROR: Python to use not found on PATH: $PYTHON_BINARY_ACTUAL"
+      exit 1
+    fi
+  fi
+  mkdir -p "$venv/bin"
+  # Match the basename; some tools, e.g. pyvenv key off the executable name
+  python_exe="$venv/bin/$(basename $PYTHON_BINARY_ACTUAL)"
+  if [[ ! -e "$python_exe" ]]; then
+    ln -s "$symlink_to" "$python_exe"
+  fi
+  if [[ ! -e "$venv/pyvenv.cfg" ]]; then
+    ln -s "$runfiles_venv/pyvenv.cfg" "$venv/pyvenv.cfg"
+  fi
+  if [[ ! -e "$venv/lib" ]]; then
+    ln -s "$runfiles_venv/lib" "$venv/lib"
+  fi
+else
+  use_exec=1
 fi
 
 # At this point, we should have a valid reference to the interpreter.
@@ -165,7 +215,6 @@ if [[ "$IS_ZIPFILE" == "1" ]]; then
   interpreter_args+=("-XRULES_PYTHON_ZIP_DIR=$zip_dir")
 fi
 
-
 export RUNFILES_DIR
 
 command=(
@@ -184,9 +233,10 @@ command=(
 # See https://github.com/bazelbuild/rules_python/issues/2043#issuecomment-2215469971
 # for more information.
 #
-# However, when running a zip file, we need to clean up the workspace after the
-# process finishes so control must return here.
-if [[ "$IS_ZIPFILE" == "1" ]]; then
+# However, we can't use exec when there is cleanup to do afterwards. Control
+# must return to this process so it can run the trap handlers. Such cases
+# occur when zip mode or recreate_venv_at_runtime creates temporary files.
+if [[ "$use_exec" == "0" ]]; then
   "${command[@]}"
   exit $?
 else

tests/bootstrap_impls/BUILD.bazel

@@ -61,6 +61,15 @@ sh_py_run_test(
     sh_src = "run_binary_zip_yes_test.sh",
 )
 
+sh_py_run_test(
+    name = "run_binary_relative_venv_symlinks_no_test",
+    bootstrap_impl = "script",
+    py_src = "bin.py",
+    relative_venv_symlinks = "no",
+    sh_src = "run_binary_relative_venv_symlinks_no_test.sh",
+    target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
+)
+
 sh_py_run_test(
     name = "run_binary_bootstrap_script_zip_yes_test",
     bootstrap_impl = "script",

tests/bootstrap_impls/bin.py

@@ -22,3 +22,4 @@
 print("PYTHONSAFEPATH:", os.environ.get("PYTHONSAFEPATH", "UNSET") or "EMPTY")
 print("sys.flags.safe_path:", sys.flags.safe_path)
 print("file:", __file__)
+print("sys.executable:", sys.executable)

tests/bootstrap_impls/run_binary_relative_venv_symlinks_no_test.sh

@@ -0,0 +1,56 @@
+# 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.
+
+# --- begin runfiles.bash initialization v3 ---
+# Copy-pasted from the Bazel Bash runfiles library v3.
+set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash
+source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$0.runfiles/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
+# --- end runfiles.bash initialization v3 ---
+set +e
+
+bin=$(rlocation $BIN_RLOCATION)
+if [[ -z "$bin" ]]; then
+  echo "Unable to locate test binary: $BIN_RLOCATION"
+  exit 1
+fi
+actual=$($bin)
+
+function expect_match() {
+  local expected_pattern=$1
+  local actual=$2
+  if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then
+    echo "expected to match: $expected_pattern"
+    echo "===== actual START ====="
+    echo "$actual"
+    echo "===== actual END ====="
+    echo
+    touch EXPECTATION_FAILED
+    return 1
+  fi
+}
+
+expect_match "sys.executable:.*tmp.*python3" "$actual"
+
+venvs_root=$(mkdir -d)
+
+actual=$(RULES_PYTHON_VENVS_ROOT=$venvs_root $bin)
+expect_match "sys.executable:.*$venvs_root" "$actual"
+
+# Exit if any of the expects failed
+[[ ! -e EXPECTATION_FAILED ]]