Skip to content

feat: data and pyi files in the venv #2936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 49 commits into from
Jun 10, 2025
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
9dd7589
wip: add symlinks for .dist-info folders in the venv
aignas May 27, 2025
1381bf8
Merge branch 'main' into exp/distinfo-venv
aignas May 29, 2025
42e5059
fixup
aignas May 29, 2025
d59f85a
add a failing test
aignas May 29, 2025
714b245
add a test for topological ordering preference
aignas May 30, 2025
31e16ee
correctly pop existing links for package overrides
aignas May 30, 2025
e0affec
finish impl
aignas May 30, 2025
c844aa0
cleanup stale docs
aignas May 30, 2025
f726341
add an .so test file
aignas May 31, 2025
ef8114e
comment: rename src to package
aignas May 31, 2025
fa1249a
comment: add an explanation
aignas May 31, 2025
30c1d01
comment: pass pkg with version
aignas May 31, 2025
dfa24b6
comment: use package as the primary key
aignas May 31, 2025
6405e4c
a little bit of cleanup
aignas May 31, 2025
4d73e04
more robust skipping of the dist_info
aignas May 31, 2025
0c594e5
make the algo more robust
aignas May 31, 2025
c70f05a
make the linking algo a little bit more robust by adding a test for e…
aignas May 31, 2025
4c497f0
fix: return depset
aignas Jun 5, 2025
20d3c25
partially revert the code
aignas Jun 5, 2025
1d34260
add comments and add an extra value into the PyInfo
aignas Jun 5, 2025
a966f28
Merge branch 'main' into exp/distinfo-venv
aignas Jun 5, 2025
e82af52
add versions_bzl to py_library deps
aignas Jun 5, 2025
db169c5
switch to postorder ordering to first process the external deps
aignas Jun 5, 2025
16f835a
Revert "switch to postorder ordering to first process the external deps"
aignas Jun 5, 2025
5b7fb40
fix logic
aignas Jun 5, 2025
0abb1df
add package to PyInfo as an experiment to make the aggregation of the…
aignas Jun 5, 2025
bcb2310
refactor: separate the function out
aignas Jun 5, 2025
7b99ff7
fixup tests
aignas Jun 5, 2025
e873c7e
simplify the code
aignas Jun 5, 2025
02cae11
fixup
aignas Jun 5, 2025
5df2411
does not work: remove sorting add topological to depset creation
aignas Jun 5, 2025
2cf9688
does not work: remove sorting and create the depset later
aignas Jun 5, 2025
15c73f3
works: put back the sorting
aignas Jun 5, 2025
c269638
Revert "works: put back the sorting"
aignas Jun 5, 2025
9f58c91
does not work: add topological to pyinfo init
aignas Jun 5, 2025
18b1edf
Reapply "works: put back the sorting"
aignas Jun 5, 2025
507985b
Revert "Reapply "works: put back the sorting""
aignas Jun 5, 2025
d1142be
add depset characterization tests
aignas Jun 6, 2025
c298f3c
add all order tests for thinking
aignas Jun 6, 2025
7ec284b
Reapply "Reapply "works: put back the sorting""
aignas Jun 7, 2025
4f4e5fb
implement the sorting of targets manually
aignas Jun 7, 2025
fbcbe6d
fix typo
aignas Jun 7, 2025
6cfa441
more notes
aignas Jun 7, 2025
2081232
docs
aignas Jun 7, 2025
cdd6105
refactor
aignas Jun 7, 2025
c1abbeb
start cleanup
aignas Jun 8, 2025
65632f8
remove outdated docs
aignas Jun 8, 2025
0c4dacd
remove depset tests
aignas Jun 10, 2025
eac2bba
update docs
aignas Jun 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
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

Expand Down
6 changes: 6 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions internal_dev_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ def rules_python_internal_deps():
path = "tests/modules/other",
)

local_repository(
name = "another_module",
path = "tests/modules/another_module",
)

