fix(runfiles): correct Python runfiles path assumption

CHANGELOG.md

@@ -88,6 +88,10 @@ END_UNRELEASED_TEMPLATE
   ({gh-issue}`3043`).
 * (pypi) The pipstar `defaults` configuration now supports any custom platform
   name.
+* (runfiles) Fix incorrect Python runfiles path assumption - the existing
+  implementation assumes that it is always four levels below the runfiles
+  directory, leading to incorrect path checks
+  ([#3085](https://github.com/bazel-contrib/rules_python/issues/3085)).
 * Multi-line python imports (e.g. with escaped newlines) are now correctly processed by Gazelle.
 
 {#v0-0-0-added}

python/private/python_bootstrap_template.txt

@@ -256,6 +256,10 @@ def RunfilesEnvvar(module_space):
     'RUNFILES_MANIFEST_FILE' and var_value is the path to that directory or
     file, or (None, None) if runfiles couldn't be found.
   """
+  # If running from a zip, there's no manifest file.
+  if IsRunningFromZip():
+    return ('RUNFILES_DIR', module_space)
+
   # If this binary is the data-dependency of another one, the other sets
   # RUNFILES_MANIFEST_FILE or RUNFILES_DIR for our sake.
   runfiles = os.environ.get('RUNFILES_MANIFEST_FILE', None)
@@ -266,10 +270,6 @@ def RunfilesEnvvar(module_space):
   if runfiles:
     return ('RUNFILES_DIR', runfiles)
 
-  # If running from a zip, there's no manifest file.
-  if IsRunningFromZip():
-    return ('RUNFILES_DIR', module_space)
-
   # Look for the runfiles "output" manifest, argv[0] + ".runfiles_manifest"
   runfiles = module_space + '_manifest'
   if os.path.exists(runfiles):

python/runfiles/runfiles.py

@@ -112,6 +112,9 @@ def RlocationChecked(self, path: str) -> str:
         # runfiles strategy on those platforms.
         return posixpath.join(self._runfiles_root, path)
 
+    def _GetRunfilesDir(self) -> str:
+        return self._runfiles_root
+
     def EnvVars(self) -> Dict[str, str]:
         return {
             "RUNFILES_DIR": self._runfiles_root,
@@ -129,7 +132,7 @@ class Runfiles:
 
     def __init__(self, strategy: Union[_ManifestBased, _DirectoryBased]) -> None:
         self._strategy = strategy
-        self._python_runfiles_root = _FindPythonRunfilesRoot()
+        self._python_runfiles_root = strategy._GetRunfilesDir()
         self._repo_mapping = _ParseRepoMapping(
             strategy.RlocationChecked("_repo_mapping")
         )
@@ -347,18 +350,6 @@ def Create(env: Optional[Dict[str, str]] = None) -> Optional["Runfiles"]:
 _Runfiles = Runfiles
 
 
-def _FindPythonRunfilesRoot() -> str:
-    """Finds the root of the Python runfiles tree."""
-    root = __file__
-    # Walk up our own runfiles path to the root of the runfiles tree from which
-    # the current file is being run. This path coincides with what the Bazel
-    # Python stub sets up as sys.path[0]. Since that entry can be changed at
-    # runtime, we rederive it here.
-    for _ in range("rules_python/python/runfiles/runfiles.py".count("/") + 1):
-        root = os.path.dirname(root)
-    return root
-
-
 def _ParseRepoMapping(repo_mapping_path: Optional[str]) -> Dict[Tuple[str, str], str]:
     """Parses the repository mapping manifest."""
     # If the repository mapping file can't be found, that is not an error: We

tests/runfiles/runfiles_test.py

@@ -532,7 +532,7 @@ def testCurrentRepository(self) -> None:
             expected = ""
         else:
             expected = "rules_python"
-        r = runfiles.Create({"RUNFILES_DIR": "whatever"})
+        r = runfiles.Create()
         assert r is not None  # mypy doesn't understand the unittest api.
         self.assertEqual(r.CurrentRepository(), expected)