From a13fcd77cd27bd131e1b4ce227372312614613f2 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 14 May 2025 07:16:11 +0900 Subject: [PATCH 1/2] feat(pypi): actually start using env_marker_setting (#2873) Summary: - `pep508_deps` is now much simpler, because the hard work is done in analysis phase - `whl_library` BUILD.bazel tests now also have a test for the legacy flow. One thing that I noticed is that now we have an implicit dependency on the python toolchain when getting all of the `whl` target tree. This is a filegroup target that includes dependent wheels. However, we fallback to the flag values if we don't have the toolchain, so we should be good in general. Overall I like how this is turning out because we don't need to pipe the `target_platforms` anymore when we enable `PIPSTAR` feature. This means that we can start creating fewer whl_library instances - e.g. a `py3-none-any` wheel can be fetched once instead of once per python interpreter version. I'll leave this optimization for a later time. Work towards #260 --------- Co-authored-by: Richard Levasseur --- python/private/pypi/BUILD.bazel | 4 - python/private/pypi/config.bzl.tmpl.bzlmod | 2 +- python/private/pypi/extension.bzl | 35 ++- .../pypi/generate_whl_library_build_bazel.bzl | 27 +- python/private/pypi/hub_repository.bzl | 2 +- python/private/pypi/pep508_deps.bzl | 131 +++------- python/private/pypi/pep508_evaluate.bzl | 4 +- .../pypi/whl_installer/wheel_installer.py | 1 - python/private/pypi/whl_library.bzl | 4 - python/private/pypi/whl_library_targets.bzl | 77 +++--- tests/pypi/extension/extension_tests.bzl | 7 +- ...generate_whl_library_build_bazel_tests.bzl | 70 ++++- tests/pypi/pep508/deps_tests.bzl | 241 +++--------------- .../whl_installer/wheel_installer_test.py | 3 +- .../whl_library_targets_tests.bzl | 28 +- 15 files changed, 249 insertions(+), 387 deletions(-) diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index f541cbe98b..06ca3a8e34 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -236,13 +236,9 @@ bzl_library( name = "pep508_deps_bzl", srcs = ["pep508_deps.bzl"], deps = [ - ":pep508_env_bzl", ":pep508_evaluate_bzl", - ":pep508_platform_bzl", ":pep508_requirement_bzl", - "//python/private:full_version_bzl", "//python/private:normalize_name_bzl", - "@pythons_hub//:versions_bzl", ], ) diff --git a/python/private/pypi/config.bzl.tmpl.bzlmod b/python/private/pypi/config.bzl.tmpl.bzlmod index deb53631d1..c3ada70d27 100644 --- a/python/private/pypi/config.bzl.tmpl.bzlmod +++ b/python/private/pypi/config.bzl.tmpl.bzlmod @@ -6,4 +6,4 @@ with your usecase. This may change in between rules_python versions without any @generated by rules_python pip.parse bzlmod extension. """ -target_platforms = %%TARGET_PLATFORMS%% +whl_map = %%WHL_MAP%% diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 9368e6539f..84caa0aee7 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -17,6 +17,7 @@ load("@bazel_features//:features.bzl", "bazel_features") load("@pythons_hub//:interpreters.bzl", "INTERPRETER_LABELS") load("@pythons_hub//:versions.bzl", "MINOR_MAPPING") +load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config") load("//python/private:auth.bzl", "AUTH_ATTRS") load("//python/private:full_version.bzl", "full_version") load("//python/private:normalize_name.bzl", "normalize_name") @@ -72,7 +73,8 @@ def _create_whl_repos( available_interpreters = INTERPRETER_LABELS, minor_mapping = MINOR_MAPPING, evaluate_markers = evaluate_markers_py, - get_index_urls = None): + get_index_urls = None, + enable_pipstar = False): """create all of the whl repositories Args: @@ -87,6 +89,7 @@ def _create_whl_repos( minor_mapping: {type}`dict[str, str]` The dictionary needed to resolve the full python version used to parse package METADATA files. evaluate_markers: the function used to evaluate the markers. + enable_pipstar: enable the pipstar feature. Returns a {type}`struct` with the following attributes: whl_map: {type}`dict[str, list[struct]]` the output is keyed by the @@ -216,7 +219,6 @@ def _create_whl_repos( enable_implicit_namespace_pkgs = pip_attr.enable_implicit_namespace_pkgs, environment = pip_attr.environment, envsubst = pip_attr.envsubst, - experimental_target_platforms = pip_attr.experimental_target_platforms, group_deps = group_deps, group_name = group_name, pip_data_exclude = pip_attr.pip_data_exclude, @@ -227,6 +229,9 @@ def _create_whl_repos( for p, args in whl_overrides.get(whl_name, {}).items() }, ) + if not enable_pipstar: + maybe_args["experimental_target_platforms"] = pip_attr.experimental_target_platforms + whl_library_args.update({k: v for k, v in maybe_args.items() if v}) maybe_args_with_default = dict( # The following values have defaults next to them @@ -249,6 +254,7 @@ def _create_whl_repos( auth_patterns = pip_attr.auth_patterns, python_version = major_minor, multiple_requirements_for_whl = len(requirements) > 1., + enable_pipstar = enable_pipstar, ).items(): repo_name = "{}_{}".format(pip_name, repo_name) if repo_name in whl_libraries: @@ -277,7 +283,7 @@ def _create_whl_repos( }, ) -def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patterns, multiple_requirements_for_whl = False, python_version): +def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patterns, multiple_requirements_for_whl = False, python_version, enable_pipstar = False): ret = {} dists = requirement.whls @@ -305,13 +311,14 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt args["urls"] = [distribution.url] args["sha256"] = distribution.sha256 args["filename"] = distribution.filename - args["experimental_target_platforms"] = [ - # Get rid of the version fot the target platforms because we are - # passing the interpreter any way. Ideally we should search of ways - # how to pass the target platforms through the hub repo. - p.partition("_")[2] - for p in requirement.target_platforms - ] + if not enable_pipstar: + args["experimental_target_platforms"] = [ + # Get rid of the version fot the target platforms because we are + # passing the interpreter any way. Ideally we should search of ways + # how to pass the target platforms through the hub repo. + p.partition("_")[2] + for p in requirement.target_platforms + ] # Pure python wheels or sdists may need to have a platform here target_platforms = None @@ -357,7 +364,11 @@ def _whl_repos(*, requirement, whl_library_args, download_only, netrc, auth_patt return ret -def parse_modules(module_ctx, _fail = fail, simpleapi_download = simpleapi_download, **kwargs): +def parse_modules( + module_ctx, + _fail = fail, + simpleapi_download = simpleapi_download, + **kwargs): """Implementation of parsing the tag classes for the extension and return a struct for registering repositories. Args: @@ -639,7 +650,7 @@ def _pip_impl(module_ctx): module_ctx: module contents """ - mods = parse_modules(module_ctx) + mods = parse_modules(module_ctx, enable_pipstar = rp_config.enable_pipstar) # Build all of the wheel modifications if the tag class is called. _whl_mods_impl(mods.whl_mods) diff --git a/python/private/pypi/generate_whl_library_build_bazel.bzl b/python/private/pypi/generate_whl_library_build_bazel.bzl index 31c9d4da60..3764e720c0 100644 --- a/python/private/pypi/generate_whl_library_build_bazel.bzl +++ b/python/private/pypi/generate_whl_library_build_bazel.bzl @@ -26,10 +26,11 @@ _RENDER = { "entry_points": render.dict, "extras": render.list, "group_deps": render.list, + "include": str, "requires_dist": render.list, "srcs_exclude": render.list, "tags": render.list, - "target_platforms": lambda x: render.list(x) if x else "target_platforms", + "target_platforms": render.list, } # NOTE @aignas 2024-10-25: We have to keep this so that files in @@ -62,28 +63,44 @@ def generate_whl_library_build_bazel( A complete BUILD file as a string """ - fn = "whl_library_targets" + loads = [] if kwargs.get("tags"): + fn = "whl_library_targets" + # legacy path unsupported_args = [ "requires", "metadata_name", "metadata_version", + "include", ] else: - fn = "{}_from_requires".format(fn) + fn = "whl_library_targets_from_requires" unsupported_args = [ "dependencies", "dependencies_by_platform", + "target_platforms", + "default_python_version", ] + dep_template = kwargs.get("dep_template") + loads.append( + """load("{}", "{}")""".format( + dep_template.format( + name = "", + target = "config.bzl", + ), + "whl_map", + ), + ) + kwargs["include"] = "whl_map" for arg in unsupported_args: if kwargs.get(arg): fail("BUG, unsupported arg: '{}'".format(arg)) - loads = [ + loads.extend([ """load("@rules_python//python/private/pypi:whl_library_targets.bzl", "{}")""".format(fn), - ] + ]) additional_content = [] if annotation: diff --git a/python/private/pypi/hub_repository.bzl b/python/private/pypi/hub_repository.bzl index d2cbf88c24..0a1e772d05 100644 --- a/python/private/pypi/hub_repository.bzl +++ b/python/private/pypi/hub_repository.bzl @@ -49,7 +49,7 @@ def _impl(rctx): "config.bzl", rctx.attr._config_template, substitutions = { - "%%TARGET_PLATFORMS%%": render.list(rctx.attr.target_platforms), + "%%WHL_MAP%%": render.dict(rctx.attr.whl_map, value_repr = lambda x: "None"), }, ) rctx.template("requirements.bzl", rctx.attr._requirements_bzl_template, substitutions = { diff --git a/python/private/pypi/pep508_deps.bzl b/python/private/pypi/pep508_deps.bzl index bcc4845cf1..e73f747bed 100644 --- a/python/private/pypi/pep508_deps.bzl +++ b/python/private/pypi/pep508_deps.bzl @@ -15,23 +15,17 @@ """This module is for implementing PEP508 compliant METADATA deps parsing. """ -load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION", "MINOR_MAPPING") -load("//python/private:full_version.bzl", "full_version") load("//python/private:normalize_name.bzl", "normalize_name") -load(":pep508_env.bzl", "env") load(":pep508_evaluate.bzl", "evaluate") -load(":pep508_platform.bzl", "platform", "platform_from_str") load(":pep508_requirement.bzl", "requirement") def deps( name, *, requires_dist, - platforms = [], extras = [], excludes = [], - default_python_version = None, - minor_mapping = MINOR_MAPPING): + include = []): """Parse the RequiresDist from wheel METADATA Args: @@ -39,12 +33,9 @@ def deps( requires_dist: {type}`list[str]` the list of RequiresDist lines from the METADATA file. excludes: {type}`list[str]` what packages should we exclude. + include: {type}`list[str]` what packages should we exclude. If it is not + specified, then we will include all deps from `requires_dist`. extras: {type}`list[str]` the requested extras to generate targets for. - platforms: {type}`list[str]` the list of target platform strings. - default_python_version: {type}`str` the host python version. - minor_mapping: {type}`type[str, str]` the minor mapping to use when - resolving to the full python version as DEFAULT_PYTHON_VERSION can by - of format `3.x`. Returns: A struct with attributes: @@ -60,39 +51,20 @@ def deps( deps_select = {} name = normalize_name(name) want_extras = _resolve_extras(name, reqs, extras) + include = [normalize_name(n) for n in include] # drop self edges excludes = [name] + [normalize_name(x) for x in excludes] - default_python_version = default_python_version or DEFAULT_PYTHON_VERSION - if default_python_version: - # if it is not bzlmod, then DEFAULT_PYTHON_VERSION may be unset - default_python_version = full_version( - version = default_python_version, - minor_mapping = minor_mapping, - ) - platforms = [ - platform_from_str(p, python_version = default_python_version) - for p in platforms - ] - - abis = sorted({p.abi: True for p in platforms if p.abi}) - if default_python_version and len(abis) > 1: - _, _, tail = default_python_version.partition(".") - default_abi = "cp3" + tail - elif len(abis) > 1: - fail( - "all python versions need to be specified explicitly, got: {}".format(platforms), - ) - else: - default_abi = None - reqs_by_name = {} for req in reqs: if req.name_ in excludes: continue + if include and req.name_ not in include: + continue + reqs_by_name.setdefault(req.name, []).append(req) for name, reqs in reqs_by_name.items(): @@ -102,55 +74,25 @@ def deps( normalize_name(name), reqs, extras = want_extras, - platforms = platforms, - default_abi = default_abi, ) return struct( deps = sorted(deps), deps_select = { - _platform_str(p): sorted(deps) - for p, deps in deps_select.items() + d: markers + for d, markers in sorted(deps_select.items()) }, ) -def _platform_str(self): - if self.abi == None: - return "{}_{}".format(self.os, self.arch) - - return "{}_{}_{}".format( - self.abi, - self.os or "anyos", - self.arch or "anyarch", - ) - -def _add(deps, deps_select, dep, platform): +def _add(deps, deps_select, dep, markers = None): dep = normalize_name(dep) - if platform == None: + if not markers: deps[dep] = True - - # If the dep is in the platform-specific list, remove it from the select. - pop_keys = [] - for p, _deps in deps_select.items(): - if dep not in _deps: - continue - - _deps.pop(dep) - if not _deps: - pop_keys.append(p) - - for p in pop_keys: - deps_select.pop(p) - return - - if dep in deps: - # If the dep is already in the main dependency list, no need to add it in the - # platform-specific dependency list. - return - - # Add the platform-specific branch - deps_select.setdefault(platform, {})[dep] = True + elif len(markers) == 1: + deps_select[dep] = markers[0] + else: + deps_select[dep] = "({})".format(") or (".join(sorted(markers))) def _resolve_extras(self_name, reqs, extras): """Resolve extras which are due to depending on self[some_other_extra]. @@ -207,37 +149,24 @@ def _resolve_extras(self_name, reqs, extras): # Poor mans set return sorted({x: None for x in extras}) -def _add_reqs(deps, deps_select, dep, reqs, *, extras, platforms, default_abi = None): +def _add_reqs(deps, deps_select, dep, reqs, *, extras): for req in reqs: if not req.marker: - _add(deps, deps_select, dep, None) + _add(deps, deps_select, dep) return - platforms_to_add = {} - for plat in platforms: - if plat in platforms_to_add: - # marker evaluation is more expensive than this check - continue - - added = False - for extra in extras: - if added: + markers = {} + for req in reqs: + for x in extras: + m = evaluate(req.marker, env = {"extra": x}, strict = False) + if m == False: + continue + elif m == True: + _add(deps, deps_select, dep) break + else: + markers[m] = None + continue - for req in reqs: - if evaluate(req.marker, env = env(target_platform = plat, extra = extra)): - platforms_to_add[plat] = True - added = True - break - - if len(platforms_to_add) == len(platforms): - # the dep is in all target platforms, let's just add it to the regular - # list - _add(deps, deps_select, dep, None) - return - - for plat in platforms_to_add: - if default_abi: - _add(deps, deps_select, dep, plat) - if plat.abi == default_abi or not default_abi: - _add(deps, deps_select, dep, platform(os = plat.os, arch = plat.arch)) + if markers: + _add(deps, deps_select, dep, sorted(markers)) diff --git a/python/private/pypi/pep508_evaluate.bzl b/python/private/pypi/pep508_evaluate.bzl index 61a5b19999..d4492a75bb 100644 --- a/python/private/pypi/pep508_evaluate.bzl +++ b/python/private/pypi/pep508_evaluate.bzl @@ -122,7 +122,9 @@ def evaluate(marker, *, env, strict = True, **kwargs): **kwargs: Extra kwargs to be passed to the expression evaluator. Returns: - The {type}`bool` If the marker is compatible with the given env. + The {type}`bool | str` If the marker is compatible with the given env. If strict is + `False`, then the output type is `str` which will represent the remaining + expression that has not been evaluated. """ tokens = tokenize(marker) diff --git a/python/private/pypi/whl_installer/wheel_installer.py b/python/private/pypi/whl_installer/wheel_installer.py index 2db03e039d..600d45f940 100644 --- a/python/private/pypi/whl_installer/wheel_installer.py +++ b/python/private/pypi/whl_installer/wheel_installer.py @@ -126,7 +126,6 @@ def _extract_wheel( _setup_namespace_pkg_compatibility(installation_dir) metadata = { - "python_version": f"{sys.version_info[0]}.{sys.version_info[1]}.{sys.version_info[2]}", "entry_points": [ { "name": name, diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index 4427c0e3ef..b370de448a 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -22,7 +22,6 @@ load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils") load(":attrs.bzl", "ATTRS", "use_isolated") load(":deps.bzl", "all_repo_names", "record_files") load(":generate_whl_library_build_bazel.bzl", "generate_whl_library_build_bazel") -load(":parse_requirements.bzl", "host_platform") load(":parse_whl_name.bzl", "parse_whl_name") load(":patch_whl.bzl", "patch_whl") load(":pypi_repo_utils.bzl", "pypi_repo_utils") @@ -352,7 +351,6 @@ def _whl_library_impl(rctx): metadata = json.decode(rctx.read("metadata.json")) rctx.delete("metadata.json") - python_version = metadata["python_version"] # NOTE @aignas 2024-06-22: this has to live on until we stop supporting # passing `twine` as a `:pkg` library via the `WORKSPACE` builds. @@ -390,9 +388,7 @@ def _whl_library_impl(rctx): entry_points = entry_points, metadata_name = metadata.name, metadata_version = metadata.version, - default_python_version = python_version, requires_dist = metadata.requires_dist, - target_platforms = rctx.attr.experimental_target_platforms or [host_platform(rctx)], # 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, diff --git a/python/private/pypi/whl_library_targets.bzl b/python/private/pypi/whl_library_targets.bzl index 21e4a54a3a..e0c03a1505 100644 --- a/python/private/pypi/whl_library_targets.bzl +++ b/python/private/pypi/whl_library_targets.bzl @@ -19,6 +19,7 @@ load("//python:py_binary.bzl", "py_binary") load("//python:py_library.bzl", "py_library") load("//python/private:glob_excludes.bzl", "glob_excludes") load("//python/private:normalize_name.bzl", "normalize_name") +load(":env_marker_setting.bzl", "env_marker_setting") load( ":labels.bzl", "DATA_LABEL", @@ -29,9 +30,7 @@ load( "WHEEL_FILE_IMPL_LABEL", "WHEEL_FILE_PUBLIC_LABEL", ) -load(":parse_whl_name.bzl", "parse_whl_name") load(":pep508_deps.bzl", "deps") -load(":whl_target_platforms.bzl", "whl_target_platforms") def whl_library_targets_from_requires( *, @@ -40,8 +39,7 @@ def whl_library_targets_from_requires( metadata_version = "", requires_dist = [], extras = [], - target_platforms = [], - default_python_version = None, + include = [], group_deps = [], **kwargs): """The macro to create whl targets from the METADATA. @@ -57,25 +55,21 @@ def whl_library_targets_from_requires( requires_dist: {type}`list[str]` The list of `Requires-Dist` values from the whl `METADATA`. extras: {type}`list[str]` The list of requested extras. This essentially includes extra transitive dependencies in the final targets depending on the wheel `METADATA`. - target_platforms: {type}`list[str]` The list of target platforms to create - dependency closures for. - default_python_version: {type}`str` The python version to assume when parsing - the `METADATA`. This is only used when the `target_platforms` do not - include the version information. + include: {type}`list[str]` The list of packages to include. **kwargs: Extra args passed to the {obj}`whl_library_targets` """ package_deps = _parse_requires_dist( - name = name, - default_python_version = default_python_version, + name = metadata_name, requires_dist = requires_dist, excludes = group_deps, extras = extras, - target_platforms = target_platforms, + include = include, ) + whl_library_targets( name = name, dependencies = package_deps.deps, - dependencies_by_platform = package_deps.deps_select, + dependencies_with_markers = package_deps.deps_select, tags = [ "pypi_name={}".format(metadata_name), "pypi_version={}".format(metadata_version), @@ -86,31 +80,16 @@ def whl_library_targets_from_requires( def _parse_requires_dist( *, name, - default_python_version, requires_dist, excludes, - extras, - target_platforms): - parsed_whl = parse_whl_name(name) - - # NOTE @aignas 2023-12-04: if the wheel is a platform specific wheel, we - # only include deps for that target platform - if parsed_whl.platform_tag != "any": - target_platforms = [ - p.target_platform - for p in whl_target_platforms( - platform_tag = parsed_whl.platform_tag, - abi_tag = parsed_whl.abi_tag.strip("tm"), - ) - ] - + include, + extras): return deps( - name = normalize_name(parsed_whl.distribution), + name = normalize_name(name), requires_dist = requires_dist, - platforms = target_platforms, excludes = excludes, + include = include, extras = extras, - default_python_version = default_python_version, ) def whl_library_targets( @@ -126,6 +105,7 @@ def whl_library_targets( }, dependencies = [], dependencies_by_platform = {}, + dependencies_with_markers = {}, group_deps = [], group_name = "", data = [], @@ -137,6 +117,7 @@ def whl_library_targets( copy_file = copy_file, py_binary = py_binary, py_library = py_library, + env_marker_setting = env_marker_setting, )): """Create all of the whl_library targets. @@ -149,6 +130,8 @@ def whl_library_targets( dependencies: {type}`list[str]` A list of dependencies. dependencies_by_platform: {type}`dict[str, list[str]]` A list of dependencies by platform key. + dependencies_with_markers: {type}`dict[str, str]` A marker to evaluate + in order for the dep to be included. filegroups: {type}`dict[str, list[str]]` A dictionary of the target names and the glob matches. group_name: {type}`str` name of the dependency group (if any) which @@ -207,10 +190,16 @@ def whl_library_targets( data.append(dest) _config_settings( - dependencies_by_platform.keys(), + dependencies_by_platform = dependencies_by_platform.keys(), + dependencies_with_markers = dependencies_with_markers, native = native, + rules = rules, visibility = ["//visibility:private"], ) + deps_conditional = { + d: "is_include_{}_true".format(d) + for d in dependencies_with_markers + } # TODO @aignas 2024-10-25: remove the entry_point generation once # `py_console_script_binary` is the only way to use entry points. @@ -290,6 +279,7 @@ def whl_library_targets( data = _deps( deps = dependencies, deps_by_platform = dependencies_by_platform, + deps_conditional = deps_conditional, tmpl = dep_template.format(name = "{}", target = WHEEL_FILE_PUBLIC_LABEL), # NOTE @aignas 2024-10-28: Actually, `select` is not part of # `native`, but in order to support bazel 6.4 in unit tests, I @@ -342,6 +332,7 @@ def whl_library_targets( deps = _deps( deps = dependencies, deps_by_platform = dependencies_by_platform, + deps_conditional = deps_conditional, tmpl = dep_template.format(name = "{}", target = PY_LIBRARY_PUBLIC_LABEL), select = getattr(native, "select", select), ), @@ -350,7 +341,7 @@ def whl_library_targets( experimental_venvs_site_packages = Label("@rules_python//python/config_settings:venvs_site_packages"), ) -def _config_settings(dependencies_by_platform, native = native, **kwargs): +def _config_settings(dependencies_by_platform, dependencies_with_markers, rules, native = native, **kwargs): """Generate config settings for the targets. Args: @@ -362,9 +353,19 @@ def _config_settings(dependencies_by_platform, native = native, **kwargs): * `@//python/config_settings:is_python_3.{minor_version}` * `{os}_{cpu}` * `cp3{minor_version}_{os}_{cpu}` + dependencies_with_markers: {type}`dict[str, str]` The markers to evaluate by + each dep. + rules: used for testing native: {type}`native` The native struct for overriding in tests. **kwargs: Extra kwargs to pass to the rule. """ + for dep, expression in dependencies_with_markers.items(): + rules.env_marker_setting( + name = "include_{}".format(dep), + expression = expression, + **kwargs + ) + for p in dependencies_by_platform: if p.startswith("@") or p.endswith("default"): continue @@ -404,9 +405,15 @@ def _plat_label(plat): else: return ":is_" + plat.replace("cp3", "python_3.") -def _deps(deps, deps_by_platform, tmpl, select = select): +def _deps(deps, deps_by_platform, deps_conditional, tmpl, select = select): deps = [tmpl.format(d) for d in sorted(deps)] + for dep, setting in deps_conditional.items(): + deps = deps + select({ + ":{}".format(setting): [tmpl.format(dep)], + "//conditions:default": [], + }) + if not deps_by_platform: return deps diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl index b4a7746271..8e325724f4 100644 --- a/tests/pypi/extension/extension_tests.bzl +++ b/tests/pypi/extension/extension_tests.bzl @@ -62,7 +62,12 @@ def _mod(*, name, parse = [], override = [], whl_mods = [], is_root = True): def _parse_modules(env, **kwargs): return env.expect.that_struct( - parse_modules(**kwargs), + parse_modules( + # TODO @aignas 2025-05-11: start integration testing the branch which + # includes this. + enable_pipstar = 0, + **kwargs + ), attrs = dict( exposed_packages = subjects.dict, hub_group_map = subjects.dict, diff --git a/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl b/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl index 83be7395d4..225b296ebf 100644 --- a/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl +++ b/tests/pypi/generate_whl_library_build_bazel/generate_whl_library_build_bazel_tests.bzl @@ -19,8 +19,73 @@ load("//python/private/pypi:generate_whl_library_build_bazel.bzl", "generate_whl _tests = [] +def _test_all_legacy(env): + want = """\ +load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets") + +package(default_visibility = ["//visibility:public"]) + +whl_library_targets( + copy_executables = { + "exec_src": "exec_dest", + }, + copy_files = { + "file_src": "file_dest", + }, + data = ["extra_target"], + data_exclude = [ + "exclude_via_attr", + "data_exclude_all", + ], + dep_template = "@pypi//{name}:{target}", + dependencies = ["foo"], + dependencies_by_platform = { + "baz": ["bar"], + }, + entry_points = { + "foo": "bar.py", + }, + group_deps = [ + "foo", + "fox", + "qux", + ], + group_name = "qux", + name = "foo.whl", + srcs_exclude = ["srcs_exclude_all"], + tags = ["tag1"], +) + +# SOMETHING SPECIAL AT THE END +""" + actual = generate_whl_library_build_bazel( + dep_template = "@pypi//{name}:{target}", + name = "foo.whl", + dependencies = ["foo"], + dependencies_by_platform = {"baz": ["bar"]}, + entry_points = { + "foo": "bar.py", + }, + data_exclude = ["exclude_via_attr"], + annotation = struct( + copy_files = {"file_src": "file_dest"}, + copy_executables = {"exec_src": "exec_dest"}, + data = ["extra_target"], + data_exclude_glob = ["data_exclude_all"], + srcs_exclude_glob = ["srcs_exclude_all"], + additive_build_content = """# SOMETHING SPECIAL AT THE END""", + ), + group_name = "qux", + group_deps = ["foo", "fox", "qux"], + tags = ["tag1"], + ) + env.expect.that_str(actual.replace("@@", "@")).equals(want) + +_tests.append(_test_all_legacy) + def _test_all(env): want = """\ +load("@pypi//:config.bzl", "whl_map") load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires") package(default_visibility = ["//visibility:public"]) @@ -47,6 +112,7 @@ whl_library_targets_from_requires( "qux", ], group_name = "qux", + include = whl_map, name = "foo.whl", requires_dist = [ "foo", @@ -54,7 +120,6 @@ whl_library_targets_from_requires( "qux", ], srcs_exclude = ["srcs_exclude_all"], - target_platforms = ["foo"], ) # SOMETHING SPECIAL AT THE END @@ -76,7 +141,6 @@ whl_library_targets_from_requires( additive_build_content = """# SOMETHING SPECIAL AT THE END""", ), group_name = "qux", - target_platforms = ["foo"], group_deps = ["foo", "fox", "qux"], ) env.expect.that_str(actual.replace("@@", "@")).equals(want) @@ -85,6 +149,7 @@ _tests.append(_test_all) def _test_all_with_loads(env): want = """\ +load("@pypi//:config.bzl", "whl_map") load("@rules_python//python/private/pypi:whl_library_targets.bzl", "whl_library_targets_from_requires") package(default_visibility = ["//visibility:public"]) @@ -111,6 +176,7 @@ whl_library_targets_from_requires( "qux", ], group_name = "qux", + include = whl_map, name = "foo.whl", requires_dist = [ "foo", diff --git a/tests/pypi/pep508/deps_tests.bzl b/tests/pypi/pep508/deps_tests.bzl index 118cd50092..aaa3b2f7dd 100644 --- a/tests/pypi/pep508/deps_tests.bzl +++ b/tests/pypi/pep508/deps_tests.bzl @@ -29,101 +29,41 @@ def test_simple_deps(env): _tests.append(test_simple_deps) def test_can_add_os_specific_deps(env): - for target in [ - struct( - platforms = [ - "linux_x86_64", - "osx_x86_64", - "osx_aarch64", - "windows_x86_64", - ], - python_version = "3.3.1", - ), - struct( - platforms = [ - "cp33_linux_x86_64", - "cp33_osx_x86_64", - "cp33_osx_aarch64", - "cp33_windows_x86_64", - ], - python_version = "", - ), - struct( - platforms = [ - "cp33.1_linux_x86_64", - "cp33.1_osx_x86_64", - "cp33.1_osx_aarch64", - "cp33.1_windows_x86_64", - ], - python_version = "", - ), - ]: - got = deps( - "foo", - requires_dist = [ - "bar", - "an_osx_dep; sys_platform=='darwin'", - "posix_dep; os_name=='posix'", - "win_dep; os_name=='nt'", - ], - platforms = target.platforms, - default_python_version = target.python_version, - ) - - env.expect.that_collection(got.deps).contains_exactly(["bar"]) - env.expect.that_dict(got.deps_select).contains_exactly({ - "linux_x86_64": ["posix_dep"], - "osx_aarch64": ["an_osx_dep", "posix_dep"], - "osx_x86_64": ["an_osx_dep", "posix_dep"], - "windows_x86_64": ["win_dep"], - }) - -_tests.append(test_can_add_os_specific_deps) - -def test_deps_are_added_to_more_specialized_platforms(env): got = deps( "foo", requires_dist = [ - "m1_dep; sys_platform=='darwin' and platform_machine=='arm64'", - "mac_dep; sys_platform=='darwin'", - ], - platforms = [ - "osx_x86_64", - "osx_aarch64", + "bar", + "an_osx_dep; sys_platform=='darwin'", + "posix_dep; os_name=='posix'", + "win_dep; os_name=='nt'", ], - default_python_version = "3.8.4", ) - env.expect.that_collection(got.deps).contains_exactly(["mac_dep"]) + env.expect.that_collection(got.deps).contains_exactly(["bar"]) env.expect.that_dict(got.deps_select).contains_exactly({ - "osx_aarch64": ["m1_dep"], + "an_osx_dep": "sys_platform == \"darwin\"", + "posix_dep": "os_name == \"posix\"", + "win_dep": "os_name == \"nt\"", }) -_tests.append(test_deps_are_added_to_more_specialized_platforms) +_tests.append(test_can_add_os_specific_deps) -def test_non_platform_markers_are_added_to_common_deps(env): +def test_deps_are_added_to_more_specialized_platforms(env): got = deps( "foo", requires_dist = [ - "bar", - "baz; implementation_name=='cpython'", "m1_dep; sys_platform=='darwin' and platform_machine=='arm64'", + "mac_dep; sys_platform=='darwin'", ], - platforms = [ - "linux_x86_64", - "osx_x86_64", - "osx_aarch64", - "windows_x86_64", - ], - default_python_version = "3.8.4", ) - env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) + env.expect.that_collection(got.deps).contains_exactly([]) env.expect.that_dict(got.deps_select).contains_exactly({ - "osx_aarch64": ["m1_dep"], + "m1_dep": "sys_platform == \"darwin\" and platform_machine == \"arm64\"", + "mac_dep": "sys_platform == \"darwin\"", }) -_tests.append(test_non_platform_markers_are_added_to_common_deps) +_tests.append(test_deps_are_added_to_more_specialized_platforms) def test_self_is_ignored(env): got = deps( @@ -167,180 +107,59 @@ def _test_can_get_deps_based_on_specific_python_version(env): "posix_dep; os_name=='posix' and python_version >= '3.8'", ] - py38 = deps( - "foo", - requires_dist = requires_dist, - platforms = ["cp38_linux_x86_64"], - ) - py373 = deps( - "foo", - requires_dist = requires_dist, - platforms = ["cp37.3_linux_x86_64"], - ) - py37 = deps( + got = deps( "foo", requires_dist = requires_dist, - platforms = ["cp37_linux_x86_64"], ) # since there is a single target platform, the deps_select will be empty - env.expect.that_collection(py37.deps).contains_exactly(["bar", "baz"]) - env.expect.that_dict(py37.deps_select).contains_exactly({}) - env.expect.that_collection(py38.deps).contains_exactly(["bar", "posix_dep"]) - env.expect.that_dict(py38.deps_select).contains_exactly({}) - env.expect.that_collection(py373.deps).contains_exactly(["bar"]) - env.expect.that_dict(py373.deps_select).contains_exactly({}) - -_tests.append(_test_can_get_deps_based_on_specific_python_version) - -def _test_no_version_select_when_single_version(env): - got = deps( - "foo", - requires_dist = [ - "bar", - "baz; python_version >= '3.8'", - "posix_dep; os_name=='posix'", - "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", - "arch_dep; platform_machine=='x86_64' and python_version >= '3.8'", - ], - platforms = [ - "cp38_linux_x86_64", - "cp38_windows_x86_64", - ], - default_python_version = "", - ) - - env.expect.that_collection(got.deps).contains_exactly(["bar", "baz", "arch_dep"]) + env.expect.that_collection(got.deps).contains_exactly(["bar"]) env.expect.that_dict(got.deps_select).contains_exactly({ - "linux_x86_64": ["posix_dep", "posix_dep_with_version"], + "baz": "python_full_version < \"3.7.3\"", + "posix_dep": "os_name == \"posix\" and python_version >= \"3.8\"", }) -_tests.append(_test_no_version_select_when_single_version) +_tests.append(_test_can_get_deps_based_on_specific_python_version) -def _test_can_get_version_select(env): +def _test_include_only_particular_deps(env): requires_dist = [ "bar", - "baz; python_version < '3.8'", - "baz_new; python_version >= '3.8'", - "posix_dep; os_name=='posix'", - "posix_dep_with_version; os_name=='posix' and python_version >= '3.8'", - "arch_dep; platform_machine=='x86_64' and python_version < '3.8'", + "baz; python_full_version < '3.7.3'", + "posix_dep; os_name=='posix' and python_version >= '3.8'", ] got = deps( "foo", requires_dist = requires_dist, - platforms = [ - "cp3{}_{}_x86_64".format(minor, os) - for minor in ["7.4", "8.8", "9.8"] - for os in ["linux", "windows"] - ], - default_python_version = "3.7", - minor_mapping = { - "3.7": "3.7.4", - }, + include = ["bar", "posix_dep"], ) + # since there is a single target platform, the deps_select will be empty env.expect.that_collection(got.deps).contains_exactly(["bar"]) env.expect.that_dict(got.deps_select).contains_exactly({ - "cp37.4_linux_x86_64": ["arch_dep", "baz", "posix_dep"], - "cp37.4_windows_x86_64": ["arch_dep", "baz"], - "cp38.8_linux_x86_64": ["baz_new", "posix_dep", "posix_dep_with_version"], - "cp38.8_windows_x86_64": ["baz_new"], - "cp39.8_linux_x86_64": ["baz_new", "posix_dep", "posix_dep_with_version"], - "cp39.8_windows_x86_64": ["baz_new"], - "linux_x86_64": ["arch_dep", "baz", "posix_dep"], - "windows_x86_64": ["arch_dep", "baz"], + "posix_dep": "os_name == \"posix\" and python_version >= \"3.8\"", }) -_tests.append(_test_can_get_version_select) +_tests.append(_test_include_only_particular_deps) -def _test_deps_spanning_all_target_py_versions_are_added_to_common(env): +def test_all_markers_are_added(env): requires_dist = [ "bar", "baz (<2,>=1.11) ; python_version < '3.8'", "baz (<2,>=1.14) ; python_version >= '3.8'", ] - default_python_version = "3.8.4" got = deps( "foo", requires_dist = requires_dist, - platforms = [ - "cp3{}_linux_x86_64".format(minor) - for minor in [7, 8, 9] - ], - default_python_version = default_python_version, - ) - - env.expect.that_collection(got.deps).contains_exactly(["bar", "baz"]) - env.expect.that_dict(got.deps_select).contains_exactly({}) - -_tests.append(_test_deps_spanning_all_target_py_versions_are_added_to_common) - -def _test_deps_are_not_duplicated(env): - default_python_version = "3.7.4" - - # See an example in - # https://files.pythonhosted.org/packages/76/9e/db1c2d56c04b97981c06663384f45f28950a73d9acf840c4006d60d0a1ff/opencv_python-4.9.0.80-cp37-abi3-win32.whl.metadata - requires_dist = [ - "bar >=0.1.0 ; python_version < '3.7'", - "bar >=0.2.0 ; python_version >= '3.7'", - "bar >=0.4.0 ; python_version >= '3.6' and platform_system == 'Linux' and platform_machine == 'aarch64'", - "bar >=0.4.0 ; python_version >= '3.9'", - "bar >=0.5.0 ; python_version <= '3.9' and platform_system == 'Darwin' and platform_machine == 'arm64'", - "bar >=0.5.0 ; python_version >= '3.10' and platform_system == 'Darwin'", - "bar >=0.5.0 ; python_version >= '3.10'", - "bar >=0.6.0 ; python_version >= '3.11'", - ] - - got = deps( - "foo", - requires_dist = requires_dist, - platforms = [ - "cp3{}_{}_{}".format(minor, os, arch) - for minor in [7, 10] - for os in ["linux", "osx", "windows"] - for arch in ["x86_64", "aarch64"] - ], - default_python_version = default_python_version, ) env.expect.that_collection(got.deps).contains_exactly(["bar"]) - env.expect.that_dict(got.deps_select).contains_exactly({}) - -_tests.append(_test_deps_are_not_duplicated) - -def _test_deps_are_not_duplicated_when_encountering_platform_dep_first(env): - # Note, that we are sorting the incoming `requires_dist` and we need to ensure that we are not getting any - # issues even if the platform-specific line comes first. - requires_dist = [ - "bar >=0.4.0 ; python_version >= '3.6' and platform_system == 'Linux' and platform_machine == 'aarch64'", - "bar >=0.5.0 ; python_version >= '3.9'", - ] - - got = deps( - "foo", - requires_dist = requires_dist, - platforms = [ - "cp37.1_linux_aarch64", - "cp37.1_linux_x86_64", - "cp310_linux_aarch64", - "cp310_linux_x86_64", - ], - default_python_version = "3.7.1", - minor_mapping = {}, - ) - - env.expect.that_collection(got.deps).contains_exactly([]) env.expect.that_dict(got.deps_select).contains_exactly({ - "cp310_linux_aarch64": ["bar"], - "cp310_linux_x86_64": ["bar"], - "cp37.1_linux_aarch64": ["bar"], - "linux_aarch64": ["bar"], + "baz": "(python_version < \"3.8\") or (python_version >= \"3.8\")", }) -_tests.append(_test_deps_are_not_duplicated_when_encountering_platform_dep_first) +_tests.append(test_all_markers_are_added) def deps_test_suite(name): # buildifier: disable=function-docstring test_suite( diff --git a/tests/pypi/whl_installer/wheel_installer_test.py b/tests/pypi/whl_installer/wheel_installer_test.py index e838047925..ef5a2483ab 100644 --- a/tests/pypi/whl_installer/wheel_installer_test.py +++ b/tests/pypi/whl_installer/wheel_installer_test.py @@ -72,7 +72,7 @@ def test_wheel_exists(self) -> None: extras={}, enable_implicit_namespace_pkgs=False, platforms=[], - enable_pipstar = False, + enable_pipstar=False, ) want_files = [ @@ -97,7 +97,6 @@ def test_wheel_exists(self) -> None: deps_by_platform={}, entry_points=[], name="example-minimal-package", - python_version="3.11.11", version="0.0.1", ) self.assertEqual(want, metadata_file_content) diff --git a/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl b/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl index 432cdbfa1b..f0e5f57ac0 100644 --- a/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl +++ b/tests/pypi/whl_library_targets/whl_library_targets_tests.bzl @@ -180,18 +180,20 @@ _tests.append(_test_entrypoints) def _test_whl_and_library_deps_from_requires(env): filegroup_calls = [] py_library_calls = [] + env_marker_setting_calls = [] whl_library_targets_from_requires( name = "foo-0-py3-none-any.whl", metadata_name = "Foo", metadata_version = "0", - dep_template = "@pypi_{name}//:{target}", + dep_template = "@pypi//{name}:{target}", requires_dist = [ "foo", # this self-edge will be ignored - "bar-baz", + "bar", + "bar-baz; python_version < \"8.2\"", + "booo", # this is effectively excluded due to the list below ], - target_platforms = ["cp38_linux_x86_64"], - default_python_version = "3.8.1", + include = ["foo", "bar", "bar_baz"], data_exclude = [], # Overrides for testing filegroups = {}, @@ -203,6 +205,7 @@ def _test_whl_and_library_deps_from_requires(env): ), rules = struct( py_library = lambda **kwargs: py_library_calls.append(kwargs), + env_marker_setting = lambda **kwargs: env_marker_setting_calls.append(kwargs), ), ) @@ -210,7 +213,10 @@ def _test_whl_and_library_deps_from_requires(env): { "name": "whl", "srcs": ["foo-0-py3-none-any.whl"], - "data": ["@pypi_bar_baz//:whl"], + "data": ["@pypi//bar:whl"] + _select({ + ":is_include_bar_baz_true": ["@pypi//bar_baz:whl"], + "//conditions:default": [], + }), "visibility": ["//visibility:public"], }, ]) # buildifier: @unsorted-dict-items @@ -233,12 +239,22 @@ def _test_whl_and_library_deps_from_requires(env): ] + glob_excludes.version_dependent_exclusions(), ), "imports": ["site-packages"], - "deps": ["@pypi_bar_baz//:pkg"], + "deps": ["@pypi//bar:pkg"] + _select({ + ":is_include_bar_baz_true": ["@pypi//bar_baz:pkg"], + "//conditions:default": [], + }), "tags": ["pypi_name=Foo", "pypi_version=0"], "visibility": ["//visibility:public"], "experimental_venvs_site_packages": Label("//python/config_settings:venvs_site_packages"), }, ]) # buildifier: @unsorted-dict-items + env.expect.that_collection(env_marker_setting_calls).contains_exactly([ + { + "name": "include_bar_baz", + "expression": "python_version < \"8.2\"", + "visibility": ["//visibility:private"], + }, + ]) # buildifier: @unsorted-dict-items _tests.append(_test_whl_and_library_deps_from_requires) From 367d09ec01ce5640ee9587398b6a8ce56a7eb0ba Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 13 May 2025 15:31:40 -0700 Subject: [PATCH 2/2] refactor: make python extension generate platform toolchains (#2875) This makes the python bzlmod extension handle generating the platform-specific toolchain entries ("python_3_10_{platform}"). This is to eventually allow adding additional toolchains that aren't part of the PLATFORMS mapping in versions.bzl and have their own custom constraints. The main things this refactor does are: 1. The bzlmod phase passes the full list of implementation toolchains to create (previously, it relied on `hub_repo` to generate the implementation names). 2. The name of a toolchain (the toolchain.name arg) and the repo that implements it (the py_toolchain_suite.user_repository_repo arg) are separate. This allows future work to mixin toolchains that point to arbitrary repos. 3. The platform meta data uses a list of target settings instead of dict of flag values. This allows more arbitrary target settings. For now, flag values on the platform metadata is still looked for because it's known that users patch python/versions.bzl. Along the way: * Factor out a platform_info helper in versions.bzl * Factor out a NOT_ACTUALLY_PUBLIC constants to better denote things that are public visibility, but actually internal. * Add some docs to some internals so we don't have to chase down their definitions. Work towards https://github.com/bazel-contrib/rules_python/issues/2081 --- internal_dev_setup.bzl | 12 ++- python/private/config_settings.bzl | 29 +++++- python/private/py_repositories.bzl | 12 ++- python/private/py_toolchain_suite.bzl | 11 ++- python/private/python.bzl | 102 ++++++++++++++------ python/private/pythons_hub.bzl | 116 +++++++++++----------- python/private/toolchains_repo.bzl | 67 ++++++++----- python/versions.bzl | 133 +++++++++++++++----------- 8 files changed, 309 insertions(+), 173 deletions(-) diff --git a/internal_dev_setup.bzl b/internal_dev_setup.bzl index f33908049f..62a11ab1d4 100644 --- a/internal_dev_setup.bzl +++ b/internal_dev_setup.bzl @@ -34,11 +34,15 @@ def rules_python_internal_setup(): name = "pythons_hub", minor_mapping = MINOR_MAPPING, default_python_version = "", - toolchain_prefixes = [], - toolchain_python_versions = [], - toolchain_set_python_version_constraints = [], - toolchain_user_repository_names = [], python_versions = sorted(TOOL_VERSIONS.keys()), + toolchain_names = [], + toolchain_repo_names = {}, + toolchain_target_compatible_with_map = {}, + toolchain_target_settings_map = {}, + toolchain_platform_keys = {}, + toolchain_python_versions = {}, + toolchain_set_python_version_constraints = {}, + base_toolchain_repo_names = [], ) runtime_env_repo(name = "rules_python_runtime_env_tc_info") diff --git a/python/private/config_settings.bzl b/python/private/config_settings.bzl index 1685195b78..5eb858e2e4 100644 --- a/python/private/config_settings.bzl +++ b/python/private/config_settings.bzl @@ -31,6 +31,10 @@ If the value is missing, then the default value is being used, see documentation {docs_url}/python/config_settings """ +# Indicates something needs public visibility so that other generated code can +# access it, but it's not intended for general public usage. +_NOT_ACTUALLY_PUBLIC = ["//visibility:public"] + def construct_config_settings(*, name, default_version, versions, minor_mapping, documented_flags): # buildifier: disable=function-docstring """Create a 'python_version' config flag and construct all config settings used in rules_python. @@ -128,7 +132,30 @@ def construct_config_settings(*, name, default_version, versions, minor_mapping, # `whl_library` in the hub repo created by `pip.parse`. flag_values = {"current_config": "will-never-match"}, # Only public so that PyPI hub repo can access it - visibility = ["//visibility:public"], + visibility = _NOT_ACTUALLY_PUBLIC, + ) + + libc = Label("//python/config_settings:py_linux_libc") + native.config_setting( + name = "_is_py_linux_libc_glibc", + flag_values = {libc: "glibc"}, + visibility = _NOT_ACTUALLY_PUBLIC, + ) + native.config_setting( + name = "_is_py_linux_libc_musl", + flag_values = {libc: "glibc"}, + visibility = _NOT_ACTUALLY_PUBLIC, + ) + freethreaded = Label("//python/config_settings:py_freethreaded") + native.config_setting( + name = "_is_py_freethreaded_yes", + flag_values = {freethreaded: "yes"}, + visibility = _NOT_ACTUALLY_PUBLIC, + ) + native.config_setting( + name = "_is_py_freethreaded_no", + flag_values = {freethreaded: "no"}, + visibility = _NOT_ACTUALLY_PUBLIC, ) def _python_version_flag_impl(ctx): diff --git a/python/private/py_repositories.bzl b/python/private/py_repositories.bzl index 46ca903df4..b5bd93b7c1 100644 --- a/python/private/py_repositories.bzl +++ b/python/private/py_repositories.bzl @@ -39,11 +39,15 @@ def py_repositories(): name = "pythons_hub", minor_mapping = MINOR_MAPPING, default_python_version = "", - toolchain_prefixes = [], - toolchain_python_versions = [], - toolchain_set_python_version_constraints = [], - toolchain_user_repository_names = [], python_versions = sorted(TOOL_VERSIONS.keys()), + toolchain_names = [], + toolchain_repo_names = {}, + toolchain_target_compatible_with_map = {}, + toolchain_target_settings_map = {}, + toolchain_platform_keys = {}, + toolchain_python_versions = {}, + toolchain_set_python_version_constraints = {}, + base_toolchain_repo_names = [], ) http_archive( name = "bazel_skylib", diff --git a/python/private/py_toolchain_suite.bzl b/python/private/py_toolchain_suite.bzl index e71882dafd..fa73d5daa3 100644 --- a/python/private/py_toolchain_suite.bzl +++ b/python/private/py_toolchain_suite.bzl @@ -34,15 +34,20 @@ def py_toolchain_suite( python_version, set_python_version_constraint, flag_values, + target_settings = [], target_compatible_with = []): """For internal use only. Args: prefix: Prefix for toolchain target names. - user_repository_name: The name of the user repository. + user_repository_name: The name of the repository with the toolchain + implementation (it's assumed to have particular target names within + it). Does not include the leading "@". python_version: The full (X.Y.Z) version of the interpreter. set_python_version_constraint: True or False as a string. - flag_values: Extra flag values to match for this toolchain. + flag_values: Extra flag values to match for this toolchain. These + are prepended to target_settings. + target_settings: Extra target_settings to match for this toolchain. target_compatible_with: list constraints the toolchains are compatible with. """ @@ -82,7 +87,7 @@ def py_toolchain_suite( match_any = match_any, visibility = ["//visibility:private"], ) - target_settings = [name] + target_settings = [name] + target_settings else: fail(("Invalid set_python_version_constraint value: got {} {}, wanted " + "either the string 'True' or the string 'False'; " + diff --git a/python/private/python.bzl b/python/private/python.bzl index f49fb26d52..53cd5e9cd2 100644 --- a/python/private/python.bzl +++ b/python/private/python.bzl @@ -22,15 +22,9 @@ load(":python_register_toolchains.bzl", "python_register_toolchains") load(":pythons_hub.bzl", "hub_repo") load(":repo_utils.bzl", "repo_utils") load(":semver.bzl", "semver") -load(":text_util.bzl", "render") load(":toolchains_repo.bzl", "multi_toolchain_aliases") load(":util.bzl", "IS_BAZEL_6_4_OR_HIGHER") -# This limit can be increased essentially arbitrarily, but doing so will cause a rebuild of all -# targets using any of these toolchains due to the changed repository name. -_MAX_NUM_TOOLCHAINS = 9999 -_TOOLCHAIN_INDEX_PAD_LENGTH = len(str(_MAX_NUM_TOOLCHAINS)) - def parse_modules(*, module_ctx, _fail = fail): """Parse the modules and return a struct for registrations. @@ -240,9 +234,6 @@ def parse_modules(*, module_ctx, _fail = fail): # toolchain. We need the default last. toolchains.append(default_toolchain) - if len(toolchains) > _MAX_NUM_TOOLCHAINS: - fail("more than {} python versions are not supported".format(_MAX_NUM_TOOLCHAINS)) - # sort the toolchains so that the toolchain versions that are in the # `minor_mapping` are coming first. This ensures that `python_version = # "3.X"` transitions work as expected. @@ -275,6 +266,9 @@ def parse_modules(*, module_ctx, _fail = fail): def _python_impl(module_ctx): py = parse_modules(module_ctx = module_ctx) + # dict[str version, list[str] platforms]; where version is full + # python version string ("3.4.5"), and platforms are keys from + # the PLATFORMS global. loaded_platforms = {} for toolchain_info in py.toolchains: # Ensure that we pass the full version here. @@ -297,30 +291,82 @@ def _python_impl(module_ctx): **kwargs ) - # Create the pythons_hub repo for the interpreter meta data and the - # the various toolchains. + # List of the base names ("python_3_10") for the toolchain repos + base_toolchain_repo_names = [] + + # list[str] The infix to use for the resulting toolchain() `name` arg. + toolchain_names = [] + + # dict[str i, str repo]; where repo is the full repo name + # ("python_3_10_unknown-linux-x86_64") for the toolchain + # i corresponds to index `i` in toolchain_names + toolchain_repo_names = {} + + # dict[str i, list[str] constraints]; where constraints is a list + # of labels for target_compatible_with + # i corresponds to index `i` in toolchain_names + toolchain_tcw_map = {} + + # dict[str i, list[str] settings]; where settings is a list + # of labels for target_settings + # i corresponds to index `i` in toolchain_names + toolchain_ts_map = {} + + # dict[str i, str set_constraint]; where set_constraint is the string + # "True" or "False". + # i corresponds to index `i` in toolchain_names + toolchain_set_python_version_constraints = {} + + # dict[str i, str python_version]; where python_version is the full + # python version ("3.4.5"). + toolchain_python_versions = {} + + # dict[str i, str platform_key]; where platform_key is the key within + # the PLATFORMS global for this toolchain + toolchain_platform_keys = {} + + # Split the toolchain info into separate objects so they can be passed onto + # the repository rule. + for i, t in enumerate(py.toolchains): + is_last = (i + 1) == len(py.toolchains) + base_name = t.name + base_toolchain_repo_names.append(base_name) + fv = full_version(version = t.python_version, minor_mapping = py.config.minor_mapping) + for platform in loaded_platforms[fv]: + if platform not in PLATFORMS: + continue + key = str(len(toolchain_names)) + + full_name = "{}_{}".format(base_name, platform) + toolchain_names.append(full_name) + toolchain_repo_names[key] = full_name + toolchain_tcw_map[key] = PLATFORMS[platform].compatible_with + + # The target_settings attribute may not be present for users + # patching python/versions.bzl. + toolchain_ts_map[key] = getattr(PLATFORMS[platform], "target_settings", []) + toolchain_platform_keys[key] = platform + toolchain_python_versions[key] = fv + + # The last toolchain is the default; it can't have version constraints + # Despite the implication of the arg name, the values are strs, not bools + toolchain_set_python_version_constraints[key] = ( + "True" if not is_last else "False" + ) + hub_repo( name = "pythons_hub", - # Last toolchain is default + toolchain_names = toolchain_names, + toolchain_repo_names = toolchain_repo_names, + toolchain_target_compatible_with_map = toolchain_tcw_map, + toolchain_target_settings_map = toolchain_ts_map, + toolchain_platform_keys = toolchain_platform_keys, + toolchain_python_versions = toolchain_python_versions, + toolchain_set_python_version_constraints = toolchain_set_python_version_constraints, + base_toolchain_repo_names = [t.name for t in py.toolchains], default_python_version = py.default_python_version, minor_mapping = py.config.minor_mapping, python_versions = list(py.config.default["tool_versions"].keys()), - toolchain_prefixes = [ - render.toolchain_prefix(index, toolchain.name, _TOOLCHAIN_INDEX_PAD_LENGTH) - for index, toolchain in enumerate(py.toolchains) - ], - toolchain_python_versions = [ - full_version(version = t.python_version, minor_mapping = py.config.minor_mapping) - for t in py.toolchains - ], - # The last toolchain is the default; it can't have version constraints - # Despite the implication of the arg name, the values are strs, not bools - toolchain_set_python_version_constraints = [ - "True" if i != len(py.toolchains) - 1 else "False" - for i in range(len(py.toolchains)) - ], - toolchain_user_repository_names = [t.name for t in py.toolchains], - loaded_platforms = loaded_platforms, ) # This is require in order to support multiple version py_test diff --git a/python/private/pythons_hub.bzl b/python/private/pythons_hub.bzl index b448d53097..53351cacb9 100644 --- a/python/private/pythons_hub.bzl +++ b/python/private/pythons_hub.bzl @@ -16,7 +16,7 @@ load("//python:versions.bzl", "PLATFORMS") load(":text_util.bzl", "render") -load(":toolchains_repo.bzl", "python_toolchain_build_file_content") +load(":toolchains_repo.bzl", "toolchain_suite_content") def _have_same_length(*lists): if not lists: @@ -24,8 +24,10 @@ def _have_same_length(*lists): return len({len(length): None for length in lists}) == 1 _HUB_BUILD_FILE_TEMPLATE = """\ -load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +# Generated by @rules_python//python/private:pythons_hub.bzl + load("@@{rules_python}//python/private:py_toolchain_suite.bzl", "py_toolchain_suite") +load("@bazel_skylib//:bzl_library.bzl", "bzl_library") bzl_library( name = "interpreters_bzl", @@ -42,44 +44,43 @@ bzl_library( {toolchains} """ -def _hub_build_file_content( - prefixes, - python_versions, - set_python_version_constraints, - user_repository_names, - workspace_location, - loaded_platforms): - """This macro iterates over each of the lists and returns the toolchain content. - - python_toolchain_build_file_content is called to generate each of the toolchain - definitions. - """ - - if not _have_same_length(python_versions, set_python_version_constraints, user_repository_names): +def _hub_build_file_content(rctx): + # Verify a precondition. If these don't match, then something went wrong. + if not _have_same_length( + rctx.attr.toolchain_names, + rctx.attr.toolchain_platform_keys, + rctx.attr.toolchain_repo_names, + rctx.attr.toolchain_target_compatible_with_map, + rctx.attr.toolchain_target_settings_map, + rctx.attr.toolchain_set_python_version_constraints, + rctx.attr.toolchain_python_versions, + ): fail("all lists must have the same length") - # Iterate over the length of python_versions and call - # build the toolchain content by calling python_toolchain_build_file_content - toolchains = "\n".join( - [ - python_toolchain_build_file_content( - prefix = prefixes[i], - python_version = python_versions[i], - set_python_version_constraint = set_python_version_constraints[i], - user_repository_name = user_repository_names[i], - loaded_platforms = { - k: v - for k, v in PLATFORMS.items() - if k in loaded_platforms[python_versions[i]] - }, - ) - for i in range(len(python_versions)) - ], - ) + #pad_length = len(str(len(rctx.attr.toolchain_names))) + 1 + pad_length = 4 + toolchains = [] + for i, base_name in enumerate(rctx.attr.toolchain_names): + key = str(i) + platform = rctx.attr.toolchain_platform_keys[key] + if platform in PLATFORMS: + flag_values = PLATFORMS[platform].flag_values + else: + flag_values = {} + + toolchains.append(toolchain_suite_content( + prefix = "_{}_{}".format(render.left_pad_zero(i, pad_length), base_name), + user_repository_name = rctx.attr.toolchain_repo_names[key], + target_compatible_with = rctx.attr.toolchain_target_compatible_with_map[key], + flag_values = flag_values, + target_settings = rctx.attr.toolchain_target_settings_map[key], + set_python_version_constraint = rctx.attr.toolchain_set_python_version_constraints[key], + python_version = rctx.attr.toolchain_python_versions[key], + )) return _HUB_BUILD_FILE_TEMPLATE.format( - toolchains = toolchains, - rules_python = workspace_location.repo_name, + toolchains = "\n".join(toolchains), + rules_python = rctx.attr._rules_python_workspace.repo_name, ) _interpreters_bzl_template = """ @@ -103,14 +104,7 @@ def _hub_repo_impl(rctx): # write them to the BUILD file. rctx.file( "BUILD.bazel", - _hub_build_file_content( - rctx.attr.toolchain_prefixes, - rctx.attr.toolchain_python_versions, - rctx.attr.toolchain_set_python_version_constraints, - rctx.attr.toolchain_user_repository_names, - rctx.attr._rules_python_workspace, - rctx.attr.loaded_platforms, - ), + _hub_build_file_content(rctx), executable = False, ) @@ -118,7 +112,7 @@ def _hub_repo_impl(rctx): # a symlink to a interpreter. interpreter_labels = "".join([ _line_for_hub_template.format(name = name) - for name in rctx.attr.toolchain_user_repository_names + for name in rctx.attr.base_toolchain_repo_names ]) rctx.file( @@ -150,13 +144,15 @@ This rule also writes out the various toolchains for the different Python versio """, implementation = _hub_repo_impl, attrs = { + "base_toolchain_repo_names": attr.string_list( + doc = "The base repo name for toolchains ('python_3_10', no " + + "platform suffix)", + mandatory = True, + ), "default_python_version": attr.string( doc = "Default Python version for the build in `X.Y` or `X.Y.Z` format.", mandatory = True, ), - "loaded_platforms": attr.string_list_dict( - doc = "The list of loaded platforms keyed by the toolchain full python version", - ), "minor_mapping": attr.string_dict( doc = "The minor mapping of the `X.Y` to `X.Y.Z` format that is used in config settings.", mandatory = True, @@ -165,20 +161,32 @@ This rule also writes out the various toolchains for the different Python versio doc = "The list of python versions to include in the `interpreters.bzl` if the toolchains are not specified. Used in `WORKSPACE` builds.", mandatory = False, ), - "toolchain_prefixes": attr.string_list( - doc = "List prefixed for the toolchains", + "toolchain_names": attr.string_list( + doc = "Names of toolchains", + mandatory = True, + ), + "toolchain_platform_keys": attr.string_dict( + doc = "The platform key in PLATFORMS for toolchains.", mandatory = True, ), - "toolchain_python_versions": attr.string_list( + "toolchain_python_versions": attr.string_dict( doc = "List of Python versions for the toolchains. In `X.Y.Z` format.", mandatory = True, ), - "toolchain_set_python_version_constraints": attr.string_list( + "toolchain_repo_names": attr.string_dict( + doc = "The repo names containing toolchain implementations.", + mandatory = True, + ), + "toolchain_set_python_version_constraints": attr.string_dict( doc = "List of version contraints for the toolchains", mandatory = True, ), - "toolchain_user_repository_names": attr.string_list( - doc = "List of the user repo names for the toolchains", + "toolchain_target_compatible_with_map": attr.string_list_dict( + doc = "The target_compatible_with settings for toolchains.", + mandatory = True, + ), + "toolchain_target_settings_map": attr.string_list_dict( + doc = "The target_settings for toolchains", mandatory = True, ), "_rules_python_workspace": attr.label(default = Label("//:does_not_matter_what_this_name_is")), diff --git a/python/private/toolchains_repo.bzl b/python/private/toolchains_repo.bzl index 23c4643c0a..d0814b66d5 100644 --- a/python/private/toolchains_repo.bzl +++ b/python/private/toolchains_repo.bzl @@ -31,6 +31,18 @@ load( load(":repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils") load(":text_util.bzl", "render") +_SUITE_TEMPLATE = """ +py_toolchain_suite( + flag_values = {flag_values}, + target_settings = {target_settings}, + prefix = {prefix}, + python_version = {python_version}, + set_python_version_constraint = {set_python_version_constraint}, + target_compatible_with = {target_compatible_with}, + user_repository_name = {user_repository_name}, +) +""".lstrip() + def python_toolchain_build_file_content( prefix, python_version, @@ -53,29 +65,40 @@ def python_toolchain_build_file_content( build_content: Text containing toolchain definitions """ - return "\n\n".join([ - """\ -py_toolchain_suite( - user_repository_name = "{user_repository_name}_{platform}", - prefix = "{prefix}{platform}", - target_compatible_with = {compatible_with}, - flag_values = {flag_values}, - python_version = "{python_version}", - set_python_version_constraint = "{set_python_version_constraint}", -)""".format( - compatible_with = render.indent(render.list(meta.compatible_with)).lstrip(), - flag_values = render.indent(render.dict( - meta.flag_values, - key_repr = lambda x: repr(str(x)), # this is to correctly display labels - )).lstrip(), - platform = platform, - set_python_version_constraint = set_python_version_constraint, - user_repository_name = user_repository_name, - prefix = prefix, + entries = [] + for platform, meta in loaded_platforms.items(): + entries.append(toolchain_suite_content( + target_compatible_with = meta.compatible_with, + flag_values = meta.flag_values, + prefix = "{}{}".format(prefix, platform), + user_repository_name = "{}_{}".format(user_repository_name, platform), python_version = python_version, - ) - for platform, meta in loaded_platforms.items() - ]) + set_python_version_constraint = set_python_version_constraint, + target_settings = [], + )) + return "\n\n".join(entries) + +def toolchain_suite_content( + *, + flag_values, + prefix, + python_version, + set_python_version_constraint, + target_compatible_with, + target_settings, + user_repository_name): + return _SUITE_TEMPLATE.format( + prefix = render.str(prefix), + user_repository_name = render.str(user_repository_name), + target_compatible_with = render.indent(render.list(target_compatible_with)).lstrip(), + flag_values = render.indent(render.dict( + flag_values, + key_repr = lambda x: repr(str(x)), # this is to correctly display labels + )).lstrip(), + target_settings = render.list(target_settings, hanging_indent = " "), + set_python_version_constraint = render.str(set_python_version_constraint), + python_version = render.str(python_version), + ) def _toolchains_repo_impl(rctx): build_content = """\ diff --git a/python/versions.bzl b/python/versions.bzl index 6343ee49c8..4a2a4cb758 100644 --- a/python/versions.bzl +++ b/python/versions.bzl @@ -682,151 +682,170 @@ MINOR_MAPPING = { "3.13": "3.13.2", } +def _platform_info( + *, + compatible_with = [], + flag_values = {}, + target_settings = [], + os_name, + arch): + """Creates a struct of platform metadata. + + Args: + compatible_with: list[str], where the values are string labels. These + are the target_compatible_with values to use with the toolchain + flag_values: dict[str|Label, Any] of config_setting.flag_values + compatible values. DEPRECATED -- use target_settings instead + target_settings: list[str], where the values are string labels. These + are the target_settings values to use with the toolchain. + os_name: str, the os name; must match the name used in `@platfroms//os` + arch: str, the cpu name; must match the name used in `@platforms//cpu` + + Returns: + A struct with attributes and values matching the args. + """ + return struct( + compatible_with = compatible_with, + flag_values = flag_values, + target_settings = target_settings, + os_name = os_name, + arch = arch, + ) + def _generate_platforms(): - libc = Label("//python/config_settings:py_linux_libc") + is_libc_glibc = str(Label("//python/config_settings:_is_py_linux_libc_glibc")) + is_libc_musl = str(Label("//python/config_settings:_is_py_linux_libc_musl")) platforms = { - "aarch64-apple-darwin": struct( + "aarch64-apple-darwin": _platform_info( compatible_with = [ "@platforms//os:macos", "@platforms//cpu:aarch64", ], - flag_values = {}, os_name = MACOS_NAME, - # Matches the value in @platforms//cpu package arch = "aarch64", ), - "aarch64-unknown-linux-gnu": struct( + "aarch64-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:aarch64", ], - flag_values = { - libc: "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "aarch64", ), - "armv7-unknown-linux-gnu": struct( + "armv7-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:armv7", ], - flag_values = { - libc: "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "arm", ), - "i386-unknown-linux-gnu": struct( + "i386-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:i386", ], - flag_values = { - libc: "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "x86_32", ), - "ppc64le-unknown-linux-gnu": struct( + "ppc64le-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:ppc", ], - flag_values = { - libc: "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "ppc", ), - "riscv64-unknown-linux-gnu": struct( + "riscv64-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:riscv64", ], - flag_values = { - Label("//python/config_settings:py_linux_libc"): "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "riscv64", ), - "s390x-unknown-linux-gnu": struct( + "s390x-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:s390x", ], - flag_values = { - Label("//python/config_settings:py_linux_libc"): "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "s390x", ), - "x86_64-apple-darwin": struct( + "x86_64-apple-darwin": _platform_info( compatible_with = [ "@platforms//os:macos", "@platforms//cpu:x86_64", ], - flag_values = {}, os_name = MACOS_NAME, - # Matches the value in @platforms//cpu package arch = "x86_64", ), - "x86_64-pc-windows-msvc": struct( + "x86_64-pc-windows-msvc": _platform_info( compatible_with = [ "@platforms//os:windows", "@platforms//cpu:x86_64", ], - flag_values = {}, os_name = WINDOWS_NAME, - # Matches the value in @platforms//cpu package arch = "x86_64", ), - "x86_64-unknown-linux-gnu": struct( + "x86_64-unknown-linux-gnu": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:x86_64", ], - flag_values = { - libc: "glibc", - }, + target_settings = [ + is_libc_glibc, + ], os_name = LINUX_NAME, - # Matches the value in @platforms//cpu package arch = "x86_64", ), - "x86_64-unknown-linux-musl": struct( + "x86_64-unknown-linux-musl": _platform_info( compatible_with = [ "@platforms//os:linux", "@platforms//cpu:x86_64", ], - flag_values = { - libc: "musl", - }, + target_settings = [ + is_libc_musl, + ], os_name = LINUX_NAME, arch = "x86_64", ), } - freethreaded = Label("//python/config_settings:py_freethreaded") + is_freethreaded_yes = str(Label("//python/config_settings:_is_py_freethreaded_yes")) + is_freethreaded_no = str(Label("//python/config_settings:_is_py_freethreaded_no")) return { - p + suffix: struct( + p + suffix: _platform_info( compatible_with = v.compatible_with, - flag_values = { - freethreaded: freethreaded_value, - } | v.flag_values, + target_settings = [ + freethreadedness, + ] + v.target_settings, os_name = v.os_name, arch = v.arch, ) for p, v in platforms.items() - for suffix, freethreaded_value in { - "": "no", - "-" + FREETHREADED: "yes", + for suffix, freethreadedness in { + "": is_freethreaded_no, + "-" + FREETHREADED: is_freethreaded_yes, }.items() }