-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat: add musl prebuild #264
feat: add musl prebuild #264
Conversation
Co-authored-by: funniray <funniray10@gmail.com> Co-authored-by: Brad Marsden <silentbot1@gmail.com>
There appears to be an issue with glibc builds still which we're working through, I will mark this ready for review once we've got these changes ready. |
Reverts back to using ubuntu-20.04 for builds to keep in-line with existing worker. Uses debian instead of ubuntu for sysroot for newer version of OpenSSL. Runs all tests on all platforms to ensure everything works, rather than just x64. Co-authored-by: funniray <funniray10@gmail.com> Co-authored-by: Brad Marsden <silentbot1@gmail.com>
This should now be fixed and ready for review! |
Thank you very much for the PR. Could you please explain me |
And if I compare the binary sizes they are different. https://github.com/SilentBot1/node-datachannel/releases/tag/v0.9.1 |
Different programs can't agree on what to call each architecture.
Alpine and Debian disagree on the architecture for the triple for armv7. Debian calls it arm, while Alpine calls it armv7. This is why there's an override on debian arm[v7] builds to set the triple's target to just
libssl.a on alpine is 6.3MB as libssl is linked statically, I'd assume that's why the filesize is larger on musl vs debian |
Thank you for the explanation. I think it will be good first to merge with another branch. So we can test it better, and then merge it with the master. I created a new branch here |
Sure, I've repointed this PR to the new branch you've mentioned. Please note, that we probably will want to bump the version for this branch once this is merged (or give it a postfix like -dev / -testing) so it doesn't interfere with the currently published release and its prebuilds. Feel free to merge whenever you are ready. |
I have created a release here. https://github.com/murat-dogan/node-datachannel/releases/tag/v0.10.0-dev |
For Linux x64 ; Less is better of course, but I wonder why. |
I checked and found out
I'm honestly unsure what could be causing the smaller builds. I don't have enough experience with C/C++ to know how to debug why the files are larger any further. As far as I'm aware however, the only difference the compilers would see is the openssl version |
The difference is pthread linking Current
New
|
Hey there, I don't seem to see the same results as you. Are you sure you're checking the right file?
old-linux.node was from 0.9.2-x64 |
Yep, you are right. I was comparing with local built version. Then, I am again at the beginning :) |
Found a diff by running command Current
new
Difference is on I have never used this lib |
In this PR, we add musl prebuilds of
libdatachannel
tonode-datachannel
to allow users on musl-based systems (Alpine, Kiss Linux) to usenode-datachannel
without needing to buildlibdatachannel
locally.This is done by generating sysroots locally for glibc (ubuntu) and musl (alpine) and using these to prebuild
libdatachannel
.An example of these builds can be seen here: https://github.com/SilentBot1/node-datachannel/releases/tag/v0.9.1
And this can be tested locally by doing
npm i https://github.com/SilentBot1/node-datachannel.git