Skip to content

test: Add _test_simple_multiple_platforms_with_extras #3193

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: Add _test_simple_multiple_platforms_with_extras #3193

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
84 changes: 84 additions & 0 deletions tests/pypi/extension/extension_tests.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,90 @@ new-package==0.0.1 --hash=sha256:deadb00f2

_tests.append(_test_simple_multiple_python_versions)

def _test_simple_multiple_platforms_with_extras(env):
"""TODO(hartikainen): Test that reproduces a multi-platform-with-extras issue."""
# This test case is based on my issue where different requirement strings for the same package
# (`jax` vs `jax[cuda12]`) for multiple platforms caused a "duplicate library" error (for details,
# see https://github.com/bazel-contrib/rules_python/issues/2797#issuecomment-3143914644).
pypi = _parse_modules(
env,
module_ctx = _mock_mctx(
_mod(
name = "rules_python",
parse = [
_parse(
hub_name = "pypi",
python_version = "3.12",
download_only = True,
requirements_by_platform = {
"requirements.linux_arm64.txt": "linux_aarch64",
"requirements.linux_x86_64.txt": "linux_x86_64",
},
experimental_index_url = "pypi.org",
),
],
),
read = lambda x: {
"requirements.linux_arm64.txt": """\
package==0.7.0 \
--hash=sha256:4dd8924f171ed73a4f1a6191e2f800ae1745069989b69fabc45593d6b6504003 \
--hash=sha256:62833036cbaf4641d66ae94c61c0446890a91b2c0d153946583a0ebe04877a76
""",
"requirements.linux_x86_64.txt": """\
package[extra]==0.7.0 \
--hash=sha256:62833036cbaf4641d66ae94c61c0446890a91b2c0d153946583a0ebe04877a76
""",
}[x],
),
available_interpreters = {
"python_3_12_host": "unit_test_interpreter_target",
},
minor_mapping = {"3.12": "3.12.11"},
simpleapi_download = lambda *_, **__: {
"package": parse_simpleapi_html(
url = "https://example.com/package",
content = """
<a href="package-0.7.0.tar.gz#sha256=4dd8924f171ed73a4f1a6191e2f800ae1745069989b69fabc45593d6b6504003">package-0.7.0.tar.gz</a>
<a href="package-0.7.0-py3-none-any.whl#sha256=62833036cbaf4641d66ae94c61c0446890a91b2c0d153946583a0ebe04877a76">package-0.7.0-py3-none-any.whl</a>
""",
),
},
)

pypi.exposed_packages().contains_exactly({"pypi": ["package"]})
# TODO(hartikainen): Check these expectations.
pypi.hub_whl_map().contains_exactly({"pypi": {
"package": {
"pypi_312_package_py3_none_any_62833036": [
whl_config_setting(
# TODO(hartikainen): I think all these platforms use the same `.whl`
# and thus all three platforms should be included in the same
# `target_platforms` here?
target_platforms = ["cp312_linux_arm64", "cp312_linux_x86_64"],
version = "3.12",
),
],
},
}})
pypi.whl_libraries().contains_exactly({
"pypi_312_package_py3_none_any_62833036": {
"dep_template": "@pypi//{name}:{target}",
"download_only": True,
"experimental_target_platforms": ["linux_arm64", "linux_x86_64"],
"filename": "package-0.7.0-py3-none-any.whl",
"python_interpreter_target": "unit_test_interpreter_target",
# NOTE(hartikainen): Perhaps this is part of the problem?
# This should say `package[extra]==0.7.0` for `linux_x86_64` platform and
# `package==0.7.0` for `linux_arm64`
"requirement": "package[extra]==0.7.0",
Comment on lines +478 to +481
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, actually this is a separate issue and I am not sure how we can fix it. So, a little bit of history here:

  1. poetry will export a requirements.txt that will have both package[extra] and package and we were asked to only consider package[extra] when doing the installation.
  2. However with the advent of universal files it means that we should do something differently - first resolve the compatible lines with the target platforms and do the filtering differently.

I remember we had this discussion with in one of @ewianda PRs, but I can't remember which one it is.

I think the issue that you are facing does not have an open issue yet.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is more, we have a test that already tests for this behaviour in the case where the extras differ, but we don't have a test where extra is present in one and in another it is not.

PRs for this are welcome and we should have enough test coverage in places that the fix would be touching.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick response and context! I'll look into the existing case where extras differ to see if that can be extended to this case. Let's see where I get.

"sha256": "62833036cbaf4641d66ae94c61c0446890a91b2c0d153946583a0ebe04877a76",
"urls": ["https://example.com/package-0.7.0-py3-none-any.whl"],
},
})
pypi.whl_mods().contains_exactly({})

_tests.append(_test_simple_multiple_platforms_with_extras)

def _test_simple_with_markers(env):
pypi = _parse_modules(
env,
Expand Down