From f8b4446eebb456e79f71e471ec4f079ed05acfcc Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 2 May 2025 15:31:25 -0700 Subject: [PATCH 1/6] refactor: make env marker config available through target and flag --- .../python/config_settings/index.md | 12 ++ docs/pypi-dependencies.md | 31 +++++ python/config_settings/BUILD.bazel | 7 ++ python/private/pypi/BUILD.bazel | 6 + python/private/pypi/env_marker_info.bzl | 26 ++++ python/private/pypi/env_marker_setting.bzl | 111 +++++++----------- python/private/pypi/flags.bzl | 67 +++++++++++ 7 files changed, 192 insertions(+), 68 deletions(-) create mode 100644 python/private/pypi/env_marker_info.bzl diff --git a/docs/api/rules_python/python/config_settings/index.md b/docs/api/rules_python/python/config_settings/index.md index ed6444298e..f4618ff967 100644 --- a/docs/api/rules_python/python/config_settings/index.md +++ b/docs/api/rules_python/python/config_settings/index.md @@ -159,6 +159,18 @@ Values: ::: :::: +::::{bzl:flag} pip_env_marker_config +The target that provides the values for pip env marker evaluation. + +Default: `//python/config_settings:_pip_env_marker_default_config` + +This flag points to a target providing {obj}`EnvMarkerInfo`, which determines +the values used when environment markers are resolved at build time. + +:::{versionadded} VERSION_NEXT_FEATURE +::: +:::: + ::::{bzl:flag} pip_whl Set what distributions are used in the `pip` integration. diff --git a/docs/pypi-dependencies.md b/docs/pypi-dependencies.md index 4ec40bc889..6f18a158a2 100644 --- a/docs/pypi-dependencies.md +++ b/docs/pypi-dependencies.md @@ -391,6 +391,31 @@ compatible indexes. This is only supported on `bzlmd`. ``` + + (bazel-downloader)= ### Bazel downloader and multi-platform wheel hub repository. @@ -487,3 +512,9 @@ Bazel will call this file like `cred_helper.sh get` and use the returned JSON to into whatever HTTP(S) request it performs against `example.com`. [rfc7617]: https://datatracker.ietf.org/doc/html/rfc7617 + + diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 872d7d1bda..24bbe665c7 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -220,3 +220,10 @@ string_flag( define_pypi_internal_flags( name = "define_pypi_internal_flags", ) + +label_flag( + name = "pip_env_marker_config", + build_setting_default = ":_pip_env_marker_default_config", + # NOTE: Only public because it is used in pip hub repos. + visibility = ["//visibility:public"], +) diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index 9216134857..21926fa13f 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -71,6 +71,11 @@ bzl_library( ], ) +bzl_lirary( + name = "env_marker_info_bzl", + srcs = ["env_marker_info.bzl"], +) + bzl_library( name = "evaluate_markers_bzl", srcs = ["evaluate_markers.bzl"], @@ -111,6 +116,7 @@ bzl_library( name = "flags_bzl", srcs = ["flags.bzl"], deps = [ + ":env_marker_info.bzl", "//python/private:enum_bzl", "@bazel_skylib//rules:common_settings", ], diff --git a/python/private/pypi/env_marker_info.bzl b/python/private/pypi/env_marker_info.bzl new file mode 100644 index 0000000000..b483436d98 --- /dev/null +++ b/python/private/pypi/env_marker_info.bzl @@ -0,0 +1,26 @@ +"""Provider for implementing environment marker values.""" + +EnvMarkerInfo = provider( + doc = """ +The values to use during environment marker evaluation. + +:::{seealso} +The {obj}`--//python/config_settings:pip_env_marker_config` flag. +::: + +:::{versionadded} VERSION_NEXT_FEATURE +""", + fields = { + "env": """ +:type: dict[str, str] + +The values to use for environment markers when evaluating an expression. + +The keys and values should be compatible with the [PyPA dependency specifiers +specification](https://packaging.python.org/en/latest/specifications/dependency-specifiers/) + +Missing values will be set to the specification's defaults or computed using +available toolchain information. +""", + }, +) diff --git a/python/private/pypi/env_marker_setting.bzl b/python/private/pypi/env_marker_setting.bzl index bbc59ab110..36b087fe87 100644 --- a/python/private/pypi/env_marker_setting.bzl +++ b/python/private/pypi/env_marker_setting.bzl @@ -2,13 +2,10 @@ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") +load(":env_marker_info.bzl", "EnvMarkerInfo") load( ":pep508_env.bzl", "env_aliases", - "os_name_select_map", - "platform_machine_select_map", - "platform_system_select_map", - "sys_platform_select_map", ) load(":pep508_evaluate.bzl", "evaluate") @@ -39,69 +36,52 @@ def env_marker_setting(*, name, expression, **kwargs): _env_marker_setting( name = name, expression = expression, - os_name = select(os_name_select_map), - sys_platform = select(sys_platform_select_map), - platform_machine = select(platform_machine_select_map), - platform_system = select(platform_system_select_map), - platform_release = select({ - "@platforms//os:osx": "USE_OSX_VERSION_FLAG", - "//conditions:default": "", - }), **kwargs ) def _env_marker_setting_impl(ctx): env = {} + env.update( + ctx.attr._env_marker_config_flag[EnvMarkerInfo].env, + ) runtime = ctx.toolchains[TARGET_TOOLCHAIN_TYPE].py3_runtime - if runtime.interpreter_version_info: - version_info = runtime.interpreter_version_info - env["python_version"] = "{major}.{minor}".format( - major = version_info.major, - minor = version_info.minor, - ) - full_version = _format_full_version(version_info) - env["python_full_version"] = full_version - env["implementation_version"] = full_version - else: - env["python_version"] = _get_flag(ctx.attr._python_version_major_minor_flag) - full_version = _get_flag(ctx.attr._python_full_version_flag) - env["python_full_version"] = full_version - env["implementation_version"] = full_version - - # We assume cpython if the toolchain doesn't specify because it's most - # likely to be true. - env["implementation_name"] = runtime.implementation_name or "cpython" - env["os_name"] = ctx.attr.os_name - env["sys_platform"] = ctx.attr.sys_platform - env["platform_machine"] = ctx.attr.platform_machine - - # The `platform_python_implementation` marker value is supposed to come - # from `platform.python_implementation()`, however, PEP 421 introduced - # `sys.implementation.name` and the `implementation_name` env marker to - # replace it. Per the platform.python_implementation docs, there's now - # essentially just two possible "registered" values: CPython or PyPy. - # Rather than add a field to the toolchain, we just special case the value - # from `sys.implementation.name` to handle the two documented values. - platform_python_impl = runtime.implementation_name - if platform_python_impl == "cpython": - platform_python_impl = "CPython" - elif platform_python_impl == "pypy": - platform_python_impl = "PyPy" - env["platform_python_implementation"] = platform_python_impl - - # NOTE: Platform release for Android will be Android version: - # https://peps.python.org/pep-0738/#platform - # Similar for iOS: - # https://peps.python.org/pep-0730/#platform - platform_release = ctx.attr.platform_release - if platform_release == "USE_OSX_VERSION_FLAG": - platform_release = _get_flag(ctx.attr._pip_whl_osx_version_flag) - env["platform_release"] = platform_release - env["platform_system"] = ctx.attr.platform_system - - # For lack of a better option, just use an empty string for now. - env["platform_version"] = "" + + if "python_version" not in env: + if runtime.interpreter_version_info: + version_info = runtime.interpreter_version_info + env["python_version"] = "{major}.{minor}".format( + major = version_info.major, + minor = version_info.minor, + ) + full_version = _format_full_version(version_info) + env["python_full_version"] = full_version + env["implementation_version"] = full_version + else: + env["python_version"] = _get_flag(ctx.attr._python_version_major_minor_flag) + full_version = _get_flag(ctx.attr._python_full_version_flag) + env["python_full_version"] = full_version + env["implementation_version"] = full_version + + if "implementation_name" not in env: + # We assume cpython if the toolchain doesn't specify because it's most + # likely to be true. + env["implementation_name"] = runtime.implementation_name or "cpython" + + if "platform_python_implementation" not in env: + # The `platform_python_implementation` marker value is supposed to come + # from `platform.python_implementation()`, however, PEP 421 introduced + # `sys.implementation.name` and the `implementation_name` env marker to + # replace it. Per the platform.python_implementation docs, there's now + # essentially just two possible "registered" values: CPython or PyPy. + # Rather than add a field to the toolchain, we just special case the value + # from `sys.implementation.name` to handle the two documented values. + platform_python_impl = runtime.implementation_name + if platform_python_impl == "cpython": + platform_python_impl = "CPython" + elif platform_python_impl == "pypy": + platform_python_impl = "PyPy" + env["platform_python_implementation"] = platform_python_impl env.update(env_aliases()) @@ -125,14 +105,9 @@ for the specification of behavior. mandatory = True, doc = "Environment marker expression to evaluate.", ), - "os_name": attr.string(), - "platform_machine": attr.string(), - "platform_release": attr.string(), - "platform_system": attr.string(), - "sys_platform": attr.string(), - "_pip_whl_osx_version_flag": attr.label( - default = "//python/config_settings:pip_whl_osx_version", - providers = [[BuildSettingInfo], [config_common.FeatureFlagInfo]], + "_env_marker_config_flag": attr.label( + default = "//python/config_settings:pip_env_marker_config", + providers = [EnvMarkerInfo], ), "_python_full_version_flag": attr.label( default = "//python/config_settings:python_version", diff --git a/python/private/pypi/flags.bzl b/python/private/pypi/flags.bzl index a25579a2b8..8801164653 100644 --- a/python/private/pypi/flags.bzl +++ b/python/private/pypi/flags.bzl @@ -20,6 +20,14 @@ unnecessary files when all that are needed are flag definitions. load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo", "string_flag") load("//python/private:enum.bzl", "enum") +load(":env_marker_info.bzl", "EnvMarkerInfo") +load( + ":pep508_env.bzl", + "os_name_select_map", + "platform_machine_select_map", + "platform_system_select_map", + "sys_platform_select_map", +) # Determines if we should use whls for third party # @@ -82,6 +90,10 @@ def define_pypi_internal_flags(name): visibility = ["//visibility:public"], ) + _default_env_marker_config( + name = "_pip_env_marker_default_config", + ) + def _allow_wheels_flag_impl(ctx): input = ctx.attr._setting[BuildSettingInfo].value value = "yes" if input in ["auto", "only"] else "no" @@ -97,3 +109,58 @@ This rule allows us to greatly reduce the number of config setting targets at no if we are duplicating some of the functionality of the `native.config_setting`. """, ) + +def _default_env_marker_config(**kwargs): + _env_marker_config( + os_name = select(os_name_select_map), + sys_platform = select(sys_platform_select_map), + platform_machine = select(platform_machine_select_map), + platform_system = select(platform_system_select_map), + platform_release = select({ + "@platforms//os:osx": "USE_OSX_VERSION_FLAG", + "//conditions:default": "", + }), + **kwargs + ) + +def _env_marker_config_impl(ctx): + env = {} + env["os_name"] = ctx.attr.os_name + env["sys_platform"] = ctx.attr.sys_platform + env["platform_machine"] = ctx.attr.platform_machine + + # NOTE: Platform release for Android will be Android version: + # https://peps.python.org/pep-0738/#platform + # Similar for iOS: + # https://peps.python.org/pep-0730/#platform + platform_release = ctx.attr.platform_release + if platform_release == "USE_OSX_VERSION_FLAG": + platform_release = _get_flag(ctx.attr._pip_whl_osx_version_flag) + env["platform_release"] = platform_release + env["platform_system"] = ctx.attr.platform_system + + # For lack of a better option, just use an empty string for now. + env["platform_version"] = "" + return [EnvMarkerInfo(env = env)] + +_env_marker_config = rule( + implementation = _env_marker_config_impl, + attrs = { + "os_name": attr.string(), + "platform_machine": attr.string(), + "platform_release": attr.string(), + "platform_system": attr.string(), + "sys_platform": attr.string(), + "_pip_whl_osx_version_flag": attr.label( + default = "//python/config_settings:pip_whl_osx_version", + providers = [[BuildSettingInfo], [config_common.FeatureFlagInfo]], + ), + }, +) + +def _get_flag(t): + if config_common.FeatureFlagInfo in t: + return t[config_common.FeatureFlagInfo].value + if BuildSettingInfo in t: + return t[BuildSettingInfo].value + fail("Should not occur: {} does not have necessary providers") From 269602f14094bf3ac1ddf1e6e1c4fa572d8849aa Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 2 May 2025 23:04:01 -0700 Subject: [PATCH 2/6] fix deps --- docs/pypi-dependencies.md | 1 - python/private/pypi/BUILD.bazel | 15 ++++++++++++++- python/private/pypi/env_marker_setting.bzl | 5 +---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/pypi-dependencies.md b/docs/pypi-dependencies.md index 6f18a158a2..b3ae7fe594 100644 --- a/docs/pypi-dependencies.md +++ b/docs/pypi-dependencies.md @@ -338,7 +338,6 @@ leg of the dependency manually. For instance by making perhaps `apache-airflow-providers-common-sql`. -(bazel-downloader)= ### Multi-platform support Multi-platform support of cross-building the wheels can be done in two ways - either diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel index 21926fa13f..d5d897ef8c 100644 --- a/python/private/pypi/BUILD.bazel +++ b/python/private/pypi/BUILD.bazel @@ -71,11 +71,23 @@ bzl_library( ], ) -bzl_lirary( +bzl_library( name = "env_marker_info_bzl", srcs = ["env_marker_info.bzl"], ) +bzl_library( + name = "env_marker_setting_bzl", + srcs = ["env_marker_setting.bzl"], + deps = [ + ":env_marker_info_bzl", + ":pep508_env_bzl", + ":pep508_evaluate_bzl", + "//python/private:toolchain_types_bzl", + "@bazel_skylib//rules:common_settings", + ], +) + bzl_library( name = "evaluate_markers_bzl", srcs = ["evaluate_markers.bzl"], @@ -117,6 +129,7 @@ bzl_library( srcs = ["flags.bzl"], deps = [ ":env_marker_info.bzl", + ":pep508_env_bzl", "//python/private:enum_bzl", "@bazel_skylib//rules:common_settings", ], diff --git a/python/private/pypi/env_marker_setting.bzl b/python/private/pypi/env_marker_setting.bzl index 36b087fe87..62d0adc883 100644 --- a/python/private/pypi/env_marker_setting.bzl +++ b/python/private/pypi/env_marker_setting.bzl @@ -3,10 +3,7 @@ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") load(":env_marker_info.bzl", "EnvMarkerInfo") -load( - ":pep508_env.bzl", - "env_aliases", -) +load(":pep508_env.bzl", "env_aliases") load(":pep508_evaluate.bzl", "evaluate") # Use capitals to hint its not an actual boolean type. From 8aa69189f61dcc65b93fdb77059cd95d5ffcbdb2 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 00:02:01 -0700 Subject: [PATCH 3/6] unify mappings --- python/private/pypi/pep508_env.bzl | 66 +++++++++++++++--------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/python/private/pypi/pep508_env.bzl b/python/private/pypi/pep508_env.bzl index 3708c46f1d..bc0166355d 100644 --- a/python/private/pypi/pep508_env.bzl +++ b/python/private/pypi/pep508_env.bzl @@ -64,23 +64,23 @@ platform_machine_select_map = { # Platform system returns results from the `uname` call. _platform_system_values = { + # See https://peps.python.org/pep-0738/#platform + "android": "Android", + "freebsd": "FreeBSD", + # See https://peps.python.org/pep-0730/#platform + # NOTE: Per Pep 730, "iPadOS" is also an acceptable value + "ios": "iOS", "linux": "Linux", + "netbsd": "NetBSD", + "openbsd": "OpenBSD", "osx": "Darwin", "windows": "Windows", } platform_system_select_map = { - # See https://peps.python.org/pep-0738/#platform - "@platforms//os:android": "Android", - "@platforms//os:freebsd": "FreeBSD", - # See https://peps.python.org/pep-0730/#platform - # NOTE: Per Pep 730, "iPadOS" is also an acceptable value - "@platforms//os:ios": "iOS", - "@platforms//os:linux": "Linux", - "@platforms//os:netbsd": "NetBSD", - "@platforms//os:openbsd": "OpenBSD", - "@platforms//os:osx": "Darwin", - "@platforms//os:windows": "Windows", + "@platforms//os:{}".format(bazel_os): py_system + for bazel_os, py_system in _platform_system_values.items() +} | { # The value is empty string if it cannot be determined: # https://docs.python.org/3/library/platform.html#platform.machine "//conditions:default": "", @@ -112,33 +112,36 @@ platform_system_select_map = { # # We are using only the subset that we actually support. _sys_platform_values = { + # These values are decided by the sys.platform docs. + "android": "android", + "emscripten": "emscripten", + # NOTE: The below values are approximations. The sys.platform() docs + # don't have documented values for these OSes. Per docs, the + # sys.platform() value reflects the OS at the time Python was *built* + # instead of the runtime (target) OS value. + "freebsd": "freebsd", + "ios": "ios", "linux": "linux", + "openbsd": "openbsd", "osx": "darwin", + "wasi": "wasi", "windows": "win32", } -# Taken from -# https://docs.python.org/3/library/sys.html#sys.platform sys_platform_select_map = { - # These values are decided by the sys.platform docs. - "@platforms//os:android": "android", - "@platforms//os:emscripten": "emscripten", - # NOTE: The below values are approximations. The sys.platform() docs - # don't have documented values for these OSes. Per docs, the - # sys.platform() value reflects the OS at the time Python was *built* - # instead of the runtime (target) OS value. - "@platforms//os:freebsd": "freebsd", - "@platforms//os:ios": "ios", - "@platforms//os:linux": "linux", - "@platforms//os:openbsd": "openbsd", - "@platforms//os:osx": "darwin", - "@platforms//os:wasi": "wasi", - "@platforms//os:windows": "win32", + "@platforms//os:{}".format(bazel_os): py_platform + for bazel_os, py_platform in _sys_platform_values.items() +} | { # For lack of a better option, use empty string. No standard doc/spec # about sys_platform value. "//conditions:default": "", } +# The "java" value is documented, but with Jython defunct, +# shouldn't occur in practice. +# The os.name value is technically a property of the runtime, not the +# targetted runtime OS, but the distinction shouldn't matter if +# things are properly configured. _os_name_values = { "linux": "posix", "osx": "posix", @@ -146,12 +149,9 @@ _os_name_values = { } os_name_select_map = { - # The "java" value is documented, but with Jython defunct, - # shouldn't occur in practice. - # The os.name value is technically a property of the runtime, not the - # targetted runtime OS, but the distinction shouldn't matter if - # things are properly configured. - "@platforms//os:windows": "nt", + "@platforms//os:{}".format(bazel_os): py_os + for bazel_os, py_os in _os_name_values.items() +} | { "//conditions:default": "posix", } From e106ef9740bfc4d09979fb54aca4cc96dc1503dc Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 01:10:32 -0700 Subject: [PATCH 4/6] add test --- .../env_marker_setting_tests.bzl | 36 ++++++++++++++++++- tests/support/support.bzl | 1 + 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl index 549c15c20b..f8414bb5d5 100644 --- a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl +++ b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl @@ -3,11 +3,45 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") load("@rules_testing//lib:util.bzl", "TestingAspectInfo") +load("//python/private/pypi:env_marker_info.bzl", "EnvMarkerInfo") # buildifier: disable=bzl-visibility load("//python/private/pypi:env_marker_setting.bzl", "env_marker_setting") # buildifier: disable=bzl-visibility -load("//tests/support:support.bzl", "PYTHON_VERSION") +load("//tests/support:support.bzl", "PIP_ENV_MARKER_CONFIG", "PYTHON_VERSION") + +def _custom_env_markers_impl(ctx): + return [EnvMarkerInfo(env = { + "os_name": "testos", + })] + +_custom_env_markers = rule( + implementation = _custom_env_markers_impl, +) _tests = [] +def _test_custom_env_markers(name): + def _impl(env, target): + env.expect.where( + expression = target[TestingAspectInfo].attrs.expression, + ).that_str( + target[config_common.FeatureFlagInfo].value, + ).equals("TRUE") + + env_marker_setting( + name = name + "_subject", + expression = "os_name == 'testos'", + ) + _custom_env_markers(name = name + "_env") + analysis_test( + name = name, + impl = _impl, + target = name + "_subject", + config_settings = { + PIP_ENV_MARKER_CONFIG: str(Label(name + "_env")), + }, + ) + +_tests.append(_test_custom_env_markers) + def _test_expr(name): def impl(env, target): env.expect.where( diff --git a/tests/support/support.bzl b/tests/support/support.bzl index 6330155d8c..7bab263c66 100644 --- a/tests/support/support.bzl +++ b/tests/support/support.bzl @@ -37,6 +37,7 @@ CROSSTOOL_TOP = Label("//tests/support/cc_toolchains:cc_toolchain_suite") ADD_SRCS_TO_RUNFILES = str(Label("//python/config_settings:add_srcs_to_runfiles")) BOOTSTRAP_IMPL = str(Label("//python/config_settings:bootstrap_impl")) EXEC_TOOLS_TOOLCHAIN = str(Label("//python/config_settings:exec_tools_toolchain")) +PIP_ENV_MARKER_CONFIG = str(Label("//python/config_settings:pip_env_marker_config")) PRECOMPILE = str(Label("//python/config_settings:precompile")) PRECOMPILE_SOURCE_RETENTION = str(Label("//python/config_settings:precompile_source_retention")) PYC_COLLECTION = str(Label("//python/config_settings:pyc_collection")) From 4e2a97e4819c432ab96b59b3f2c7c5145c1bb36c Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 01:10:58 -0700 Subject: [PATCH 5/6] cleanup --- tests/pypi/env_marker_setting/env_marker_setting_tests.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl index f8414bb5d5..e16f2c8ef6 100644 --- a/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl +++ b/tests/pypi/env_marker_setting/env_marker_setting_tests.bzl @@ -8,6 +8,7 @@ load("//python/private/pypi:env_marker_setting.bzl", "env_marker_setting") # bu load("//tests/support:support.bzl", "PIP_ENV_MARKER_CONFIG", "PYTHON_VERSION") def _custom_env_markers_impl(ctx): + _ = ctx # @unused return [EnvMarkerInfo(env = { "os_name": "testos", })] From e8bccc101c6f6afd84d78772f7b54c327ec34227 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 3 May 2025 13:24:34 -0700 Subject: [PATCH 6/6] unify setting defaults --- python/private/pypi/env_marker_setting.bzl | 28 +++--------- python/private/pypi/flags.bzl | 7 +-- python/private/pypi/pep508_env.bzl | 51 ++++++++++++++++------ 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/python/private/pypi/env_marker_setting.bzl b/python/private/pypi/env_marker_setting.bzl index 62d0adc883..2bfdf42ef0 100644 --- a/python/private/pypi/env_marker_setting.bzl +++ b/python/private/pypi/env_marker_setting.bzl @@ -3,7 +3,7 @@ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") load(":env_marker_info.bzl", "EnvMarkerInfo") -load(":pep508_env.bzl", "env_aliases") +load(":pep508_env.bzl", "create_env", "set_missing_env_defaults") load(":pep508_evaluate.bzl", "evaluate") # Use capitals to hint its not an actual boolean type. @@ -37,7 +37,7 @@ def env_marker_setting(*, name, expression, **kwargs): ) def _env_marker_setting_impl(ctx): - env = {} + env = create_env() env.update( ctx.attr._env_marker_config_flag[EnvMarkerInfo].env, ) @@ -60,28 +60,10 @@ def _env_marker_setting_impl(ctx): env["python_full_version"] = full_version env["implementation_version"] = full_version - if "implementation_name" not in env: - # We assume cpython if the toolchain doesn't specify because it's most - # likely to be true. - env["implementation_name"] = runtime.implementation_name or "cpython" - - if "platform_python_implementation" not in env: - # The `platform_python_implementation` marker value is supposed to come - # from `platform.python_implementation()`, however, PEP 421 introduced - # `sys.implementation.name` and the `implementation_name` env marker to - # replace it. Per the platform.python_implementation docs, there's now - # essentially just two possible "registered" values: CPython or PyPy. - # Rather than add a field to the toolchain, we just special case the value - # from `sys.implementation.name` to handle the two documented values. - platform_python_impl = runtime.implementation_name - if platform_python_impl == "cpython": - platform_python_impl = "CPython" - elif platform_python_impl == "pypy": - platform_python_impl = "PyPy" - env["platform_python_implementation"] = platform_python_impl - - env.update(env_aliases()) + if "implementation_name" not in env and runtime.implementation_name: + env["implementation_name"] = runtime.implementation_name + set_missing_env_defaults(env) if evaluate(ctx.attr.expression, env = env): value = _ENV_MARKER_TRUE else: diff --git a/python/private/pypi/flags.bzl b/python/private/pypi/flags.bzl index 8801164653..037383910e 100644 --- a/python/private/pypi/flags.bzl +++ b/python/private/pypi/flags.bzl @@ -23,6 +23,7 @@ load("//python/private:enum.bzl", "enum") load(":env_marker_info.bzl", "EnvMarkerInfo") load( ":pep508_env.bzl", + "create_env", "os_name_select_map", "platform_machine_select_map", "platform_system_select_map", @@ -124,7 +125,7 @@ def _default_env_marker_config(**kwargs): ) def _env_marker_config_impl(ctx): - env = {} + env = create_env() env["os_name"] = ctx.attr.os_name env["sys_platform"] = ctx.attr.sys_platform env["platform_machine"] = ctx.attr.platform_machine @@ -139,8 +140,8 @@ def _env_marker_config_impl(ctx): env["platform_release"] = platform_release env["platform_system"] = ctx.attr.platform_system - # For lack of a better option, just use an empty string for now. - env["platform_version"] = "" + # NOTE: We intentionally do not call set_missing_env_defaults() here because + # `env_marker_setting()` computes missing values using the toolchain. return [EnvMarkerInfo(env = env)] _env_marker_config = rule( diff --git a/python/private/pypi/pep508_env.bzl b/python/private/pypi/pep508_env.bzl index bc0166355d..5be4658194 100644 --- a/python/private/pypi/pep508_env.bzl +++ b/python/private/pypi/pep508_env.bzl @@ -158,6 +158,9 @@ os_name_select_map = { def env(target_platform, *, extra = None): """Return an env target platform + NOTE: This is for use during the loading phase. For the analysis phase, + `env_marker_setting()` constructs the env dict. + Args: target_platform: {type}`str` the target platform identifier, e.g. `cp33_linux_aarch64` @@ -166,16 +169,9 @@ def env(target_platform, *, extra = None): Returns: A dict that can be used as `env` in the marker evaluation. """ - - # TODO @aignas 2025-02-13: consider moving this into config settings. - - env = {"extra": extra} if extra != None else {} - env = env | { - "implementation_name": "cpython", - "platform_python_implementation": "CPython", - "platform_release": "", - "platform_version": "", - } + env = create_env() + if extra != None: + env["extra"] = extra if type(target_platform) == type(""): target_platform = platform_from_str(target_platform, python_version = "") @@ -196,13 +192,42 @@ def env(target_platform, *, extra = None): "platform_system": _platform_system_values.get(os, ""), "sys_platform": _sys_platform_values.get(os, ""), } + set_missing_env_defaults(env) - # This is split by topic - return env | env_aliases() + return env -def env_aliases(): +def create_env(): return { + # This is split by topic "_aliases": { "platform_machine": platform_machine_aliases, }, } + +def set_missing_env_defaults(env): + """Sets defaults based on existing values. + + Args: + env: dict; NOTE: modified in-place + """ + if "implementation_name" not in env: + # Use cpython as the default because it's likely the correct value. + env["implementation_name"] = "cpython" + if "platform_python_implementation" not in env: + # The `platform_python_implementation` marker value is supposed to come + # from `platform.python_implementation()`, however, PEP 421 introduced + # `sys.implementation.name` and the `implementation_name` env marker to + # replace it. Per the platform.python_implementation docs, there's now + # essentially just two possible "registered" values: CPython or PyPy. + # Rather than add a field to the toolchain, we just special case the value + # from `sys.implementation.name` to handle the two documented values. + platform_python_impl = env["implementation_name"] + if platform_python_impl == "cpython": + platform_python_impl = "CPython" + elif platform_python_impl == "pypy": + platform_python_impl = "PyPy" + env["platform_python_implementation"] = platform_python_impl + if "platform_release" not in env: + env["platform_release"] = "" + if "platform_version" not in env: + env["platform_version"] = "0"