From 5a06d9480cc8c365493f5793ebc63d3e1e854c2e Mon Sep 17 00:00:00 2001 From: vfdev-5 Date: Thu, 3 Oct 2024 10:00:43 +0000 Subject: [PATCH 1/2] Added flag_values for free_threading argument to point to correct paths of the headers and the library. --- python/config_settings/BUILD.bazel | 8 ++++++ .../private/hermetic_runtime_repo_setup.bzl | 27 +++++++++++-------- python/private/pypi/flags.bzl | 8 ++++++ python/private/python.bzl | 16 +++++++++++ python/private/python_register_toolchains.bzl | 7 +++++ python/private/python_repository.bzl | 10 ++++++- 6 files changed, 64 insertions(+), 12 deletions(-) diff --git a/python/config_settings/BUILD.bazel b/python/config_settings/BUILD.bazel index 9fb395741b..76282915f1 100644 --- a/python/config_settings/BUILD.bazel +++ b/python/config_settings/BUILD.bazel @@ -12,6 +12,7 @@ load( load( "//python/private/pypi:flags.bzl", "UniversalWhlFlag", + "UseFreeThreadingFlag", "UseWhlFlag", "WhlLibcFlag", "define_pypi_internal_flags", @@ -170,3 +171,10 @@ string_flag( define_pypi_internal_flags( name = "define_pypi_internal_flags", ) + +string_flag( + name = "free_threading", + build_setting_default = UseFreeThreadingFlag.NO, + values = sorted(UseFreeThreadingFlag.__members__.values()), + visibility = ["//visibility:public"], +) diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl index 4b5a3c6810..02fbf0c145 100644 --- a/python/private/hermetic_runtime_repo_setup.bzl +++ b/python/private/hermetic_runtime_repo_setup.bzl @@ -27,7 +27,8 @@ def define_hermetic_runtime_toolchain_impl( extra_files_glob_exclude, python_version, python_bin, - coverage_tool): + coverage_tool, + free_threading = False): """Define a toolchain implementation for a python-build-standalone repo. It expected this macro is called in the top-level package of an extracted @@ -44,13 +45,17 @@ def define_hermetic_runtime_toolchain_impl( python_version: {type}`str` The Python version, in `major.minor.micro` format. python_bin: {type}`str` The path to the Python binary within the - repositoroy. + repository. coverage_tool: {type}`str` optional target to the coverage tool to use. + free_threading: {type}`bool` optional free-threading support. + Default, False. """ _ = name # @unused version_info = semver(python_version) version_dict = version_info.to_dict() + version_dict["ft_postfix"] = "t" if free_threading else "" + native.filegroup( name = "files", srcs = native.glob( @@ -67,19 +72,19 @@ def define_hermetic_runtime_toolchain_impl( "**/* *", # Bazel does not support spaces in file names. # Unused shared libraries. `python` executable and the `:libpython` target # depend on `libpython{python_version}.so.1.0`. - "lib/libpython{major}.{minor}.so".format(**version_dict), + "lib/libpython{major}.{minor}{ft_postfix}.so".format(**version_dict), # static libraries "lib/**/*.a", # tests for the standard libraries. - "lib/python{major}.{minor}/**/test/**".format(**version_dict), - "lib/python{major}.{minor}/**/tests/**".format(**version_dict), + "lib/python{major}.{minor}{ft_postfix}/**/test/**".format(**version_dict), + "lib/python{major}.{minor}{ft_postfix}/**/tests/**".format(**version_dict), "**/__pycache__/*.pyc.*", # During pyc creation, temp files named *.pyc.NNN are created ] + extra_files_glob_exclude, ), ) cc_import( name = "interface", - interface_library = "libs/python{major}{minor}.lib".format(**version_dict), + interface_library = "libs/python{major}{minor}{ft_postfix}.lib".format(**version_dict), system_provided = True, ) @@ -96,7 +101,7 @@ def define_hermetic_runtime_toolchain_impl( hdrs = [":includes"], includes = [ "include", - "include/python{major}.{minor}".format(**version_dict), + "include/python{major}.{minor}{ft_postfix}".format(**version_dict), "include/python{major}.{minor}m".format(**version_dict), ], ) @@ -105,11 +110,11 @@ def define_hermetic_runtime_toolchain_impl( hdrs = [":includes"], srcs = select({ "@platforms//os:linux": [ - "lib/libpython{major}.{minor}.so".format(**version_dict), - "lib/libpython{major}.{minor}.so.1.0".format(**version_dict), + "lib/libpython{major}.{minor}{ft_postfix}.so".format(**version_dict), + "lib/libpython{major}.{minor}{ft_postfix}.so.1.0".format(**version_dict), ], - "@platforms//os:macos": ["lib/libpython{major}.{minor}.dylib".format(**version_dict)], - "@platforms//os:windows": ["python3.dll", "libs/python{major}{minor}.lib".format(**version_dict)], + "@platforms//os:macos": ["lib/libpython{major}.{minor}{ft_postfix}.dylib".format(**version_dict)], + "@platforms//os:windows": ["python3.dll", "libs/python{major}{minor}{ft_postfix}.lib".format(**version_dict)], }), ) diff --git a/python/private/pypi/flags.bzl b/python/private/pypi/flags.bzl index 1e380625ce..d357cd4dc1 100644 --- a/python/private/pypi/flags.bzl +++ b/python/private/pypi/flags.bzl @@ -70,6 +70,14 @@ INTERNAL_FLAGS = [ "whl_pycp3x_abicp", ] +# Determines if we use CPython with free-threading (disabled GIL) +# +# buildifier: disable=name-conventions +UseFreeThreadingFlag = enum( + YES = "yes", + NO = "no", +) + def define_pypi_internal_flags(name): for flag in INTERNAL_FLAGS: string_flag( diff --git a/python/private/python.bzl b/python/private/python.bzl index 83bc43f92e..b482ee28e1 100644 --- a/python/private/python.bzl +++ b/python/private/python.bzl @@ -357,6 +357,13 @@ def _process_single_version_overrides(*, tag, _fail = fail, default): for platform in sha256 } + if tag.flag_values: + # Normalize flag keys to strings + flag_values = {str(k): v for k, v in tag.flag_values.items()} + override["flag_values"] = flag_values + if tag.suffix: + override["suffix"] = tag.suffix + available_versions[tag.python_version] = {k: v for k, v in override.items() if v} if tag.distutils_content: @@ -789,6 +796,15 @@ class. mandatory = False, doc = "The URL template to fetch releases for this Python version. See {attr}`python.single_version_platform_override.urls` for documentation.", ), + "flag_values": attr.string_dict( + mandatory = False, + doc = "TODO", + ), + "suffix": attr.string( + mandatory = False, + doc = "TODO", + default = "", + ) }, ) diff --git a/python/private/python_register_toolchains.bzl b/python/private/python_register_toolchains.bzl index d20e049610..7549702ddc 100644 --- a/python/private/python_register_toolchains.bzl +++ b/python/private/python_register_toolchains.bzl @@ -129,6 +129,12 @@ def python_register_toolchains( )], ) + flag_values = tool_versions[python_version].get("flag_values", None) + free_threading_label = "@rules_python//python/config_settings:free_threading" + free_threading = False + if flag_values: + free_threading = flag_values.get(free_threading_label, False) == "yes" + suffix = tool_versions[python_version].get("suffix", "") python_repository( name = "{name}_{platform}".format( name = name, @@ -143,6 +149,7 @@ def python_register_toolchains( urls = urls, strip_prefix = strip_prefix, coverage_tool = coverage_tool, + free_threading = free_threading, **kwargs ) if register_toolchains: diff --git a/python/private/python_repository.bzl b/python/private/python_repository.bzl index e44bdd151d..01b14a4aa5 100644 --- a/python/private/python_repository.bzl +++ b/python/private/python_repository.bzl @@ -67,6 +67,7 @@ def _python_repository_impl(rctx): release_filename = rctx.attr.release_filename urls = rctx.attr.urls or [rctx.attr.url] auth = get_auth(rctx, urls) + free_threading = rctx.attr.free_threading if release_filename.endswith(".zst"): rctx.download( @@ -130,7 +131,8 @@ def _python_repository_impl(rctx): if "windows" in platform: distutils_path = "Lib/distutils/distutils.cfg" else: - distutils_path = "lib/python{}/distutils/distutils.cfg".format(python_short_version) + ft_postfix = "t" if free_threading else "" + distutils_path = "lib/python{}{}/distutils/distutils.cfg".format(python_short_version, ft_postfix) if rctx.attr.distutils: rctx.file(distutils_path, rctx.read(rctx.attr.distutils)) elif rctx.attr.distutils_content: @@ -255,6 +257,7 @@ define_hermetic_runtime_toolchain_impl( python_version = {python_version}, python_bin = {python_bin}, coverage_tool = {coverage_tool}, + free_threading = {free_threading}, ) """.format( extra_files_glob_exclude = render.list(glob_exclude), @@ -262,6 +265,7 @@ define_hermetic_runtime_toolchain_impl( python_bin = render.str(python_bin), python_version = render.str(rctx.attr.python_version), coverage_tool = render.str(coverage_tool), + free_threading = free_threading, ) rctx.delete("python") rctx.symlink(python_bin, "python") @@ -321,6 +325,10 @@ For more information see {attr}`py_runtime.coverage_tool`. "Either distutils or distutils_content can be specified, but not both.", mandatory = False, ), + "free_threading": attr.bool( + doc = "Whether we use CPython interpreter in free-threading mode (disabled GIL).", + default = False, + ), "ignore_root_user_error": attr.bool( default = False, doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.", From be242fa34992788cded66198bbe6230ae15516de Mon Sep 17 00:00:00 2001 From: vfdev-5 Date: Thu, 10 Oct 2024 12:32:02 +0200 Subject: [PATCH 2/2] WIP adding support for cpython build_option --- python/private/py_exec_tools_toolchain.bzl | 2 +- python/private/python_register_toolchains.bzl | 28 ++++++++++--------- python/private/python_repository.bzl | 4 +-- python/versions.bzl | 16 ++++++++--- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/python/private/py_exec_tools_toolchain.bzl b/python/private/py_exec_tools_toolchain.bzl index 957448f421..d6db14016d 100644 --- a/python/private/py_exec_tools_toolchain.bzl +++ b/python/private/py_exec_tools_toolchain.bzl @@ -43,7 +43,7 @@ py_exec_tools_toolchain = rule( Provides a toolchain for build time tools. This provides `ToolchainInfo` with the following attributes: -* `exec_tools`: {type}`PyExecToolsInfo` +* `exec_tools`: {type}`PyExecToolsInfo` * `toolchain_label`: {type}`Label` _only present when `--visibile_for_testing=True` for internal testing_. The rule's label; this allows identifying what toolchain implmentation was selected for testing purposes. diff --git a/python/private/python_register_toolchains.bzl b/python/private/python_register_toolchains.bzl index 7549702ddc..1a9f30127a 100644 --- a/python/private/python_register_toolchains.bzl +++ b/python/private/python_register_toolchains.bzl @@ -109,35 +109,37 @@ def python_register_toolchains( if not sha256: continue + build_option = tool_versions[python_version].get("build_option", None) loaded_platforms.append(platform) - (release_filename, urls, strip_prefix, patches, patch_strip) = get_release_info(platform, python_version, base_url, tool_versions) + (release_filename, urls, strip_prefix, patches, patch_strip, free_threading) = get_release_info( + platform, python_version, base_url, tool_versions, build_option=build_option + ) + build_opt_str = ("_" + build_option.replace("+", "-")) if build_option else "" + name_with_build_opt = "{name}{build_opt}".format( + name = name, + build_opt = build_opt_str + ) # allow passing in a tool version coverage_tool = None coverage_tool = tool_versions[python_version].get("coverage_tool", {}).get(platform, None) if register_coverage_tool and coverage_tool == None: coverage_tool = coverage_dep( name = "{name}_{platform}_coverage".format( - name = name, + name = name_with_build_opt, platform = platform, ), python_version = python_version, platform = platform, visibility = ["@{name}_{platform}//:__subpackages__".format( - name = name, + name = name_with_build_opt, platform = platform, )], ) - flag_values = tool_versions[python_version].get("flag_values", None) - free_threading_label = "@rules_python//python/config_settings:free_threading" - free_threading = False - if flag_values: - free_threading = flag_values.get(free_threading_label, False) == "yes" - suffix = tool_versions[python_version].get("suffix", "") python_repository( name = "{name}_{platform}".format( - name = name, + name = name_with_build_opt, platform = platform, ), sha256 = sha256, @@ -166,12 +168,12 @@ def python_register_toolchains( platform = platform, )) - host_toolchain(name = name + "_host") + host_toolchain(name = name_with_build_opt + "_host") toolchain_aliases( name = name, python_version = python_version, - user_repository_name = name, + user_repository_name = name_with_build_opt, platforms = loaded_platforms, ) @@ -183,5 +185,5 @@ def python_register_toolchains( name = toolchain_repo_name, python_version = python_version, set_python_version_constraint = set_python_version_constraint, - user_repository_name = name, + user_repository_name = name_with_build_opt, ) diff --git a/python/private/python_repository.bzl b/python/private/python_repository.bzl index 01b14a4aa5..0ad7001339 100644 --- a/python/private/python_repository.bzl +++ b/python/private/python_repository.bzl @@ -127,11 +127,11 @@ def _python_repository_impl(rctx): for patch in patches: rctx.patch(patch, strip = rctx.attr.patch_strip) + ft_postfix = "t" if free_threading else "" # Write distutils.cfg to the Python installation. if "windows" in platform: distutils_path = "Lib/distutils/distutils.cfg" else: - ft_postfix = "t" if free_threading else "" distutils_path = "lib/python{}{}/distutils/distutils.cfg".format(python_short_version, ft_postfix) if rctx.attr.distutils: rctx.file(distutils_path, rctx.read(rctx.attr.distutils)) @@ -145,7 +145,7 @@ def _python_repository_impl(rctx): # dyld lookup errors. To fix, set the full path to the dylib as # it appears in the Bazel workspace as its LC_ID_DYLIB using # the `install_name_tool` bundled with macOS. - dylib = "libpython{}.dylib".format(python_short_version) + dylib = "libpython{}{}.dylib".format(python_short_version, ft_postfix) repo_utils.execute_checked( rctx, op = "python_repository.FixUpDyldIdPath", diff --git a/python/versions.bzl b/python/versions.bzl index ae017e3d12..ddb95c15a9 100644 --- a/python/versions.bzl +++ b/python/versions.bzl @@ -703,7 +703,9 @@ PLATFORMS = { ), } -def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_URL, tool_versions = TOOL_VERSIONS): +def get_release_info( + platform, python_version, base_url = DEFAULT_RELEASE_BASE_URL, tool_versions = TOOL_VERSIONS, build_option = None +): """Resolve the release URL for the requested interpreter version Args: @@ -711,12 +713,18 @@ def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_U python_version: The version of the interpreter to get base_url: The URL to prepend to the 'url' attr in the tool_versions dict tool_versions: A dict listing the interpreter versions, their SHAs and URL + build_option: Python build option, default: "install_only" Returns: - A tuple of (filename, url, archive strip prefix, patches, patch_strip) + A tuple of (filename, url, archive strip prefix, patches, patch_strip, free_threading) """ url = tool_versions[python_version]["url"] + if not build_option: + build_option = "shared-install_only" if (WINDOWS_NAME in platform) else "install_only" + free_threading = False + else: + free_threading = True if build_option.startswith("freethreaded") else False if type(url) == type({}): url = url[platform] @@ -734,7 +742,7 @@ def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_U release_filename = u.format( platform = platform, python_version = python_version, - build = "shared-install_only" if (WINDOWS_NAME in platform) else "install_only", + build = build_option, ) if "://" in release_filename: # is absolute url? rendered_urls.append(release_filename) @@ -757,7 +765,7 @@ def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_U else: patch_strip = None - return (release_filename, rendered_urls, strip_prefix, patches, patch_strip) + return (release_filename, rendered_urls, strip_prefix, patches, patch_strip, free_threading) def print_toolchains_checksums(name): native.genrule(