fix(runfiles): correct Python runfiles path assumption (#3086)
The current _FindPythonRunfilesRoot() implementation assumes that
the Python module has been unpacked four levels below the runfiles
directory. This is not the case in multiple situations, for example when
rules_pycross is in use and has installed the module via pypi (in which
case it is five levels below runfiles).
Both strategies already know where the runfiles directory exists -
implement _GetRunfilesDir() on the _DirectoryBased strategy, then call
_GetRunfilesDir() in order to populate self._python_runfiles_dir.
Stop passing a bogus path to runfiles.Create() in
testCurrentRepository(),
such that the test actually uses the appropriate runfiles path.
Fixes #3085
---------
Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index bae9823..9c6a6b6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,10 @@
### Fixed
* (gazelle) Remove {obj}`py_binary` targets with invalid `srcs`. This includes files
that are not generated or regular files.
+* (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)).
{#v0-0-0-added}
### Added
diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py
index fc79427..bfa9d0d 100644
--- a/python/runfiles/runfiles.py
+++ b/python/runfiles/runfiles.py
@@ -229,6 +229,9 @@
# 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,
@@ -246,7 +249,7 @@
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 = _RepositoryMapping.create_from_file(
strategy.RlocationChecked("_repo_mapping")
)
@@ -469,18 +472,6 @@
_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 CreateManifestBased(manifest_path: str) -> Runfiles:
return Runfiles.CreateManifestBased(manifest_path)
diff --git a/tests/runfiles/BUILD.bazel b/tests/runfiles/BUILD.bazel
index 5c92026..84602d2 100644
--- a/tests/runfiles/BUILD.bazel
+++ b/tests/runfiles/BUILD.bazel
@@ -5,6 +5,9 @@
py_test(
name = "runfiles_test",
srcs = ["runfiles_test.py"],
+ data = [
+ "//tests/support:current_build_settings",
+ ],
env = {
"BZLMOD_ENABLED": "1" if BZLMOD_ENABLED else "0",
},
diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py
index b8a3d5f..165ab8c 100644
--- a/tests/runfiles/runfiles_test.py
+++ b/tests/runfiles/runfiles_test.py
@@ -12,7 +12,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import json
import os
+import pathlib
import tempfile
import unittest
from typing import Any, List, Optional
@@ -63,6 +65,16 @@
lambda: r.Rlocation("\\foo"),
)
+ def testRlocationWithData(self) -> None:
+ r = runfiles.Create()
+ assert r is not None # mypy doesn't understand the unittest api.
+ settings_path = r.Rlocation(
+ "rules_python/tests/support/current_build_settings.json"
+ )
+ assert settings_path is not None
+ settings = json.loads(pathlib.Path(settings_path).read_text())
+ self.assertIn("bootstrap_impl", settings)
+
def testCreatesManifestBasedRunfiles(self) -> None:
with _MockFile(contents=["a/b c/d"]) as mf:
r = runfiles.Create(
@@ -692,7 +704,7 @@
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)
diff --git a/tests/runtime_env_toolchain/toolchain_runs_test.py b/tests/runtime_env_toolchain/toolchain_runs_test.py
index c66b0bb..6f0948f 100644
--- a/tests/runtime_env_toolchain/toolchain_runs_test.py
+++ b/tests/runtime_env_toolchain/toolchain_runs_test.py
@@ -10,10 +10,20 @@
class RunTest(unittest.TestCase):
def test_ran(self):
rf = runfiles.Create()
- settings_path = rf.Rlocation(
- "rules_python/tests/support/current_build_settings.json"
- )
+ try:
+ settings_path = rf.Rlocation(
+ "rules_python/tests/support/current_build_settings.json"
+ )
+ except ValueError as e:
+ # The current toolchain being used has a buggy zip file bootstrap, which
+ # leaves RUNFILES_DIR pointing at the first stage path and not the module
+ # path.
+ if platform.system() != "Windows" or "does not lie under the runfiles root" not in str(e):
+ raise e
+ settings_path = "./tests/support/current_build_settings.json"
+
settings = json.loads(pathlib.Path(settings_path).read_text())
+
if platform.system() == "Windows":
self.assertEqual(
"/_magic_pyruntime_sentinel_do_not_use", settings["interpreter_path"]
