switch to internal attr to trigger behavior

Author: rickeylev

CHANGELOG.md

@@ -91,6 +91,10 @@ Unreleased changes template.
   please check the {obj}`uv.configure` tag class.
 * Add support for riscv64 linux platform.
 * (toolchains) Add python 3.13.2 and 3.12.9 toolchains
+* (providers) (experimental) {obj}`PyInfo.site_packages_symlinks` field added to
+  allow specifying links to create within the venv site packages (only
+  applicable with {obj}`--bootstrap_impl=script`)
+  ([#2156](https://github.com/bazelbuild/rules_python/issues/2156)).
 
 {#v0-0-0-removed}
 ### Removed
@@ -156,10 +160,6 @@ Unreleased changes template.
   which allows pass arguments to the interpreter before the regular args.
 * (rules) Added {obj}`main_module` attribute to `py_binary` and `py_test`,
   which allows specifying a module name to run (i.e. `python -m <module>`).
-* (providers) (experimental) {obj}`PyInfo.site_packages_symlinks` field added to
-  allow specifying links to create within the venv site packages (only
-  applicable with {obj}`--bootstrap_impl=script`)
-  ([#2156](https://github.com/bazelbuild/rules_python/issues/2156)).
 
 {#v1-3-0-removed}
 ### Removed

docs/_includes/experimental_api.md

@@ -0,0 +1,5 @@
+:::{warning}
+
+**Experimental API.** This API is still under development and may change or be
+removed without notice.
+:::

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

@@ -215,13 +215,19 @@ Values:
 
 ::::
 
-:::{flag} venv_site_packages
+:::{flag} venvs_site_packages
+
+Determines if libraries use a site-packages layout for their files.
+
+Note this flag only affects PyPI dependencies of `--bootstrap_impl=script` binaries
+
+:::{include} /_includes/experimental_api.md
+:::
 
-Determines if
 
 Values:
-* `no` (default)
-* `yes`
+* `no` (default): Make libraries importable by adding to `sys.path`
+* `yes`: Make libraries importable by creating paths in a binary's site-packages directory.
 ::::
 
 ::::{bzl:flag} bootstrap_impl

python/config_settings/BUILD.bazel

@@ -9,7 +9,7 @@ load(
     "LibcFlag",
     "PrecompileFlag",
     "PrecompileSourceRetentionFlag",
-    "VenvSitePackages",
+    "VenvsSitePackages",
     "VenvsUseDeclareSymlinkFlag",
 )
 load(
@@ -197,8 +197,8 @@ string_flag(
 )
 
 string_flag(
-    name = "venv_site_packages",
-    build_setting_default = VenvSitePackages.NO,
+    name = "venvs_site_packages",
+    build_setting_default = VenvsSitePackages.NO,
     # NOTE: Only public because it is used in pip hub repos.
     visibility = ["//visibility:public"],
 )

python/features.bzl

@@ -31,15 +31,6 @@ def _features_typedef():
     :::
     ::::
 
-    ::::{field} site_packages_root_attr
-    :type: bool
-
-    True if the {obj}`site_packages_root` attribute is available.
-
-    :::{versionadded} VERSION_NEXT_FEATURE
-    :::
-    ::::
-
     ::::{field} uses_builtin_rules
     :type: bool
 
@@ -61,7 +52,6 @@ def _features_typedef():
 features = struct(
     TYPEDEF = _features_typedef,
     precompile = True,
-    site_packages_root_attr = True,
     uses_builtin_rules = not config.enable_pystar,
     version = _VERSION_PRIVATE if "$Format" not in _VERSION_PRIVATE else "",
 )

python/private/attributes.bzl

@@ -236,8 +236,6 @@ that depend on this rule. The strings are repo-runfiles-root relative,
 
 Absolute paths (paths that start with `/`) and paths that references a path
 above the execution root are not allowed and will result in an error.
-
-This attribute is mutually exclusive with the {attr}`site_packages_root` attribute.
 """,
     ),
 }

python/private/flags.bzl

@@ -118,10 +118,20 @@ VenvsUseDeclareSymlinkFlag = FlagEnum(
     get_value = _venvs_use_declare_symlink_flag_get_value,
 )
 
-# Decides if libraries
-VenvSitePackages = FlagEnum(
+def _venvs_site_packages_is_enabled(ctx):
+    if not ctx.attr.experimental_venvs_site_packages:
+        return False
+    flag_value = ctx.attr.experimental_venvs_site_packages[BuildSettingInfo].value
+    return flag_value == VenvsSitePackages.YES
+
+# Decides if libraries try to use a site-packages layout using site_packages_symlinks
+# buildifier: disable=name-conventions
+VenvsSitePackages = FlagEnum(
+    # Use site_packages_symlinks
     YES = "yes",
+    # Don't use site_packages_symlinks
     NO = "no",
+    is_enabled = _venvs_site_packages_is_enabled,
 )
 
 # Used for matching freethreaded toolchains and would have to be used in wheels

python/private/py_info.bzl

@@ -158,6 +158,9 @@ Tuples of `(runfiles_path, site_packages_path)`. Where
   the code in `runfiles_path` available for import. Note that this
   is created as a "raw" symlink (via `declare_symlink`).
 
+:::{include} /_includes/experimental_api.md
+:::
+
 :::{tip}
 The topological ordering means dependencies earlier and closer to the consumer
 have precedence. This allows e.g. a binary to add dependencies that override

python/private/py_library.bzl

@@ -39,9 +39,8 @@ load(
     "filter_to_py_srcs",
     "get_imports",
     "runfiles_root_path",
-    "union_attrs",
 )
-load(":flags.bzl", "AddSrcsToRunfilesFlag", "PrecompileFlag")
+load(":flags.bzl", "AddSrcsToRunfilesFlag", "PrecompileFlag", "VenvsSitePackages")
 load(":precompile.bzl", "maybe_precompile")
 load(":py_cc_link_params_info.bzl", "PyCcLinkParamsInfo")
 load(":py_internal.bzl", "py_internal")
@@ -59,38 +58,25 @@ LIBRARY_ATTRS = dicts.add(
     PY_SRCS_ATTRS,
     IMPORTS_ATTRS,
     {
-        "experimental_venv_site_packages": lambda: attrb.Bool(
+        "experimental_venvs_site_packages": lambda: attrb.Label(
             doc = """
-Internal attribute. Should only be set by rules_python-internal code.
+**INTERNAL ATTRIBUTE. SHOULD ONLY BE SET BY rules_python-INTERNAL CODE.**
 
 :::{include} /_includes/experimental_api.md
 :::
 
-If true, the library consults {flag}`//python/config_settings:venv_site_packages`
-to decide if `srcs` is a site-packages relative layout.
-""",
-            default = False,
-        ),
-        "XXsite_packages_root": lambda: attrb.String(
-            doc = """
-Package relative prefix to remove from `srcs` for site-packages layouts.
-
-This attribute is mutually exclusive with the {attr}`imports` attribute.
+A flag that decides whether the library should treat its sources as a
+site-packages layout.
 
-When set, `srcs` are interpreted to have a file layout as if they were installed
-in site-packages. This attribute specifies the directory within `srcs` to treat
-as the site-packages root so the correct site-packages relative paths for
-the files can be computed.
+When the flag is `yes`, then the `srcs` files are treated as a site-packages
+layout that is relative to the `imports` attribute. The `imports` attribute
+can have only a single element. It is a repo-relative runfiles path.
 
-:::{note}
-This string is relative to the target's *Bazel package*. e.g. Relative to the
-directory with the BUILD file that defines the target (the same as how e.g.
-`srcs`).
-:::
-
-For example, given `srcs=["site-packages/foo/bar.py"]`, specifying
-`site_packages_root="site-packages/" means `foo/bar.py` is the file path
-under the binary's venv site-packages directory that should be made available.
+For example, in the `my/pkg/BUILD.bazel` file, given
+`srcs=["site-packages/foo/bar.py"]`, specifying
+`imports=["my/pkg/site-packages"]` means `foo/bar.py` is the file path
+under the binary's venv site-packages directory that should be made available (i.e.
+`import foo.bar` will work).
 
 `__init__.py` files are treated specially to provide basic support for [implicit
 namespace packages](
@@ -169,17 +155,7 @@ def py_library_impl(ctx, *, semantics):
     imports = []
     site_packages_symlinks = []
 
-    ##if ctx.attr.imports and ctx.attr.site_packages_root:
-    ##    fail(("Only one of the `imports` or `site_packages_root` attributes " +
-    ##          "can be set: site_packages_root={}, imports={}").format(
-    ##        ctx.attr.site_packages_root,
-    ##        ctx.attr.imports,
-    ##    ))
-    ##elif ctx.attr.site_packages_root:
-    ##    site_packages_symlinks = _get_site_packages_symlinks(ctx)
-    ##elif ctx.attr.imports:
-    ##    imports = collect_imports(ctx, semantics)
-    imports, site_packages_symlinks = _get_imports_and_site_packages_symlinks(ctx)
+    imports, site_packages_symlinks = _get_imports_and_site_packages_symlinks(ctx, semantics)
 
     cc_info = semantics.get_cc_info_for_library(ctx)
     py_info, deps_transitive_sources, builtins_py_info = create_py_info(
@@ -231,8 +207,7 @@ Source files are no longer added to the runfiles directly.
 def _get_imports_and_site_packages_symlinks(ctx, semantics):
     imports = depset()
     site_packages_symlinks = depset()
-    if (ctx.attr.experimental_venv_site_packages and
-        ctx.attr._venv_site_packages_flag[BuildSettingInfo].value):
+    if VenvsSitePackages.is_enabled(ctx):
         site_packages_symlinks = _get_site_packages_symlinks(ctx)
     else:
         imports = collect_imports(ctx, semantics)
@@ -247,6 +222,14 @@ def _get_site_packages_symlinks(ctx):
     else:
         site_packages_root = imports[0]
 
+    if site_packages_root.endswith("/"):
+        fail("should not end in slash")
+    if site_packages_root.startswith("/"):
+        fail("cannot start with slash")
+
+    # Append slash to prevent incorrectly prefix-string matches
+    site_packages_root += "/"
+
     # We have to build a list of (runfiles path, site-packages path) pairs of
     # the files to create in the consuming binary's venv site-packages directory.
     # To minimize the number of files to create, we just return the paths
@@ -262,7 +245,7 @@ def _get_site_packages_symlinks(ctx):
     # directories that _do_ have an `__init__.py` file and treat those as
     # the path to symlink to.
 
-    site_packages_root = paths.join(ctx.label.package, site_packages_root)
+    ##site_packages_root = paths.join(ctx.label.package, site_packages_root)
     repo_runfiles_dirname = None
     dirs_with_init = {}  # dirname -> runfile path
     for src in ctx.files.srcs:
@@ -302,6 +285,8 @@ def _get_site_packages_symlinks(ctx):
             paths.join(repo_runfiles_dirname, site_packages_root, dirname),
             dirname,
         ))
+    if not site_packages_symlinks:
+        fail("empty?", ctx.label, site_packages_root, ctx.files.srcs[0])
     return site_packages_symlinks
 
 def _repo_relative_short_path(short_path):

python/private/pypi/whl_library_targets.bzl

@@ -266,6 +266,7 @@ def whl_library_targets(
             ),
             tags = tags,
             visibility = impl_vis,
+            experimental_venvs_site_packages = Label("@rules_python//python/config_settings:venvs_site_packages"),
         )
 
 def _config_settings(dependencies_by_platform, native = native, **kwargs):