http_archive(
name = "bazel_skylib",
sha256 = "bc283cdfcd526a52c3201279cda4bc298652efa898b10b4db0837dc51652756f",
Expand Down
2 changes: 2 additions & 0 deletions python/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,13 @@ bzl_library(
":attributes_bzl",
":common_bzl",
":flags_bzl",
":normalize_name_bzl",
":precompile_bzl",
":py_cc_link_params_info_bzl",
":py_internal_bzl",
":rule_builders_bzl",
":toolchain_types_bzl",
":version_bzl",
"@bazel_skylib//lib:dicts",
"@bazel_skylib//rules:common_settings",
],
Expand Down
26 changes: 20 additions & 6 deletions python/private/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,19 @@ def collect_runfiles(ctx, files = depset()):
collect_default = True,
)

def _third_party_first(targets):
"""Sort targets to allow for deterministic depset merging.

First return the third party deps so that the depsets get merged in a way to allow
topological traversal so that the first dependency that is met during traversing
will be a third party dep.

This is because the DAG is going from first-party deps to third-party deps and usually
no third-party deps include first-party deps.
"""
return targets
# return sorted(targets, lambda x: PyInfo in x and not x[PyInfo].package)

def create_py_info(
ctx,
*,
Expand All @@ -378,6 +391,7 @@ def create_py_info(
implicit_pyc_files,
implicit_pyc_source_files,
imports,
package = None,
venv_symlinks = []):
"""Create PyInfo provider.

Expand All @@ -396,9 +410,9 @@ def create_py_info(
implicit_pyc_files: {type}`depset[File]` Implicitly generated pyc files
that a binary can choose to include.
imports: depset of strings; the import path values to propagate.
venv_symlinks: {type}`list[tuple[str, str]]` tuples of
`(runfiles_path, site_packages_path)` for symlinks to create
in the consuming binary's venv site packages.
package: TODO
venv_symlinks: {type}`list[VenvSymlinkEntry]` instances for
symlinks to create in the consuming binary's venv.

Returns:
A tuple of the PyInfo instance and a depset of the
Expand All @@ -419,7 +433,7 @@ def create_py_info(
py_info.merge_has_py2_only_sources(ctx.attr.srcs_version in ("PY2", "PY2ONLY"))
py_info.merge_has_py3_only_sources(ctx.attr.srcs_version in ("PY3", "PY3ONLY"))

for target in ctx.attr.deps:
for target in _third_party_first(ctx.attr.deps):
# PyInfo may not be present e.g. cc_library rules.
if PyInfo in target or (BuiltinPyInfo != None and BuiltinPyInfo in target):
py_info.merge(_get_py_info(target))
Expand All @@ -431,7 +445,7 @@ def create_py_info(
if f.extension == "py":
py_info.transitive_sources.add(f)
py_info.merge_uses_shared_libraries(cc_helper.is_valid_shared_library_artifact(f))
for target in ctx.attr.pyi_deps:
for target in _third_party_first(ctx.attr.pyi_deps):
# PyInfo may not be present e.g. cc_library rules.
if PyInfo in target or (BuiltinPyInfo != None and BuiltinPyInfo in target):
py_info.merge(_get_py_info(target))
Expand All @@ -458,7 +472,7 @@ def create_py_info(
if py_info.get_uses_shared_libraries():
break

return py_info.build(), deps_transitive_sources, py_info.build_builtin_py_info()
return py_info.build(package), deps_transitive_sources, py_info.build_builtin_py_info()

def _get_py_info(target):
return target[PyInfo] if PyInfo in target or BuiltinPyInfo == None else target[BuiltinPyInfo]
Expand Down
53 changes: 32 additions & 21 deletions python/private/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -680,43 +680,54 @@ def _create_venv_symlinks(ctx, venv_dir_map):
return venv_files

def _build_link_map(entries):
# dict[str kind, dict[str rel_path, str link_to_path]]
link_map = {}
# 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:
kind = entry.kind
kind_map = link_map.setdefault(kind, {})
if entry.venv_path in kind_map:
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. The dependency closer to the
# binary gets precedence due to the topological ordering.
continue
elif entry.venv_path in kind_map:
# NOTE @aignas 2025-06-05: This branch should be hit only for first party
# packages, maybe we should error here?
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 topological ordering, this allows binaries to remove
# entries by having an earlier dependency produce empty link_to 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)
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 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)
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):
Expand Down
33 changes: 20 additions & 13 deletions python/private/py_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,24 @@ the venv to create the path under.

A runfiles-root relative path that `venv_path` will symlink to. If `None`,
it means to not create a symlink.
""",
"package": """
:type: str | None

Represents the PyPI package name that the code originates from. It is normalized according to the
PEP440 with all `-` replaced with `_`, i.e. the same as the package name in the hub repository that
it would come from.
""",
"venv_path": """
:type: str

A path relative to the `kind` directory within the venv.
""",
"version": """
:type: str | None

Represents the PyPI package version that the code originates from. It is normalized according to the
PEP440 standard.
""",
},
)
Expand All @@ -94,14 +107,15 @@ def _PyInfo_init(
has_py2_only_sources = False,
has_py3_only_sources = False,
direct_pyc_files = depset(),
package = None,
transitive_pyc_files = depset(),
transitive_implicit_pyc_files = depset(),
transitive_implicit_pyc_source_files = depset(),
direct_original_sources = depset(),
transitive_original_sources = depset(),
direct_pyi_files = depset(),
transitive_pyi_files = depset(),
venv_symlinks = depset()):
venv_symlinks = depset(order = "topological")):
_check_arg_type("transitive_sources", "depset", transitive_sources)

