-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
libjxl: migrate to Conan v2, add v0.10.2 #18812
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi, any progress? |
be88ee6
to
b28d312
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Bump! Is it ready for merging? |
@uilianries @RubenRBS @franramirez688 pinging for a review, in case you have missed this one |
This comment has been minimized.
This comment has been minimized.
@valgur What does this mean? Is this closed because you are also quitting cci? Forgive me as a I am somewhat confused :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 4 (
Conan v2 pipeline ✔️
All green in build 6 ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Built locally, looks okay.
conan-io#18812 been merged. also upgraded libjxl from 0.10.2 to 0.10.3
del self.options.avx512_zen4 | ||
# /~https://github.com/libjxl/libjxl/blob/v0.9.1/CMakeLists.txt#L52-L59 | ||
if self.settings.os in ["Linux", "FreeBSD"] and self.settings.arch == "x86_64": | ||
self.options.with_tcmalloc = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for reviving this but I just got a surprise when updating this package with allocator mismatches due to this change in my static build and I just wanted to ask; Should we really do this here? From the linked cmake I gather that libjxl tries to use tcmalloc by default if it exists, but if we don't add it as a dependency, it shouldn't exists unless it is available on the system? Which I assume is a common problem in general with many libs. Could a better solution be to be able to disable it completely upstream instead either via a patch or a PR to them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reporting this. Please open it as a separate issue for more visibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, added one #26707
.