basic impl

Author: rickeylev

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

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)
@@ -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("../"):

tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl

@@ -4,7 +4,25 @@ load("@bazel_skylib//lib:paths.bzl", "paths")
 load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
 load("@rules_testing//lib:test_suite.bzl", "test_suite")
 load("//python/private:py_info.bzl", "VenvSymlinkEntry", "VenvSymlinkKind")  # buildifier: disable=bzl-visibility
-load("//python/private:venv_runfiles.bzl", "build_link_map")  # buildifier: disable=bzl-visibility
+load("//python/private:venv_runfiles.bzl", "build_link_map", "get_venv_symlinks")  # buildifier: disable=bzl-visibility
+
+def _empty_files_impl(ctx):
+    files = []
+    for p in ctx.attr.paths:
+        f = ctx.actions.declare_file(p)
+        ctx.actions.write(output = f, content = "")
+        files.append(f)
+    return [DefaultInfo(files = depset(files))]
+
+empty_files = rule(
+    implementation = _empty_files_impl,
+    attrs = {
+        "paths": attr.string_list(
+            doc = "A list of paths to create as files.",
+            mandatory = True,
+        ),
+    },
+)
 
 _tests = []
 
@@ -240,6 +258,62 @@ def _test_multiple_venv_symlink_kinds_impl(env, _):
         VenvSymlinkKind.INCLUDE,
     ])
 
+def _test_shared_library_symlinking(name):
+    empty_files(
+        name = name + "_files",
+        # NOTE: Test relies upon order
+        paths = [
+            "site-packages/bar/libs/liby.so",
+            "site-packages/bar/x.py",
+            "site-packages/bar/y.so",
+            "site-packages/foo.libs/libx.so",
+            "site-packages/foo/a.py",
+            "site-packages/foo/b.so",
+        ],
+    )
+    analysis_test(
+        name = name,
+        impl = _test_shared_library_symlinking_impl,
+        target = name + "_files",
+    )
+
+_tests.append(_test_shared_library_symlinking)
+
+def _test_shared_library_symlinking_impl(env, target):
+    srcs = target.files.to_list()
+    actual_entries = get_venv_symlinks(
+        _ctx(),
+        srcs,
+        package = "foo",
+        version_str = "1.0",
+        site_packages_root = env.ctx.label.package + "/site-packages",
+    )
+
+    actual = [e for e in actual_entries if e.venv_path == "foo.libs/libx.so"]
+    if not actual:
+        fail("Did not find VenvSymlinkEntry with venv_path equal to foo.libs/libx.so. " +
+             "Found: {}".format(actual_entries))
+    elif len(actual) > 1:
+        fail("Found multiple entries with venv_path=foo.libs/libx.so. " +
+             "Found: {}".format(actual_entries))
+    actual = actual[0]
+
+    actual_files = actual.files.to_list()
+    expected_lib_dso = [f for f in srcs if f.basename == "libx.so"]
+    env.expect.that_collection(actual_files).contains_exactly(expected_lib_dso)
+
+    entries = actual_entries
+    actual = build_link_map(_ctx(), entries)
+
+    expected_libs = {
+        "bar/libs/liby.so": srcs[0],
+        "bar/x.py": srcs[1],
+        "bar/y.so": srcs[2],
+        "foo": "_main/tests/venv_site_packages_libs/app_files_building/site-packages/foo",
+        "foo.libs/libx.so": srcs[3],
+    }
+    env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs)
+
 def app_files_building_test_suite(name):
     test_suite(
         name = name,