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

patch a pip_install #553

Closed
knzivid opened this issue Oct 20, 2021 · 20 comments
Closed

patch a pip_install #553

knzivid opened this issue Oct 20, 2021 · 20 comments

Comments

@knzivid
Copy link

knzivid commented Oct 20, 2021

🚀 feature request

Relevant Rules

pip_install

Description

I want to patch a python package after it is installed.

Describe the solution you'd like

Similar to patches argument in http_archive, I would like an extra argument in pip_install.

Describe alternatives you've considered

  • extra_pip_args argument - I did not find a way to request pip to patch a package
  • Forking upstream with my tiny patch - I am open to workarounds but this is my last resort

Potential problems

  • pyc files will be invalidated
@hrfuller
Copy link
Contributor

Hey @knzivid Have you considered writing a small shim library which consumes the pip requirement you need and depending on that in your repository instead?

@knzivid
Copy link
Author

knzivid commented Oct 21, 2021

The python file I intend to patch is not a part of the public interface. By a shim, do you mean a wrapper that calls this python module, but with my customizations?

@thundergolfer
Copy link

Yes, I think Henry means basically that. I think wouldn't work well though if you wish that all packages in your target's transitive closer see the customization, as they of course wouldn't import the wrapper.

@UebelAndre
Copy link
Contributor

UebelAndre commented Jan 21, 2022

I've been able to modify the package_annotation functionality (from #589) to add some patching support to whl_library but the patching applies after the wheel is installed. I think it'd be ideal to patch before but I'm not sure how to go about setting up an extracted wheel. Any suggestions or opinions there?

edit:
The reason it'd be nice to patch a wheel before installing it would be to modify setup.py and avoid having python even attempt to do things like compile some C++ source.

@philsc
Copy link
Contributor

philsc commented Oct 31, 2022

@UebelAndre , do you have a PR for your wheel patches via annotations? If not, I have an internal patch that I can clean up and put up a PR. My internal patch also works with annotations post build. I don't know how one would implement pre-build patches because it would require intercepting pip. Also, we don't have any use cases for it at the moment.

@groodt
Copy link
Collaborator

groodt commented Nov 2, 2022

I prefer a user-space solution, similar to this in the JVM ecosystem:
https://github.com/johnynek/bazel_jar_jar

I believe it's entirely reasonable to want to patch wheels (and possibly sdist), but Im not really a massive fan of the present "annotation" approach. Is there a way to create this functionality without using the annotations?

@UebelAndre
Copy link
Contributor

I prefer a user-space solution, similar to this in the JVM ecosystem: https://github.com/johnynek/bazel_jar_jar

I believe it's entirely reasonable to want to patch wheels (and possibly sdist), but Im not really a massive fan of the present "annotation" approach. Is there a way to create this functionality without using the annotations?

Can you expand on what you don't like about the "annotation" approach and what "bazel_jar_jar" does that you prefer?

@UebelAndre
Copy link
Contributor

@UebelAndre , do you have a PR for your wheel patches via annotations? I

I do not, I was able to use the new download_only feature with custom built wheels to remove the need for patching.

@creste
Copy link

creste commented Dec 5, 2022

I have an internal patch that I can clean up and put up a PR. My internal patch also works with annotations post build.

@philsc - That would be very helpful for me. I am also interested in patching python packages after they are built.

@philsc
Copy link
Contributor

philsc commented Dec 20, 2022

@creste , apologies for the delay. Here are the "non-cleaned-up" patches: https://github.com/frc971/971-Robot-Code/tree/master/third_party/rules_python
These patches (specifically, the second one) allow you to create a file like this: https://github.com/frc971/971-Robot-Code/blob/master/tools/python/patches.json
There you can specify the patches you care about.
You can point the WORKSPACE at the patches.json file like this: https://github.com/frc971/971-Robot-Code/blob/3eaca7e070c0b4a6909eaf6eec14ac26e5364dce/WORKSPACE#L441
(EDIT: I believe I set it up so that the values in patches.json have to be normalized: name.lower().replace("-","_").replace(".","_").)

It's not ideal, but this works for us at the moment.

I ended up abandoning the annotations approach because @groodt convinced me to avoid adding more annotations features. Instead, I am working on a proposal for a different approach. I am finding less time for that proposal than I was hoping so I don't have anything to show you, unfortunately.

@creste
Copy link

creste commented Dec 27, 2022

@philsc - Thank you! That is extremely helpful!

@Topher-the-Geek
Copy link

The patches above are for python_rules v0.16.1. There's been significant changes to the wheel installer since then. I think there's an easier way to go about this, especially if the source for the package is open on GitHub.

  1. Fork the source of the package
  2. Make your changes
  3. In requirements.txt, write something like git+https://github.com/<you or your org>/<your repo>@<your tag or hash>

You're probably going to do the first 2 steps anyways to submit a PR to the source.

@kersson
Copy link

kersson commented Aug 16, 2023

For anyone interested, I adapted @philsc's patch to the latest rules_python release, v0.24.0.

patch_wheels.patch

Which allows you to do this:

http_archive(
    name = "rules_python",
    patch_args = ["-p1"],
    patches = ["//path/to:patch_wheels.patch"],
    sha256 = "0a8003b044294d7840ac7d9d73eef05d6ceb682d7516781a4ec62eeb34702578",
    strip_prefix = "rules_python-0.24.0",
    url = "https://github.com/bazelbuild/rules_python/releases/download/0.24.0/rules_python-0.24.0.tar.gz",
)

pip_parse(
    ...
    # Assumes `-p1` for all patch files.
    patches = {
        "foo-pkg": [
            "//path/to/some:patch",
            "//path/to/some/other:patch",
        ],
    },
)

tingilee pushed a commit to tingilee/rules_python that referenced this issue Sep 29, 2023
@lazcamus
Copy link
Contributor

forward ported patch: patch_wheels_0.26.0.patch

@aignas
Copy link
Collaborator

aignas commented Nov 20, 2023

Support of wheel patching should work on bzlmod now with 0.27.0: bazelbuild/bazel-central-registry#1138

@aignas
Copy link
Collaborator

aignas commented Feb 5, 2024

FYI, for legacy WORKSPACE users it is technically possible to pass whl_patches argument to install_deps() call in your WORKSPACE. The API is not something we want to make a promise to maintain backwards compatibility for, hence the documentation that it is internal use only. As we evolve the APIs to better support patching and cross-platform dependency resolution, this may change, so please be warned, but at least this can give a stop-gap solution without requiring users to patch rules_python itself.

The source code documenting on what you would need to pass is https://github.com/bazelbuild/rules_python/blob/main/python/pip_install/pip_repository.bzl#L847.

@aignas
Copy link
Collaborator

aignas commented May 13, 2024

Since bzlmod is becoming the standard, I'll close this issue since the patching support is here.

@aignas aignas closed this as completed May 13, 2024
@nikonikolov
Copy link

@aignas I am not a bazel pro and I am struggling to setup a patch in the legacy WORKSPACE structure. Could you provide an example? Last I tried, I remember rules_python didn't yet fully work with bzlmod so I couldn't migrate yet.

@aignas
Copy link
Collaborator

aignas commented Sep 7, 2024

Whilst some bzlmod APIs may still change nearing 1.0, it has greatly matured, so feel free to give it a try. If you are having issues, please create new issues.

@nikonikolov
Copy link

That's good to hear, I will give it a try as soon as I can, but transition will definitely take non-trivial time and I am in a big rush to get some other things done, so an example would be really helpful so I can do a quick fix and move on. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests