Skip to content

Commit c7aa989

Browse files
Wyveraldrickeylev
Wyverald
and
rickeylev
authored
fix: Downgrade "running as root" error to a warning by default (bazel-contrib#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 bazel-contrib#713, bazel-contrib#749, bazel-contrib#907, bazel-contrib#1008, bazel-contrib#1169, etc. Fixes bazel-contrib#1169. --------- Co-authored-by: Richard Levasseur <rlevasseur@google.com>
1 parent bb6249b commit c7aa989

File tree

4 files changed

+56
-92
lines changed

4 files changed

+56
-92
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ Unreleased changes template.
5959
* (pypi) The `ppc64le` is now pointing to the right target in the `platforms` package.
6060
* (gazelle) No longer incorrectly merge `py_binary` targets during partial updates in
6161
`file` generation mode. Fixed in [#2619](https://github.com/bazelbuild/rules_python/pull/2619).
62+
* (bzlmod) Running as root is no longer an error. `ignore_root_user_error=True`
63+
is now the default. Note that running as root may still cause spurious
64+
Bazel cache invalidation
65+
([#1169](https://github.com/bazelbuild/rules_python/issues/1169)).
6266

6367
{#v0-0-0-added}
6468
### Added

‎python/private/python.bzl

Lines changed: 16 additions & 23 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,17 +692,8 @@ dependencies are introduced.
690692
default = DEFAULT_RELEASE_BASE_URL,
691693
),
692694
"ignore_root_user_error": attr.bool(
693-
default = False,
694-
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.
703-
""",
695+
default = True,
696+
doc = """Deprecated; do not use. This attribute has no effect.""",
704697
mandatory = False,
705698
),
706699
"minor_mapping": attr.string_dict(

‎python/private/python_repository.bzl

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -127,37 +127,36 @@ 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"],
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 = logger.warn if rctx.attr.ignore_root_user_error else logger.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"],
143+
logger = logger,
144+
)
145+
146+
# The issue with running as root is the installation is no longer
147+
# read-only, so the problems due to pyc can resurface.
148+
if exec_result.return_code == 0:
149+
stdout = repo_utils.execute_checked_stdout(
141150
rctx,
142-
op = "python_repository.TestReadOnly",
143-
arguments = [repo_utils.which_checked(rctx, "touch"), "{}/.test".format(lib_dir)],
151+
op = "python_repository.GetUserId",
152+
arguments = [repo_utils.which_checked(rctx, "id"), "-u"],
144153
logger = logger,
145154
)
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.")
155+
uid = int(stdout.strip())
156+
if uid == 0:
157+
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.")
158+
else:
159+
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.")
161160

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

@@ -294,7 +293,7 @@ For more information see {attr}`py_runtime.coverage_tool`.
294293
mandatory = False,
295294
),
296295
"ignore_root_user_error": attr.bool(
297-
default = False,
296+
default = True,
298297
doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.",
299298
mandatory = False,
300299
),

‎tests/python/python_tests.bzl

Lines changed: 9 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def _override(
6262
auth_patterns = {},
6363
available_python_versions = [],
6464
base_url = "",
65-
ignore_root_user_error = False,
65+
ignore_root_user_error = True,
6666
minor_mapping = {},
6767
netrc = "",
6868
register_all_versions = False):
@@ -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(
@@ -238,49 +238,17 @@ def _test_default_non_rules_python_ignore_root_user_error(env):
238238

239239
_tests.append(_test_default_non_rules_python_ignore_root_user_error)
240240

241-
def _test_default_non_rules_python_ignore_root_user_error_override(env):
242-
py = parse_modules(
243-
module_ctx = _mock_mctx(
244-
_mod(
245-
name = "my_module",
246-
toolchain = [_toolchain("3.12")],
247-
override = [_override(ignore_root_user_error = True)],
248-
),
249-
_mod(name = "rules_python", toolchain = [_toolchain("3.11")]),
250-
),
251-
)
252-
253-
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True)
254-
env.expect.that_str(py.default_python_version).equals("3.12")
255-
256-
my_module_toolchain = struct(
257-
name = "python_3_12",
258-
python_version = "3.12",
259-
register_coverage_tool = False,
260-
)
261-
rules_python_toolchain = struct(
262-
name = "python_3_11",
263-
python_version = "3.11",
264-
register_coverage_tool = False,
265-
)
266-
env.expect.that_collection(py.toolchains).contains_exactly([
267-
rules_python_toolchain,
268-
my_module_toolchain,
269-
]).in_order()
270-
271-
_tests.append(_test_default_non_rules_python_ignore_root_user_error_override)
272-
273241
def _test_default_non_rules_python_ignore_root_user_error_non_root_module(env):
274242
py = parse_modules(
275243
module_ctx = _mock_mctx(
276244
_mod(name = "my_module", toolchain = [_toolchain("3.13")]),
277-
_mod(name = "some_module", toolchain = [_toolchain("3.12", ignore_root_user_error = True)]),
245+
_mod(name = "some_module", toolchain = [_toolchain("3.12", ignore_root_user_error = False)]),
278246
_mod(name = "rules_python", toolchain = [_toolchain("3.11")]),
279247
),
280248
)
281249

282250
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)
251+
env.expect.that_bool(py.config.default["ignore_root_user_error"]).equals(True)
284252

285253
my_module_toolchain = struct(
286254
name = "python_3_13",
@@ -338,8 +306,8 @@ def _test_first_occurance_of_the_toolchain_wins(env):
338306

339307
env.expect.that_dict(py.debug_info).contains_exactly({
340308
"toolchains_registered": [
341-
{"ignore_root_user_error": False, "module": {"is_root": True, "name": "my_module"}, "name": "python_3_12"},
342-
{"ignore_root_user_error": False, "module": {"is_root": False, "name": "rules_python"}, "name": "python_3_11"},
309+
{"ignore_root_user_error": True, "module": {"is_root": True, "name": "my_module"}, "name": "python_3_12"},
310+
{"ignore_root_user_error": True, "module": {"is_root": False, "name": "rules_python"}, "name": "python_3_11"},
343311
],
344312
})
345313

@@ -364,7 +332,7 @@ def _test_auth_overrides(env):
364332

365333
env.expect.that_dict(py.config.default).contains_at_least({
366334
"auth_patterns": {"foo": "bar"},
367-
"ignore_root_user_error": False,
335+
"ignore_root_user_error": True,
368336
"netrc": "/my/netrc",
369337
})
370338
env.expect.that_str(py.default_python_version).equals("3.12")

0 commit comments

Comments
 (0)