Skip to content
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

multi_pip_parse should support platform specific requirement files #2653

Open
cosinequanon opened this issue Mar 8, 2025 · 2 comments
Open
Labels
help wanted P4 type: pip pip/pypi integration

Comments

@cosinequanon
Copy link

🚀 feature request

Relevant Rules

I think we should extend the functionality of multi_pip_parse to deal with platform specific requirement files better.

Description

Currently you can do this to have different lock files on different versions of python

multi_pip_parse(
    ...
    requirements_lock = {
        "3.10": "//:requirements_lock_3_10.txt",
        "3.12": "//:requirements_lock_3_12.txt",
    },
)

but the platform specific attributes take strings (and not dict<str, lable>) so you can only have a single requirements file on a specific platform

multi_pip_parse(
    ...
    requirements_linux = "//requirements:requirements_lock_3_12_linux.txt",
    requirements_lock = {
        "3.10": "//:requirements_lock_3_10.txt",
        "3.12": "//:requirements_lock_3_12.txt",
    },
)

The platform specific requirements should also allow you to specify a dict so we can express the proper requirements files on various platforms

Describe the solution you'd like

Ideally it would just look like this

multi_pip_parse(
    ...
    requirements_linux = {
        "3.10": "//:requirements_lock_3_10_linux.txt",
        "3.12": "//:requirements_lock_3_12_linux.txt",
    },
    requirements_lock = {
        "3.10": "//:requirements_lock_3_10.txt",
        "3.12": "//:requirements_lock_3_12.txt",
    },
)

Describe alternatives you've considered

I'm not really sure there are any good alternative solutions, this feels like it is just missing functionality

@aignas
Copy link
Collaborator

aignas commented Mar 8, 2025

Sorry to be very blunt here, but bzlmod does support what you are searching for and it is unlikely that multi_pip_parse is going to be updated before WORKSPACE support gets dropped entirely (which is likely to happen when bazel 9 is released).

That said, if someone decides to do this and starts contributing code with tests, I am happy to review it. :)

@aignas aignas added help wanted P4 type: pip pip/pypi integration labels Mar 8, 2025
@cosinequanon
Copy link
Author

@aignas that totally makes sense! Appreciate the response and I'm glad to hear it is supported in bzlmod as we will eventually move over to that system (hopefully soon).

In the meantime I'm not sure I will have time to submit a proper PR for this feature but if anyone else runs into this, I was able to easily add this functionality for my project with a patch file. Something like this will do the trick if you just need the linux lock files:

--- python/private/pypi/multi_pip_parse.bzl
+++ python/private/pypi/multi_pip_parse.bzl
@@ -124,7 +124,7 @@ _multi_pip_parse = repository_rule(
     },
 )

-def multi_pip_parse(name, default_version, python_versions, python_interpreter_target, requirements_lock, minor_mapping, **kwargs):
+def multi_pip_parse(name, default_version, python_versions, python_interpreter_target, requirements_lock, requirements_linux, minor_mapping, **kwargs):
     """NOT INTENDED FOR DIRECT USE!

     This is intended to be used by the multi_pip_parse implementation in the template of the
@@ -136,6 +136,7 @@ def multi_pip_parse(name, default_version, python_versions, python_interpreter_t
         python_versions: {type}`list[str]` all Python toolchain versions currently registered.
         python_interpreter_target: {type}`dict[str, Label]` a dictionary which keys are Python versions and values are resolved host interpreters.
         requirements_lock: {type}`dict[str, Label]` a dictionary which keys are Python versions and values are locked requirements files.
+        requirements_linux: {type}`dict[str, Label]` a dictionary which keys are Python versions and values are locked requirements files.
         minor_mapping: {type}`dict[str, str]` mapping between `X.Y` to `X.Y.Z` format.
         **kwargs: extra arguments passed to all wrapped pip_parse.

@@ -148,12 +149,16 @@ def multi_pip_parse(name, default_version, python_versions, python_interpreter_t
             fail("Missing python_interpreter_target for Python version %s in '%s'" % (python_version, name))
         if not python_version in requirements_lock:
             fail("Missing requirements_lock for Python version %s in '%s'" % (python_version, name))
+        if not python_version in requirements_linux:
+            fail("Missing requirements_linux for Python version %s in '%s'" % (python_version, name))

         pip_parse_name = name + "_" + python_version.replace(".", "_")
         pip_parse(
             name = pip_parse_name,
             python_interpreter_target = python_interpreter_target[python_version],
             requirements_lock = requirements_lock[python_version],
+            requirements_linux = requirements_linux[python_version],
             **kwargs
         )
         pip_parses[python_version] = pip_parse_name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted P4 type: pip pip/pypi integration
Projects
None yet
Development

No branches or pull requests

2 participants