10000 fix: Downgrade "running as root" error to a warning by default · bazel-contrib/rules_python@550174d · GitHub
[go: up one dir, main page]
Skip to content

Commit 550174d

Browse files
WyveraldWyverald
committed
fix: Downgrade "running as root" error to a warning by default
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 #1169.
1 parent effdce8 commit 550174d

File tree

3 files changed

+58
-56
lines changed

3 files changed

+58
-56
lines changed

‎python/private/python.bzl

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ def parse_modules(*, module_ctx, _fail = fail):
7272
logger = repo_utils.logger(module_ctx, "python")
7373

7474
# if the root module does not register any toolchain then the
75-
# ignore_root_user_error takes its default value: False
75+
# ignore_root_user_error takes its default value: True
7676
if not module_ctx.modules[0].tags.toolchain:
77-
ignore_root_user_error = False
77+
ignore_root_user_error = True
7878

7979
config = _get_toolchain_config(modules = module_ctx.modules, _fail = _fail)
8080

@@ -559,7 +559,7 @@ def _create_toolchain_attrs_struct(*, tag = None, python_version = None, toolcha
559559
is_default = is_default,
560560
python_version = python_version if python_version else tag.python_version,
561561
configure_coverage_tool = getattr(tag, "configure_coverage_tool", False),
562-
ignore_root_user_error = getattr(tag, "ignore_root_user_error", False),
562+
ignore_root_user_error = getattr(tag, "ignore_root_user_error", True),
563563
)
564564

565565
def _get_bazel_version_specific_kwargs():
@@ -636,16 +636,18 @@ Then the python interpreter will be available as `my_python_name`.
636636
doc = "Whether or not to configure the default coverage tool provided by `rules_python` for the compatible toolchains.",
637637
),
638638
"ignore_root_user_error": attr.bool(
639-
default = False,
639+
default = True,
640640
doc = """\
641-
If `False`, the Python runtime installation will be made read only. This improves
642-
the ability for Bazel to cache it, but prevents the interpreter from creating
643-
`.pyc` files for the standard library dynamically at runtime as they are loaded.
644-
645-
If `True`, the Python runtime installation is read-write. This allows the
646-
interpreter to create `.pyc` files for the standard library, but, because they are
647-
created as needed, it adversely affects Bazel's ability to cache the runtime and
648-
can result in spurious build failures.
641+
The Python runtime installation is made read only. This improves the ability for
642+
Bazel to cache it by preventing the interpreter from creating `.pyc` files for
643+
the standard library dynamically at runtime as they are loaded (this often leads
644+
to spurious cache misses or build failures).
645+
646+
However, if the user is running Bazel as root, this read-onlyness is not
647+
respected. Bazel will print a warning message when it detects that the runtime
648+
installation is writable despite being made read only (i.e. it's running with
649+
root access). If this attribute is set to `False`, Bazel will make it a hard
650+
error to run with root access instead.
649651
""",
650652
mandatory = False,
651653
),
@@ -690,16 +692,18 @@ dependencies are introduced.
690692
default = DEFAULT_RELEASE_BASE_URL,
691693
),
692< 10000 code>694
"ignore_root_user_error": attr.bool(
693-
default = False,
695+
default = True,
694696
doc = """\
695-
If `False`, the Python runtime installation will be made read only. This improves
696-
the ability for Bazel to cache it, but prevents the interpreter from creating
697-
`.pyc` files for the standard library dynamically at runtime as they are loaded.
698-
699-
If `True`, the Python runtime installation is read-write. This allows the
700-
interpreter to create `.pyc` files for the standard library, but, because they are
701-
created as needed, it adversely affects Bazel's ability to cache the runtime and
702-
can result in spurious build failures.
697+
The Python runtime installation is made read only. This improves the ability for
698+
Bazel to cache it by preventing the interpreter from creating `.pyc` files for
699+
the standard library dynamically at runtime as they are loaded (this often leads
700+
to spurious cache misses or build failures).
701+
702+
However, if the user is running Bazel as root, this read-onlyness is not
703+
respected. Bazel will print a warning message when it detects that the runtime
704+
installation is writable despite being made read only (i.e. it's running with
705+
root access). If this attribute is set to `False`, Bazel will make it a hard
706+
error to run with root access instead.
703707
""",
704708
mandatory = False,
705709
),

‎python/private/python_repository.bzl

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -127,37 +127,35 @@ def _python_repository_impl(rctx):
127127
# pycs being generated at runtime:
128128
# * The pycs are not deterministic (they contain timestamps)
129129
# * Multiple processes trying to write the same pycs can result in errors.
130-
if not rctx.attr.ignore_root_user_error:
131-
if "windows" not in platform:
132-
lib_dir = "lib" if "windows" not in platform else "Lib"
130+
if "windows" not in platform:
131+
repo_utils.execute_checked(
132+
rctx,
133+
op = "python_repository.MakeReadOnly",
134+
arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", lib_dir],
135+
logger = logger,
136+
)
133137

