make the linking algo a little bit more robust by adding a test for external data inclusion.

Author: aignas

.bazelrc

@@ -4,8 +4,8 @@
 # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it)
 # To update these lines, execute
 # `bazel run @rules_bazel_integration_test//tools:update_deleted_packages`
-build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma,tests/modules/other/nspkg_single,tests/modules/other/simple_v1,tests/modules/other/simple_v2
-query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma,tests/modules/other/nspkg_single,tests/modules/other/simple_v1,tests/modules/other/simple_v2
+build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/another_module,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma,tests/modules/other/nspkg_single,tests/modules/other/simple_v1,tests/modules/other/simple_v2,tests/modules/other/with_external_data
+query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,gazelle,gazelle/manifest,gazelle/manifest/generate,gazelle/manifest/hasher,gazelle/manifest/test,gazelle/modules_mapping,gazelle/python,gazelle/pythonconfig,gazelle/python/private,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/custom_commands,tests/integration/ignore_root_user_error,tests/integration/ignore_root_user_error/submodule,tests/integration/local_toolchains,tests/integration/pip_parse,tests/integration/pip_parse/empty,tests/integration/py_cc_toolchain_registered,tests/modules/another_module,tests/modules/other,tests/modules/other/nspkg_delta,tests/modules/other/nspkg_gamma,tests/modules/other/nspkg_single,tests/modules/other/simple_v1,tests/modules/other/simple_v2,tests/modules/other/with_external_data
 
 test --test_output=errors
 

MODULE.bazel

@@ -86,6 +86,7 @@ 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)
 bazel_dep(name = "other", version = "0", dev_dependency = True)
+bazel_dep(name = "another_module", version = "0", 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.
@@ -116,6 +117,11 @@ local_path_override(
     path = "tests/modules/other",
 )
 
+local_path_override(
+    module_name = "another_module",
+    path = "tests/modules/another_module",
+)
+
 dev_python = use_extension(
     "//python/extensions:python.bzl",
     "python",

python/private/py_library.bzl

@@ -269,7 +269,6 @@ def _get_venv_symlinks(ctx, dist_info_metadata):
     # directories that _do_ have an `__init__.py` file and treat those as
     # the path to symlink to.
 
-    repo_runfiles_dirname = None
     dir_symlinks = {}  # dirname -> runfile path
     venv_symlinks = []
     package = None
@@ -286,45 +285,36 @@ def _get_venv_symlinks(ctx, dist_info_metadata):
             version.normalize(version_str),
         )
 
-        repo_runfiles_dirname = runfiles_root_path(ctx, dist_info_metadata.short_path).partition("/")[0]
-        venv_symlinks.append(VenvSymlinkEntry(
-            kind = VenvSymlinkKind.LIB,
-            link_to_path = paths.join(repo_runfiles_dirname, site_packages_root, dist_info_dir),
-            package = package,
-            venv_path = dist_info_dir,
-        ))
-
     for src in ctx.files.srcs + ctx.files.data:
         path = _repo_relative_short_path(src.short_path)
         if not path.startswith(site_packages_root):
             continue
         path = path.removeprefix(site_packages_root)
         dir_name, _, filename = path.rpartition("/")
 
-        if dist_info_metadata and dir_name in path:
-            # we have already handled the stuff
+        if dir_name in dir_symlinks:
+            # we already have this dir.
             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)
             # or a directory with `__init__.py` file that potentially also needs to be
             # symlinked.
             #
             # This could be also regular files, that just need to be symlinked, so we will
             # add the directory here.
-            repo_runfiles_dirname = runfiles_root_path(ctx, src.short_path).partition("/")[0]
-            dir_symlinks[dir_name] = repo_runfiles_dirname
+            dir_symlinks[dir_name] = runfiles_dir_name
         elif src.extension in PYTHON_FILE_EXTENSIONS:
-            repo_runfiles_dirname = runfiles_root_path(ctx, src.short_path).partition("/")[0]
-
             # This would be files that do not have directories and we just need to add
             # direct symlinks to them as is:
-            venv_symlinks.append(VenvSymlinkEntry(
+            entry = VenvSymlinkEntry(
                 kind = VenvSymlinkKind.LIB,
-                link_to_path = paths.join(repo_runfiles_dirname, site_packages_root, filename),
+                link_to_path = paths.join(runfiles_dir_name, site_packages_root, filename),
                 package = package,
                 venv_path = filename,
-            ))
+            )
+            venv_symlinks.append(entry)
 
     # Sort so that we encounter `foo` before `foo/bar`. This ensures we
     # see the top-most explicit package first.
@@ -342,12 +332,14 @@ def _get_venv_symlinks(ctx, dist_info_metadata):
 
     for dirname in first_level_explicit_packages:
         prefix = dir_symlinks[dirname]
-        venv_symlinks.append(VenvSymlinkEntry(
+        entry = VenvSymlinkEntry(
             kind = VenvSymlinkKind.LIB,
             link_to_path = paths.join(prefix, site_packages_root, dirname),
             package = package,
             venv_path = dirname,
-        ))
+        )
+        venv_symlinks.append(entry)
+
     return venv_symlinks
 
 def _repo_relative_short_path(short_path):

tests/modules/another_module/BUILD.bazel

@@ -0,0 +1,5 @@
+filegroup(
+    name = "data",
+    srcs = ["another_module_data.txt"],
+    visibility = ["//visibility:public"],
+)

tests/modules/another_module/MODULE.bazel

@@ -0,0 +1 @@
+module(name = "another_module")

tests/modules/another_module/another_module_data.txt

@@ -0,0 +1 @@
+print("token")

tests/modules/other/MODULE.bazel

@@ -1,3 +1,5 @@
 module(name = "other")
 
 bazel_dep(name = "rules_python", version = "0")
+bazel_dep(name = "bazel_skylib", version = "1.7.1")
+bazel_dep(name = "another_module", version = "0")

tests/modules/other/with_external_data/BUILD.bazel

@@ -0,0 +1,23 @@
+load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
+load("@rules_python//python:py_library.bzl", "py_library")
+
+package(default_visibility = ["//visibility:public"])
+
+# The users may include data through other repos via annotations and copy_file
+# just add this edge case.
+#
+# NOTE: if the data is not copied to `site-packages/<some dir>` then it will not
+# appear.
+copy_file(
+    name = "external_data",
+    src = "@another_module//:data",
+    out = "site-packages/external_data/another_module_data.txt",
+)
+
+py_library(
+    name = "with_external_data",
+    srcs = ["site-packages/with_external_data.py"],
+    data = [":external_data"],
+    experimental_venvs_site_packages = "@rules_python//python/config_settings:venvs_site_packages",
+    imports = [package_name() + "/site-packages"],
+)

tests/modules/other/with_external_data/site-packages/with_external_data.py

@@ -0,0 +1 @@
+# Intentionally blank

tests/venv_site_packages_libs/BUILD.bazel

@@ -29,5 +29,6 @@ py_reconfig_test(
         "@other//nspkg_delta",
         "@other//nspkg_gamma",
         "@other//nspkg_single",
+        "@other//with_external_data",
     ],
 )