chore(toolchain): remove chmod and disable ignore_root_error (#3421)

As discussed in the #2024 messaged we decided to remove the chmoding and
simplify the setup for all of our users. This in effect unifies how the
toolchains are created across all of the platforms reducing the need for
some
of the integration tests.

Summary:
- `python_repository`: stop chmoding
- `python_repository`: make `ignore_root_user_error` noop
- `python(bzlmod)`: stop using `ignore_root_user_error`.
- `tests`: remove `ignore_root_user_error` tests.

Fixes #2016
Fixes #2053
Closes #2024

Author: aignas

.bazelci/presubmit.yml

@@ -615,19 +615,6 @@ tasks:
     - "git diff --exit-code"
 
 
-  integration_test_ignore_root_user_error_macos_workspace:
-    <<: *reusable_build_test_all
-    <<: *common_workspace_flags
-    name: "ignore_root_user_error: macOS, workspace"
-    working_directory: tests/integration/ignore_root_user_error
-    platform: macos
-  integration_test_ignore_root_user_error_windows_workspace:
-    <<: *reusable_build_test_all
-    <<: *common_workspace_flags
-    name: "ignore_root_user_error: Windows, workspace"
-    working_directory: tests/integration/ignore_root_user_error
-    platform: windows
-
   integration_compile_pip_requirements_test_from_external_repo_ubuntu_min_workspace:
     <<: *minimum_supported_version
     <<: *common_workspace_flags_min_bazel

.bazelignore

@@ -31,6 +31,5 @@ gazelle/examples/bzlmod_build_file_generation/bazel-bzlmod_build_file_generation
 gazelle/examples/bzlmod_build_file_generation/bazel-out
 gazelle/examples/bzlmod_build_file_generation/bazel-testlog
 tests/integration/compile_pip_requirements/bazel-compile_pip_requirements
-tests/integration/ignore_root_user_error/bazel-ignore_root_user_error
 tests/integration/local_toolchains/bazel-local_toolchains
 tests/integration/py_cc_toolchain_registered/bazel-py_cc_toolchain_registered

.bazelrc.deleted_packages

@@ -40,8 +40,6 @@ common --deleted_packages=gazelle/python/private
 common --deleted_packages=tests/integration/compile_pip_requirements
 common --deleted_packages=tests/integration/compile_pip_requirements_test_from_external_repo
 common --deleted_packages=tests/integration/custom_commands
-common --deleted_packages=tests/integration/ignore_root_user_error
-common --deleted_packages=tests/integration/ignore_root_user_error/submodule
 common --deleted_packages=tests/integration/local_toolchains
 common --deleted_packages=tests/integration/pip_parse
 common --deleted_packages=tests/integration/pip_parse/empty

CHANGELOG.md

@@ -62,6 +62,11 @@ END_UNRELEASED_TEMPLATE
 * (toolchain) Remove all of the python 3.9 toolchain versions except for the `3.9.25`.
   This version has reached EOL and will no longer receive any security fixes, please update to
   `3.10` or above. ([#2704](https://github.com/bazel-contrib/rules_python/issues/2704))
+* (toolchain) `ignore_root_user_error` has now been flipped to be always enabled and
+  the `chmod` of the python toolchain directories have been removed. From now on `rules_python`
+  always adds the `pyc` files to the glob excludes and in order to avoid any problems when using
+  the toolchains in the repository phase, ensure that you pass `-B` to the python interpreter.
+  ([#2016](https://github.com/bazel-contrib/rules_python/issues/2016))
 
 {#v0-0-0-changed}
 ### Changed

python/private/python.bzl

@@ -76,13 +76,6 @@ def parse_modules(*, module_ctx, logger, _fail = fail):
     # Map of string Major.Minor or Major.Minor.Patch to the toolchain_info struct
     global_toolchain_versions = {}
 
-    ignore_root_user_error = None
-
-    # if the root module does not register any toolchain then the
-    # ignore_root_user_error takes its default value: True
-    if not module_ctx.modules[0].tags.toolchain:
-        ignore_root_user_error = True
-
     config = _get_toolchain_config(modules = module_ctx.modules, _fail = _fail)
 
     default_python_version = _compute_default_python_version(module_ctx)
@@ -115,15 +108,6 @@ def parse_modules(*, module_ctx, logger, _fail = fail):
                 # * The root module is allowed to override the rules_python default.
                 is_default = default_python_version == toolchain_version
 
-                # Also only the root module should be able to decide ignore_root_user_error.
-                # Modules being depended upon don't know the final environment, so they aren't
-                # in the right position to know or decide what the correct setting is.
-
-                # If an inconsistency in the ignore_root_user_error among multiple toolchains is detected, fail.
-                if ignore_root_user_error != None and toolchain_attr.ignore_root_user_error != ignore_root_user_error:
-                    fail("Toolchains in the root module must have consistent 'ignore_root_user_error' attributes")
-
-                ignore_root_user_error = toolchain_attr.ignore_root_user_error
             elif mod.name == "rules_python" and not default_toolchain:
                 # This branch handles when the root module doesn't declare a
                 # Python toolchain
@@ -166,7 +150,6 @@ def parse_modules(*, module_ctx, logger, _fail = fail):
                 global_toolchain_versions[toolchain_version] = toolchain_info
                 if debug_info:
                     debug_info["toolchains_registered"].append({
-                        "ignore_root_user_error": ignore_root_user_error,
                         "module": {"is_root": mod.is_root, "name": mod.name},
                         "name": toolchain_name,
                     })
@@ -185,8 +168,6 @@ def parse_modules(*, module_ctx, logger, _fail = fail):
             elif toolchain_info:
                 toolchains.append(toolchain_info)
 
-    config.default.setdefault("ignore_root_user_error", ignore_root_user_error)
-
     # A default toolchain is required so that the non-version-specific rules
     # are able to match a toolchain.
     if default_toolchain == None:
@@ -722,7 +703,6 @@ def _process_global_overrides(*, tag, default, _fail = fail):
         default["minor_mapping"] = tag.minor_mapping
 
     forwarded_attrs = sorted(AUTH_ATTRS) + [
-        "ignore_root_user_error",
         "base_url",
         "register_all_versions",
     ]
@@ -974,7 +954,6 @@ def _create_toolchain_attrs_struct(
         is_default = is_default,
         python_version = python_version if python_version else tag.python_version,
         configure_coverage_tool = getattr(tag, "configure_coverage_tool", False),
-        ignore_root_user_error = getattr(tag, "ignore_root_user_error", True),
     )
 
 _defaults = tag_class(
@@ -1086,16 +1065,9 @@ Then the python interpreter will be available as `my_python_name`.
         "ignore_root_user_error": attr.bool(
             default = True,
             doc = """\
-The Python runtime installation is made read only. This improves the ability for
-Bazel to cache it by preventing the interpreter from creating `.pyc` files for
-the standard library dynamically at runtime as they are loaded (this often leads
-to spurious cache misses or build failures).
-
-However, if the user is running Bazel as root, this read-onlyness is not
-respected. Bazel will print a warning message when it detects that the runtime
-installation is writable despite being made read only (i.e. it's running with
-root access) while this attribute is set `False`, however this messaging can be ignored by setting
-this to `False`.
+:::{versionchanged} VERSION_NEXT_FEATURE
+Noop, will be removed in the next major release.
+:::
 """,
             mandatory = False,
         ),

python/private/python_repository.bzl

@@ -123,45 +123,6 @@ def _python_repository_impl(rctx):
             logger = logger,
         )
 
-    # Make the Python installation read-only. This is to prevent issues due to
-    # pycs being generated at runtime:
-    # * The pycs are not deterministic (they contain timestamps)
-    # * Multiple processes trying to write the same pycs can result in errors.
-    #
-    # Note, when on Windows the `chmod` may not work
-    if "windows" not in platform and "windows" != repo_utils.get_platforms_os_name(rctx):
-        repo_utils.execute_checked(
-            rctx,
-            op = "python_repository.MakeReadOnly",
-            arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", "lib"],
-            logger = logger,
-        )
-
-        # If the user is not ignoring the warnings, then proceed to run a check,
-        # otherwise these steps can be skipped, as they both result in some warning.
-        if not rctx.attr.ignore_root_user_error:
-            exec_result = repo_utils.execute_unchecked(
-                rctx,
-                op = "python_repository.TestReadOnly",
-                arguments = [repo_utils.which_checked(rctx, "touch"), "lib/.test"],
-                logger = logger,
-            )
-
-            # The issue with running as root is the installation is no longer
-            # read-only, so the problems due to pyc can resurface.
-            if exec_result.return_code == 0:
-                stdout = repo_utils.execute_checked_stdout(
-                    rctx,
-                    op = "python_repository.GetUserId",
-                    arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
-                    logger = logger,
-                )
-                uid = int(stdout.strip())
-                if uid == 0:
-                    logger.warn("The current user is root, which can cause spurious cache misses or build failures with the hermetic Python interpreter. See https://github.com/bazel-contrib/rules_python/pull/713.")
-                else:
-                    logger.warn("The current user has CAP_DAC_OVERRIDE set, which can cause spurious cache misses or build failures with the hermetic Python interpreter. See https://github.com/bazel-contrib/rules_python/pull/713.")
-
     python_bin = "python.exe" if ("windows" in platform) else "bin/python3"
 
     if "linux" in platform:
@@ -186,17 +147,15 @@ def _python_repository_impl(rctx):
                 break
 
     glob_include = []
-    glob_exclude = []
-    if rctx.attr.ignore_root_user_error or "windows" in platform:
-        glob_exclude += [
-            # These pycache files are created on first use of the associated python files.
-            # Exclude them from the glob because otherwise between the first time and second time a python toolchain is used,"
-            # the definition of this filegroup will change, and depending rules will get invalidated."
-            # See https://github.com/bazel-contrib/rules_python/issues/1008 for unconditionally adding these to toolchains so we can stop ignoring them."
-            # pyc* is ignored because pyc creation creates temporary .pyc.NNNN files
-            "**/__pycache__/*.pyc*",
-            "**/__pycache__/*.pyo*",
-        ]
+    glob_exclude = [
+        # These pycache files are created on first use of the associated python files.
+        # Exclude them from the glob because otherwise between the first time and second time a python toolchain is used,"
+        # the definition of this filegroup will change, and depending rules will get invalidated."
+        # See https://github.com/bazel-contrib/rules_python/issues/1008 for unconditionally adding these to toolchains so we can stop ignoring them."
+        # pyc* is ignored because pyc creation creates temporary .pyc.NNNN files
+        "**/__pycache__/*.pyc*",
+        "**/__pycache__/*.pyo*",
+    ]
 
     if "windows" in platform:
         glob_include += [
@@ -249,7 +208,6 @@ define_hermetic_runtime_toolchain_impl(
         "coverage_tool": rctx.attr.coverage_tool,
         "distutils": rctx.attr.distutils,
         "distutils_content": rctx.attr.distutils_content,
-        "ignore_root_user_error": rctx.attr.ignore_root_user_error,
         "name": rctx.attr.name,
         "netrc": rctx.attr.netrc,
         "patch_strip": rctx.attr.patch_strip,
@@ -299,7 +257,7 @@ For more information see {attr}`py_runtime.coverage_tool`.
         ),
         "ignore_root_user_error": attr.bool(
             default = True,
-            doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.",
+            doc = "Noop, will be removed in the next major release",
             mandatory = False,
         ),
         "netrc": attr.string(

tests/integration/BUILD.bazel

@@ -75,22 +75,6 @@ rules_python_integration_test(
     workspace_path = "compile_pip_requirements",
 )
 
-rules_python_integration_test(
-    name = "ignore_root_user_error_test",
-    env = {
-        "RULES_PYTHON_BZLMOD_DEBUG": "1",
-    },
-)
-
-rules_python_integration_test(
-    name = "ignore_root_user_error_workspace_test",
-    bzlmod = False,
-    env = {
-        "RULES_PYTHON_BZLMOD_DEBUG": "1",
-    },
-    workspace_path = "ignore_root_user_error",
-)
-
 rules_python_integration_test(
     name = "local_toolchains_test",
     env = {