fix: treat ignore_root_user_error either ignored or warning #2739
Conversation
| @@ -803,8 +803,8 @@ to spurious cache misses or 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. | ||
| root access) while this attribute is set `False`, however this messaging can be ignored by setting |
There was a problem hiding this comment.
Perhaps this should be flipped back to False, so the user gets the warning (like they do now with the default), and True is then a true ignore.
Another option would be to make this a tri-state in a non-breaking way, or perhaps add yet another attribute (likely not preferable).
There was a problem hiding this comment.
Maybe we can have it print a warning if the root module is what configured the toolchain?
The situation I want to avoid is warning spam -- if a downstream user is running as root (for whatever reason), and happens to depend on rules_python (unbeknowst to them, and out of their control), getting warning spam is somewhat annoying.
I do wish we understood the cause here better. I'm mostly sure that the various glob-excludes have fixed this. And there's always the alternative to generate hash-based pyc files at repo-time (when possible) to help avoid the issue further.
There was a problem hiding this comment.
The situation I want to avoid is warning spam
That's excellent to hear, and was the motivation for this diff.
We have quite a number of toolchains registered (arguably too many), and received a lot of warnings (we actually fail builds if too many new log lines are introduced). The change in semantics in #2636 made it hard to remove the warnings, which led to me patching the logger and disabling warnings (now upstreamed as #2737)
| @@ -803,8 +803,8 @@ to spurious cache misses or 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. | ||
| root access) while this attribute is set `False`, however this messaging can be ignored by setting |
There was a problem hiding this comment.
Maybe we can have it print a warning if the root module is what configured the toolchain?
The situation I want to avoid is warning spam -- if a downstream user is running as root (for whatever reason), and happens to depend on rules_python (unbeknowst to them, and out of their control), getting warning spam is somewhat annoying.
I do wish we understood the cause here better. I'm mostly sure that the various glob-excludes have fixed this. And there's always the alternative to generate hash-based pyc files at repo-time (when possible) to help avoid the issue further.
Previously #2636 changed the semantics of
ignore_root_user_errorfrom "ignore" to "warning". This is now flipped back to ignoring the issue, and will only emit a warning when the attribute is setFalse.This does also change the semantics of what #2636 did by flipping the attribute, as now there is no warning, and the user would have to explicitly set it to
False(they don't want to ignore the error) to see the warning.