Skip to content

Commit

Permalink
fix: dependency resolver on windows only works when --enable_runfiles…
Browse files Browse the repository at this point in the history
… and --windows_enable_symlinks is used (#2457)

`compile_pip_requirements` doesn't work as expected on Windows unless
both `--enable_runfiles` and `--windows_enable_symlinks` are used. Both
options default to off on Windows because the filesystem on Windows
makes setting up the runfiles directories with actual files very slow.
This means that anyone on Windows with a default set up has to search
around the Github issues to try and figure out why things don't work as
advertised.

The `dependency_resolver.py` doesn't inherently require these options,
it just had two bugs that prevented it from working.

1. calling pip_compile exits the whole program so it never gets to run
the code that should copy the output to the source tree. Things just
happen to work on linux because the runfiles are symlinks, and it does
not need to copy anything.
2. it assumed the `runfiles` resolved file would be in the runfiles
tree, but on Windows, when `--enable_runfiles` is not set, it actually
gets resolved to a file in the source tree.

Before:
```sh
bazel run //third_party/python:requirements.update
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 8aa3e832-78ce-4999-912b-c43e7ca3212b
INFO: Analyzed target //third_party/python:requirements.update (129 packages loaded, 9563 targets configured).
INFO: Found 1 target...
Target //third_party/python:requirements.update up-to-date:
  bazel-bin/third_party/python/requirements.update.zip
  bazel-bin/third_party/python/requirements.update.exe
INFO: Elapsed time: 60.964s, Critical Path: 0.77s
INFO: 8 processes: 2 remote cache hit, 6 internal.
INFO: Build completed successfully, 8 total actions
INFO: Running command line: bazel-bin/third_party/python/requirements.update.exe '--src=_main/third_party/python/requirements.txt' _main/third_party/python/requirements_lock.txt //third_party/python:requirements.update '--resolver=backtracking' --allow-unsafe --generate-hashes '--requirements-windows=_main/third_party/python/requirements_windows.txt' --strip-extras
Updating third_party/python/requirements_windows.txt
Error: Could not open file 'third_party/python/requirements_windows.txt': No such file or directory
```

After:
```sh
bazel run //third_party/python:requirements.update
INFO: Invocation ID: 39f999a0-6c1d-4b2c-a1be-3d71e838916a
INFO: Analyzed target //third_party/python:requirements.update (5 packages loaded, 45 targets configured).
INFO: Found 1 target...
Target //third_party/python:requirements.update up-to-date:
  bazel-bin/third_party/python/requirements.update.zip
  bazel-bin/third_party/python/requirements.update.exe
INFO: Elapsed time: 5.410s, Critical Path: 4.79s
INFO: 2 processes: 1 internal, 1 local.
INFO: Build completed successfully, 2 total actions
INFO: Running command line: bazel-bin/third_party/python/requirements.update.exe '--src=_main/third_party/python/requirements.txt' _main/third_party/python/requirements_lock.txt //third_party/python:requirements.update '--resolver=backtracking' --allow-unsafe --generate-hashes '--requirements-windows=_main/third_party/python/requirements_windows.txt' --strip-extras
Updating third_party/python/requirements_windows.txt
#
# This file is autogenerated by pip-compile with Python 3.13
# by the following command:
#
#    bazel run //third_party/python:requirements.update
#
mpmath==1.3.0 \
    --hash=sha256:7a28eb2a9774d00c7bc92411c19a89209d5da7c4c9a9e227be8330a23a25b91f \
    --hash=sha256:a0b2b9fe80bbcd81a6647ff13108738cfb482d481d826cc0e02f5b35e5c88d2c
    # via sympy
sympy==1.13.3 \
    --hash=sha256:54612cf55a62755ee71824ce692986f23c88ffa77207b30c1368eda4a7060f73 \
    --hash=sha256:b27fd2c6530e0ab39e275fc9b683895367e51d5da91baa8d3d64db2565fec4d9
    # via -r G:/projects/bedrock-engine/third_party/python/requirements.txt
```
And `//third_part/python:requirements_windows.txt` is updated.

Fixes #1943 
Fixes #1431

---------

Co-authored-by: Ignas Anikevicius <[email protected]>
  • Loading branch information
ChewyGumball and aignas authored Dec 1, 2024
1 parent d5a595c commit 084865d
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ Other changes:
([2169](https://github.com/bazelbuild/rules_python/issues/2169)).
* (workspace) Corrected protobuf's name to com_google_protobuf, the name is
hardcoded in Bazel, WORKSPACE mode.
* (pypi): {bzl:obj}`compile_pip_requirements` no longer fails on Windows when `--enable_runfiles` is not enabled.
* (pypi): {bzl:obj}`compile_pip_requirements` now correctly updates files in the source tree on Windows when `--windows_enable_symlinks` is not enabled.
* (repositories): Add libs/python3.lib and pythonXY.dll to the `libpython` target
defined by a repository template. This enables stable ABI builds of Python extensions
on Windows (by defining Py_LIMITED_API).
Expand Down
13 changes: 10 additions & 3 deletions python/private/pypi/dependency_resolver/dependency_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,19 +170,26 @@ def main(

if UPDATE:
print("Updating " + requirements_file_relative)

# Make sure the output file for pip_compile exists. It won't if we are on Windows and --enable_runfiles is not set.
if not os.path.exists(requirements_file_relative):
os.makedirs(os.path.dirname(requirements_file_relative), exist_ok=True)
shutil.copy(resolved_requirements_file, requirements_file_relative)

if "BUILD_WORKSPACE_DIRECTORY" in os.environ:
workspace = os.environ["BUILD_WORKSPACE_DIRECTORY"]
requirements_file_tree = os.path.join(workspace, requirements_file_relative)
absolute_output_file = Path(requirements_file_relative).absolute()
# In most cases, requirements_file will be a symlink to the real file in the source tree.
# If symlinks are not enabled (e.g. on Windows), then requirements_file will be a copy,
# and we should copy the updated requirements back to the source tree.
if not os.path.samefile(resolved_requirements_file, requirements_file_tree):
if not absolute_output_file.samefile(requirements_file_tree):
atexit.register(
lambda: shutil.copy(
resolved_requirements_file, requirements_file_tree
absolute_output_file, requirements_file_tree
)
)
cli(argv)
cli(argv, standalone_mode = False)
requirements_file_relative_path = Path(requirements_file_relative)
content = requirements_file_relative_path.read_text()
content = content.replace(absolute_path_prefix, "")
Expand Down

0 comments on commit 084865d

Please sign in to comment.