Skip to content

fix(whl_library): stop duplicating deps in whl_library #1874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ A brief description of the categories of changes:
* (whl_library): Fix the experimental_target_platforms overriding for platform
specific wheels when the wheels are for any python interpreter version. Fixes
[#1810](https://github.com/bazelbuild/rules_python/issues/1810).
* (whl_library): Stop generating duplicate dependencies when encountering
duplicates in the METADATA. Fixes
[#1873](https://github.com/bazelbuild/rules_python/issues/1873).
* (gazelle) In `project` or `package` generation modes, do not generate `py_test`
rules when there are no test files and do not set `main = "__test__.py"` when
that file doesn't exist.
Expand Down
18 changes: 18 additions & 0 deletions python/pip_install/tools/wheel_installer/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,24 @@ def _add(self, dep: str, platform: Optional[Platform]):

if not platform:
self._deps.add(dep)

# If the dep is in the platform-specific list, remove it from the select.
pop_keys = []
for p, deps in self._select.items():
if dep not in deps:
continue

deps.remove(dep)
if not deps:
pop_keys.append(p)

for p in pop_keys:
self._select.pop(p)
return

if dep in self._deps:
# If the dep is already in the main dependency list, no need to add it in the
# platform-specific dependency list.
return

# Add the platform-specific dep
Expand Down
42 changes: 42 additions & 0 deletions python/pip_install/tools/wheel_installer/wheel_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,48 @@ def test_deps_spanning_all_target_py_versions_are_added_to_common(self):
self.assertEqual(["bar", "baz"], got.deps)
self.assertEqual({}, got.deps_select)

def test_deps_are_not_duplicated(self):
# See an example in
# https://files.pythonhosted.org/packages/76/9e/db1c2d56c04b97981c06663384f45f28950a73d9acf840c4006d60d0a1ff/opencv_python-4.9.0.80-cp37-abi3-win32.whl.metadata
requires_dist = [
"bar >=0.1.0 ; python_version < '3.7'",
"bar >=0.2.0 ; python_version >= '3.7'",
"bar >=0.4.0 ; python_version >= '3.6' and platform_system == 'Linux' and platform_machine == 'aarch64'",
"bar >=0.4.0 ; python_version >= '3.9'",
"bar >=0.5.0 ; python_version <= '3.9' and platform_system == 'Darwin' and platform_machine == 'arm64'",
"bar >=0.5.0 ; python_version >= '3.10' and platform_system == 'Darwin'",
"bar >=0.5.0 ; python_version >= '3.10'",
"bar >=0.6.0 ; python_version >= '3.11'",
]

deps = wheel.Deps(
"foo",
requires_dist=requires_dist,
platforms=wheel.Platform.from_string(["cp310_*"]),
)
got = deps.build()

self.assertEqual(["bar"], got.deps)
self.assertEqual({}, got.deps_select)

def test_deps_are_not_duplicated_when_encountering_platform_dep_first(self):
# Note, that we are sorting the incoming `requires_dist` and we need to ensure that we are not getting any
# issues even if the platform-specific line comes first.
requires_dist = [
"bar >=0.4.0 ; python_version >= '3.6' and platform_system == 'Linux' and platform_machine == 'aarch64'",
"bar >=0.5.0 ; python_version >= '3.9'",
]

deps = wheel.Deps(
"foo",
requires_dist=requires_dist,
platforms=wheel.Platform.from_string(["cp310_*"]),
)
got = deps.build()

self.assertEqual(["bar"], got.deps)
self.assertEqual({}, got.deps_select)


class MinorVersionTest(unittest.TestCase):
def test_host(self):
Expand Down
Loading