fix(runfiles): correct Python runfiles path assumption by jsing-canva · Pull Request #3086 · bazel-contrib/rules_python

fix(runfiles): correct Python runfiles path assumption by jsing-canva · Pull Request #3086 · bazel-contrib/rules_python

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 bazel-contrib#3085

Merged via the queue into bazel-contrib:main with commit a506d77

4 checks passed

rdesgroppes added a commit to rdesgroppes/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11
- so assuming the main repository on Windows as well.

Removing the workaround from `CurrentRepository()`, would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py", line 13, in test_ran
    settings_path = rf.Rlocation(
        "rules_python/tests/support/current_build_settings.json"
    )
  File "C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\python\runfiles\runfiles.py", line 306, in Rlocation
    source_repo = self.CurrentRepository(frame=2)
  File "C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\python\runfiles\runfiles.py", line 404, in CurrentRepository
    raise ValueError(
    ...<3 lines>...
    )
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

rdesgroppes added a commit to rdesgroppes/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11
- so assuming the main repository on Windows as well.

Removing the workaround from `CurrentRepository()`, would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py", line 13, in test_ran
    settings_path = rf.Rlocation(
        "rules_python/tests/support/current_build_settings.json"
    )
  File "C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\python\runfiles\runfiles.py", line 306, in Rlocation
    source_repo = self.CurrentRepository(frame=2)
  File "C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\python\runfiles\runfiles.py", line 404, in CurrentRepository
    raise ValueError(
    ...<3 lines>...
    )
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

rdesgroppes added a commit to rdesgroppes/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11
- so assuming the main repository on Windows as well.

Removing the workaround from `CurrentRepository()`, would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
[...]
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

rdesgroppes added a commit to rdesgroppes/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11
- so assuming the main repository on Windows as well.

Removing the workaround from `CurrentRepository()`, would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
[...]
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

rdesgroppes added a commit to rdesgroppes/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11.
It therefore leads to assuming the main module path on Windows as well.

Removing the workaround from `CurrentRepository()` would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
[...]
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

pull bot pushed a commit to garymm/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11. It
therefore leads to assuming the main module path on Windows as well.

Removing the workaround from `CurrentRepository()` would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
[...]
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

Fixes bazel-contrib#3579.

phst added a commit to phst/rules_elisp that referenced this pull request

phst added a commit to phst/rules_elisp that referenced this pull request

phst mentioned this pull request

rickeylev pushed a commit to rickeylev/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11. It
therefore leads to assuming the main module path on Windows as well.

Removing the workaround from `CurrentRepository()` would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
[...]
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

Fixes bazel-contrib#3579.

(cherry picked from commit f78add7)

aignas pushed a commit to aignas/rules_python that referenced this pull request

With any Python version on Windows, `RUNFILES_*` environment variables
still point to an intermediate stage path instead of the module path,
causing `CurrentRepository()` to raise a `ValueError` since
`rules_python` v1.8.0:
```
ValueError: C:\Users\bot\AppData\Local\Temp\Bazel.runfiles_gh34ij5_\runfiles\+_repo_rules+cpython\my_script.py does not lie under the runfiles root C:/cache/ab1cdef2/execroot/_main/bazel-out/x64_windows-fastbuild/bin/external/+_repo_rules+cpython/install.exe.runfiles
```

This was not the case with `rules_python` v1.7.0. The issue stems from
bazel-contrib#3086 which, by eliminating a wrong assumption, also brought a stricter
behavior.

Since bazel-contrib#3086 came up with a corresponding workaround in
`//tests/runtime_env_toolchain:toolchain_runs_test`, the proposed fix
simply consists in moving it to `CurrentRepository()`, thus adding
another case to the workaround introduced by bazel-contrib#1634 for Python < 3.11. It
therefore leads to assuming the main module path on Windows as well.

Removing the workaround from `CurrentRepository()` would make the test
fail as follows:
```
==================== Test output for //tests/runtime_env_toolchain:toolchain_runs_test:
E
======================================================================
ERROR: test_ran (__main__.RunTest.test_ran)
----------------------------------------------------------------------
Traceback (most recent call last):
[...]
ValueError: C:\Users\user\AppData\Local\Temp\Bazel.runfiles_1f08smy6\runfiles\_main\tests\runtime_env_toolchain\toolchain_runs_test.py does not lie under the runfiles root c:\users\user\_bazel_user\cxxeswjo\execroot\_main\bazel-out\x64_windows-fastbuild-st-c530e4918e48\bin\tests\runtime_env_toolchain\toolchain_runs_test.exe.runfiles
```

Fixes bazel-contrib#3579.

phst added a commit to phst/rules_elisp that referenced this pull request