Skip to content

Commit e8dd503

Browse files
arrdemmyrrlyn
andauthored
feat(py_venv): Replace untennable copying with symlinks (#644)
As reported by customers, the naive but correct strategy of using copies in `py_venv_*` can lead to laughable disk usage. Some clients are reporting order 10min slowdowns and order 100GiB disk usage wasted copying inputs into binaries. We need a more scalable strategy such as symlinking. Thankfully we can generate symlinks from tools driven by Bazel into a TreeArtifact so long as the symlinks aren't dangling. By carefully crafting relative symlinks we're able to produce a tree of links which is valid both at and after action time. When relocating a `.runfiles` tree containing such links (for instance into a OCI later tar) these links must be dereferenced but that Just Works. While I'm at it, refactor the venv machinery to operate in terms of strategies and combinators on strategies so that it's simpler to talk about the production-grade behavior we want which is: * `site-packages` trees in 1stparty code get relocated/linked into the venv * `bin` sibling trees in 1stparty code get relocated/patched into the venv * General trees in 1stparty code are referred to by `.pth` file entries * General trees in 3rdparty code get relocated/linked into the venv * `bin` sibling trees in 3rdparty code get relocated/patched into the venv This makes the venv builder significantly more flexible, allows for better error reporting and opens the door to more flexible error handling. Incorporates an implementation of #606, but testing is required. Should include an implementation of #635, but testing is required. ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes `py_venv_*` now use symlinks rather than hard file copies which radically reduce disk usage while improving venv building performance. ### Test plan - Covered by existing test cases - New test cases added - Manual testing; please provide instructions so we can reproduce: TODO. ### Remaining work - [x] Strip debug prints - [x] Improve collision handling - [x] Rework the command interpreter to implement the last-wins semantics - [x] Mitigate spooky dangling symlink issues - [x] Fix a regression which can cause a `site-packages/__init__.py` file to be linked - [x] Add sha256-sum based collision ignoring - [ ] Add a test covering that a `site-packages/__init__.py` file will not be linked - [ ] Add a test covering bin shebang patching - [ ] Integrate the test case from #635 - [ ] Manually test that linked venvs still work; should just be fine --------- Co-authored-by: Alexander Payne <[email protected]>
1 parent 30d82dc commit e8dd503

File tree

7 files changed

+575
-290
lines changed

7 files changed

+575
-290
lines changed

py/private/py_venv/py_venv.bzl

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def _py_venv_base_impl(ctx):
128128
"--bin-dir=" + ctx.bin_dir.path,
129129
"--collision-strategy=" + ctx.attr.package_collisions,
130130
"--venv-name=" + venv_name,
131-
"--mode=static-copy",
131+
"--mode=" + ctx.attr.mode,
132132
"--version={}.{}".format(
133133
py_toolchain.interpreter_version_info.major,
134134
py_toolchain.interpreter_version_info.minor,
@@ -276,6 +276,15 @@ A collision can occur when multiple packages providing the same file are install
276276
default = "error",
277277
values = ["error", "warning", "ignore"],
278278
),
279+
"mode": attr.string(
280+
doc = """The venv assembly mode.
281+
282+
* "static-pth": Efficient. Just use a .pth file. Ignore binaries.
283+
* "static-symlink": Efficient. Use .pth entries for firstparty and symlinks for 3rdparty. Copies and patches binaries.
284+
""",
285+
default = "static-symlink",
286+
values = ["static-pth", "static-symlink"],
287+
),
279288
"interpreter_options": attr.string_list(
280289
doc = "Additional options to pass to the Python interpreter.",
281290
default = [],

py/tests/py_venv_conflict/test_import_roots.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ def tree(dir_path: Path, prefix: str=''):
4848
print(sys.prefix)
4949

5050
import conflict
51-
print(conflict.__file__)
51+
print("conflict.__file__", conflict.__file__)
5252
assert conflict.__file__.startswith(sys.prefix)
5353

5454
import noconflict
55-
print(noconflict.__file__)
55+
print("noconflict.__file__", noconflict.__file__)
5656
assert noconflict.__file__.startswith(sys.prefix)
5757

5858
import py_venv_conflict.lib as srclib
59-
print(srclib.__file__)
59+
print("srclib.__file__", srclib.__file__)
6060
assert not srclib.__file__.startswith(sys.prefix)

py/tests/py_venv_image_layer/my_app_amd64_layers_listing.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2442,7 +2442,7 @@ files:
24422442
layer: 1
24432443
files:
24442444
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/
2445-
- -rwxr-xr-x 0 0 0 328 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_aspect.pth
2445+
- -rwxr-xr-x 0 0 0 280 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_aspect.pth
24462446
- -rwxr-xr-x 0 0 0 19 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_virtualenv.pth
24472447
- -rwxr-xr-x 0 0 0 4342 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_virtualenv.py
24482448
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/colorama-0.4.6.dist-info/

py/tests/py_venv_image_layer/my_app_arm64_layers_listing.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2423,7 +2423,7 @@ files:
24232423
layer: 1
24242424
files:
24252425
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/
2426-
- -rwxr-xr-x 0 0 0 328 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_aspect.pth
2426+
- -rwxr-xr-x 0 0 0 280 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_aspect.pth
24272427
- -rwxr-xr-x 0 0 0 19 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_virtualenv.pth
24282428
- -rwxr-xr-x 0 0 0 4342 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/_virtualenv.py
24292429
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_venv_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_venv_image_layer/.my_app_bin/lib/python3.9/site-packages/colorama-0.4.6.dist-info/

0 commit comments

Comments
 (0)