-
Notifications
You must be signed in to change notification settings - Fork 166
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
Use cmake to require ICU version >= 56. #347
base: master
Are you sure you want to change the base?
Conversation
Hi, thanks for the second contribution :) Similar story -- we've opened an internal ticket (GODT-2410) to check and hopefully merge this. Will keep you posted. |
Hello @themobiusproject . Your pull request has been cherry picked and merged into our internal repository. It will be part of an upcoming release. |
Actually we need more work on that side since it appears that even if your PR builds perfectly. |
@rlejeune74 I am trying to figure out the use case where this would be an issue. If the binary is prebuilt and distributed with libicuxxx.so.56 as it has been, then it should be able to continue running with .so.56. If, such as on my machine (Gentoo), I am building with libicuxxx.so.72.1, I already have it on my machine. If I upgrade icu, I am already going to have to rebuild a fair portion of my system as it is. Even though I am manually compiling proton-bridge (not using portage), when I started it up after an icu upgrade it would yell at me that it couldn't load the library and I would rebuild it with the current version of icu, which also wouldn't be a surprise since I have already recompiled many other packages due to this dependency. I recognize that I am theorizing here and could very well be wrong, but the two main scenarios that I see are the binary being prebuilt and distributed with the correct version of icu or the end user (me) building it and dealing with it myself. |
@themobiusproject
With your PR, and the current way we are bundling deps, we end up with |
@rlejeune74 I am assuming something like
Would it be sufficient to also have the a link for |
Any update on this? |
Currently the cmake build is specifically requiring icu version 56. This will allow versions greater than 56 (I am using 72).