-
-
Notifications
You must be signed in to change notification settings - Fork 636
refactor: add rule to do analysis time evaluation of environment markers #2832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
799f9c3
wip: flag to evaluate dep specs
rickeylev a658e8f
add some todo, fake some more values
rickeylev e7ea75b
Merge branch 'main' into pep508.flag
rickeylev e65c973
source values from select and flags
rickeylev 6181509
sorta fix platform_python_implementation
rickeylev bf2173c
wip: analysis tests basic
rickeylev 1e23941
trying to get tests to work
rickeylev ef61724
got basic test passing
rickeylev d1b9c0d
Merge branch 'main' of https://github.com/bazel-contrib/rules_python …
rickeylev 77decbc
parameterize test
rickeylev bda07f7
expand testing a bit
rickeylev 5d37d7d
a few notes and links and continue with impl
aignas 38e6f5b
rename to env_marker_setting
rickeylev be7e007
Merge remote-tracking branch 'origin/pep508.flag' into pep508.flag
rickeylev 3f794c2
move select mappings to constants
rickeylev 7dae5d0
remove extras_flag; various code/comment cleanup
rickeylev 2f1b528
minor cleanup, doc
rickeylev 7ff220e
fixup: forgot to pass args
rickeylev b340de7
code share with pep508_env.bzl, add config_setting,
rickeylev 104c27b
Merge branch 'main' of https://github.com/bazel-contrib/rules_python …
rickeylev b2c73c0
more cleanup
rickeylev ba05d71
more cleanup
rickeylev 5316f2a
remove platform_version and platform_release flags for now
rickeylev 2b30b54
wire platform_release back to osx flag
rickeylev a6bc605
cleanup comment
rickeylev 764879d
Merge branch 'main' of https://github.com/bazel-contrib/rules_python …
rickeylev 72a8f1e
update dleted packages
rickeylev 8a210d2
restore bazelrc
rickeylev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,265 @@ | ||
"""Implement a flag for matching the dependency specifiers at analysis time.""" | ||
|
||
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") | ||
load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") | ||
load(":pep508_evaluate.bzl", "evaluate") | ||
|
||
# todo: copied from pep508_env.bzl | ||
_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", | ||
"//conditions:default": "posix", | ||
} | ||
|
||
# TODO @aignas 2025-04-29: this is copied from ./pep508_env.bzl | ||
_platform_machine_aliases = { | ||
# These pairs mean the same hardware, but different values may be used | ||
# on different host platforms. | ||
"amd64": "x86_64", | ||
"arm64": "aarch64", | ||
"i386": "x86_32", | ||
"i686": "x86_32", | ||
} | ||
|
||
# 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", | ||
"@platforms//os:ios": "ios", | ||
"@platforms//os:linux": "linux", | ||
"@platforms//os:osx": "darwin", | ||
"@platforms//os:windows": "win32", | ||
"@platforms//os:wasi": "wasi", | ||
# 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:openbsd": "openbsd", | ||
# For lack of a better option, use empty string. No standard doc/spec | ||
# about sys_platform value. | ||
"//conditions:default": "", | ||
} | ||
|
||
# todo: copied from pep508_env.bzl | ||
# TODO: there are many cpus and unfortunately, it doesn't look like | ||
# the value is directly accessible to starlark. It might be possible to | ||
# get it via CcToolchain.cpu though. | ||
_platform_machine_select_map = { | ||
"@platforms//cpu:aarch32": "aarch32", | ||
"@platforms//cpu:aarch64": "aarch64", | ||
"@platforms//cpu:arm": "arm", | ||
"@platforms//cpu:arm64": "arm64", | ||
"@platforms//cpu:arm64_32": "arm64_32", | ||
"@platforms//cpu:arm64e": "arm64e", | ||
"@platforms//cpu:armv6-m": "armv6-m", | ||
"@platforms//cpu:armv7": "armv7", | ||
"@platforms//cpu:armv7-m": "armv7-m", | ||
"@platforms//cpu:armv7e-m": "armv7e-m", | ||
"@platforms//cpu:armv7e-mf": "armv7e-mf", | ||
"@platforms//cpu:armv7k": "armv7k", | ||
"@platforms//cpu:armv8-m": "armv8-m", | ||
"@platforms//cpu:cortex-r52": "cortex-r52", | ||
"@platforms//cpu:cortex-r82": "cortex-r82", | ||
"@platforms//cpu:i386": "i386", | ||
"@platforms//cpu:mips64": "mips64", | ||
"@platforms//cpu:ppc": "ppc", | ||
"@platforms//cpu:ppc32": "ppc32", | ||
"@platforms//cpu:ppc64le": "ppc64le", | ||
"@platforms//cpu:riscv32": "riscv32", | ||
"@platforms//cpu:riscv64": "riscv64", | ||
"@platforms//cpu:s390x": "s390x", | ||
"@platforms//cpu:wasm32": "wasm32", | ||
"@platforms//cpu:wasm64": "wasm64", | ||
"@platforms//cpu:x86_32": "x86_32", | ||
"@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": "", | ||
} | ||
|
||
# todo: copied from pep508_env.bzl | ||
_platform_system_select_map = { | ||
rickeylev marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
# 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", | ||
# The value is empty string if it cannot be determined: | ||
# https://docs.python.org/3/library/platform.html#platform.machine | ||
"//conditions:default": "", | ||
} | ||
|
||
def env_marker_setting(*, name, expression, **kwargs): | ||
aignas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Creates an env_marker setting. | ||
Args: | ||
name: {type}`str` target name | ||
expression: {type}`str` the environment marker string to evaluate | ||
**kwargs: {type}`dict` additionally common 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), | ||
**kwargs | ||
) | ||
|
||
# todo: maybe put all the env into a single target and have a | ||
# PyPiEnvMarkersInfo provider? Have --pypi_env=//some:target? | ||
def _env_marker_setting_impl(ctx): | ||
# todo: should unify with pep508_env.bzl | ||
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) | ||
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 | ||
aignas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
# The `platform_python_implementation` marker value is supposed to come from | ||
# `platform.python_implementation()`, however, PEP 421 introduced | ||
# `sys.implementation.name` to replace it. There's now essentially just two | ||
# possible values it might have: CPython or PyPy. Rather than add a field to | ||
# the toolchain, we just special case the value from | ||
# `sys.implementation.name` | ||
platform_python_impl = runtime.implementation_name | ||
if platform_python_impl == "cpython": | ||
platform_python_impl = "CPython" | ||
elif platform_python_impl == "pypy": | ||
platform_python_impl = "PyPy" | ||
aignas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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 | ||
env["platform_release"] = _get_flag(ctx.attr._platform_release_config_flag) | ||
env["platform_system"] = ctx.attr.platform_system | ||
env["platform_version"] = _get_flag(ctx.attr._platform_version_config_flag) | ||
|
||
# TODO @aignas 2025-04-29: figure out how to correctly share the aliases | ||
# between the two. Maybe the select statements above should be part of the | ||
# `pep508_env.bzl` file? | ||
env = env | { | ||
"_aliases": { | ||
"platform_machine": _platform_machine_aliases, | ||
}, | ||
} | ||
|
||
if evaluate(ctx.attr.expression, env = env): | ||
# todo: better return value than "yes" and "no" | ||
# matched/unmatched, satisfied/unsatisfied ? | ||
value = "yes" | ||
else: | ||
value = "no" | ||
return [config_common.FeatureFlagInfo(value = value)] | ||
|
||
_env_marker_setting = rule( | ||
doc = """ | ||
Evaluates an environment marker expression using target configuration info. | ||
See | ||
https://packaging.python.org/en/latest/specifications/dependency-specifiers | ||
for the specification of behavior. | ||
""", | ||
implementation = _env_marker_setting_impl, | ||
attrs = { | ||
"expression": attr.string( | ||
mandatory = True, | ||
doc = "Environment marker expression to evaluate.", | ||
), | ||
"os_name": attr.string(), | ||
"platform_machine": attr.string(), | ||
"platform_system": attr.string(), | ||
"sys_platform": attr.string(), | ||
"_platform_release_config_flag": attr.label( | ||
default = "//python/config_settings:pip_platform_release_config", | ||
providers = [[config_common.FeatureFlagInfo], [BuildSettingInfo]], | ||
), | ||
"_platform_version_config_flag": attr.label( | ||
default = "//python/config_settings:pip_platform_version_config", | ||
providers = [[config_common.FeatureFlagInfo], [BuildSettingInfo]], | ||
), | ||
"_python_full_version_flag": attr.label( | ||
default = "//python/config_settings:python_version", | ||
providers = [config_common.FeatureFlagInfo], | ||
), | ||
"_python_version_major_minor_flag": attr.label( | ||
default = "//python/config_settings:python_version_major_minor", | ||
providers = [config_common.FeatureFlagInfo], | ||
), | ||
}, | ||
provides = [config_common.FeatureFlagInfo], | ||
toolchains = [ | ||
TARGET_TOOLCHAIN_TYPE, | ||
], | ||
) | ||
|
||
def _format_full_version(info): | ||
"""Format the full python interpreter version. | ||
Adapted from spec code at: | ||
https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers | ||
Args: | ||
info: The provider from the Python runtime. | ||
Returns: | ||
a {type}`str` with the version | ||
""" | ||
kind = info.releaselevel | ||
if kind == "final": | ||
kind = "" | ||
serial = "" | ||
else: | ||
kind = kind[0] if kind else "" | ||
serial = str(info.serial) if info.serial else "" | ||
|
||
return "{major}.{minor}.{micro}{kind}{serial}".format( | ||
v = info, | ||
major = info.major, | ||
minor = info.minor, | ||
micro = info.micro, | ||
kind = kind, | ||
serial = serial, | ||
) | ||
|
||
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") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
load(":env_marker_setting_tests.bzl", "env_marker_setting_test_suite") | ||
|
||
env_marker_setting_test_suite( | ||
name = "env_marker_setting_tests", | ||
) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am reading it correctly, this value might be
freebsd8
and I am wondering if we should do some trickery to combine this value with theplatform_release
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs give "freebsd8" as an example from combining the uname calls. We don't know what freebsd version is being targeted, so I think it's best to just omit the version entirely.
re: combining with platform_release: platform_release is so poorly defined I don't think that's a good idea. In order to make it work, it'd have to be specific to a particular platform, i.e. the equivalent of "if //os:freebsd -> then concat('freebsd', $platform_release)". (Possible to express using select(), but if we go through that length, adding it to the toolchain is better IMHO).
The most correct thing would be to get the "undefined" cases from the toolchain; that's the best equivalent of "value when python was built".