-
Notifications
You must be signed in to change notification settings - Fork 622
Zlibcloudflare #3946
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
Zlibcloudflare #3946
Conversation
Z/Zlibcloudflare/build_tarballs.jl
Outdated
|
||
# The products that we will ensure are always built | ||
products = [ | ||
LibraryProduct("libz", :libz) |
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.
I can't say I like having multiple libraries around with same name (and likely soname?), point which I made repeatedly in #3801. That's going to be brittle, to say the least.
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.
I'm fine with renaming it since I think few ppl would use it
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.
Do you plan to link other libraries to this or you just want to ccall
into this library directly?
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.
I probably need to copy the source code of https://github.com/JuliaIO/CodecZlib.jl
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.
When trying to build for aarch64:
-- Performing Test ARM_CRC
-- Performing Test ARM_CRC - Failed
-- Performing Test HAS_SSE2
-- Performing Test HAS_SSE2 - Success
-- Performing Test HAS_SSSE3
-- Performing Test HAS_SSSE3 - Success
-- Performing Test HAS_SSE42
-- Performing Test HAS_SSE42 - Success
-- Performing Test HAS_PCLMUL
-- Performing Test HAS_PCLMUL - Success
sigh... 🤦
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.
Judging by the plethora of -march
and other -m
options they really like to use, this might be a good exercise for our totally-untested-in-the-wild multiarchitecture infrastructure 🙃
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.
Renaming the variable :libz
-> :libzcloudfare
is hardly a problem at all 🙂 My suggestion was to rename the library, with, most importantly, its SONAME everywhere in the build system and inside the pkgconfig file, which should also be renamed 🙂 Unless you can show that you can build the JLL and call into the right libz as it is now (which I have many doubts with, especially in relation with JuliaPackaging/JLLWrappers.jl#31).
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.
since the output is a single SO file, can we just rename it after make
?
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.
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, not familiar with ABI/linker, guess this is too much patching work for too little gain. Thanks for the explanation.
No description provided.