fix: ignore_root_user_error should be taken from root module only (#1662)
Only the root module should be able to decide `ignore_root_user_error`.
Modules being depended upon don't know the final environment, so they
aren't in the right position to know or decide what the correct setting
is. This PR implements a solution to take the `ignore_root_user_error`
value from the root module only.
Fixes #1658
---------
Co-authored-by: Richard Levasseur <rlevasseur@google.com>
diff --git a/.bazelrc b/.bazelrc
index 250377a..9ecb892 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -4,8 +4,8 @@
# (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it)
# To update these lines, execute
# `bazel run @rules_bazel_integration_test//tools:update_deleted_packages`
-build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered
-query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered
+build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered,tests/integration/ignore_root_user_error/submodule
+query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_points,examples/bzlmod/entry_points/tests,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/patches,examples/bzlmod/py_proto_library,examples/bzlmod/py_proto_library/example.com/another_proto,examples/bzlmod/py_proto_library/example.com/proto,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/dupe_requirements,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,examples/py_proto_library/example.com/another_proto,examples/py_proto_library/example.com/proto,tests/integration/compile_pip_requirements,tests/integration/compile_pip_requirements_test_from_external_repo,tests/integration/ignore_root_user_error,tests/integration/pip_repository_entry_points,tests/integration/py_cc_toolchain_registered,tests/integration/ignore_root_user_error/submodule
test --test_output=errors
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b861902..07b68ec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -124,6 +124,9 @@
* (toolchains) Workspace builds register the py cc toolchain (bzlmod already
was). This makes e.g. `//python/cc:current_py_cc_headers` Just Work.
([#1669](https://github.com/bazelbuild/rules_python/issues/1669))
+* (bzlmod python.toolchain) The value of `ignore_root_user_error` is now decided
+ by the root module only.
+ ([#1658](https://github.com/bazelbuild/rules_python/issues/1658))
### Added
diff --git a/python/extensions/BUILD.bazel b/python/extensions/BUILD.bazel
index 88e3984..a9dede4 100644
--- a/python/extensions/BUILD.bazel
+++ b/python/extensions/BUILD.bazel
@@ -35,5 +35,8 @@
name = "python_bzl",
srcs = ["python.bzl"],
visibility = ["//:__subpackages__"],
- deps = ["//python/private/bzlmod:python_bzl"],
+ deps = [
+ "//python/private:util_bzl",
+ "//python/private/bzlmod:python_bzl",
+ ],
)
diff --git a/python/private/bzlmod/python.bzl b/python/private/bzlmod/python.bzl
index 3b59d5b..5862f00 100644
--- a/python/private/bzlmod/python.bzl
+++ b/python/private/bzlmod/python.bzl
@@ -16,6 +16,7 @@
load("//python:repositories.bzl", "python_register_toolchains")
load("//python/private:toolchains_repo.bzl", "multi_toolchain_aliases")
+load("//python/private:util.bzl", "IS_BAZEL_6_4_OR_HIGHER")
load(":pythons_hub.bzl", "hub_repo")
# This limit can be increased essentially arbitrarily, but doing so will cause a rebuild of all
@@ -43,14 +44,14 @@
def _print_warn(msg):
print("WARNING:", msg)
-def _python_register_toolchains(name, toolchain_attr, module):
+def _python_register_toolchains(name, toolchain_attr, module, ignore_root_user_error):
"""Calls python_register_toolchains and returns a struct used to collect the toolchains.
"""
python_register_toolchains(
name = name,
python_version = toolchain_attr.python_version,
register_coverage_tool = toolchain_attr.configure_coverage_tool,
- ignore_root_user_error = toolchain_attr.ignore_root_user_error,
+ ignore_root_user_error = ignore_root_user_error,
)
return struct(
python_version = toolchain_attr.python_version,
@@ -59,6 +60,13 @@
)
def _python_impl(module_ctx):
+ if module_ctx.os.environ.get("RULES_PYTHON_BZLMOD_DEBUG", "0") == "1":
+ debug_info = {
+ "toolchains_registered": [],
+ }
+ else:
+ debug_info = None
+
# The toolchain_info structs to register, in the order to register them in.
# NOTE: The last element is special: it is treated as the default toolchain,
# so there is special handling to ensure the last entry is the correct one.
@@ -72,6 +80,13 @@
# Map of string Major.Minor to the toolchain_info struct
global_toolchain_versions = {}
+ ignore_root_user_error = None
+
+ # if the root module does not register any toolchain then the
+ # ignore_root_user_error takes its default value: False
+ if not module_ctx.modules[0].tags.toolchain:
+ ignore_root_user_error = False
+
for mod in module_ctx.modules:
module_toolchain_versions = []
@@ -84,16 +99,27 @@
_fail_duplicate_module_toolchain_version(toolchain_version, mod.name)
module_toolchain_versions.append(toolchain_version)
- # Only the root module and rules_python are allowed to specify the default
- # toolchain for a couple reasons:
- # * It prevents submodules from specifying different defaults and only
- # one of them winning.
- # * rules_python needs to set a soft default in case the root module doesn't,
- # e.g. if the root module doesn't use Python itself.
- # * The root module is allowed to override the rules_python default.
if mod.is_root:
+ # Only the root module and rules_python are allowed to specify the default
+ # toolchain for a couple reasons:
+ # * It prevents submodules from specifying different defaults and only
+ # one of them winning.
+ # * rules_python needs to set a soft default in case the root module doesn't,
+ # e.g. if the root module doesn't use Python itself.
+ # * The root module is allowed to override the rules_python default.
+
# A single toolchain is treated as the default because it's unambiguous.
is_default = toolchain_attr.is_default or len(mod.tags.toolchain) == 1
+
+ # Also only the root module should be able to decide ignore_root_user_error.
+ # Modules being depended upon don't know the final environment, so they aren't
+ # in the right position to know or decide what the correct setting is.
+
+ # If an inconsistency in the ignore_root_user_error among multiple toolchains is detected, fail.
+ if ignore_root_user_error != None and toolchain_attr.ignore_root_user_error != ignore_root_user_error:
+ fail("Toolchains in the root module must have consistent 'ignore_root_user_error' attributes")
+
+ ignore_root_user_error = toolchain_attr.ignore_root_user_error
elif mod.name == "rules_python" and not default_toolchain:
# We don't do the len() check because we want the default that rules_python
# sets to be clearly visible.
@@ -128,8 +154,14 @@
toolchain_name,
toolchain_attr,
module = mod,
+ ignore_root_user_error = ignore_root_user_error,
)
global_toolchain_versions[toolchain_version] = toolchain_info
+ if debug_info:
+ debug_info["toolchains_registered"].append({
+ "ignore_root_user_error": ignore_root_user_error,
+ "name": toolchain_name,
+ })
if is_default:
# This toolchain is setting the default, but the actual
@@ -192,6 +224,12 @@
},
)
+ if debug_info != None:
+ _debug_repo(
+ name = "rules_python_bzlmod_debug",
+ debug_info = json.encode_indent(debug_info),
+ )
+
def _fail_duplicate_module_toolchain_version(version, module):
fail(("Duplicate module toolchain version: module '{module}' attempted " +
"to use version '{version}' multiple times in itself").format(
@@ -220,6 +258,14 @@
second = second,
))
+def _get_bazel_version_specific_kwargs():
+ kwargs = {}
+
+ if IS_BAZEL_6_4_OR_HIGHER:
+ kwargs["environ"] = ["RULES_PYTHON_BZLMOD_DEBUG"]
+
+ return kwargs
+
python = module_extension(
doc = """Bzlmod extension that is used to register Python toolchains.
""",
@@ -263,7 +309,16 @@
),
"ignore_root_user_error": attr.bool(
default = False,
- doc = "Whether the check for root should be ignored or not. This causes cache misses with .pyc files.",
+ 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.
+""",
mandatory = False,
),
"is_default": attr.bool(
@@ -279,4 +334,23 @@
},
),
},
+ **_get_bazel_version_specific_kwargs()
+)
+
+_DEBUG_BUILD_CONTENT = """
+package(
+ default_visibility = ["//visibility:public"],
+)
+exports_files(["debug_info.json"])
+"""
+
+def _debug_repo_impl(repo_ctx):
+ repo_ctx.file("BUILD.bazel", _DEBUG_BUILD_CONTENT)
+ repo_ctx.file("debug_info.json", repo_ctx.attr.debug_info)
+
+_debug_repo = repository_rule(
+ implementation = _debug_repo_impl,
+ attrs = {
+ "debug_info": attr.string(),
+ },
)
diff --git a/python/private/util.bzl b/python/private/util.bzl
index 71476f9..16b8ff8 100644
--- a/python/private/util.bzl
+++ b/python/private/util.bzl
@@ -89,3 +89,16 @@
# Bazel 5.4 has a bug where every access of testing.ExecutionInfo is a
# different object that isn't equal to any other. This is fixed in bazel 6+.
IS_BAZEL_6_OR_HIGHER = testing.ExecutionInfo == testing.ExecutionInfo
+
+_marker_rule_to_detect_bazel_6_4_or_higher = rule(implementation = lambda ctx: None)
+
+# Bazel 6.4 and higher have a bug fix where rule names show up in the str()
+# of a rule. See
+# https://github.com/bazelbuild/bazel/commit/002490b9a2376f0b2ea4a37102c5e94fc50a65ba
+# https://github.com/bazelbuild/bazel/commit/443cbcb641e17f7337ccfdecdfa5e69bc16cae55
+# This technique is done instead of using native.bazel_version because,
+# under stardoc, the native.bazel_version attribute is entirely missing, which
+# prevents doc generation from being able to correctly generate docs.
+IS_BAZEL_6_4_OR_HIGHER = "_marker_rule_to_detect_bazel_6_4_or_higher" in str(
+ _marker_rule_to_detect_bazel_6_4_or_higher,
+)
diff --git a/tests/integration/ignore_root_user_error/.bazelrc b/tests/integration/ignore_root_user_error/.bazelrc
index f23315a..fde94c7 100644
--- a/tests/integration/ignore_root_user_error/.bazelrc
+++ b/tests/integration/ignore_root_user_error/.bazelrc
@@ -1,3 +1,5 @@
+common --action_env=RULES_PYTHON_BZLMOD_DEBUG=1
+common --lockfile_mode=off
test --test_output=errors
# Windows requires these for multi-python support:
diff --git a/tests/integration/ignore_root_user_error/BUILD.bazel b/tests/integration/ignore_root_user_error/BUILD.bazel
index f907624..6e3b7b9 100644
--- a/tests/integration/ignore_root_user_error/BUILD.bazel
+++ b/tests/integration/ignore_root_user_error/BUILD.bazel
@@ -1,7 +1,32 @@
-load("@rules_python//python:defs.bzl", "py_test")
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load("@rules_python//python:py_test.bzl", "py_test")
+load("@rules_python//python/private:bzlmod_enabled.bzl", "BZLMOD_ENABLED") # buildifier: disable=bzl-visibility
py_test(
name = "foo_test",
srcs = ["foo_test.py"],
visibility = ["//visibility:public"],
)
+
+py_test(
+ name = "bzlmod_test",
+ srcs = ["bzlmod_test.py"],
+ data = [
+ "@rules_python//python/runfiles",
+ "@rules_python_bzlmod_debug//:debug_info.json",
+ ],
+ target_compatible_with = [] if BZLMOD_ENABLED else ["@platforms//:incompatible"],
+)
diff --git a/tests/integration/ignore_root_user_error/MODULE.bazel b/tests/integration/ignore_root_user_error/MODULE.bazel
new file mode 100644
index 0000000..15c37c4
--- /dev/null
+++ b/tests/integration/ignore_root_user_error/MODULE.bazel
@@ -0,0 +1,20 @@
+module(name = "ignore_root_user_error")
+
+bazel_dep(name = "rules_python", version = "0.0.0")
+local_path_override(
+ module_name = "rules_python",
+ path = "../../..",
+)
+
+bazel_dep(name = "submodule")
+local_path_override(
+ module_name = "submodule",
+ path = "submodule",
+)
+
+python = use_extension("@rules_python//python/extensions:python.bzl", "python")
+python.toolchain(
+ ignore_root_user_error = True,
+ python_version = "3.11",
+)
+use_repo(python, "rules_python_bzlmod_debug")
diff --git a/tests/integration/ignore_root_user_error/bzlmod_test.py b/tests/integration/ignore_root_user_error/bzlmod_test.py
new file mode 100644
index 0000000..62bc4f4
--- /dev/null
+++ b/tests/integration/ignore_root_user_error/bzlmod_test.py
@@ -0,0 +1,37 @@
+# Copyright 2024 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import unittest
+import pathlib
+import json
+
+from python.runfiles import runfiles
+
+class BzlmodTest(unittest.TestCase):
+ def test_toolchains(self):
+ rf = runfiles.Create()
+ debug_path = pathlib.Path(rf.Rlocation(
+ "rules_python_bzlmod_debug/debug_info.json"
+ ))
+ debug_info = json.loads(debug_path.read_bytes())
+
+ expected = [
+ {'ignore_root_user_error': True, 'name': 'python_3_11', },
+ {'ignore_root_user_error': True, 'name': 'python_3_10'}
+ ]
+ self.assertCountEqual(debug_info["toolchains_registered"],
+ expected)
+
+if __name__ == "__main__":
+ unittest.main()
diff --git a/tests/integration/ignore_root_user_error/submodule/BUILD.bazel b/tests/integration/ignore_root_user_error/submodule/BUILD.bazel
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/integration/ignore_root_user_error/submodule/BUILD.bazel
diff --git a/tests/integration/ignore_root_user_error/submodule/MODULE.bazel b/tests/integration/ignore_root_user_error/submodule/MODULE.bazel
new file mode 100644
index 0000000..f128709
--- /dev/null
+++ b/tests/integration/ignore_root_user_error/submodule/MODULE.bazel
@@ -0,0 +1,9 @@
+module(name = "submodule")
+
+bazel_dep(name = "rules_python", version = "0.0.0")
+
+python = use_extension("@rules_python//python/extensions:python.bzl", "python")
+python.toolchain(
+ ignore_root_user_error = False,
+ python_version = "3.10",
+)
diff --git a/tests/integration/ignore_root_user_error/submodule/WORKSPACE b/tests/integration/ignore_root_user_error/submodule/WORKSPACE
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/integration/ignore_root_user_error/submodule/WORKSPACE