# Verify it's postorder compatible, but retain is original ordering.
Expand Down Expand Up @@ -129,6 +143,7 @@ def _PyInfo_init(
"has_py2_only_sources": has_py2_only_sources,
"has_py3_only_sources": has_py2_only_sources,
"imports": imports,
"package": package,
"transitive_implicit_pyc_files": transitive_implicit_pyc_files,
"transitive_implicit_pyc_source_files": transitive_implicit_pyc_source_files,
"transitive_original_sources": transitive_original_sources,
Expand Down Expand Up @@ -205,6 +220,7 @@ Python targets. These are accumulated from the transitive `deps`.
The order of the depset is not guaranteed and may be changed in the future. It
is recommended to use `default` order (the default).
""",
"package": "TODO",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related discussion in Bazel Slack about asking if we should add this field: https://bazelbuild.slack.com/archives/CA306CEV6/p1743702775560119?thread_ts=1743702761.960199&cid=CA306CEV6

"transitive_implicit_pyc_files": """
:type: depset[File]

Expand Down Expand Up @@ -298,17 +314,6 @@ This field is currently unused in Bazel and may go away in the future.

A depset with `topological` ordering.


Tuples of `(runfiles_path, site_packages_path)`. Where
* `runfiles_path` is a runfiles-root relative path. It is the path that
has the code to make importable. If `None` or empty string, then it means
to not create a site packages directory with the `site_packages_path`
name.
* `site_packages_path` is a path relative to the site-packages directory of
the venv for whatever creates the venv (typically py_binary). It makes
the code in `runfiles_path` available for import. Note that this
is created as a "raw" symlink (via `declare_symlink`).

:::{include} /_includes/experimental_api.md
:::

Expand Down Expand Up @@ -623,11 +628,12 @@ def _PyInfoBuilder_merge_targets(self, targets):
self.merge_target(t)
return self

def _PyInfoBuilder_build(self):
def _PyInfoBuilder_build(self, package = None):
"""Builds into a {obj}`PyInfo` object.

Args:
self: implicitly added.
package: TODO

Returns:
{type}`PyInfo`
Expand All @@ -637,6 +643,7 @@ def _PyInfoBuilder_build(self):
direct_original_sources = self.direct_original_sources.build(),
direct_pyc_files = self.direct_pyc_files.build(),
direct_pyi_files = self.direct_pyi_files.build(),
package = package,
transitive_implicit_pyc_files = self.transitive_implicit_pyc_files.build(),
transitive_implicit_pyc_source_files = self.transitive_implicit_pyc_source_files.build(),
transitive_original_sources = self.transitive_original_sources.build(),
Expand Down
Loading