fix: Downgrade "running as root" error to a warning by default (#2636)
Currently, by default, rules_python immediately fails when Bazel is run
as root. The reasoning behind this involves .pyc files being generated
for hermetic toolchains when they're first used, causing cache misses;
to work around this, rules_python opts to make the toolchain
installation directory read-only, but running Bazel as root would
circumvent this. So rules_python actively detects if the current user is
root, and hard fails. This check can be disabled by the root module by
setting `python.override(ignore_root_user_error=True)`. (See more
context in the linked issues/PRs.)
This causes a reverberating effect across the Bazel ecosystem, as
rules_python is essentially a dependency of every single Bazel project
through protobuf. Effectively, any Bazel project wishing to run as root
need to add the override tag above, even if they don't have anything to
do with Python at all.
This PR changes the default value of the `ignore_root_user_error` to
True instead. Besides, it now unconditionally tries to make the
toolchain installation directory read-only, and only outputs a warning
if it's detected that the current user is root.
See previous discussions at #713, #749, #907, #1008, #1169, etc.
Fixes https://github.com/bazelbuild/rules_python/issues/1169.
---------
Co-authored-by: Richard Levasseur <rlevasseur@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e447012..849b458 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -59,6 +59,10 @@
* (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 ec6f73e..304a1d7 100644
--- a/python/private/python.bzl
+++ b/python/private/python.bzl
@@ -72,9 +72,9 @@
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 @@
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 @@
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.
+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).
-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.
+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 @@
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 c7407c8..075d4b1 100644
--- a/python/private/python_repository.bzl
+++ b/python/private/python_repository.bzl
@@ -127,37 +127,36 @@
# 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(
+ 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.MakeReadOnly",
- arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", lib_dir],
+ op = "python_repository.GetUserId",
+ arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
logger = logger,
)
- exec_result = repo_utils.execute_unchecked(
- rctx,
- op = "python_repository.TestReadOnly",
- arguments = [repo_utils.which_checked(rctx, "touch"), "{}/.test".format(lib_dir)],
- 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 @@
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 e7828b9..6552251 100644
--- a/tests/python/python_tests.bzl
+++ b/tests/python/python_tests.bzl
@@ -62,7 +62,7 @@
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 @@
"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 @@
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 @@
_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 @@
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 @@
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")
