py_proto_library prepends too aggressively to the PYTHONPATH

[tpudlik]

🐞 bug report

Affected Rule

py_proto_library

Is this a regression?

Yes, this is a regression due to #1406. (The failing provided below passes if you check out rules_python from the previous commit.)

Description

#1406 added support for generating Python code for proto libraries with strip_import_prefix attributes. This is great! Our project (http://pigweed.dev) uses such attributes (to my chargin, but they're here to stay), so I'm excited for it.

But the solution adopted in #1406 involves quite aggressive prepending to the PYTHONPATH. In particular, I find that if a generated Python proto library is added to the transitive dependencies of a target, then the repository root, and the roots of external repositories like com_google_protobuf, get added close to the top of the PYTHONPATH, ahead of paths coming from import attributes. This can lead to module name collisions at import time.

The description above is a little hard to parse, so I created a small repro (see below).

🔬 Minimal Reproduction

https://github.com/tpudlik/rules_python_path/

🔥 Exception or Error

Depending on whether you're importing a module or an object from a module, you'll hit a ModuleNotFoundError or an ImportError.

🌍 Your Environment

Operating System:

Linux

Output of bazel version:

  
Build label: 6.4.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Thu Oct 19 17:07:43 2023 (1697735263)
Build timestamp: 1697735263
Build timestamp as int: 1697735263
  

Rules_python version:

  
bee35ef2abb0a1b59123500528c7d4ca0cd8a688
  

Anything else relevant?

https://issues.pigweed.dev/issues/266950138 tracks our migration to the rules_python version of py_proto_library.

[tpudlik]

I think I have a fix: instead of unconditionally adding [proto_root] to the imports, only do so if it contains _virtual_imports (i.e., we're actually dealing with a strip_import_prefix situation). I'll send a PR.

[tpudlik]

The fix is trivial, but I had a tough time creating a regression test inside rules_python. It turns out this issue is much less likely to bite if legacy_create_init is set to False (i.e., if --incompatible_default_to_explicit_init_py is set), since fewer directories will be considered packages by Python. In rules_python this flag is enabled in .bazelrc.