-
-
Notifications
You must be signed in to change notification settings - Fork 594
refactor: reimplement writing namespace pkgs in Starlark #2882
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
# __path__ manipulation added by bazel-contrib/rules_python to support namespace pkgs. | ||
__path__ = __import__("pkgutil").extend_path(__path__, __name__) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name implies it's a template, but I don't see any template variable names? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
"""Utilities to get where we should write namespace pkg paths.""" | ||
|
||
load("@bazel_skylib//rules:copy_file.bzl", "copy_file") | ||
|
||
_ext = struct( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .dylib (mac) and .dll (windows) are additional extension module filename extensions |
||
py = ".py", | ||
pyd = ".pyd", | ||
so = ".so", | ||
pyc = ".pyc", | ||
) | ||
|
||
_TEMPLATE = Label("//python/private/pypi:namespace_pkg_tmpl.py") | ||
|
||
def _add_all(dirname, dirs): | ||
dir_path = "." | ||
for dir_name in dirname.split("/"): | ||
dir_path = "{}/{}".format(dir_path, dir_name) | ||
dirs[dir_path[2:]] = None | ||
|
||
def get_files(*, srcs, ignored_dirnames = [], root = None): | ||
"""Get the list of filenames to write the namespace pkg files. | ||
|
||
Args: | ||
srcs: {type}`src` a list of files to be passed to {bzl:obj}`py_library` | ||
as `srcs` and `data`. This is usually a result of a {obj}`glob`. | ||
ignored_dirnames: {type}`str` a list of patterns to ignore. | ||
root: {type}`str` the prefix to use as the root. | ||
|
||
Returns: | ||
{type}`src` a list of paths to write the namespace pkg `__init__.py` file. | ||
""" | ||
dirs = {} | ||
ignored = {i: None for i in ignored_dirnames} | ||
|
||
if root: | ||
_add_all(root, ignored) | ||
|
||
for file in srcs: | ||
dirname, _, filename = file.rpartition("/") | ||
|
||
if filename == "__init__.py": | ||
ignored[dirname] = None | ||
dirname, _, _ = dirname.rpartition("/") | ||
elif filename.endswith(_ext.py): | ||
pass | ||
elif filename.endswith(_ext.pyc): | ||
pass | ||
elif filename.endswith(_ext.pyd): | ||
pass | ||
elif filename.endswith(_ext.so): | ||
pass | ||
else: | ||
continue | ||
|
||
if dirname in dirs or not dirname: | ||
continue | ||
|
||
_add_all(dirname, dirs) | ||
|
||
return sorted([d for d in dirs if d not in ignored]) | ||
|
||
def create_inits(**kwargs): | ||
"""Create init files and return the list to be included `py_library` srcs. | ||
|
||
Args: | ||
**kwargs: passed to {obj}`get_files`. | ||
|
||
Returns: | ||
{type}`list[str]` to be included as part of `py_library`. | ||
""" | ||
srcs = [] | ||
for out in get_files(**kwargs): | ||
src = "{}/__init__.py".format(out) | ||
srcs.append(srcs) | ||
|
||
copy_file( | ||
name = "_cp_{}_namespace".format(out), | ||
src = _TEMPLATE, | ||
out = src, | ||
**kwargs | ||
) | ||
|
||
return srcs |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,7 @@ | |
|
||
from pip._vendor.packaging.utils import canonicalize_name | ||
|
||
from python.private.pypi.whl_installer import arguments, namespace_pkgs, wheel | ||
from python.private.pypi.whl_installer import arguments, wheel | ||
|
||
|
||
def _configure_reproducible_wheels() -> None: | ||
|
@@ -77,35 +77,10 @@ def _parse_requirement_for_extra( | |
return None, None | ||
|
||
|
||
def _setup_namespace_pkg_compatibility(wheel_dir: str) -> None: | ||
"""Converts native namespace packages to pkgutil-style packages | ||
|
||
Namespace packages can be created in one of three ways. They are detailed here: | ||
https://packaging.python.org/guides/packaging-namespace-packages/#creating-a-namespace-package | ||
|
||
'pkgutil-style namespace packages' (2) and 'pkg_resources-style namespace packages' (3) works in Bazel, but | ||
'native namespace packages' (1) do not. | ||
|
||
We ensure compatibility with Bazel of method 1 by converting them into method 2. | ||
Comment on lines
-83
to
-89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please preserve this comment somewhere (namespace_pkgs.bzl ?). The link and why this legacy method is used were really helpful when I came across this code. |
||
|
||
Args: | ||
wheel_dir: the directory of the wheel to convert | ||
""" | ||
|
||
namespace_pkg_dirs = namespace_pkgs.implicit_namespace_packages( | ||
wheel_dir, | ||
ignored_dirnames=["%s/bin" % wheel_dir], | ||
) | ||
|
||
for ns_pkg_dir in namespace_pkg_dirs: | ||
namespace_pkgs.add_pkgutil_style_namespace_pkg_init(ns_pkg_dir) | ||
|
||
|
||
def _extract_wheel( | ||
wheel_file: str, | ||
extras: Dict[str, Set[str]], | ||
enable_pipstar: bool, | ||
enable_implicit_namespace_pkgs: bool, | ||
platforms: List[wheel.Platform], | ||
installation_dir: Path = Path("."), | ||
) -> None: | ||
|
@@ -116,15 +91,11 @@ def _extract_wheel( | |
installation_dir: the destination directory for installation of the wheel. | ||
extras: a list of extras to add as dependencies for the installed wheel | ||
enable_pipstar: if true, turns off certain operations. | ||
enable_implicit_namespace_pkgs: if true, disables conversion of implicit namespace packages and will unzip as-is | ||
""" | ||
|
||
whl = wheel.Wheel(wheel_file) | ||
whl.unzip(installation_dir) | ||
|
||
if not enable_implicit_namespace_pkgs: | ||
_setup_namespace_pkg_compatibility(installation_dir) | ||
|
||
metadata = { | ||
"entry_points": [ | ||
{ | ||
|
@@ -168,7 +139,6 @@ def main() -> None: | |
wheel_file=whl, | ||
extras=extras, | ||
enable_pipstar=args.enable_pipstar, | ||
enable_implicit_namespace_pkgs=args.enable_implicit_namespace_pkgs, | ||
platforms=arguments.get_platforms(args), | ||
) | ||
return | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -173,9 +173,6 @@ def _parse_optional_attrs(rctx, args, extra_pip_args = None): | |||
json.encode(struct(arg = rctx.attr.pip_data_exclude)), | ||||
] | ||||
|
||||
if rctx.attr.enable_implicit_namespace_pkgs: | ||||
args.append("--enable_implicit_namespace_pkgs") | ||||
|
||||
env = {} | ||||
if rctx.attr.environment != None: | ||||
for key, value in rctx.attr.environment.items(): | ||||
|
@@ -389,6 +386,8 @@ def _whl_library_impl(rctx): | |||
metadata_name = metadata.name, | ||||
metadata_version = metadata.version, | ||||
requires_dist = metadata.requires_dist, | ||||
# TODO @aignas 2025-05-17: maybe have a build flag for this instead | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless there's a commitment to resolve a TODO, leave them out. (IMHO: no build flag needed)
Suggested change
|
||||
enable_implicit_namespace_pkgs = rctx.attr.enable_implicit_namespace_pkgs, | ||||
# TODO @aignas 2025-04-14: load through the hub: | ||||
annotation = None if not rctx.attr.annotation else struct(**json.decode(rctx.read(rctx.attr.annotation))), | ||||
data_exclude = rctx.attr.pip_data_exclude, | ||||
|
@@ -457,6 +456,8 @@ def _whl_library_impl(rctx): | |||
name = whl_path.basename, | ||||
dep_template = rctx.attr.dep_template or "@{}{{name}}//:{{target}}".format(rctx.attr.repo_prefix), | ||||
entry_points = entry_points, | ||||
# TODO @aignas 2025-05-17: maybe have a build flag for this instead | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
enable_implicit_namespace_pkgs = rctx.attr.enable_implicit_namespace_pkgs, | ||||
# TODO @aignas 2025-04-14: load through the hub: | ||||
dependencies = metadata["deps"], | ||||
dependencies_by_platform = metadata["deps_by_platform"], | ||||
|
@@ -580,7 +581,6 @@ attr makes `extra_pip_args` and `download_only` ignored.""", | |||
Label("//python/private/pypi/whl_installer:wheel.py"), | ||||
Label("//python/private/pypi/whl_installer:wheel_installer.py"), | ||||
Label("//python/private/pypi/whl_installer:arguments.py"), | ||||
Label("//python/private/pypi/whl_installer:namespace_pkgs.py"), | ||||
] + record_files.values(), | ||||
), | ||||
"_rule_name": attr.string(default = "whl_library"), | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
load(":namespace_pkgs_tests.bzl", "namespace_pkgs_test_suite") | ||
|
||
namespace_pkgs_test_suite( | ||
name = "namespace_pkgs_tests", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use an internal name, or factor out a define_internal_flags() function (similar to what pypi).