fix(pypi): reuse select dicts for constructing the env (#3108)
Before this PR we would be constructing slightly different environments
when the `env_marker_setting` is doing it in the analysis phase and when
we are doing it in the repo phase due to how the defaults are handled.
In this change we simply reuse the same select statements and add an
extra
helper that is allowing us to process that.
Work towards #2949
Prep for #3058
Co-authored-by: Richard Levasseur <richardlev@gmail.com>
diff --git a/python/private/pypi/BUILD.bazel b/python/private/pypi/BUILD.bazel
index b098f29..e5a916b 100644
--- a/python/private/pypi/BUILD.bazel
+++ b/python/private/pypi/BUILD.bazel
@@ -252,6 +252,9 @@
bzl_library(
name = "pep508_env_bzl",
srcs = ["pep508_env.bzl"],
+ deps = [
+ "//python/private:version_bzl",
+ ],
)
bzl_library(
@@ -264,11 +267,6 @@
)
bzl_library(
- name = "pep508_platform_bzl",
- srcs = ["pep508_platform.bzl"],
-)
-
-bzl_library(
name = "pep508_requirement_bzl",
srcs = ["pep508_requirement.bzl"],
deps = [
diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl
index 096256e..08e1af4 100644
--- a/python/private/pypi/extension.bzl
+++ b/python/private/pypi/extension.bzl
@@ -76,11 +76,12 @@
for platform, values in config.platforms.items():
key = "{}_{}".format(abi, platform)
- platforms[key] = env(struct(
- abi = abi,
+ platforms[key] = env(
+ env = values.env,
os = values.os_name,
arch = values.arch_name,
- )) | values.env
+ python_version = python_version,
+ )
return platforms
def _create_whl_repos(
diff --git a/python/private/pypi/pep508_env.bzl b/python/private/pypi/pep508_env.bzl
index c2d404b..5031eba 100644
--- a/python/private/pypi/pep508_env.bzl
+++ b/python/private/pypi/pep508_env.bzl
@@ -15,6 +15,20 @@
"""This module is for implementing PEP508 environment definition.
"""
+load("//python/private:version.bzl", "version")
+
+_DEFAULT = "//conditions:default"
+
+# Here we store the aliases in the platform so that the users can specify any valid target in
+# there.
+_cpu_aliases = {
+ "arm": "aarch32",
+ "arm64": "aarch64",
+}
+_os_aliases = {
+ "macos": "osx",
+}
+
# See https://stackoverflow.com/a/45125525
platform_machine_aliases = {
# These pairs mean the same hardware, but different values may be used
@@ -59,7 +73,7 @@
"@platforms//cpu:x86_64": "x86_64",
# The value is empty string if it cannot be determined:
# https://docs.python.org/3/library/platform.html#platform.machine
- "//conditions:default": "",
+ _DEFAULT: "",
}
# Platform system returns results from the `uname` call.
@@ -73,7 +87,7 @@
"linux": "Linux",
"netbsd": "NetBSD",
"openbsd": "OpenBSD",
- "osx": "Darwin",
+ "osx": "Darwin", # NOTE: macos is an alias to osx, we handle it through _os_aliases
"windows": "Windows",
}
@@ -83,7 +97,7 @@
} | {
# The value is empty string if it cannot be determined:
# https://docs.python.org/3/library/platform.html#platform.machine
- "//conditions:default": "",
+ _DEFAULT: "",
}
# The copy of SO [answer](https://stackoverflow.com/a/13874620) containing
@@ -123,18 +137,19 @@
"ios": "ios",
"linux": "linux",
"openbsd": "openbsd",
- "osx": "darwin",
+ "osx": "darwin", # NOTE: macos is an alias to osx, we handle it through _os_aliases
"wasi": "wasi",
"windows": "win32",
}
sys_platform_select_map = {
+ # These values are decided by the sys.platform docs.
"@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": "",
+ _DEFAULT: "",
}
# The "java" value is documented, but with Jython defunct,
@@ -142,53 +157,58 @@
# 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",
- "windows": "nt",
-}
-
os_name_select_map = {
- "@platforms//os:{}".format(bazel_os): py_os
- for bazel_os, py_os in _os_name_values.items()
-} | {
- "//conditions:default": "posix",
+ "@platforms//os:windows": "nt",
+ _DEFAULT: "posix",
}
-def env(target_platform, *, extra = None):
+def _set_default(env, env_key, m, key):
+ """Set the default value in the env if it is not already set."""
+ default = m.get(key, m[_DEFAULT])
+ env.setdefault(env_key, default)
+
+def env(*, env = None, os, arch, python_version = "", 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`
+ env: {type}`str` the environment.
+ os: {type}`str` the OS name.
+ arch: {type}`str` the CPU name.
+ python_version: {type}`str` the full python version.
extra: {type}`str` the extra value to be added into the env.
Returns:
A dict that can be used as `env` in the marker evaluation.
"""
- env = create_env()
+ env = env or {}
+ env = env | create_env()
if extra != None:
env["extra"] = extra
- if target_platform.abi:
- minor_version, _, micro_version = target_platform.abi[3:].partition(".")
- micro_version = micro_version or "0"
+ if python_version:
+ v = version.parse(python_version)
+ major = v.release[0]
+ minor = v.release[1]
+ micro = v.release[2] if len(v.release) > 2 else 0
env = env | {
- "implementation_version": "3.{}.{}".format(minor_version, micro_version),
- "python_full_version": "3.{}.{}".format(minor_version, micro_version),
- "python_version": "3.{}".format(minor_version),
+ "implementation_version": "{}.{}.{}".format(major, minor, micro),
+ "python_full_version": "{}.{}.{}".format(major, minor, micro),
+ "python_version": "{}.{}".format(major, minor),
}
- if target_platform.os and target_platform.arch:
- os = target_platform.os
- env = env | {
- "os_name": _os_name_values.get(os, ""),
- "platform_machine": target_platform.arch,
- "platform_system": _platform_system_values.get(os, ""),
- "sys_platform": _sys_platform_values.get(os, ""),
- }
+
+ if os:
+ os = "@platforms//os:{}".format(_os_aliases.get(os, os))
+ _set_default(env, "os_name", os_name_select_map, os)
+ _set_default(env, "platform_system", platform_system_select_map, os)
+ _set_default(env, "sys_platform", sys_platform_select_map, os)
+
+ if arch:
+ arch = "@platforms//cpu:{}".format(_cpu_aliases.get(arch, arch))
+ _set_default(env, "platform_machine", platform_machine_select_map, arch)
+
set_missing_env_defaults(env)
return env
diff --git a/python/private/pypi/pep508_platform.bzl b/python/private/pypi/pep508_platform.bzl
deleted file mode 100644
index 381a8d7..0000000
--- a/python/private/pypi/pep508_platform.bzl
+++ /dev/null
@@ -1,57 +0,0 @@
-# Copyright 2025 The Bazel Authors. All rights reserved.
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-# http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-
-"""The platform abstraction
-"""
-
-def platform(*, abi = None, os = None, arch = None):
- """platform returns a struct for the platform.
-
- Args:
- abi: {type}`str | None` the target ABI, e.g. `"cp39"`.
- os: {type}`str | None` the target os, e.g. `"linux"`.
- arch: {type}`str | None` the target CPU, e.g. `"aarch64"`.
-
- Returns:
- A struct.
- """
-
- # Note, this is used a lot as a key in dictionaries, so it cannot contain
- # methods.
- return struct(
- abi = abi,
- os = os,
- arch = arch,
- )
-
-def platform_from_str(p, python_version):
- """Return a platform from a string.
-
- Args:
- p: {type}`str` the actual string.
- python_version: {type}`str` the python version to add to platform if needed.
-
- Returns:
- A struct that is returned by the `_platform` function.
- """
- if p.startswith("cp"):
- abi, _, p = p.partition("_")
- elif python_version:
- major, _, tail = python_version.partition(".")
- abi = "cp{}{}".format(major, tail)
- else:
- abi = None
-
- os, _, arch = p.partition("_")
- return platform(abi = abi, os = os or None, arch = arch or None)
diff --git a/tests/pypi/pep508/BUILD.bazel b/tests/pypi/pep508/BUILD.bazel
index 7eab2e0..36fce0f 100644
--- a/tests/pypi/pep508/BUILD.bazel
+++ b/tests/pypi/pep508/BUILD.bazel
@@ -1,4 +1,5 @@
load(":deps_tests.bzl", "deps_test_suite")
+load(":env_tests.bzl", "env_test_suite")
load(":evaluate_tests.bzl", "evaluate_test_suite")
load(":requirement_tests.bzl", "requirement_test_suite")
@@ -6,6 +7,10 @@
name = "deps_tests",
)
+env_test_suite(
+ name = "env_tests",
+)
+
evaluate_test_suite(
name = "evaluate_tests",
)
diff --git a/tests/pypi/pep508/env_tests.bzl b/tests/pypi/pep508/env_tests.bzl
new file mode 100644
index 0000000..cfd94a1
--- /dev/null
+++ b/tests/pypi/pep508/env_tests.bzl
@@ -0,0 +1,69 @@
+"""Tests to check for env construction."""
+
+load("@rules_testing//lib:test_suite.bzl", "test_suite")
+load("//python/private/pypi:pep508_env.bzl", pep508_env = "env") # buildifier: disable=bzl-visibility
+
+_tests = []
+
+def _test_env_defaults(env):
+ got = pep508_env(os = "exotic", arch = "exotic", python_version = "3.1.1")
+ got.pop("_aliases")
+ env.expect.that_dict(got).contains_exactly({
+ "implementation_name": "cpython",
+ "implementation_version": "3.1.1",
+ "os_name": "posix",
+ "platform_machine": "",
+ "platform_python_implementation": "CPython",
+ "platform_release": "",
+ "platform_system": "",
+ "platform_version": "0",
+ "python_full_version": "3.1.1",
+ "python_version": "3.1",
+ "sys_platform": "",
+ })
+
+_tests.append(_test_env_defaults)
+
+def _test_env_freebsd(env):
+ got = pep508_env(os = "freebsd", arch = "arm64", python_version = "3.1.1")
+ got.pop("_aliases")
+ env.expect.that_dict(got).contains_exactly({
+ "implementation_name": "cpython",
+ "implementation_version": "3.1.1",
+ "os_name": "posix",
+ "platform_machine": "aarch64",
+ "platform_python_implementation": "CPython",
+ "platform_release": "",
+ "platform_system": "FreeBSD",
+ "platform_version": "0",
+ "python_full_version": "3.1.1",
+ "python_version": "3.1",
+ "sys_platform": "freebsd",
+ })
+
+_tests.append(_test_env_freebsd)
+
+def _test_env_macos(env):
+ got = pep508_env(os = "macos", arch = "arm64", python_version = "3.1.1")
+ got.pop("_aliases")
+ env.expect.that_dict(got).contains_exactly({
+ "implementation_name": "cpython",
+ "implementation_version": "3.1.1",
+ "os_name": "posix",
+ "platform_machine": "aarch64",
+ "platform_python_implementation": "CPython",
+ "platform_release": "",
+ "platform_system": "Darwin",
+ "platform_version": "0",
+ "python_full_version": "3.1.1",
+ "python_version": "3.1",
+ "sys_platform": "darwin",
+ })
+
+_tests.append(_test_env_macos)
+
+def env_test_suite(name): # buildifier: disable=function-docstring
+ test_suite(
+ name = name,
+ basic_tests = _tests,
+ )
diff --git a/tests/pypi/pep508/evaluate_tests.bzl b/tests/pypi/pep508/evaluate_tests.bzl
index cc867f3..7843f88 100644
--- a/tests/pypi/pep508/evaluate_tests.bzl
+++ b/tests/pypi/pep508/evaluate_tests.bzl
@@ -16,7 +16,6 @@
load("@rules_testing//lib:test_suite.bzl", "test_suite")
load("//python/private/pypi:pep508_env.bzl", pep508_env = "env") # buildifier: disable=bzl-visibility
load("//python/private/pypi:pep508_evaluate.bzl", "evaluate", "tokenize") # buildifier: disable=bzl-visibility
-load("//python/private/pypi:pep508_platform.bzl", "platform_from_str") # buildifier: disable=bzl-visibility
_tests = []
@@ -244,26 +243,37 @@
def _evaluate_with_aliases(env):
# When
- for target_platform, tests in {
+ for (os, cpu), tests in {
# buildifier: @unsorted-dict-items
- "osx_aarch64": {
+ ("osx", "aarch64"): {
"platform_system == 'Darwin' and platform_machine == 'arm64'": True,
"platform_system == 'Darwin' and platform_machine == 'aarch64'": True,
"platform_system == 'Darwin' and platform_machine == 'amd64'": False,
},
- "osx_x86_64": {
+ ("osx", "x86_64"): {
"platform_system == 'Darwin' and platform_machine == 'amd64'": True,
"platform_system == 'Darwin' and platform_machine == 'x86_64'": True,
},
- "osx_x86_32": {
+ ("osx", "x86_32"): {
"platform_system == 'Darwin' and platform_machine == 'i386'": True,
"platform_system == 'Darwin' and platform_machine == 'i686'": True,
"platform_system == 'Darwin' and platform_machine == 'x86_32'": True,
"platform_system == 'Darwin' and platform_machine == 'x86_64'": False,
},
+ ("freebsd", "x86_32"): {
+ "platform_system == 'FreeBSD' and platform_machine == 'i386'": True,
+ "platform_system == 'FreeBSD' and platform_machine == 'i686'": True,
+ "platform_system == 'FreeBSD' and platform_machine == 'x86_32'": True,
+ "platform_system == 'FreeBSD' and platform_machine == 'x86_64'": False,
+ "platform_system == 'FreeBSD' and os_name == 'posix'": True,
+ },
}.items(): # buildifier: @unsorted-dict-items
for input, want in tests.items():
- _check_evaluate(env, input, want, pep508_env(platform_from_str(target_platform, "")))
+ _check_evaluate(env, input, want, pep508_env(
+ os = os,
+ arch = cpu,
+ python_version = "3.2",
+ ))
_tests.append(_evaluate_with_aliases)
