diff --git a/CHANGELOG.md b/CHANGELOG.md index e447012c98..849b458745 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,10 @@ Unreleased changes template. * (pypi) The `ppc64le` is now pointing to the right target in the `platforms` package. * (gazelle) No longer incorrectly merge `py_binary` targets during partial updates in `file` generation mode. Fixed in [#2619](https://github.com/bazelbuild/rules_python/pull/2619). +* (bzlmod) Running as root is no longer an error. `ignore_root_user_error=True` + is now the default. Note that running as root may still cause spurious + Bazel cache invalidation + ([#1169](https://github.com/bazelbuild/rules_python/issues/1169)). {#v0-0-0-added} ### Added diff --git a/python/private/python.bzl b/python/private/python.bzl index ec6f73e41f..304a1d7745 100644 --- a/python/private/python.bzl +++ b/python/private/python.bzl @@ -72,9 +72,9 @@ def parse_modules(*, module_ctx, _fail = fail): logger = repo_utils.logger(module_ctx, "python") # if the root module does not register any toolchain then the - # ignore_root_user_error takes its default value: False + # ignore_root_user_error takes its default value: True if not module_ctx.modules[0].tags.toolchain: - ignore_root_user_error = False + ignore_root_user_error = True config = _get_toolchain_config(modules = module_ctx.modules, _fail = _fail) @@ -559,7 +559,7 @@ def _create_toolchain_attrs_struct(*, tag = None, python_version = None, toolcha 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", False), + ignore_root_user_error = getattr(tag, "ignore_root_user_error", True), ) def _get_bazel_version_specific_kwargs(): @@ -636,16 +636,18 @@ Then the python interpreter will be available as `my_python_name`. doc = "Whether or not to configure the default coverage tool provided by `rules_python` for the compatible toolchains.", ), "ignore_root_user_error": attr.bool( - default = False, + default = True, doc = """\ -If `False`, the Python runtime installation will be made read only. This improves -the ability for Bazel to cache it, but prevents the interpreter from creating -`.pyc` files for the standard library dynamically at runtime as they are loaded. - -If `True`, the Python runtime installation is read-write. This allows the -interpreter to create `.pyc` files for the standard library, but, because they are -created as needed, it adversely affects Bazel's ability to cache the runtime and -can result in spurious build failures. +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). If this attribute is set to `False`, Bazel will make it a hard +error to run with root access instead. """, mandatory = False, ), @@ -690,17 +692,8 @@ dependencies are introduced. default = DEFAULT_RELEASE_BASE_URL, ), "ignore_root_user_error": attr.bool( - default = False, - doc = """\ -If `False`, the Python runtime installation will be made read only. This improves -the ability for Bazel to cache it, but prevents the interpreter from creating -`.pyc` files for the standard library dynamically at runtime as they are loaded. - -If `True`, the Python runtime installation is read-write. This allows the -interpreter to create `.pyc` files for the standard library, but, because they are -created as needed, it adversely affects Bazel's ability to cache the runtime and -can result in spurious build failures. -""", + default = True, + doc = """Deprecated; do not use. This attribute has no effect.""", mandatory = False, ), "minor_mapping": attr.string_dict( diff --git a/python/private/python_repository.bzl b/python/private/python_repository.bzl index c7407c8f2c..075d4b1195 100644 --- a/python/private/python_repository.bzl +++ b/python/private/python_repository.bzl @@ -127,37 +127,36 @@ def _python_repository_impl(rctx): # 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. - if not rctx.attr.ignore_root_user_error: - if "windows" not in platform: - lib_dir = "lib" if "windows" not in platform else "Lib" + if "windows" not in platform: + repo_utils.execute_checked( + rctx, + op = "python_repository.MakeReadOnly", + arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", "lib"], + logger = logger, + ) - repo_utils.execute_checked( - rctx, - op = "python_repository.MakeReadOnly", - arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", lib_dir], - logger = logger, - ) - exec_result = repo_utils.execute_unchecked( + fail_or_warn = logger.warn if rctx.attr.ignore_root_user_error else logger.fail + 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.TestReadOnly", - arguments = [repo_utils.which_checked(rctx, "touch"), "{}/.test".format(lib_dir)], + op = "python_repository.GetUserId", + arguments = [repo_utils.which_checked(rctx, "id"), "-u"], 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: - fail("The current user is root, please run as non-root when using the hermetic Python interpreter. See https://github.com/bazelbuild/rules_python/pull/713.") - else: - fail("The current user has CAP_DAC_OVERRIDE set, please drop this capability when using the hermetic Python interpreter. See https://github.com/bazelbuild/rules_python/pull/713.") + uid = int(stdout.strip()) + if uid == 0: + fail_or_warn("The current user is root, which can cause spurious cache misses or build failures with the hermetic Python interpreter. See https://github.com/bazelbuild/rules_python/pull/713.") + else: + fail_or_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/bazelbuild/rules_python/pull/713.") python_bin = "python.exe" if ("windows" in platform) else "bin/python3" @@ -294,7 +293,7 @@ For more information see {attr}`py_runtime.coverage_tool`. mandatory = False, ), "ignore_root_user_error": attr.bool( - default = False, + default = True, doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.", mandatory = False, ), diff --git a/tests/python/python_tests.bzl b/tests/python/python_tests.bzl index e7828b92f5..6552251331 100644 --- a/tests/python/python_tests.bzl +++ b/tests/python/python_tests.bzl @@ -62,7 +62,7 @@ def _override( auth_patterns = {}, available_python_versions = [], base_url = "", - ignore_root_user_error = False, + ignore_root_user_error = True, minor_mapping = {}, netrc = "", register_all_versions = False): @@ -139,7 +139,7 @@ def _test_default(env): "ignore_root_user_error", "tool_versions", ]) - env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False) + env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True) env.expect.that_str(py.default_python_version).equals("3.11") want_toolchain = struct( @@ -212,13 +212,13 @@ def _test_default_non_rules_python_ignore_root_user_error(env): module_ctx = _mock_mctx( _mod( name = "my_module", - toolchain = [_toolchain("3.12", ignore_root_user_error = True)], + toolchain = [_toolchain("3.12", ignore_root_user_error = False)], ), _mod(name = "rules_python", toolchain = [_toolchain("3.11")]), ), ) - env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True) + env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False) env.expect.that_str(py.default_python_version).equals("3.12") my_module_toolchain = struct( @@ -238,49 +238,17 @@ def _test_default_non_rules_python_ignore_root_user_error(env): _tests.append(_test_default_non_rules_python_ignore_root_user_error) -def _test_default_non_rules_python_ignore_root_user_error_override(env): - py = parse_modules( - module_ctx = _mock_mctx( - _mod( - name = "my_module", - toolchain = [_toolchain("3.12")], - override = [_override(ignore_root_user_error = True)], - ), - _mod(name = "rules_python", toolchain = [_toolchain("3.11")]), - ), - ) - - env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True) - env.expect.that_str(py.default_python_version).equals("3.12") - - my_module_toolchain = struct( - name = "python_3_12", - python_version = "3.12", - register_coverage_tool = False, - ) - rules_python_toolchain = struct( - name = "python_3_11", - python_version = "3.11", - register_coverage_tool = False, - ) - env.expect.that_collection(py.toolchains).contains_exactly([ - rules_python_toolchain, - my_module_toolchain, - ]).in_order() - -_tests.append(_test_default_non_rules_python_ignore_root_user_error_override) - def _test_default_non_rules_python_ignore_root_user_error_non_root_module(env): py = parse_modules( module_ctx = _mock_mctx( _mod(name = "my_module", toolchain = [_toolchain("3.13")]), - _mod(name = "some_module", toolchain = [_toolchain("3.12", ignore_root_user_error = True)]), + _mod(name = "some_module", toolchain = [_toolchain("3.12", ignore_root_user_error = False)]), _mod(name = "rules_python", toolchain = [_toolchain("3.11")]), ), ) env.expect.that_str(py.default_python_version).equals("3.13") - env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False) + env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True) my_module_toolchain = struct( name = "python_3_13", @@ -338,8 +306,8 @@ def _test_first_occurance_of_the_toolchain_wins(env): env.expect.that_dict(py.debug_info).contains_exactly({ "toolchains_registered": [ - {"ignore_root_user_error": False, "module": {"is_root": True, "name": "my_module"}, "name": "python_3_12"}, - {"ignore_root_user_error": False, "module": {"is_root": False, "name": "rules_python"}, "name": "python_3_11"}, + {"ignore_root_user_error": True, "module": {"is_root": True, "name": "my_module"}, "name": "python_3_12"}, + {"ignore_root_user_error": True, "module": {"is_root": False, "name": "rules_python"}, "name": "python_3_11"}, ], }) @@ -364,7 +332,7 @@ def _test_auth_overrides(env): env.expect.that_dict(py.config.default).contains_at_least({ "auth_patterns": {"foo": "bar"}, - "ignore_root_user_error": False, + "ignore_root_user_error": True, "netrc": "/my/netrc", }) env.expect.that_str(py.default_python_version).equals("3.12")