Skip to content

Commit 33fd322

Browse files
authored
fix(venv): group venv prefixes by path component, not raw path (#3333)
When files overlap between packages, and dist-info directories are present, it results in a prefix list like `foo foo-bar foo/bar`. When sorted as raw strings, hyphen sorts before slash, so the continuity of path prefixes is violated and they are grouped separately. An error then occurs because both `foo/` and `foo/bar` are created, but the latter is a sub-path of the former. To fix, change the sort key to a tuple of path components. This makes `foo foo-bar foo/bar` sort as `(foo,) (foo, bar), (foo-bar, )`, resulting in the correct order. Fixes #3204
1 parent ec1df01 commit 33fd322

File tree

2 files changed

+5
-1
lines changed

2 files changed

+5
-1
lines changed

python/private/venv_runfiles.bzl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ def _group_venv_path_entries(entries):
138138
"""
139139

140140
# Sort so order is top-down, ensuring grouping by short common prefix
141-
entries = sorted(entries, key = lambda e: e.venv_path)
141+
# Split it into path components so `foo foo-bar foo/bar` sorts as
142+
# `foo foo/bar foo-bar`
143+
entries = sorted(entries, key = lambda e: tuple(e.venv_path.split("/")))
142144

143145
groups = []
144146
current_group = None

tests/venv_site_packages_libs/app_files_building/app_files_building_tests.bzl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ _tests.append(_test_conflict_merging)
6161
def _test_conflict_merging_impl(env, _):
6262
entries = [
6363
_entry("a", "+pypi_a/site-packages/a", ["a.txt"]),
64+
_entry("a-1.0.dist-info", "+pypi_a/site-packages/a-1.0.dist-info", ["METADATA"]),
6465
_entry("a/b", "+pypi_a_b/site-packages/a/b", ["b.txt"]),
6566
_entry("x", "_main/src/x", ["x.txt"]),
6667
_entry("x/p", "_main/src-dev/x/p", ["p.txt"]),
@@ -72,6 +73,7 @@ def _test_conflict_merging_impl(env, _):
7273

7374
actual = build_link_map(_ctx(), entries)
7475
expected_libs = {
76+
"a-1.0.dist-info": "+pypi_a/site-packages/a-1.0.dist-info",
7577
"a/a.txt": _file("../+pypi_a/site-packages/a/a.txt"),
7678
"a/b/b.txt": _file("../+pypi_a_b/site-packages/a/b/b.txt"),
7779
"duplicate/d.py": _file("../+dupe_a/site-packages/duplicate/d.py"),

0 commit comments

Comments
 (0)