Unvendor boost #231
Unvendor boost #231
Conversation
mattip
requested review from
apmorton,
dHannasch,
h-vetinari,
timkpaine and
vnlitvinov
as code owners
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
@conda-forge-admin, please rerender |
|
fyi, I'm working on unvendoring more stuff locally; was planning to land them all at once |
|
Ahh, cool. Should I close this, or do you want to see if the robocopy works on windows |
|
lets see if robocopy works; I'll open a PR once I have stuff building on linux with the major painful libraries unvendored |
|
|
||
| echo ========================================================== | ||
| echo calling pip to install | ||
| echo ========================================================== | ||
| cd python | ||
| echo startup --output_user_root=D:/tmp >> ..\.bazelrc | ||
| echo build --jobs=1 >> ..\.bazelrc |
There was a problem hiding this comment.
Bazel is crashing while building. Maybe this is needed?
|
osx_64 is failing with a missing boost symbol |
That sounds like a stdlib symbol. Looking upstream, boost expects that symbol unconditionally. You could try setting |
|
unvendoring protobuf/grpc are shaping up to be a monumental feat that likely requires non-trivial upstream changes to land. ray transitively pulls in many bazel rules it required from grpc This is quite the mess to untangle. |
|
At some point conda-forge’s build will diverge so far from upstream we may as well use meson to build what we need |
|
Patches like this also effectively render it impossible to unvendor libraries. That patch isn't fixing a compilation bug or backporting some feature, its changing behavior. |
|
potentially hot take - should this feedstock switch to binary repackaging upstream wheels? Maintaining ray on conda-forge is getting more complicated as conda-forge moved forward with bazel, protobuf, grpc, etc and ray does not. There is fairly little upstream movement on modernization of the build stack, and we're already several grpc migrations behind with no clear path forward IMO. I'm not actually sure if binary repackaging solves the grpc version problem, but it's maybe worth a try? |
|
This to me is (unfortunately) the cost of bazel, for all it's other benefits. Well, that and patching third-party dependencies, which should be a stop-gap measure, not a long-term strategy (but of course, who has time to draw down all the tech debt...). Perhaps a minimal meson build is actually the right approach. If that works, perhaps you could even convince upstream to carry it in the repo as a not-officially-supported build orchestrator. Of course, I think that bazel really should allow local overrides without jumping through these ridiculous hoops. But try to tell them that not everyone needs or wants hermetically isolated builds... I'm not in favour of binary repackaging here, but if there's no other option and it doesn't cause worse problems overall... 🤷 |
+1, repackage and be done with the mess. Hopefully all thirdparty symbols are hidden upstream, if not that is probably easy to accomplish. For a brief few months, it was working great 😅 |
|
If we repackage we would have to run many more tests. |
The catch with this is all the behavior-changing patches ray applies. Right now they have: The grpc one is pretty recent; the spdlog one has been around forever. I think all the current ones can be worked around/fixed upstream, but more philosophically, if we maintain a parallel build (even upstream) and ray developers continue to patch their deps with careless abandon, we'll have bugs that don't exist in the official binaries. |
That's kind of the main argument in favour of unvendoring IMO. That way you can consistently build against one version. The mixing between versions can still happen if you build a local copy that's linked statically, unless you take very thorough care that grpc gets completely absorbed. |
|
Running a binary repackaged ray with conda-forge grpcio results in the following segfault that is the root cause of the aforementioned test failure: The notable section is:
@timkpaine turns out this isn't true:
Any ideas how to resolve this upstream? As it stands this means any python process which loads I fear this may also be true for other libraries vendored into |
|
Ahh, cool debugging. Do you mean the test failure in ray-project/ray#51673? Curious how you got a debug stack out of the tests?
|
I mean the following fails:
Investigation of the ray logs reveals some message about a raylet falling over:
apport (on my linux system) produced a core file, and loading it up in gdb gave that backtrace: |
Unvendor boost, refactored from #224. Closes #229. Also clean up some leftover comments and debug cruft.
Checklist
0(if the version changed)conda-smithy(Use the phrase@conda-forge-admin, please rerenderin a comment in this PR for automated rerendering)