Skip to content

chore(pip): Check for whl extract compatibility in internal_config_repo.bzl #3456

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.

Sign up for GitHub

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
aignas merged 1 commit into from
Dec 14, 2025
Merged

chore(pip): Check for whl extract compatibility in internal_config_repo.bzl #3456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion python/private/internal_config_repo.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ _ENABLE_DEPRECATION_WARNINGS_DEFAULT = "0"
_CONFIG_TEMPLATE = """
config = struct(
build_python_zip_default = {build_python_zip_default},
supports_whl_extraction = {supports_whl_extraction},
enable_pystar = True,
enable_pipstar = {enable_pipstar},
enable_deprecation_warnings = {enable_deprecation_warnings},
Expand Down Expand Up @@ -91,8 +92,20 @@ _TRANSITION_SETTINGS_DEBUG_TEMPLATE = """
def _internal_config_repo_impl(rctx):
# An empty version signifies a development build, which is treated as
# the latest version.
bazel_major_version = int(native.bazel_version.split(".")[0]) if native.bazel_version else 99999
if native.bazel_version:
version_parts = native.bazel_version.split(".")
bazel_major_version = int(version_parts[0])
bazel_minor_version = int(version_parts[1])
Comment on lines +96 to +98
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current version parsing logic is a bit fragile. If native.bazel_version is a string like "8.3-rc1", version_parts[1] would be "3-rc1", and int("3-rc1") would raise a ValueError. Similarly, if the version string doesn't contain a . (e.g., "8"), accessing version_parts[1] would cause an IndexError. It's safer to handle these edge cases to make the parsing more robust.

Suggested change
version_parts = native.bazel_version.split(".")
bazel_major_version = int(version_parts[0])
bazel_minor_version = int(version_parts[1])
version_parts = native.bazel_version.split(".")
bazel_major_version = int(version_parts[0])
bazel_minor_version = int(version_parts[1].split("-")[0]) if len(version_parts) > 1 else 0

Copy link
Contributor Author

@armandomontanez armandomontanez Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? I'd assume if the Bazel version is set we're guaranteed both that:

  • The version string is three parts.
  • The 2nd part of version string will be an integer.

Copy link
Collaborator

@aignas aignas Dec 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually the stranger versions numbers may happen with pre-build. That said, I am not fully sure about what would happen.

If the UI is happy (and it is running with 9.0 rc version, I'd say we can merge.

else:
bazel_major_version = 99999
bazel_minor_version = 99999

supports_whl_extraction = False
if bazel_major_version >= 8:
# Extracting .whl files requires Bazel 8.3.0 or later.
if bazel_major_version > 8 or bazel_minor_version >= 3:
supports_whl_extraction = True

builtin_py_info_symbol = "None"
builtin_py_runtime_info_symbol = "None"
builtin_py_cc_link_params_provider = "None"
Expand All @@ -107,6 +120,7 @@ def _internal_config_repo_impl(rctx):
enable_deprecation_warnings = _bool_from_environ(rctx, _ENABLE_DEPRECATION_WARNINGS_ENVVAR_NAME, _ENABLE_DEPRECATION_WARNINGS_DEFAULT),
builtin_py_info_symbol = builtin_py_info_symbol,
builtin_py_runtime_info_symbol = builtin_py_runtime_info_symbol,
supports_whl_extraction = str(supports_whl_extraction),
builtin_py_cc_link_params_provider = builtin_py_cc_link_params_provider,
bazel_8_or_later = str(bazel_major_version >= 8),
bazel_9_or_later = str(bazel_major_version >= 9),
Expand Down
7 changes: 2 additions & 5 deletions python/private/pypi/whl_library.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,7 @@ def _whl_library_impl(rctx):
#
# Remove non-pipstar and config_load check when we release rules_python 2.
if enable_pipstar:
# Extracting .whl files requires Bazel 8.3.0 or later, so require a
# minimum of Bazel 9.0.0 to ensure compatibilty with earlier versions
# of Bazel 8.
if rp_config.bazel_9_or_later:
if rp_config.supports_whl_extraction:
extract_path = whl_path
else:
extract_path = rctx.path(whl_path.basename + ".zip")
Expand All @@ -389,7 +386,7 @@ def _whl_library_impl(rctx):
archive = extract_path,
output = "site-packages",
)
if not rp_config.bazel_9_or_later:
if not rp_config.supports_whl_extraction:
rctx.delete(extract_path)

metadata = whl_metadata(
Expand Down