diff --git a/CHANGELOG.md b/CHANGELOG.md index 7248e1f9f2..81ecc5f6ce 100644 --- a/CHANGELOG.md +++ b/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} diff --git a/python/private/python_bootstrap_template.txt b/python/private/python_bootstrap_template.txt index a979fd4422..f5ff469610 100644 --- a/python/private/python_bootstrap_template.txt +++ b/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): diff --git a/python/runfiles/runfiles.py b/python/runfiles/runfiles.py index 3943be5646..e9ec2f1e87 100644 --- a/python/runfiles/runfiles.py +++ b/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 diff --git a/tests/runfiles/runfiles_test.py b/tests/runfiles/runfiles_test.py index a3837ac842..efe00940fc 100644 --- a/tests/runfiles/runfiles_test.py +++ b/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)