From ce34fefb4d9885237c4805538c6297099ca32f54 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 26 Jun 2026 07:58:14 +0900 Subject: [PATCH 1/2] feat: stop using chmod on Bazel 8.6+ Add an extract_needs_chmod config flag that controls whether chmod is run after extracting wheel files. The flag is False for Bazel >= 8.6 (where the fix for file permissions is built into Bazel) and True for older versions. Move _maybe_fix_permissions from whl_extract.bzl into repo_utils.bzl and have _extract call it conditionally based on the config flag. This ensures both whl_extract and patch_whl benefit from the chmod fix when needed. Fixes #3585 --- news/3855.changed.md | 1 + python/private/internal_config_repo.bzl | 3 + python/private/pypi/patch_whl.bzl | 1 + python/private/pypi/whl_extract.bzl | 24 +----- python/private/repo_utils.bzl | 23 +++++- tests/repo_utils/repo_utils_test.bzl | 104 ++++++++++++++++++++++++ 6 files changed, 132 insertions(+), 24 deletions(-) create mode 100644 news/3855.changed.md diff --git a/news/3855.changed.md b/news/3855.changed.md new file mode 100644 index 0000000000..a92fc52226 --- /dev/null +++ b/news/3855.changed.md @@ -0,0 +1 @@ +(pypi) chmod after wheel extraction is now skipped on Bazel 8.6+ diff --git a/python/private/internal_config_repo.bzl b/python/private/internal_config_repo.bzl index 72970cf100..f26556bce1 100644 --- a/python/private/internal_config_repo.bzl +++ b/python/private/internal_config_repo.bzl @@ -30,6 +30,7 @@ config = struct( supports_whl_extraction = {supports_whl_extraction}, enable_pystar = True, enable_deprecation_warnings = {enable_deprecation_warnings}, + extract_needs_chmod = {extract_needs_chmod}, bazel_8_or_later = {bazel_8_or_later}, bazel_9_or_later = {bazel_9_or_later}, bazel_10_or_later = {bazel_10_or_later}, @@ -86,6 +87,7 @@ def _internal_config_repo_impl(rctx): bazel_minor_version = 99999 supports_whl_extraction = False + extract_needs_chmod = bazel_major_version < 8 or (bazel_major_version == 8 and bazel_minor_version < 6) 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: @@ -105,6 +107,7 @@ def _internal_config_repo_impl(rctx): 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), + extract_needs_chmod = str(extract_needs_chmod), 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), diff --git a/python/private/pypi/patch_whl.bzl b/python/private/pypi/patch_whl.bzl index 98a5ad49c8..e8d76eeba5 100644 --- a/python/private/pypi/patch_whl.bzl +++ b/python/private/pypi/patch_whl.bzl @@ -80,6 +80,7 @@ def patch_whl(rctx, *, whl_path, patches): rctx, archive = whl_input, supports_whl_extraction = rp_config.supports_whl_extraction, + extract_needs_chmod = rp_config.extract_needs_chmod, ) if not patches: diff --git a/python/private/pypi/whl_extract.bzl b/python/private/pypi/whl_extract.bzl index 2ebb61a83a..0d61b9a07b 100644 --- a/python/private/pypi/whl_extract.bzl +++ b/python/private/pypi/whl_extract.bzl @@ -18,10 +18,9 @@ def whl_extract(rctx, *, whl_path, logger): archive = whl_path, output = install_dir_path, supports_whl_extraction = rp_config.supports_whl_extraction, + extract_needs_chmod = rp_config.extract_needs_chmod, ) - _maybe_fix_permissions(rctx, whl_path = whl_path, logger = logger) - metadata_file = find_whl_metadata( install_dir = install_dir_path, logger = logger, @@ -65,27 +64,6 @@ def whl_extract(rctx, *, whl_path, logger): # Ensure that there is no data dir left rctx.delete(data_dir) -# TODO: This can be removed when Bazel 8.6+ is the minimum supported version. -def _maybe_fix_permissions(rctx, *, whl_path, logger): - # Fix permissions on extracted files. Some wheels have files without read permissions set, - # which causes errors when trying to read them later. - # We apply this to the root directory to ensure that everything in bin/, site-packages/, - # etc. is readable and executable where appropriate. - os_name = repo_utils.get_platforms_os_name(rctx) - if os_name != "windows": - # On Unix-like systems, recursively add read permissions to all files - # and ensure directories are traversable (need execute permission) - result = repo_utils.execute_unchecked( - rctx, - op = "Fixing wheel permissions {}".format(whl_path), - arguments = ["chmod", "-R", "a+rX", "."], - logger = logger, - ) - if result.return_code != 0: - # It's possible chmod is not available or the filesystem doesn't support it. - # This is fine, we just want to try to fix permissions if possible. - logger.warn(lambda: "Failed to fix file permissions: {}".format(result.stderr)) - def merge_trees(src, dest): """Merge src into the destination path. diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl index 6fa851b7b3..b53373b9ac 100644 --- a/python/private/repo_utils.bzl +++ b/python/private/repo_utils.bzl @@ -511,7 +511,7 @@ def _get_platforms_cpu_name(mrctx): return "riscv64" return arch -def _extract(mrctx, *, archive, supports_whl_extraction = False, **kwargs): +def _extract(mrctx, *, archive, supports_whl_extraction = False, extract_needs_chmod = False, **kwargs): """Extract an archive TODO: remove when the earliest supported bazel version is at least 8.3. @@ -533,6 +533,26 @@ def _extract(mrctx, *, archive, supports_whl_extraction = False, **kwargs): if not mrctx.delete(archive): fail("Failed to remove the symlink after extracting") + if extract_needs_chmod: + _maybe_fix_permissions(mrctx, whl_path = archive) + +def _maybe_fix_permissions(mrctx, *, whl_path, logger = None): + if not logger and hasattr(mrctx, "attr"): + logger = _logger(mrctx) + elif not logger: + fail("logger must be specified when using 'module_ctx'") + + os_name = _get_platforms_os_name(mrctx) + if os_name != "windows": + result = _execute_unchecked( + mrctx, + op = "Fixing wheel permissions {}".format(whl_path), + arguments = ["chmod", "-R", "a+rX", "."], + logger = logger, + ) + if result.return_code != 0: + logger.warn(lambda: "Failed to fix file permissions: {}".format(result.stderr)) + def _rename(mrctx, src, dest): """Rename a file or directory. @@ -575,6 +595,7 @@ repo_utils = struct( get_platforms_os_name = _get_platforms_os_name, is_repo_debug_enabled = _is_repo_debug_enabled, logger = _logger, + maybe_fix_permissions = _maybe_fix_permissions, mkdir = _mkdir, norm_path = _norm_path, relative_to = _relative_to, diff --git a/tests/repo_utils/repo_utils_test.bzl b/tests/repo_utils/repo_utils_test.bzl index ce9e48b5a6..294a1b5b2b 100644 --- a/tests/repo_utils/repo_utils_test.bzl +++ b/tests/repo_utils/repo_utils_test.bzl @@ -6,6 +6,48 @@ load("//tests/support:mocks.bzl", "mocks") _tests = [] +def _make_rctx(*, os_name, mock_extracts = None, mock_files = None): + mock_extracts = mock_extracts or {} + mock_files = mock_files or {} + execute_calls = [] + + def _execute(arguments, **kwargs): + _ = kwargs # buildifier: disable=unused-variable + execute_calls.append(arguments) + return struct(return_code = 0, stdout = "", stderr = "") + + def _extract(archive, output = "", **kwargs): + _ = kwargs # buildifier: disable=unused-variable + archive_str = str(archive) + if archive_str in mock_extracts: + for f, c in mock_extracts[archive_str].items(): + out_path = "{}/{}".format(output, f) if output else str(f) + mock_files[out_path] = c + + def _path(x): + return struct( + exists = str(x) in mock_files, + basename = str(x).split("/")[-1], + dirname = "/".join(str(x).split("/")[:-1]), + ) + + rctx = struct( + execute = _execute, + extract = _extract, + getenv = lambda key: "", + path = _path, + report_progress = lambda msg: None, + os = struct(name = os_name), + delete = lambda x: True, + symlink = lambda target, link_name: None, + name = "test_repo", + attr = struct( + _rule_name = "test_rule", + ), + _execute_calls = execute_calls, + ) + return rctx + def _test_get_platforms_os_name(env): mock_mrctx = mocks.rctx(os_name = "Mac OS X") got = repo_utils.get_platforms_os_name(mock_mrctx) @@ -50,6 +92,68 @@ def _test_is_relative_to(env): _tests.append(_test_is_relative_to) +def _test_extract_calls_chmod_when_enabled(env): + mock_rctx = _make_rctx( + os_name = "linux", + mock_extracts = {"test.whl": {"f1": "c1"}}, + ) + + repo_utils.extract( + mock_rctx, + archive = mock_rctx.path("test.whl"), + output = "out", + supports_whl_extraction = True, + extract_needs_chmod = True, + ) + + env.expect.that_bool(len(mock_rctx._execute_calls) > 0).equals(True) + env.expect.that_str(mock_rctx._execute_calls[0][0]).equals("chmod") + +_tests.append(_test_extract_calls_chmod_when_enabled) + +def _test_extract_skips_chmod_when_disabled(env): + mock_rctx = _make_rctx( + os_name = "linux", + mock_extracts = {"test.whl": {"f1": "c1"}}, + ) + + repo_utils.extract( + mock_rctx, + archive = mock_rctx.path("test.whl"), + output = "out", + supports_whl_extraction = True, + extract_needs_chmod = False, + ) + + env.expect.that_collection(mock_rctx._execute_calls).contains_exactly([]) + +_tests.append(_test_extract_skips_chmod_when_disabled) + +def _test_maybe_fix_permissions_calls_chmod_on_linux(env): + mock_rctx = _make_rctx(os_name = "linux") + + repo_utils.maybe_fix_permissions( + mock_rctx, + whl_path = mock_rctx.path("test.whl"), + ) + + env.expect.that_bool(len(mock_rctx._execute_calls) > 0).equals(True) + env.expect.that_str(mock_rctx._execute_calls[0][0]).equals("chmod") + +_tests.append(_test_maybe_fix_permissions_calls_chmod_on_linux) + +def _test_maybe_fix_permissions_skips_on_windows(env): + mock_rctx = _make_rctx(os_name = "windows") + + repo_utils.maybe_fix_permissions( + mock_rctx, + whl_path = mock_rctx.path("test.whl"), + ) + + env.expect.that_collection(mock_rctx._execute_calls).contains_exactly([]) + +_tests.append(_test_maybe_fix_permissions_skips_on_windows) + def repo_utils_test_suite(name): """Create the test suite. From f4174082b023440b6c67f9dfba9c4480fc41383c Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 27 Jun 2026 14:30:24 +0900 Subject: [PATCH 2/2] fix: filter out from dir --- python/private/repo_utils.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/repo_utils.bzl b/python/private/repo_utils.bzl index b53373b9ac..0e83fda28c 100644 --- a/python/private/repo_utils.bzl +++ b/python/private/repo_utils.bzl @@ -202,7 +202,7 @@ def _execute_internal( output = _outputs_to_str(result, log_stdout = log_stdout, log_stderr = log_stderr), )) - result_kwargs = {k: getattr(result, k) for k in dir(result)} + result_kwargs = {k: getattr(result, k) for k in dir(result) if k not in ["to_json", "to_proto"]} return struct( describe_failure = lambda: _execute_describe_failure( op = op,