setting ignore_root_user_error to True in root module is not enough to avoid the root user error (bzlmod)

[ziad-fernride]

🐞 bug report

Affected Rule

The issue is caused by the rule: rule_python, python extension.

Description

Even when setting ignore_root_user_error = True in my root module, I still get the:
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.") when using a submodule that does not have the ignore_root_user_error attr set to True.

Shouldn't it be such that the root module decides that for the entire dep graph ? Similar to the is_default attribute ?

Rules_python version:
0.27.1

[shabanzd]

If we agree that the ignore_root_user_error attr should be picked up from the root module only, I am happy to draft a PR for the change.

[aignas]

I am +1 for respecting this attribute only in the root module and emitting a warning it is used by non-root modules. @rickeylev, what do you think?

[ziad-fernride]

rebased the PR onto @rickeylev 's latest changes ... The PR needs a maintainer approval to trigger the workflow.

[rickeylev]

Yeah, 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. We should just silently ignore it, as we do with is_default.

Shouldn't it be such that the root module decides that for the entire dep graph ? Similar to the is_default attribute ?

Yeah, that makes sense, I think. So, more precisely: only the root module and rules_python module can set it, and the root module gets precedence. (just like is_default).

As you mentioned in the PR, the root module could have a mixture of True/False values. But I don't know why one would do that in practice? Or if that would even work?

(as an aside, this seems like another case where a python.root_config() tag class would help; a similar issue comes up with controlling what python version is the default version)

(And just to clarify, it's not that the root module is deciding the value for the whole dep graph. It's more that a breadth-first traversal is occurring, and the first found wins; others are ignored.)

[ziad-fernride]

(as an aside, this seems like another case where a python.root_config() tag class would help; a similar issue comes up with controlling what python version is the default version)

Agreed !

But I don't know why one would do that in practice? Or if that would even work?

For now, I added a logic to raise an error in case of inconsistent ignore_root_user_error attributes. The PR is ready for a second round of reviews I think 😊

Please let me know if I'm missing something. Thanks @aignas and @rickeylev 🙏