Incorrect platform-specific dependency selected from universal requirements.txt using pip.parse

[ewianda]

🐞 bug report

Affected Rule

pip.parse

Is this a regression?

Unknown — this is observed with the current behavior in the latest release.

Description

I'm using a universal requirements.txt generated with uv

I want to use platform-specific dependencies like this:

requirements.in:

optimum[onnxruntime]; sys_platform == 'darwin' and sys_platform != 'linux'
optimum[onnxruntime-gpu]; sys_platform == 'linux'

requirements.txt (generated):

optimum[onnxruntime]==1.17.1 ; python_full_version < '3.12' and platform_python_implementation == 'CPython' and sys_platform == 'darwin'
optimum[onnxruntime-gpu]==1.17.1 ; python_full_version < '3.12' and platform_python_implementation == 'CPython' and sys_platform == 'linux'

MODULE.bazel:

pip.parse(
    requirements_by_platform = {
        "//:requirements.txt": "linux_*,osx_*",
    },
)

However, when building on macOS, Bazel attempts to use optimum[onnxruntime-gpu] instead of the appropriate optimum[onnxruntime].

It seems that the logic in the following file prioritizes the wrong requirement:

🔗

rules_python/python/private/pypi/parse_requirements.bzl

Lines 114 to 126 in 032f6aa

	requirements_dict = {
	(normalize_name(entry[0]), _extract_version(entry[1])): entry
	for entry in sorted(
	parse_result.requirements,
	# Get the longest match and fallback to original WORKSPACE sorting,
	# which should get us the entry with most extras.
	#
	# FIXME @aignas 2024-05-13: The correct behaviour might be to get an
	# entry with all aggregated extras, but it is unclear if we
	# should do this now.
	key = lambda x: (len(x[1].partition("==")[0]), x),
	)
	}.values()

As a result, the build fails with:

🔥 Exception or Error

rules_python~~pip~py_deps/onnxruntime_gpu/BUILD.bazel:6:12: configurable attribute "actual" in @@rules_python~~pip~py_deps//onnxruntime_gpu:_no_matching_repository doesn't match this configuration: No matching wheel for current configuration's Python version.

The current build configuration's Python version doesn't match any of the Python
wheels available for this distribution. This distribution supports the following Python
configuration settings:
    //_config:is_cp311_cp311_manylinux_2_28_x86_64
    //_config:is_cp311_cp311_manylinux_2_31_x86_64
    //_config:is_cp311_cp311_manylinux_x86_64

To determine the current config, I ran:

bazel config <config_id>

🔬 Minimal Reproduction

Repro steps:

1. Use a universal requirements.in as shown above.
2. Generate requirements.txt with uv pip compile requirements.in.
3. Use pip.parse(requirements_by_platform=...) in MODULE.bazel.
4. Build on macOS.

🌍 Your Environment

Operating System:

macOS 14.x

Output of bazel version:

Bazel 7.x

Rules_python version:

1.2

Anything else relevant?

A workaround mentioned by @aignas in Slack is to split requirements into separate files for different platforms. That works, but it would be ideal if this could be handled correctly from a single universal requirements file.

Thanks!

[aignas]

So the solution is simple - swap the order of evaluation of the markers and the linked line so that we are first evaluating the markers and only then merging multiple requirements lines into one.

So instead of doing:

1. rules_python/python/private/pypi/parse_requirements.bzl

   Lines 114 to 126 in 032f6aa

   	requirements_dict = {
   	(normalize_name(entry[0]), _extract_version(entry[1])): entry
   	for entry in sorted(
   	parse_result.requirements,
   	# Get the longest match and fallback to original WORKSPACE sorting,
   	# which should get us the entry with most extras.
   	#
   	# FIXME @aignas 2024-05-13: The correct behaviour might be to get an
   	# entry with all aggregated extras, but it is unclear if we
   	# should do this now.
   	key = lambda x: (len(x[1].partition("==")[0]), x),
   	)
   	}.values()

2. rules_python/python/private/pypi/parse_requirements.bzl

   Line 171 in 032f6aa

   env_marker_target_platforms = evaluate_markers(ctx, reqs_with_env_markers)

We should evaluate the markers as we go. This won't be super efficient, but it will be correct. It will become much more efficient once I have time to finish #2629.

Writing my thoughts here in case someone wants to take a stab.

[aignas]

Since the marker evaluation is done entirely in starlark, this should not be too hard. Feel free submit a PR if your hands get to this faster.

[ewianda]

After spending some time on this, I started wondering why this comment is relevant

         # Get the longest match and fallback to original WORKSPACE sorting, 
         # which should get us the entry with most extras. 
         # 
         # FIXME @aignas 2024-05-13: The correct behaviour might be to get an 
         # entry with all aggregated extras, but it is unclear if we 
         # should do this now. 

I think this should be handled by the user, i.e., aggregating extras.
requirements.txt should not have two lines with different extras for the same platform

However, including the logic of selecting the longest extra makes it trickier to implement this because the extra is platform-dependent, so there is no way to select one or aggregate both. The logic now becomes select longest if the same platform or keep both if different platforms.

IMHO, the solution is just to delete the sorting.

[ewianda]

ping @aignas

[aignas]

The sorting and selecting the longest match roughly approximates how WORKSPACE would work in the past where a requirements file like:

my_dep ...hashes...
my_dep[foo] ...hashes...

would always work in a way where only the last entry survives.

When I was modifying the code, I thought that in case there was a file:

my_dep
my_dep[foo]
my_dep[bar]
my_dep[foo,bar]

We should select my_dep[foo,bar].

If I was solving this with the env markers I would first resolve the markers and then still do the sorting just to ensure that people with such requirements files do not have regressions. At least in the past such requirements files would be produced by poetry and rules_poetry used to break just because of rules_python not handling such requirements files.

[ewianda]

That is correct. I think optimum is different since the extras are completely different and not just a subset. Another case (probably hypothetical) is say

my_dep[bar]; sys_platform == 'linux'
my_dep[foo,bar]; sys_platform == 'darwin'

In which just selecting the longest will be inaccurate.

I will take a stab with this in mind.

[ewianda]

@aignas I came up with #2766