unify setting defaults

Author: rickeylev

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:

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(

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"