From add0c262ed6cabe50df9266157ac74aca384f3f9 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 9 Oct 2025 14:44:07 -0700 Subject: [PATCH] basic impl --- python/private/py_info.bzl | 17 +++++++++--- python/private/venv_runfiles.bzl | 46 ++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/python/private/py_info.bzl b/python/private/py_info.bzl index 9318347819..6118ba3974 100644 --- a/python/private/py_info.bzl +++ b/python/private/py_info.bzl @@ -47,12 +47,17 @@ VenvSymlinkKind = struct( INCLUDE = "INCLUDE", ) +def _VenvSymlinkEntry_init(**kwargs): + kwargs.setdefault("link_to_file", None) + return kwargs + # A provider is used for memory efficiency. # buildifier: disable=name-conventions -VenvSymlinkEntry = provider( +VenvSymlinkEntry, _ = provider( doc = """ An entry in `PyInfo.venv_symlinks` """, + init = _VenvSymlinkEntry_init, fields = { "files": """ :type: depset[File] @@ -67,12 +72,18 @@ if one adds files to `venv_path=a/` and another adds files to `venv_path=a/b/`. One of the {obj}`VenvSymlinkKind` values. It represents which directory within the venv to create the path under. +""", + "link_to_file": """ +:type: File | None + +A file that `venv_path` should point to. The file to link to should also be in +`files`. """, "link_to_path": """ :type: str | None -A runfiles-root relative path that `venv_path` will symlink to. If `None`, -it means to not create a symlink. +A runfiles-root relative path that `venv_path` will symlink to (if +`link_to_file` is None). If `None`, it means to not create it in the venv. """, "package": """ :type: str | None diff --git a/python/private/venv_runfiles.bzl b/python/private/venv_runfiles.bzl index 291920b848..1cdd1e82e8 100644 --- a/python/private/venv_runfiles.bzl +++ b/python/private/venv_runfiles.bzl @@ -81,7 +81,7 @@ def build_link_map(ctx, entries): Returns: {type}`dict[str, dict[str, str|File]]` Mappings of venv paths to their backing files. The first key is a `VenvSymlinkKind` value. - The inner dict keys are venv paths relative to the kind's diretory. The + The inner dict keys are venv paths relative to the kind's directory. The inner dict values are strings or Files to link to. """ @@ -116,7 +116,10 @@ def build_link_map(ctx, entries): # If there's just one group, we can symlink to the directory if len(group) == 1: entry = group[0] - keep_kind_link_map[entry.venv_path] = entry.link_to_path + if entry.link_to_file: + keep_kind_link_map[entry.venv_path] = entry.link_to_file + else: + keep_kind_link_map[entry.venv_path] = entry.link_to_path else: # Merge a group of overlapping prefixes _merge_venv_path_group(ctx, group, keep_kind_link_map) @@ -138,7 +141,7 @@ def _group_venv_path_entries(entries): """ # Sort so order is top-down, ensuring grouping by short common prefix - entries = sorted(entries, key = lambda e: e.venv_path) + entries = sorted(entries, key = lambda e: tuple(e.venv_path.split("/"))) groups = [] current_group = None @@ -170,7 +173,9 @@ def _merge_venv_path_group(ctx, group, keep_map): # TODO: Compute the minimum number of entries to create. This can't avoid # flattening the files depset, but can lower the number of materialized # files significantly. Usually overlaps are limited to a small number - # of directories. + # of directories. Note that, when doing so, shared libraries need to + # be symlinked directory, not the directory containing them, due to + # symlink resolution semantics on Linux. for entry in group: prefix = entry.venv_path for file in entry.files.to_list(): @@ -247,13 +252,27 @@ def get_venv_symlinks(ctx, files, package, version_str, site_packages_root): continue path = path.removeprefix(site_packages_root) dir_name, _, filename = path.rpartition("/") + runfiles_dir_name, _, _ = runfiles_root_path(ctx, src.short_path).partition("/") + + if _is_linker_loaded_library(filename): + entry = VenvSymlinkEntry( + kind = VenvSymlinkKind.LIB, + # todo: omit setting link_to_path + link_to_path = paths.join(runfiles_dir_name, site_packages_root, filename), + link_to_file = src, + package = package, + version = version_str, + venv_path = path, + files = depset([src]), + ) + venv_symlinks.append(entry) + continue if dir_name in dir_symlinks: # we already have this dir, this allows us to short-circuit since most of the # ctx.files.data might share the same directories as ctx.files.srcs continue - runfiles_dir_name, _, _ = runfiles_root_path(ctx, src.short_path).partition("/") if dir_name: # This can be either: # * a directory with libs (e.g. numpy.libs, created by auditwheel) @@ -276,6 +295,8 @@ def get_venv_symlinks(ctx, files, package, version_str, site_packages_root): files = depset([src]), ) venv_symlinks.append(entry) + else: + fail("hit", src) # Sort so that we encounter `foo` before `foo/bar`. This ensures we # see the top-most explicit package first. @@ -310,6 +331,21 @@ def get_venv_symlinks(ctx, files, package, version_str, site_packages_root): return venv_symlinks +def _is_linker_loaded_library(filename): + """Tells if a filename is one that `dlopen()` or the runtime linker handle. + + Notably, it should return False for Python C extension modules. + """ + if not filename.startswith("lib"): + return False + if filename.endswith((".so", ".dylib")): + return True + + # Versioned library, e.g. libfoo.so.1.2.3 + if ".so." in filename: + return True + return False + def _repo_relative_short_path(short_path): # Convert `../+pypi+foo/some/file.py` to `some/file.py` if short_path.startswith("../"):