-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
Unvendor grpc/protobuf #90
Comments
Noticed the following the logs (on main, passing builds, https://github.com/conda-forge/ray-packages-feedstock/runs/10863439621). Pretty interesting stuff!
|
Ahh, hang on, that is the warning that is failing the aarch64 builds in #92. So it was there all the time and the difference is a |
Do we have any bazel experts around who could either remove the build altogether or figure out how to ignore that error? |
You mean in conda-forge or upstream? We will try to adapt this build to make it work (with bazel). We have a specific toolchain that we likely have to use https://github.com/conda-forge/bazel-toolchain-feedstock (an example of using this toolchain successfully is jaxlib, and the tensorflow build relies on a similarly modified toolchain) |
Do you have specific needs, @mattip? I am planning to attempt fixing this in the coming weeks, but I can also try make some effort sooner |
Our bazel expert is @xhochy who may not be free these days (we miss you if you see this!) |
It seems tensorflow has a whole scheme to allow using system libraries. Is this build deps the parallel in ray? How would that look for a local grpc? |
We use this sort of thing in jaxlib: https://github.com/conda-forge/jaxlib-feedstock/blob/77c8ef863a48afae4654c4adc5962232f807cf8e/recipe/build.sh#L70 We also tend to edit bazelrc files like this: https://github.com/conda-forge/jaxlib-feedstock/blob/77c8ef863a48afae4654c4adc5962232f807cf8e/recipe/build.sh#L14-L26 Another good example is the tensorflow build: https://github.com/conda-forge/tensorflow-feedstock/blob/main/recipe/build.sh |
The main issue for me is whether or not we will have to do a lot of deep patching to get this to work. I am not that familiar with the build setup of ray yet |
That passes |
Is anyone still working on this? Having such an old version pinned here is starting to cause some problems for us. |
Not that I'm aware of. Please feel free to have a go and tag people in this issue so that we can keep track and help if we could |
Ray 2.4.0 pins to <1.49 like upstream ray on darwin. Would changing to exactly the upstream pinning ( |
Yes, this would help a lot actually. |
Good news: dealing with external deps in bazel might finallyyyyyyy be getting easier: conda-forge/tensorflow-feedstock#332 |
It requires bazel 6, which does not seem to work. See ray-project/ray#31504 |
Yeah, but compatibility with modern bazel is mostly just a question of time. The important update here IMO is the new capabilities that'll allow to finally improve the (un)vendoring situation here. |
@mattip is there anything preventing an update of ray to 2.9.0. that should bring grpcio version to 1.59 EDIT: we should guard at <1.59 not pin it. |
I found this article about using native libaries in bazel builds. It seems we could add some patches to replace the grpc and protobuf builds with the conda-provided ones? |
I think we could try to use these from conda: |
The biggest problem with this is how hard it is (for me at least) to tell bazel to use "foreign" libraries.
Originally posted by @h-vetinari in #87 (comment)
The text was updated successfully, but these errors were encountered: