Skip to content

Commit a535daa

Browse files
authored
Merge branch 'main' into repo-utils-python-b
2 parents dea6493 + c7aa989 commit a535daa

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)