From df77fa51efec0411521e7be5eec79763b1978392 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 9 Sep 2025 00:51:24 -0700 Subject: [PATCH 01/17] fix: site packages with pkgutil --- python/private/common.bzl | 34 ++++ python/private/py_executable.bzl | 125 +----------- python/private/py_info.bzl | 8 + python/private/py_library.bzl | 16 +- python/private/venv_runfiles.bzl | 186 ++++++++++++++++++ tests/venv_site_packages_libs/BUILD.bazel | 2 + .../app_files_building/BUILD.bazel | 5 + .../app_files_building_tests.bzl | 77 ++++++++ tests/venv_site_packages_libs/bin.py | 12 ++ .../pkgutil_top/BUILD.bazel | 10 + .../site-packages/pkgutil_top/__init__.py | 2 + .../site-packages/pkgutil_top/top.py | 0 .../pkgutil_top_sub/BUILD.bazel | 10 + .../site-packages/pkgutil_top/sub/__init__.py | 1 + .../site-packages/pkgutil_top/sub/suba.py | 0 15 files changed, 365 insertions(+), 123 deletions(-) create mode 100644 python/private/venv_runfiles.bzl create mode 100644 tests/venv_site_packages_libs/app_files_building/BUILD.bazel create mode 100644 tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl create mode 100644 tests/venv_site_packages_libs/pkgutil_top/BUILD.bazel create mode 100644 tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/__init__.py create mode 100644 tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/top.py create mode 100644 tests/venv_site_packages_libs/pkgutil_top_sub/BUILD.bazel create mode 100644 tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/__init__.py create mode 100644 tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/suba.py diff --git a/python/private/common.bzl b/python/private/common.bzl index 9fc366818d..33b175c247 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl @@ -495,6 +495,9 @@ _BOOL_TYPE = type(True) def is_bool(v): return type(v) == _BOOL_TYPE +def is_file(v): + return type(v) == "File" + def target_platform_has_any_constraint(ctx, constraints): """Check if target platform has any of a list of constraints. @@ -511,6 +514,37 @@ def target_platform_has_any_constraint(ctx, constraints): return True return False +def relative_path(from_, to): + """Compute a relative path from one path to another. + + Args: + from_: {type}`str` the starting directory. Note that it should be + a directory because relative-symlinks are relative to the + directory the symlink resides in. + to: {type}`str` the path that `from_` wants to point to + + Returns: + {type}`str` a relative path + """ + from_parts = from_.split("/") + to_parts = to.split("/") + + # Strip common leading parts from both paths + n = min(len(from_parts), len(to_parts)) + for _ in range(n): + if from_parts[0] == to_parts[0]: + from_parts.pop(0) + to_parts.pop(0) + else: + break + + # Impossible to compute a relative path without knowing what ".." is + if from_parts and from_parts[0] == "..": + fail("cannot compute relative path from '%s' to '%s'", from_, to) + + parts = ([".."] * len(from_parts)) + to_parts + return paths.join(*parts) + def runfiles_root_path(ctx, short_path): """Compute a runfiles-root relative path from `File.short_path` diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index fa80ea5105..40b2a43252 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -48,6 +48,7 @@ load( "filter_to_py_srcs", "get_imports", "is_bool", + "relative_path", "runfiles_root_path", "target_platform_has_any_constraint", ) @@ -67,6 +68,7 @@ load( TOOLCHAIN_TYPE = "TARGET_TOOLCHAIN_TYPE", ) load(":transition_labels.bzl", "TRANSITION_LABELS") +load(":venv_runfiles.bzl", "create_venv_app_files") _py_builtins = py_internal _EXTERNAL_PATH_PREFIX = "external" @@ -504,37 +506,6 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv): ) return output -def relative_path(from_, to): - """Compute a relative path from one path to another. - - Args: - from_: {type}`str` the starting directory. Note that it should be - a directory because relative-symlinks are relative to the - directory the symlink resides in. - to: {type}`str` the path that `from_` wants to point to - - Returns: - {type}`str` a relative path - """ - from_parts = from_.split("/") - to_parts = to.split("/") - - # Strip common leading parts from both paths - n = min(len(from_parts), len(to_parts)) - for _ in range(n): - if from_parts[0] == to_parts[0]: - from_parts.pop(0) - to_parts.pop(0) - else: - break - - # Impossible to compute a relative path without knowing what ".." is - if from_parts and from_parts[0] == "..": - fail("cannot compute relative path from '%s' to '%s'", from_, to) - - parts = ([".."] * len(from_parts)) + to_parts - return paths.join(*parts) - # Create a venv the executable can use. # For venv details and the venv startup process, see: # * https://docs.python.org/3/library/venv.html @@ -641,9 +612,9 @@ def _create_venv(ctx, output_prefix, imports, runtime_details): VenvSymlinkKind.BIN: bin_dir, VenvSymlinkKind.LIB: site_packages, } - venv_symlinks = _create_venv_symlinks(ctx, venv_dir_map) + venv_app_files = create_venv_app_files(ctx, venv_dir_map) - files_without_interpreter = [pth, site_init] + venv_symlinks + files_without_interpreter = [pth, site_init] + venv_app_files if pyvenv_cfg: files_without_interpreter.append(pyvenv_cfg) @@ -668,94 +639,6 @@ def _create_venv(ctx, output_prefix, imports, runtime_details): ), ) -def _create_venv_symlinks(ctx, venv_dir_map): - """Creates symlinks within the venv. - - Args: - ctx: current rule ctx - venv_dir_map: mapping of VenvSymlinkKind constants to the - venv path. - - Returns: - {type}`list[File]` list of the File symlink objects created. - """ - - # maps venv-relative path to the runfiles path it should point to - entries = depset( - transitive = [ - dep[PyInfo].venv_symlinks - for dep in ctx.attr.deps - if PyInfo in dep - ], - ).to_list() - - link_map = _build_link_map(entries) - venv_files = [] - for kind, kind_map in link_map.items(): - base = venv_dir_map[kind] - for venv_path, link_to in kind_map.items(): - venv_link = ctx.actions.declare_symlink(paths.join(base, venv_path)) - venv_link_rf_path = runfiles_root_path(ctx, venv_link.short_path) - rel_path = relative_path( - # dirname is necessary because a relative symlink is relative to - # the directory the symlink resides within. - from_ = paths.dirname(venv_link_rf_path), - to = link_to, - ) - ctx.actions.symlink(output = venv_link, target_path = rel_path) - venv_files.append(venv_link) - - return venv_files - -def _build_link_map(entries): - # dict[str package, dict[str kind, dict[str rel_path, str link_to_path]]] - pkg_link_map = {} - - # dict[str package, str version] - version_by_pkg = {} - - for entry in entries: - link_map = pkg_link_map.setdefault(entry.package, {}) - kind_map = link_map.setdefault(entry.kind, {}) - - if version_by_pkg.setdefault(entry.package, entry.version) != entry.version: - # We ignore duplicates by design. - continue - elif entry.venv_path in kind_map: - # We ignore duplicates by design. - continue - else: - kind_map[entry.venv_path] = entry.link_to_path - - # An empty link_to value means to not create the site package symlink. Because of the - # ordering, this allows binaries to remove entries by having an earlier dependency produce - # empty link_to values. - for link_map in pkg_link_map.values(): - for kind, kind_map in link_map.items(): - for dir_path, link_to in kind_map.items(): - if not link_to: - kind_map.pop(dir_path) - - # dict[str kind, dict[str rel_path, str link_to_path]] - keep_link_map = {} - - # Remove entries that would be a child path of a created symlink. - # Earlier entries have precedence to match how exact matches are handled. - for link_map in pkg_link_map.values(): - for kind, kind_map in link_map.items(): - keep_kind_map = keep_link_map.setdefault(kind, {}) - for _ in range(len(kind_map)): - if not kind_map: - break - dirname, value = kind_map.popitem() - keep_kind_map[dirname] = value - prefix = dirname + "/" # Add slash to prevent /X matching /XY - for maybe_suffix in kind_map.keys(): - maybe_suffix += "/" # Add slash to prevent /X matching /XY - if maybe_suffix.startswith(prefix) or prefix.startswith(maybe_suffix): - kind_map.pop(maybe_suffix) - return keep_link_map - def _map_each_identity(v): return v diff --git a/python/private/py_info.bzl b/python/private/py_info.bzl index 31df5cfbde..f96dec554b 100644 --- a/python/private/py_info.bzl +++ b/python/private/py_info.bzl @@ -56,6 +56,14 @@ VenvSymlinkEntry = provider( An entry in `PyInfo.venv_symlinks` """, fields = { + "files": """ +:type: depset[File] + +Files under `link_to_path`. + +This is only used when multiple targets have overlapping `venv_path` paths. e.g. +if one adds files to `venv_path=a/` and another adds files to `venv_path=a/b/`. +""", "kind": """ :type: str diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 1f3e4d88d4..36b5a50e5d 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -277,7 +277,12 @@ def _get_venv_symlinks(ctx, package, version_str): dir_symlinks = {} # dirname -> runfile path venv_symlinks = [] - for src in ctx.files.srcs + ctx.files.data + ctx.files.pyi_srcs: + all_files = sorted( + ctx.files.srcs + ctx.files.data + ctx.files.pyi_srcs, + key = lambda f: f.short_path, + ) + + for src in all_files: path = _repo_relative_short_path(src.short_path) if not path.startswith(site_packages_root): continue @@ -309,6 +314,7 @@ def _get_venv_symlinks(ctx, package, version_str): package = package, version = version_str, venv_path = filename, + files = depset([src]), ) venv_symlinks.append(entry) @@ -328,12 +334,18 @@ def _get_venv_symlinks(ctx, package, version_str): for dirname in first_level_explicit_packages: prefix = dir_symlinks[dirname] + link_to_path = paths.join(prefix, site_packages_root, dirname) entry = VenvSymlinkEntry( kind = VenvSymlinkKind.LIB, - link_to_path = paths.join(prefix, site_packages_root, dirname), + link_to_path = link_to_path, package = package, version = version_str, venv_path = dirname, + files = depset([ + f + for f in all_files + if runfiles_root_path(ctx, f.short_path).startswith(link_to_path + "/") + ]), ) venv_symlinks.append(entry) diff --git a/python/private/venv_runfiles.bzl b/python/private/venv_runfiles.bzl new file mode 100644 index 0000000000..0b4e336383 --- /dev/null +++ b/python/private/venv_runfiles.bzl @@ -0,0 +1,186 @@ +"""Code for constructing venvs.""" + +load("@bazel_skylib//lib:paths.bzl", "paths") +load( + ":common.bzl", + "is_file", + "relative_path", + "runfiles_root_path", +) +load(":py_info.bzl", "PyInfo") + +def create_venv_app_files(ctx, venv_dir_map): + """Creates the tree of app-specific files for a venv for a binary. + + App specific files are the files that come from dependencies. + + Args: + ctx: {type}`ctx` current ctx. + venv_dir_map: mapping of VenvSymlinkKind constants to the + venv path. This tells the directory name of + platform/configuration-dependent directories. The values are + paths within the current ctx's venv (e.g. `_foo.venv/bin`). + + Returns: + {type}`list[File}` of the files that were created. + """ + + # maps venv-relative path to the runfiles path it should point to + entries = depset( + transitive = [ + dep[PyInfo].venv_symlinks + for dep in ctx.attr.deps + if PyInfo in dep + ], + ).to_list() + + link_map = build_link_map(ctx, entries) + venv_files = [] + for kind, kind_map in link_map.items(): + base = venv_dir_map[kind] + for venv_path, link_to in kind_map.items(): + bin_venv_path = paths.join(base, venv_path) + if is_file(link_to): + if link_to.is_directory: + venv_link = ctx.actions.declare_directory(bin_venv_path) + else: + venv_link = ctx.actions.declare_file(bin_venv_path) + ctx.actions.symlink(output = venv_link, target_file = link_to) + else: + venv_link = ctx.actions.declare_symlink(bin_venv_path) + venv_link_rf_path = runfiles_root_path(ctx, venv_link.short_path) + rel_path = relative_path( + # dirname is necessary because a relative symlink is relative to + # the directory the symlink resides within. + from_ = paths.dirname(venv_link_rf_path), + to = link_to, + ) + ctx.actions.symlink(output = venv_link, target_path = rel_path) + venv_files.append(venv_link) + + return venv_files + +# Visible for testing +def build_link_map(ctx, entries): + """Compute the mapping of venv paths to their backing objects. + + + Args: + ctx: {type}`ctx` current ctx. + entries: {type}`list[VenvSymlinkEntry]` the entries that describe the + venv-relative + + 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 + inner dict values are strings or Files to link to. + """ + + version_by_pkg = {} # dict[str pkg, str version] + entries_by_kind = {} # dict[str kind, list[entry]] + + # Group by path kind and reduce to a single package's version of entries + for entry in entries: + entries_by_kind.setdefault(entry.kind, []) + if not entry.package: + entries_by_kind[entry.kind].append(entry) + continue + if entry.package not in version_by_pkg: + version_by_pkg[entry.package] = entry.version + entries_by_kind[entry.kind].append(entry) + continue + if entry.version == version_by_pkg[entry.package]: + entries_by_kind[entry.kind].append(entry) + continue + + # else: ignore it; not the selected version + + # final paths to keep, grouped by kind + keep_link_map = {} # dict[str kind, dict[path, str|File]] + for kind, entries in entries_by_kind.items(): + # dict[str kind-relative path, str|File link_to] + keep_kind_link_map = {} + + groups = _group_venv_path_entries(entries) + + for group in groups: + # 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 + else: + # Merge a group of overlapping prefixes + _merge_venv_path_group(ctx, group, keep_kind_link_map) + + keep_link_map[kind] = keep_kind_link_map + + return keep_link_map + +def _group_venv_path_entries(entries): + """Group entries by VenvSymlinkEntry.venv_path overlap. + + This does an initial grouping by the top-level venv path an entry wants. + Entries that are underneath another entry are put into the same group. + + Returns: + {type}`list[list[VenvSymlinkEntry]]` The inner list is the entries under + a common venv path. The inner list is ordered from shortest to longest + path. + """ + + # Sort so order is top-down, ensuring grouping by short common prefix + entries = sorted(entries, key = lambda e: e.venv_path) + + groups = [] + current_group = None + current_group_prefix = None + for entry in entries: + prefix = entry.venv_path + anchored_prefix = prefix + "/" + if (current_group_prefix == None or + not anchored_prefix.startswith(current_group_prefix)): + current_group_prefix = anchored_prefix + current_group = [entry] + groups.append(current_group) + else: + current_group.append(entry) + + return groups + +def _merge_venv_path_group(ctx, group, keep_map): + """Merges a group of overlapping prefixes. + + Args: + ctx: {type}`ctx` current ctx. + group: {type}`dict[str, VenvSymlinkEntry]` map of prefixes and their + values. Keys are the venv kind relative prefix. + keep_map: {type}`dict[str, str|File]` files kept after merging are + populated into this 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. + for entry in group: + prefix = entry.venv_path + for file in entry.files.to_list(): + # Compute the file-specific venv path. i.e. the relative + # path of the file under entry.venv_path, joined with + # entry.venv_path + rf_root_path = runfiles_root_path(ctx, file.short_path) + if not rf_root_path.startswith(entry.link_to_path): + # This generally shouldn't occur in practice, but just + # in case, skip them, for lack of a better option. + continue + venv_path = "{}/{}".format( + prefix, + rf_root_path.removeprefix(entry.link_to_path + "/"), + ) + + # For lack of a better option, first added wins. We happen to + # go in top-down prefix order, so the highest level namespace + # package typically wins. + if venv_path not in keep_map: + keep_map[venv_path] = file diff --git a/tests/venv_site_packages_libs/BUILD.bazel b/tests/venv_site_packages_libs/BUILD.bazel index e64299e1ad..92d5dec6d3 100644 --- a/tests/venv_site_packages_libs/BUILD.bazel +++ b/tests/venv_site_packages_libs/BUILD.bazel @@ -26,6 +26,8 @@ py_reconfig_test( ":closer_lib", "//tests/venv_site_packages_libs/nspkg_alpha", "//tests/venv_site_packages_libs/nspkg_beta", + "//tests/venv_site_packages_libs/pkgutil_top", + "//tests/venv_site_packages_libs/pkgutil_top_sub", "@other//nspkg_delta", "@other//nspkg_gamma", "@other//nspkg_single", diff --git a/tests/venv_site_packages_libs/app_files_building/BUILD.bazel b/tests/venv_site_packages_libs/app_files_building/BUILD.bazel new file mode 100644 index 0000000000..60afd34c38 --- /dev/null +++ b/tests/venv_site_packages_libs/app_files_building/BUILD.bazel @@ -0,0 +1,5 @@ +load(":app_files_building_tests.bzl", "app_files_building_test_suite") + +app_files_building_test_suite( + name = "app_files_building_tests", +) diff --git a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl new file mode 100644 index 0000000000..eb323c1085 --- /dev/null +++ b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl @@ -0,0 +1,77 @@ +"" + +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 + +_tests = [] + +def _ctx(workspace_name): + return struct( + workspace_name = workspace_name, + ) + +def _file(short_path): + return struct( + short_path = short_path, + ) + +def _entry(venv_path, link_to_path, files = [], **kwargs): + kwargs.setdefault("kind", VenvSymlinkKind.LIB) + kwargs.setdefault("package", None) + kwargs.setdefault("version", None) + + # Treat paths starting with "+" as external references. This matches + # how bzlmod names things. + if link_to_path.startswith("+"): + # File.short_path to external repos have `../` prefixed + short_path_prefix = "../{}".format(link_to_path) + else: + # File.short_path in main repo is main-repo relative + _, _, short_path_prefix = link_to_path.partition("/") + + files = depset([ + _file(paths.join(short_path_prefix, f)) + for f in files + ]) + return VenvSymlinkEntry( + venv_path = venv_path, + link_to_path = link_to_path, + files = files, + **kwargs + ) + +def _test_build_link_map(name): + analysis_test( + name = name, + impl = _test_build_link_map_impl, + target = "//python:none", + ) + +_tests.append(_test_build_link_map) + +def _test_build_link_map_impl(env, _): + entries = [ + _entry("a", "+pypi_a/site-packages/a", ["a.txt"]), + _entry("a/b", "+pypi_a_b/site-packages/a/b", ["b.txt"]), + _entry("x", "_main/src/x", ["x.txt"]), + _entry("x/p", "_main/src-dev/x/p", ["p.txt"]), + ] + + actual = build_link_map(_ctx("_main"), entries) + expected_libs = { + "a/a.txt": _file("../+pypi_a/site-packages/a/a.txt"), + "a/b/b.txt": _file("../+pypi_a_b/site-packages/a/b/b.txt"), + "x/p/p.txt": _file("src-dev/x/p/p.txt"), + "x/x.txt": _file("src/x/x.txt"), + } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + env.expect.that_dict(actual).keys().contains_exactly([VenvSymlinkKind.LIB]) + +def app_files_building_test_suite(name): + test_suite( + name = name, + tests = _tests, + ) diff --git a/tests/venv_site_packages_libs/bin.py b/tests/venv_site_packages_libs/bin.py index 7e5838d2c2..772925f00e 100644 --- a/tests/venv_site_packages_libs/bin.py +++ b/tests/venv_site_packages_libs/bin.py @@ -14,14 +14,26 @@ def setUp(self): def assert_imported_from_venv(self, module_name): module = importlib.import_module(module_name) self.assertEqual(module.__name__, module_name) + self.assertIsNotNone( + module.__file__, + f"Expected module {module_name!r} to have" + + f"__file__ set, but got None. {module=}", + ) self.assertTrue( module.__file__.startswith(self.venv), f"\n{module_name} was imported, but not from the venv.\n" + f"venv : {self.venv}\n" + f"actual: {module.__file__}", ) + return module def test_imported_from_venv(self): + m = self.assert_imported_from_venv("pkgutil_top") + self.assertEqual(m.WHOAMI, "pkgutil_top") + + m = self.assert_imported_from_venv("pkgutil_top.sub") + self.assertEqual(m.WHOAMI, "pkgutil_top.sub") + self.assert_imported_from_venv("nspkg.subnspkg.alpha") self.assert_imported_from_venv("nspkg.subnspkg.beta") self.assert_imported_from_venv("nspkg.subnspkg.gamma") diff --git a/tests/venv_site_packages_libs/pkgutil_top/BUILD.bazel b/tests/venv_site_packages_libs/pkgutil_top/BUILD.bazel new file mode 100644 index 0000000000..c805b1ad53 --- /dev/null +++ b/tests/venv_site_packages_libs/pkgutil_top/BUILD.bazel @@ -0,0 +1,10 @@ +load("//python:py_library.bzl", "py_library") + +package(default_visibility = ["//visibility:public"]) + +py_library( + name = "pkgutil_top", + srcs = glob(["site-packages/**/*.py"]), + experimental_venvs_site_packages = "//python/config_settings:venvs_site_packages", + imports = [package_name() + "/site-packages"], +) diff --git a/tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/__init__.py b/tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/__init__.py new file mode 100644 index 0000000000..e1809a325a --- /dev/null +++ b/tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/__init__.py @@ -0,0 +1,2 @@ +WHOAMI = "pkgutil_top" +__path__ = __import__("pkgutil").extend_path(__path__, __name__) diff --git a/tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/top.py b/tests/venv_site_packages_libs/pkgutil_top/site-packages/pkgutil_top/top.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/venv_site_packages_libs/pkgutil_top_sub/BUILD.bazel b/tests/venv_site_packages_libs/pkgutil_top_sub/BUILD.bazel new file mode 100644 index 0000000000..9d771628a0 --- /dev/null +++ b/tests/venv_site_packages_libs/pkgutil_top_sub/BUILD.bazel @@ -0,0 +1,10 @@ +load("//python:py_library.bzl", "py_library") + +package(default_visibility = ["//visibility:public"]) + +py_library( + name = "pkgutil_top_sub", + srcs = glob(["site-packages/**/*.py"]), + experimental_venvs_site_packages = "//python/config_settings:venvs_site_packages", + imports = [package_name() + "/site-packages"], +) diff --git a/tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/__init__.py b/tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/__init__.py new file mode 100644 index 0000000000..e7fb2340ea --- /dev/null +++ b/tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/__init__.py @@ -0,0 +1 @@ +WHOAMI = "pkgutil_top.sub" diff --git a/tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/suba.py b/tests/venv_site_packages_libs/pkgutil_top_sub/site-packages/pkgutil_top/sub/suba.py new file mode 100644 index 0000000000..e69de29bb2 From ee5e02b9ab1a1f3df8d3b44200935aaab02112bc Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 13:50:35 -0700 Subject: [PATCH 02/17] Update python/private/venv_runfiles.bzl Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- python/private/venv_runfiles.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/private/venv_runfiles.bzl b/python/private/venv_runfiles.bzl index 0b4e336383..220c5ae879 100644 --- a/python/private/venv_runfiles.bzl +++ b/python/private/venv_runfiles.bzl @@ -153,8 +153,8 @@ def _merge_venv_path_group(ctx, group, keep_map): Args: ctx: {type}`ctx` current ctx. - group: {type}`dict[str, VenvSymlinkEntry]` map of prefixes and their - values. Keys are the venv kind relative prefix. + group: {type}`list[VenvSymlinkEntry]` a group of entries with overlapping + `venv_path` prefixes, ordered from shortest to longest path. keep_map: {type}`dict[str, str|File]` files kept after merging are populated into this map. """ From 4b03c2a573e19ce64750d918cf2cf58d00341d79 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 13:55:58 -0700 Subject: [PATCH 03/17] fix test --- tests/bootstrap_impls/venv_relative_path_tests.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bootstrap_impls/venv_relative_path_tests.bzl b/tests/bootstrap_impls/venv_relative_path_tests.bzl index ad4870fe08..72b5012809 100644 --- a/tests/bootstrap_impls/venv_relative_path_tests.bzl +++ b/tests/bootstrap_impls/venv_relative_path_tests.bzl @@ -15,7 +15,7 @@ "Unit tests for relative_path computation" load("@rules_testing//lib:test_suite.bzl", "test_suite") -load("//python/private:py_executable.bzl", "relative_path") # buildifier: disable=bzl-visibility +load("//python/private:common.bzl", "relative_path") # buildifier: disable=bzl-visibility _tests = [] From 01bea90d904541285398da4442c2a10994651423 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 14:07:02 -0700 Subject: [PATCH 04/17] add bzl_library for venv_runfiles.bzl --- python/private/BUILD.bazel | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index f31b56ec50..9a6eb7882d 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -718,6 +718,16 @@ bzl_library( ], ) +bzl_library( + name = "venv_runfiles_bzl", + srcs = ["venv_runfiles.bzl"], + deps = [ + ":common_bzl", + ":py_info.bzl", + "@bazel_skylib//lib:paths.bzl", + ], +) + # Needed to define bzl_library targets for docgen. (We don't define the # bzl_library target here because it'd give our users a transitive dependency # on Skylib.) From e9971c74f5dbc9201547efc8f6be3761e93938c7 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 14:30:32 -0700 Subject: [PATCH 05/17] provide some bzl library guidance --- AGENTS.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e21b15bd03..3d5219c2bc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,10 +21,16 @@ into the sentence, not verbatim. ### bzl_library targets for bzl source files -* `.bzl` files should have `bzl_library` defined for them. +* A `bzl_library` target should be defined for every `.bzl` file outside + of the `tests/` directory. * They should have a single `srcs` file and be named after the file with `_bzl` appended. -* Their deps should be based on the `load()` statements in the source file. +* Their deps should be based on the `load()` statements in the source file + and refer to the `bzl_library` target containing the loaded file. + * For files in rules_python: replace `.bzl` with `_bzl`. + e.g. given `load("//foo:bar.bzl", ...)`, the target is `//foo:bar_bzl`. + * For files outside rules_python: remove the `.bzl` suffix. e.g. given + `load("@foo//foo:bar.bzl", ...)`, the target is `@foo//foo:bar`. * `bzl_library()` targets should be kept in alphabetical order by name. Example: From 342bfe53e0b71afdcddaeab753a1bcc309fdd903 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 14:30:41 -0700 Subject: [PATCH 06/17] add missing dep --- python/private/BUILD.bazel | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 9a6eb7882d..c9f8efa90a 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -415,6 +415,7 @@ bzl_library( ":rules_cc_srcs_bzl", ":toolchain_types_bzl", ":transition_labels_bzl", + ":venv_runfiles_bzl", "@bazel_skylib//lib:dicts", "@bazel_skylib//lib:paths", "@bazel_skylib//lib:structs", @@ -724,7 +725,7 @@ bzl_library( deps = [ ":common_bzl", ":py_info.bzl", - "@bazel_skylib//lib:paths.bzl", + "@bazel_skylib//lib:paths", ], ) From 7d5de8772d6cf8776ba68c2d3fc0f6cabf793efa Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 14:58:20 -0700 Subject: [PATCH 07/17] move py_library code to venv_runfiles --- python/private/BUILD.bazel | 1 + python/private/py_library.bzl | 126 +----------------------------- python/private/venv_runfiles.bzl | 128 ++++++++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 125 deletions(-) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index c9f8efa90a..bb548c4d24 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -464,6 +464,7 @@ bzl_library( ":py_internal_bzl", ":rule_builders_bzl", ":toolchain_types_bzl", + ":venv_runfiles_bzl", ":version_bzl", "@bazel_skylib//lib:dicts", "@bazel_skylib//rules:common_settings", diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 36b5a50e5d..1f5ed5b245 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -28,7 +28,6 @@ load( load(":builders.bzl", "builders") load( ":common.bzl", - "PYTHON_FILE_EXTENSIONS", "collect_cc_info", "collect_imports", "collect_runfiles", @@ -38,13 +37,12 @@ load( "create_py_info", "filter_to_py_srcs", "get_imports", - "runfiles_root_path", ) load(":flags.bzl", "AddSrcsToRunfilesFlag", "PrecompileFlag", "VenvsSitePackages") load(":normalize_name.bzl", "normalize_name") load(":precompile.bzl", "maybe_precompile") load(":py_cc_link_params_info.bzl", "PyCcLinkParamsInfo") -load(":py_info.bzl", "PyInfo", "VenvSymlinkEntry", "VenvSymlinkKind") +load(":py_info.bzl", "PyInfo") load(":reexports.bzl", "BuiltinPyInfo") load(":rule_builders.bzl", "ruleb") load( @@ -52,6 +50,7 @@ load( "EXEC_TOOLS_TOOLCHAIN_TYPE", TOOLCHAIN_TYPE = "TARGET_TOOLCHAIN_TYPE", ) +load(":venv_runfiles.bzl", "get_venv_symlinks") load(":version.bzl", "version") LIBRARY_ATTRS = dicts.add( @@ -234,130 +233,11 @@ def _get_imports_and_venv_symlinks(ctx, semantics): venv_symlinks = [] if VenvsSitePackages.is_enabled(ctx): package, version_str = _get_package_and_version(ctx) - venv_symlinks = _get_venv_symlinks(ctx, package, version_str) + venv_symlinks = get_venv_symlinks(ctx, package, version_str, ctx.attr.imports) else: imports = collect_imports(ctx, semantics) return imports, venv_symlinks -def _get_venv_symlinks(ctx, package, version_str): - imports = ctx.attr.imports - if len(imports) == 0: - fail("When venvs_site_packages is enabled, exactly one `imports` " + - "value must be specified, got 0") - elif len(imports) > 1: - fail("When venvs_site_packages is enabled, exactly one `imports` " + - "value must be specified, got {}".format(imports)) - else: - site_packages_root = imports[0] - - if site_packages_root.endswith("/"): - fail("The site packages root value from `imports` cannot end in " + - "slash, got {}".format(site_packages_root)) - if site_packages_root.startswith("/"): - fail("The site packages root value from `imports` cannot start with " + - "slash, got {}".format(site_packages_root)) - - # 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 to the directories containing the code of - # interest. - # - # However, namespace packages complicate matters: multiple distributions install in the - # same directory in site-packages. This works out because they don't overlap in their - # files. Typically, they install to different directories within the namespace package - # directory. We also need to ensure that we can handle a case where the main package (e.g. - # airflow) has directories only containing data files and then namespace packages coming - # along and being next to it. - # - # Lastly we have to assume python modules just being `.py` files (e.g. typing-extensions) - # is just a single Python file. - - dir_symlinks = {} # dirname -> runfile path - venv_symlinks = [] - all_files = sorted( - ctx.files.srcs + ctx.files.data + ctx.files.pyi_srcs, - key = lambda f: f.short_path, - ) - - for src in all_files: - 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 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) - # * a directory with `__init__.py` file that potentially also needs to be - # symlinked. - # * `.dist-info` directory - # - # This could be also regular files, that just need to be symlinked, so we will - # add the directory here. - dir_symlinks[dir_name] = runfiles_dir_name - elif src.extension in PYTHON_FILE_EXTENSIONS: - # This would be files that do not have directories and we just need to add - # direct symlinks to them as is, we only allow Python files in here - entry = VenvSymlinkEntry( - kind = VenvSymlinkKind.LIB, - link_to_path = paths.join(runfiles_dir_name, site_packages_root, filename), - package = package, - version = version_str, - venv_path = filename, - files = depset([src]), - ) - venv_symlinks.append(entry) - - # Sort so that we encounter `foo` before `foo/bar`. This ensures we - # see the top-most explicit package first. - dirnames = sorted(dir_symlinks.keys()) - first_level_explicit_packages = [] - for d in dirnames: - is_sub_package = False - for existing in first_level_explicit_packages: - # Suffix with / to prevent foo matching foobar - if d.startswith(existing + "/"): - is_sub_package = True - break - if not is_sub_package: - first_level_explicit_packages.append(d) - - for dirname in first_level_explicit_packages: - prefix = dir_symlinks[dirname] - link_to_path = paths.join(prefix, site_packages_root, dirname) - entry = VenvSymlinkEntry( - kind = VenvSymlinkKind.LIB, - link_to_path = link_to_path, - package = package, - version = version_str, - venv_path = dirname, - files = depset([ - f - for f in all_files - if runfiles_root_path(ctx, f.short_path).startswith(link_to_path + "/") - ]), - ) - venv_symlinks.append(entry) - - return venv_symlinks - -def _repo_relative_short_path(short_path): - # Convert `../+pypi+foo/some/file.py` to `some/file.py` - if short_path.startswith("../"): - return short_path[3:].partition("/")[2] - else: - return short_path - _MaybeBuiltinPyInfo = [BuiltinPyInfo] if BuiltinPyInfo != None else [] # NOTE: Exported publicaly diff --git a/python/private/venv_runfiles.bzl b/python/private/venv_runfiles.bzl index 220c5ae879..1d687e0f81 100644 --- a/python/private/venv_runfiles.bzl +++ b/python/private/venv_runfiles.bzl @@ -3,11 +3,17 @@ load("@bazel_skylib//lib:paths.bzl", "paths") load( ":common.bzl", + "PYTHON_FILE_EXTENSIONS", "is_file", "relative_path", "runfiles_root_path", ) -load(":py_info.bzl", "PyInfo") +load( + ":py_info.bzl", + "PyInfo", + "VenvSymlinkEntry", + "VenvSymlinkKind", +) def create_venv_app_files(ctx, venv_dir_map): """Creates the tree of app-specific files for a venv for a binary. @@ -22,7 +28,7 @@ def create_venv_app_files(ctx, venv_dir_map): paths within the current ctx's venv (e.g. `_foo.venv/bin`). Returns: - {type}`list[File}` of the files that were created. + {type}`list[File]` of the files that were created. """ # maps venv-relative path to the runfiles path it should point to @@ -184,3 +190,121 @@ def _merge_venv_path_group(ctx, group, keep_map): # package typically wins. if venv_path not in keep_map: keep_map[venv_path] = file + +def get_venv_symlinks(ctx, package, version_str, imports): + if len(imports) == 0: + fail("When venvs_site_packages is enabled, exactly one `imports` " + + "value must be specified, got 0") + elif len(imports) > 1: + fail("When venvs_site_packages is enabled, exactly one `imports` " + + "value must be specified, got {}".format(imports)) + else: + site_packages_root = imports[0] + + if site_packages_root.endswith("/"): + fail("The site packages root value from `imports` cannot end in " + + "slash, got {}".format(site_packages_root)) + if site_packages_root.startswith("/"): + fail("The site packages root value from `imports` cannot start with " + + "slash, got {}".format(site_packages_root)) + + # 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 to the directories containing the code of + # interest. + # + # However, namespace packages complicate matters: multiple distributions install in the + # same directory in site-packages. This works out because they don't overlap in their + # files. Typically, they install to different directories within the namespace package + # directory. We also need to ensure that we can handle a case where the main package (e.g. + # airflow) has directories only containing data files and then namespace packages coming + # along and being next to it. + # + # Lastly we have to assume python modules just being `.py` files (e.g. typing-extensions) + # is just a single Python file. + + dir_symlinks = {} # dirname -> runfile path + venv_symlinks = [] + all_files = sorted( + ctx.files.srcs + ctx.files.data + ctx.files.pyi_srcs, + key = lambda f: f.short_path, + ) + + for src in all_files: + 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 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) + # * a directory with `__init__.py` file that potentially also needs to be + # symlinked. + # * `.dist-info` directory + # + # This could be also regular files, that just need to be symlinked, so we will + # add the directory here. + dir_symlinks[dir_name] = runfiles_dir_name + elif src.extension in PYTHON_FILE_EXTENSIONS: + # This would be files that do not have directories and we just need to add + # direct symlinks to them as is, we only allow Python files in here + entry = VenvSymlinkEntry( + kind = VenvSymlinkKind.LIB, + link_to_path = paths.join(runfiles_dir_name, site_packages_root, filename), + package = package, + version = version_str, + venv_path = filename, + files = depset([src]), + ) + venv_symlinks.append(entry) + + # Sort so that we encounter `foo` before `foo/bar`. This ensures we + # see the top-most explicit package first. + dirnames = sorted(dir_symlinks.keys()) + first_level_explicit_packages = [] + for d in dirnames: + is_sub_package = False + for existing in first_level_explicit_packages: + # Suffix with / to prevent foo matching foobar + if d.startswith(existing + "/"): + is_sub_package = True + break + if not is_sub_package: + first_level_explicit_packages.append(d) + + for dirname in first_level_explicit_packages: + prefix = dir_symlinks[dirname] + link_to_path = paths.join(prefix, site_packages_root, dirname) + entry = VenvSymlinkEntry( + kind = VenvSymlinkKind.LIB, + link_to_path = link_to_path, + package = package, + version = version_str, + venv_path = dirname, + files = depset([ + f + for f in all_files + if runfiles_root_path(ctx, f.short_path).startswith(link_to_path + "/") + ]), + ) + venv_symlinks.append(entry) + + return venv_symlinks + +def _repo_relative_short_path(short_path): + # Convert `../+pypi+foo/some/file.py` to `some/file.py` + if short_path.startswith("../"): + return short_path[3:].partition("/")[2] + else: + return short_path \ No newline at end of file From 58aae63408646b36281f91d7e77c6adf02da7226 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 15:51:52 -0700 Subject: [PATCH 08/17] fixup after moving --- python/private/py_executable.bzl | 2 +- python/private/py_library.bzl | 16 +++++++++++- python/private/venv_runfiles.bzl | 43 ++++++++++++++++++-------------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 40b2a43252..bb21c65ec7 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -612,7 +612,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details): VenvSymlinkKind.BIN: bin_dir, VenvSymlinkKind.LIB: site_packages, } - venv_app_files = create_venv_app_files(ctx, venv_dir_map) + venv_app_files = create_venv_app_files(ctx, ctx.attr.deps, venv_dir_map) files_without_interpreter = [pth, site_init] + venv_app_files if pyvenv_cfg: diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 1f5ed5b245..f78d0eb40e 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -233,7 +233,21 @@ def _get_imports_and_venv_symlinks(ctx, semantics): venv_symlinks = [] if VenvsSitePackages.is_enabled(ctx): package, version_str = _get_package_and_version(ctx) - venv_symlinks = get_venv_symlinks(ctx, package, version_str, ctx.attr.imports) + imports = ctx.attr.imports + if len(imports) == 0: + fail("When venvs_site_packages is enabled, exactly one `imports` " + + "value must be specified, got 0") + elif len(imports) > 1: + fail("When venvs_site_packages is enabled, exactly one `imports` " + + "value must be specified, got {}".format(imports)) + + venv_symlinks = get_venv_symlinks( + ctx, + ctx.files.srcs + ctx.files.data + ctx.files.pyi_srcs, + package, + version_str, + site_packages_root = imports[0], + ) else: imports = collect_imports(ctx, semantics) return imports, venv_symlinks diff --git a/python/private/venv_runfiles.bzl b/python/private/venv_runfiles.bzl index 1d687e0f81..66e1a4fd8e 100644 --- a/python/private/venv_runfiles.bzl +++ b/python/private/venv_runfiles.bzl @@ -15,13 +15,15 @@ load( "VenvSymlinkKind", ) -def create_venv_app_files(ctx, venv_dir_map): +def create_venv_app_files(ctx, deps, venv_dir_map): """Creates the tree of app-specific files for a venv for a binary. App specific files are the files that come from dependencies. Args: ctx: {type}`ctx` current ctx. + deps: {type}`list[Target]` the targets whose venv information + to put into the returned venv files. venv_dir_map: mapping of VenvSymlinkKind constants to the venv path. This tells the directory name of platform/configuration-dependent directories. The values are @@ -35,7 +37,7 @@ def create_venv_app_files(ctx, venv_dir_map): entries = depset( transitive = [ dep[PyInfo].venv_symlinks - for dep in ctx.attr.deps + for dep in deps if PyInfo in dep ], ).to_list() @@ -191,24 +193,28 @@ def _merge_venv_path_group(ctx, group, keep_map): if venv_path not in keep_map: keep_map[venv_path] = file -def get_venv_symlinks(ctx, package, version_str, imports): - if len(imports) == 0: - fail("When venvs_site_packages is enabled, exactly one `imports` " + - "value must be specified, got 0") - elif len(imports) > 1: - fail("When venvs_site_packages is enabled, exactly one `imports` " + - "value must be specified, got {}".format(imports)) - else: - site_packages_root = imports[0] +def get_venv_symlinks(ctx, files, package, version_str, site_packages_root): + """Compute the VenvSymlinkEntry objects for a library. + + Args: + ctx: {type}`ctx` the current ctx. + package: {type}`str` the Python distribution name. + version_str: {type}`str` the distribution's version. + site_packages_root: {type}`str` prefix under which files are + considered to be part of the installed files. + Returns: + {type}`list[VenvSymlinkEntry]` the entries that describe how + to map the files into a venv. + """ if site_packages_root.endswith("/"): - fail("The site packages root value from `imports` cannot end in " + + fail("The `site_packages_root` value cannot end in " + "slash, got {}".format(site_packages_root)) if site_packages_root.startswith("/"): - fail("The site packages root value from `imports` cannot start with " + + fail("The `site_packages_root` cannot start with " + "slash, got {}".format(site_packages_root)) - # Append slash to prevent incorrectly prefix-string matches + # Append slash to prevent incorrect prefix-string matches site_packages_root += "/" # We have to build a list of (runfiles path, site-packages path) pairs of the files to @@ -228,10 +234,9 @@ def get_venv_symlinks(ctx, package, version_str, imports): dir_symlinks = {} # dirname -> runfile path venv_symlinks = [] - all_files = sorted( - ctx.files.srcs + ctx.files.data + ctx.files.pyi_srcs, - key = lambda f: f.short_path, - ) + + # Sort so order is top-down + all_files = sorted(files, key = lambda f: f.short_path) for src in all_files: path = _repo_relative_short_path(src.short_path) @@ -307,4 +312,4 @@ def _repo_relative_short_path(short_path): if short_path.startswith("../"): return short_path[3:].partition("/")[2] else: - return short_path \ No newline at end of file + return short_path From 3562973a4dbdfe85fd0db954a8c835eb92267959 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 16:46:46 -0700 Subject: [PATCH 09/17] remove bad to_list() call, add missing doc --- python/private/py_library.bzl | 4 ++-- python/private/venv_runfiles.bzl | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index f78d0eb40e..84718cf37c 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -234,10 +234,10 @@ def _get_imports_and_venv_symlinks(ctx, semantics): if VenvsSitePackages.is_enabled(ctx): package, version_str = _get_package_and_version(ctx) imports = ctx.attr.imports - if len(imports) == 0: + if len(imports.to_list()) == 0: fail("When venvs_site_packages is enabled, exactly one `imports` " + "value must be specified, got 0") - elif len(imports) > 1: + elif len(imports.to_list()) > 1: fail("When venvs_site_packages is enabled, exactly one `imports` " + "value must be specified, got {}".format(imports)) diff --git a/python/private/venv_runfiles.bzl b/python/private/venv_runfiles.bzl index 66e1a4fd8e..291920b848 100644 --- a/python/private/venv_runfiles.bzl +++ b/python/private/venv_runfiles.bzl @@ -198,6 +198,9 @@ def get_venv_symlinks(ctx, files, package, version_str, site_packages_root): Args: ctx: {type}`ctx` the current ctx. + files: {type}`list[File]` the underlying files that are under + `site_packages_root` and intended to be part of the venv + contents. package: {type}`str` the Python distribution name. version_str: {type}`str` the distribution's version. site_packages_root: {type}`str` prefix under which files are From 2c4270416f3698159841146f04a807d9e1f85383 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 17:07:29 -0700 Subject: [PATCH 10/17] expand tests a bit --- .../app_files_building_tests.bzl | 142 +++++++++++++++++- 1 file changed, 139 insertions(+), 3 deletions(-) diff --git a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl index eb323c1085..c0501df6b9 100644 --- a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl +++ b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl @@ -8,7 +8,7 @@ load("//python/private:venv_runfiles.bzl", "build_link_map") # buildifier: disa _tests = [] -def _ctx(workspace_name): +def _ctx(workspace_name = "_main"): return struct( workspace_name = workspace_name, ) @@ -52,24 +52,160 @@ def _test_build_link_map(name): _tests.append(_test_build_link_map) -def _test_build_link_map_impl(env, _): +def _test_overlapping_and_merging(env, _): entries = [ _entry("a", "+pypi_a/site-packages/a", ["a.txt"]), _entry("a/b", "+pypi_a_b/site-packages/a/b", ["b.txt"]), _entry("x", "_main/src/x", ["x.txt"]), _entry("x/p", "_main/src-dev/x/p", ["p.txt"]), + _entry("duplicate", "+dupe_a/site-packages/duplicate", ["d.py"]), + # This entry also provides a/x.py, but since the "a" entry is shorter + # and comes first, its version of x.py should win. + _entry("duplicate", "+dupe_b/site-packages/duplicate", ["d.py"]), ] - actual = build_link_map(_ctx("_main"), entries) + actual = build_link_map(_ctx(), entries) expected_libs = { "a/a.txt": _file("../+pypi_a/site-packages/a/a.txt"), "a/b/b.txt": _file("../+pypi_a_b/site-packages/a/b/b.txt"), "x/p/p.txt": _file("src-dev/x/p/p.txt"), "x/x.txt": _file("src/x/x.txt"), + "duplicate/d.py": _file("../+dupe_a/site-packages/duplicate/d.py"), } env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) env.expect.that_dict(actual).keys().contains_exactly([VenvSymlinkKind.LIB]) +def _test_package_version_filtering(name): + analysis_test( + name = name, + impl = _test_package_version_filtering_impl, + target = "//python:none", + ) + +_tests.append(_test_package_version_filtering) + +def _test_package_version_filtering_impl(env, _): + entries = [ + _entry("foo", "+pypi_foo_v1/site-packages/foo", ["foo.txt"], package = "foo", version = "1.0"), + _entry("foo", "+pypi_foo_v2/site-packages/foo", ["bar.txt"], package = "foo", version = "2.0"), + ] + + actual = build_link_map(_ctx(), entries) + expected_libs = { + "foo/foo.txt": _file("../+pypi_foo/site-packages/foo/foo.txt"), + } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + + actual = build_link_map(_ctx(), entries) + expected_libs = { + "a/x.py": _file("../+pypi_a/site-packages/a/x.py"), + } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + +def _test_malformed_entry(name): + analysis_test( + name = name, + impl = _test_malformed_entry_impl, + target = "//python:none", + ) + +_tests.append(_test_malformed_entry) + +def _test_malformed_entry_impl(env, _): + entries = [ + _entry( + "a", + "+pypi_a/site-packages/a", + # This file is outside the link_to_path, so it should be ignored. + ["../outside.txt"], + ), + ] + + actual = build_link_map(_ctx(), entries) + env.expect.that_dict(actual).is_empty() + +def _test_complex_namespace_packages(name): + analysis_test( + name = name, + impl = _test_complex_namespace_packages_impl, + target = "//python:none", + ) + +_tests.append(_test_complex_namespace_packages) + +def _test_complex_namespace_packages_impl(env, _): + entries = [ + _entry("a/b", "+pypi_a_b/site-packages/a/b", ["b.txt"]), + _entry("a/c", "+pypi_a_c/site-packages/a/c", ["c.txt"]), + _entry("x/y/z", "+pypi_x_y_z/site-packages/x/y/z", ["z.txt"]), + _entry("foo", "+pypi_foo/site-packages/foo", ["foo.txt"]), + _entry("foobar", "+pypi_foobar/site-packages/foobar", ["foobar.txt"]), + ] + + actual = build_link_map(_ctx(), entries) + expected_libs = { + "a/b/b.txt": _file("../+pypi_a_b/site-packages/a/b/b.txt"), + "a/c/c.txt": _file("../+pypi_a_c/site-packages/a/c/c.txt"), + "x/y/z/z.txt": _file("../+pypi_x_y_z/site-packages/x/y/z/z.txt"), + "foo/foo.txt": _file("../+pypi_foo/site-packages/foo/foo.txt"), + "foobar/foobar.txt": _file("../+pypi_foobar/site-packages/foobar/foobar.txt"), + } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + +def _test_empty_and_trivial_inputs(name): + analysis_test( + name = name, + impl = _test_empty_and_trivial_inputs_impl, + target = "//python:none", + ) + +_tests.append(_test_empty_and_trivial_inputs) + +def _test_empty_and_trivial_inputs_impl(env, _): + # Test with empty list of entries + actual = build_link_map(_ctx(), []) + env.expect.that_dict(actual).is_empty() + + # Test with an entry with no files + entries = [_entry("a", "+pypi_a/site-packages/a", [])] + actual = build_link_map(_ctx(), entries) + env.expect.that_dict(actual).is_empty() + +def _test_multiple_venv_symlink_kinds(name): + analysis_test( + name = name, + impl = _test_multiple_venv_symlink_kinds_impl, + target = "//python:none", + ) + +_tests.append(_test_multiple_venv_symlink_kinds) + +def _test_multiple_venv_symlink_kinds_impl(env, _): + entries = [ + _entry("libfile", "+pypi_lib/site-packages/libfile", ["lib.txt"], kind = VenvSymlinkKind.LIB), + _entry("binfile", "+pypi_bin/bin/binfile", ["bin.txt"], kind = VenvSymlinkKind.BIN), + _entry("includefile", "+pypi_include/include/includefile", ["include.h"], kind = VenvSymlinkKind.INCLUDE), + ] + + actual = build_link_map(_ctx(), entries) + expected_libs = { + "libfile/lib.txt": _file("../+pypi_lib/site-packages/libfile/lib.txt"), + } + expected_bins = { + "binfile/bin.txt": _file("../+pypi_bin/bin/binfile/bin.txt"), + } + expected_includes = { + "includefile/include.h": _file("../+pypi_include/include/includefile/include.h"), + } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + env.expect.that_dict(actual[VenvSymlinkKind.BIN]).contains_exactly(expected_bins) + env.expect.that_dict(actual[VenvSymlinkKind.INCLUDE]).contains_exactly(expected_includes) + env.expect.that_dict(actual).keys().contains_exactly([ + VenvSymlinkKind.LIB, + VenvSymlinkKind.BIN, + VenvSymlinkKind.INCLUDE, + ]) + def app_files_building_test_suite(name): test_suite( name = name, From dbfc90ac78de01752fa366bd12a464d594bd9783 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 20:12:05 -0700 Subject: [PATCH 11/17] fixing tests --- .../app_files_building_tests.bzl | 118 +++++++++++------- 1 file changed, 76 insertions(+), 42 deletions(-) diff --git a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl index c0501df6b9..b9779822ec 100644 --- a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl +++ b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl @@ -23,36 +23,42 @@ def _entry(venv_path, link_to_path, files = [], **kwargs): kwargs.setdefault("package", None) kwargs.setdefault("version", None) - # Treat paths starting with "+" as external references. This matches - # how bzlmod names things. - if link_to_path.startswith("+"): - # File.short_path to external repos have `../` prefixed - short_path_prefix = "../{}".format(link_to_path) - else: - # File.short_path in main repo is main-repo relative - _, _, short_path_prefix = link_to_path.partition("/") - - files = depset([ - _file(paths.join(short_path_prefix, f)) - for f in files - ]) + def short_pathify(path): + path = paths.join(link_to_path, path) + + # In tests, `../` is used to step out of the link_to_path scope. + path = paths.normalize(path) + + # Treat paths starting with "+" as external references. This matches + # how bzlmod names things. + if link_to_path.startswith("+"): + # File.short_path to external repos have `../` prefixed + path = paths.join("../", path) + else: + # File.short_path in main repo is main-repo relative + _, _, path = path.partition("/") + return path + return VenvSymlinkEntry( venv_path = venv_path, link_to_path = link_to_path, - files = files, + files = depset([ + _file(short_pathify(f)) + for f in files + ]), **kwargs ) -def _test_build_link_map(name): +def _test_conflict_merging(name): analysis_test( name = name, - impl = _test_build_link_map_impl, + impl = _test_conflict_merging_impl, target = "//python:none", ) -_tests.append(_test_build_link_map) +_tests.append(_test_conflict_merging) -def _test_overlapping_and_merging(env, _): +def _test_conflict_merging_impl(env, _): entries = [ _entry("a", "+pypi_a/site-packages/a", ["a.txt"]), _entry("a/b", "+pypi_a_b/site-packages/a/b", ["b.txt"]), @@ -86,19 +92,14 @@ _tests.append(_test_package_version_filtering) def _test_package_version_filtering_impl(env, _): entries = [ - _entry("foo", "+pypi_foo_v1/site-packages/foo", ["foo.txt"], package = "foo", version = "1.0"), - _entry("foo", "+pypi_foo_v2/site-packages/foo", ["bar.txt"], package = "foo", version = "2.0"), + _entry("foo", "+pypi_v1/site-packages/foo", ["foo.txt"], package = "foo", version = "1.0"), + _entry("foo", "+pypi_v2/site-packages/foo", ["bar.txt"], package = "foo", version = "2.0"), ] actual = build_link_map(_ctx(), entries) - expected_libs = { - "foo/foo.txt": _file("../+pypi_foo/site-packages/foo/foo.txt"), - } - env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) - actual = build_link_map(_ctx(), entries) expected_libs = { - "a/x.py": _file("../+pypi_a/site-packages/a/x.py"), + "foo": "+pypi_v1/site-packages/foo", } env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) @@ -119,10 +120,20 @@ def _test_malformed_entry_impl(env, _): # This file is outside the link_to_path, so it should be ignored. ["../outside.txt"], ), + # A second, conflicting, entry is added to force merging of the known + # files. Without this, there's no conflict, so files is never + # considered. + _entry( + "a", + "+pypi_b/site-packages/a", + ["../outside.txt"], + ), ] actual = build_link_map(_ctx(), entries) - env.expect.that_dict(actual).is_empty() + env.expect.that_dict(actual).contains_exactly({ + VenvSymlinkKind.LIB: {}, + }) def _test_complex_namespace_packages(name): analysis_test( @@ -144,11 +155,11 @@ def _test_complex_namespace_packages_impl(env, _): actual = build_link_map(_ctx(), entries) expected_libs = { - "a/b/b.txt": _file("../+pypi_a_b/site-packages/a/b/b.txt"), - "a/c/c.txt": _file("../+pypi_a_c/site-packages/a/c/c.txt"), - "x/y/z/z.txt": _file("../+pypi_x_y_z/site-packages/x/y/z/z.txt"), - "foo/foo.txt": _file("../+pypi_foo/site-packages/foo/foo.txt"), - "foobar/foobar.txt": _file("../+pypi_foobar/site-packages/foobar/foobar.txt"), + "a/b": "+pypi_a_b/site-packages/a/b", + "a/c": "+pypi_a_c/site-packages/a/c", + "foo": "+pypi_foo/site-packages/foo", + "foobar": "+pypi_foobar/site-packages/foobar", + "x/y/z": "+pypi_x_y_z/site-packages/x/y/z", } env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) @@ -164,12 +175,14 @@ _tests.append(_test_empty_and_trivial_inputs) def _test_empty_and_trivial_inputs_impl(env, _): # Test with empty list of entries actual = build_link_map(_ctx(), []) - env.expect.that_dict(actual).is_empty() + env.expect.that_dict(actual).contains_exactly({}) # Test with an entry with no files entries = [_entry("a", "+pypi_a/site-packages/a", [])] actual = build_link_map(_ctx(), entries) - env.expect.that_dict(actual).is_empty() + env.expect.that_dict(actual).contains_exactly({ + VenvSymlinkKind.LIB: {"a": "+pypi_a/site-packages/a"}, + }) def _test_multiple_venv_symlink_kinds(name): analysis_test( @@ -182,24 +195,45 @@ _tests.append(_test_multiple_venv_symlink_kinds) def _test_multiple_venv_symlink_kinds_impl(env, _): entries = [ - _entry("libfile", "+pypi_lib/site-packages/libfile", ["lib.txt"], kind = VenvSymlinkKind.LIB), - _entry("binfile", "+pypi_bin/bin/binfile", ["bin.txt"], kind = VenvSymlinkKind.BIN), - _entry("includefile", "+pypi_include/include/includefile", ["include.h"], kind = VenvSymlinkKind.INCLUDE), + _entry( + "libfile", + "+pypi_lib/site-packages/libfile", + ["lib.txt"], + kind = + VenvSymlinkKind.LIB, + ), + _entry( + "binfile", + "+pypi_bin/bin/binfile", + ["bin.txt"], + kind = VenvSymlinkKind.BIN, + ), + _entry( + "includefile", + "+pypi_include/include/includefile", + ["include.h"], + kind = + VenvSymlinkKind.INCLUDE, + ), ] actual = build_link_map(_ctx(), entries) + expected_libs = { - "libfile/lib.txt": _file("../+pypi_lib/site-packages/libfile/lib.txt"), + "libfile": "+pypi_lib/site-packages/libfile", } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + expected_bins = { - "binfile/bin.txt": _file("../+pypi_bin/bin/binfile/bin.txt"), + "binfile": "+pypi_bin/bin/binfile", } + env.expect.that_dict(actual[VenvSymlinkKind.BIN]).contains_exactly(expected_bins) + expected_includes = { - "includefile/include.h": _file("../+pypi_include/include/includefile/include.h"), + "includefile": "+pypi_include/include/includefile", } - env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) - env.expect.that_dict(actual[VenvSymlinkKind.BIN]).contains_exactly(expected_bins) env.expect.that_dict(actual[VenvSymlinkKind.INCLUDE]).contains_exactly(expected_includes) + env.expect.that_dict(actual).keys().contains_exactly([ VenvSymlinkKind.LIB, VenvSymlinkKind.BIN, From 259f0a9771a6021d1fce6142b66e590b359a9c9b Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 21:06:53 -0700 Subject: [PATCH 12/17] remove bad to_list_call() --- python/private/py_library.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 84718cf37c..f78d0eb40e 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -234,10 +234,10 @@ def _get_imports_and_venv_symlinks(ctx, semantics): if VenvsSitePackages.is_enabled(ctx): package, version_str = _get_package_and_version(ctx) imports = ctx.attr.imports - if len(imports.to_list()) == 0: + if len(imports) == 0: fail("When venvs_site_packages is enabled, exactly one `imports` " + "value must be specified, got 0") - elif len(imports.to_list()) > 1: + elif len(imports) > 1: fail("When venvs_site_packages is enabled, exactly one `imports` " + "value must be specified, got {}".format(imports)) From fa958035983c6d014bb1a44173a75f6a61f537d4 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 14 Sep 2025 21:25:02 -0700 Subject: [PATCH 13/17] add tests covering omission behavior --- .../app_files_building_tests.bzl | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl index b9779822ec..54b59d4c09 100644 --- a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl +++ b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl @@ -240,6 +240,57 @@ def _test_multiple_venv_symlink_kinds_impl(env, _): VenvSymlinkKind.INCLUDE, ]) +def _test_omit_entry(name): + analysis_test( + name = name, + impl = _test_omit_entry_impl, + target = "//python:none", + ) + +_tests.append(_test_omit_entry) + +def _test_omit_entry_impl(env, _): + entries = [ + # Omit by venv_path + _entry("foo", None), + _entry("foo", "+pypi_foo/site-packages/foo", ["foo.txt"]), + + # Omit by venv_path and package + _entry("bar", None, package = "bar"), + _entry("bar", "+pypi_bar_1/site-packages/bar", ["bar.txt"], package = "bar"), + _entry( + "bar", + "+pypi_bar_2/site-packages/bar", + ["bar_2.txt"], + package = "bar-two", + ), + + # Omit by package and version + _entry("baz", None, package = "baz", version = "1.0"), + _entry( + "baz", + "+pypi_baz_v1/site-packages/baz", + ["baz_v1.txt"], + package = "baz", + version = "1.0", + ), + _entry( + "baz", + "+pypi_baz_v2/site-packages/baz", + ["baz_v2.txt"], + package = "baz", + version = "2.0", + ), + ] + + actual = build_link_map(_ctx(), entries) + + expected_libs = { + "bar": "+pypi_bar_2/site-packages/bar", + "baz": "+pypi_baz_v2/site-packages/baz", + } + env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) + def app_files_building_test_suite(name): test_suite( name = name, From c2ec3853df9cecf2e1582b2956e02ff028a3f2c7 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Sep 2025 18:57:01 -0700 Subject: [PATCH 14/17] Revert "add tests covering omission behavior" This reverts commit fa958035983c6d014bb1a44173a75f6a61f537d4. --- .../app_files_building_tests.bzl | 51 ------------------- 1 file changed, 51 deletions(-) diff --git a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl index 54b59d4c09..b9779822ec 100644 --- a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl +++ b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl @@ -240,57 +240,6 @@ def _test_multiple_venv_symlink_kinds_impl(env, _): VenvSymlinkKind.INCLUDE, ]) -def _test_omit_entry(name): - analysis_test( - name = name, - impl = _test_omit_entry_impl, - target = "//python:none", - ) - -_tests.append(_test_omit_entry) - -def _test_omit_entry_impl(env, _): - entries = [ - # Omit by venv_path - _entry("foo", None), - _entry("foo", "+pypi_foo/site-packages/foo", ["foo.txt"]), - - # Omit by venv_path and package - _entry("bar", None, package = "bar"), - _entry("bar", "+pypi_bar_1/site-packages/bar", ["bar.txt"], package = "bar"), - _entry( - "bar", - "+pypi_bar_2/site-packages/bar", - ["bar_2.txt"], - package = "bar-two", - ), - - # Omit by package and version - _entry("baz", None, package = "baz", version = "1.0"), - _entry( - "baz", - "+pypi_baz_v1/site-packages/baz", - ["baz_v1.txt"], - package = "baz", - version = "1.0", - ), - _entry( - "baz", - "+pypi_baz_v2/site-packages/baz", - ["baz_v2.txt"], - package = "baz", - version = "2.0", - ), - ] - - actual = build_link_map(_ctx(), entries) - - expected_libs = { - "bar": "+pypi_bar_2/site-packages/bar", - "baz": "+pypi_baz_v2/site-packages/baz", - } - env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) - def app_files_building_test_suite(name): test_suite( name = name, From f254401bfe466c11e6aed5704bf2a2ac75b1bc4b Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Sep 2025 19:00:33 -0700 Subject: [PATCH 15/17] format --- .../app_files_building/app_files_building_tests.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl index b9779822ec..0a0265eb8c 100644 --- a/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl +++ b/tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl @@ -74,9 +74,9 @@ def _test_conflict_merging_impl(env, _): expected_libs = { "a/a.txt": _file("../+pypi_a/site-packages/a/a.txt"), "a/b/b.txt": _file("../+pypi_a_b/site-packages/a/b/b.txt"), + "duplicate/d.py": _file("../+dupe_a/site-packages/duplicate/d.py"), "x/p/p.txt": _file("src-dev/x/p/p.txt"), "x/x.txt": _file("src/x/x.txt"), - "duplicate/d.py": _file("../+dupe_a/site-packages/duplicate/d.py"), } env.expect.that_dict(actual[VenvSymlinkKind.LIB]).contains_exactly(expected_libs) env.expect.that_dict(actual).keys().contains_exactly([VenvSymlinkKind.LIB]) From 7717a7b30fc13511284b9a9d8ba6b00ad6bbe77d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Sep 2025 19:02:46 -0700 Subject: [PATCH 16/17] format fix --- python/private/py_library.bzl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index a9ce09b8b0..b2a9fdd3be 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -234,7 +234,10 @@ def _get_imports_and_venv_symlinks(ctx, semantics): venv_symlinks = [] if VenvsSitePackages.is_enabled(ctx): package, version_str = _get_package_and_version(ctx) - imports = ctx.attr.imports + + # NOTE: Already a list, but buildifier thinks its a depset and + # adds to_list() calls later. + imports = list(ctx.attr.imports) if len(imports) == 0: fail("When venvs_site_packages is enabled, exactly one `imports` " + "value must be specified, got 0") From ffdd2deb0d611e359e8a5a7df79b6274d6a90a80 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 15 Sep 2025 19:31:01 -0700 Subject: [PATCH 17/17] add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abbd5f5cf1..0adb458bd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,9 @@ END_UNRELEASED_TEMPLATE length errors due to too long environment variables. * (bootstrap) {obj}`--bootstrap_impl=script` now supports the `-S` interpreter setting. +* (venvs) {obj}`--vens_site_packages=yes` no longer errors when packages with + overlapping files or directories are used together. + ([#3204](https://github.com/bazel-contrib/rules_python/issues/3204)). {#v0-0-0-added} ### Added