134-
repo_utils.execute_checked(
135-
rctx,
136-
op = "python_repository.MakeReadOnly",
137-
arguments = [repo_utils.which_checked(rctx, "chmod"), "-R", "ugo-w", lib_dir],
138-
logger = logger,
139-
)
140-
exec_result = repo_utils.execute_unchecked(
138+
fail_or_warn = print if rctx.attr.ignore_root_user_error else fail
139+
exec_result = repo_utils.execute_unchecked(
140+
rctx,
141+
op = "python_repository.TestReadOnly",
142+
arguments = [repo_utils.which_checked(rctx, "touch"), "lib/.test".format(lib_dir)],
143+
logger = logger,
144+
)
145+
# The issue with running as root is the installation is no longer
146+
# read-only, so the problems due to pyc can resurface.
147+
if exec_result.return_code == 0:
148+
stdout = repo_utils.execute_checked_stdout(
141149
rctx,
142-
op = "python_repository.TestReadOnly",
143-
arguments = [repo_utils.which_checked(rctx, "touch"), "{}/.test".format(lib_dir)],
150+
op = "python_repository.GetUserId",
151+
arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
144152
logger = logger,
145153
)
146-
147-
# The issue with running as root is the installation is no longer
148-
# read-only, so the problems due to pyc can resurface.
149-
if exec_result.return_code == 0:
150-
stdout = repo_utils.execute_checked_stdout(
151-
rctx,
152-
op = "python_repository.GetUserId",
153-
arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
154-
logger = logger,
155-
)
156-
uid = int(stdout.strip())
157-
if uid == 0:
158-
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.")
159-
else:
160-
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.")
154+
uid = int(stdout.strip())
155+
if uid == 0:
156+
fail_or_warn("The current user is root, which can cause 10000 spurious cache misses or build failures with the hermetic Python interpreter. See https://github.com/bazelbuild/rules_python/pull/713.")
157+
else:
158+
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.")
161159

162160
python_bin = "python.exe" if ("windows" in platform) else "bin/python3"
163161

@@ -294,7 +292,7 @@ For more information see {attr}`py_runtime.coverage_tool`.
294292
mandatory = False,
295293
),
296294
"ignore_root_user_error": attr.bool(
297-
default = False,
295+
default = True,
298296
doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.",
299297
mandatory = False,
300298
),

‎tests/python/python_tests.bzl

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ def _test_default(env):
139139
"ignore_root_user_error",
140140
"tool_versions",
141141
])
142-
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False)
142+
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True)
143143
env.expect.that_str(py.default_python_version).equals("3.11")
144144

145145
want_toolchain = struct(
@@ -212,13 +212,13 @@ def _test_default_non_rules_python_ignore_root_user_error(env):
212212
module_ctx = _mock_mctx(
213213
_mod(
214214
name = "my_module",
215-
toolchain = [_toolchain("3.12", ignore_root_user_error = True)],
215+
toolchain = [_toolchain("3.12", ignore_root_user_error = False)],
216216
),
217217
_mod(name = "rules_python", toolchain = [_toolchain("3.11")]),
218218
),
219219
)
220220

221-
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True)
221+
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False)
222222
env.expect.that_str(py.default_python_version).equals("3.12")
223223

224224
my_module_toolchain = struct(
@@ -244,13 +244,13 @@ def _test_default_non_rules_python_ignore_root_user_error_override(env):
244244
_mod(
245245
name = "my_module",
246246
toolchain = [_toolchain("3.12")],
247-
override = [_override(ignore_root_user_error = True)],
247+
override = [_override(ignore_root_user_error = False)],
248248
),
249249
_mod(name = "rules_python", toolchain = [_toolchain("3.11")]),
250250
),
251251
)
252252

253-
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True)
253+
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False)
254254
env.expect.that_str(py.default_python_version).equals("3.12")
255255

256256
my_module_toolchain = struct(
@@ -274,13 +274,13 @@ def _test_default_non_rules_python_ignore_root_user_error_non_root_module(env):
274274
py = parse_modules(
275275
module_ctx = _mock_mctx(
276276
_mod(name = "my_module", toolchain = [_toolchain("3.13")]),
277-
_mod(name = "some_module", toolchain = [_toolchain("3.12", ignore_root_user_error = True)]),
277+
_mod(name = "some_module", toolchain = [_toolchain("3.12", ignore_root_user_error = False)]),
278278
_mod(name = "rules_python", toolchain = [_toolchain("3.11")]),
279279
),
280280
)
281281

282282
env.expect.that_str(py.default_python_version).equals("3.13")
283-
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(False)
283+
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True)
284284

285285
my_module_toolchain = struct(
286286
name = "python_3_13",

0 commit comments

Comments
 (0)
0