feat: stop using chmod on Bazel 8.6+

news/3855.changed.md

@@ -0,0 +1 @@
+(pypi) chmod after wheel extraction is now skipped on Bazel 8.6+

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),

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:

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.
 

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,
@@ -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,

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.