-
Notifications
You must be signed in to change notification settings - Fork 549
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
[open62541] Add OpenSSL encryption, update Julia compat and increase version number (1.4.0) #7889
Conversation
You should declare a dependency on Also, @giordano is mbedtls still a stdlib where version compats have to be kept in line with Julia versions? |
How would I check this? The open62541 docs just use apt get install on Debian to install libmedtls (https://www.open62541.org/doc/1.3/building.html). Checking debian pages anything above 2.28.3 should be fine (counting from stable release onwards), see here: https://packages.debian.org/search?keywords=mbedtls |
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
I hope patch version increase is fine for this. If not (and v1.4.0 would be the one to do), then I will wait for the open62541 v1.4.0 release that should happen in a month or so and keep numbers in sync like this. |
This may be a nicer option, since the new release isn't too far away |
I agree; changed title to WIP and will update with correct commit hash once the release is out. |
There has been a large effort recently to move away from MbedTLS and to OpenSSL instead. Can open62541 use OpenSSL? |
Both is possible (in the sense of "either or"), as detailed here: https://www.open62541.org/doc/master/building.html Maybe it would be helpful to feed the arguments for either MBEDTLS or OPENSSL here or add a cross reference to relevant discussions? I was for example able to find this: JuliaLang/julia#48799 |
The effort has mainly started because the upstream MbedTLS seems to not publish their plans for future LTS versions (so Julia doesn't know when they are coming), and also can have ABI compatibility issues between releases. With the effort going into switching the stdlibs that ship with Julia from MbedTLS to OpenSSL (#8386, #8377), that means OpenSSL will already be a necessary part of Julia and so won't require the user getting another dependency. @fxcoudert and @giordano can probably explain more though. |
These sound like very valid reasons to me and in the end I just wanted to provide potential (future) users of open62541.jl with the possibility to have encryption activated - it's of course nicer if it's aligned with the general direction that Julia is taking in this matter! There's some time before a decision is "necessary", because the 1.4 release of open62541 is taking a little bit more time than expected and, as agreed above, I will aim to have this PR done only after that release happened. |
open62541 has released version 1.4.0 a couple of days ago, but I can't seem to compile it with If I set the flag to OFF (d3343e9), then it builds on most platforms (but it would mean that I need to alter my Clang.jl generator script for my wrapper package?), but not all. On the platforms where it fails to build:
x86_64-w64-mingw32:
Maybe one of the wizards around here can help me? Obviously, I probably could get x86_64-w64-mingw32 work without OpenSSL included in the library and I would have to live with updating my Clang.jl generator script in that case. My preferred solution would of course be to have |
It's less weird if you git blame the relevant line and see that the error was introduced three weeks ago: open62541/open62541#6369. It looks very deliberate. |
Thanks for finding that discussion. I think I understand the reasoning behind the change and it makes sense to me (just means that I have to update the generator script for my wrapper - that's an "OK" bit of work and it seems I can't avoid it :) ). |
I haven't tested it locally, but according to the FreeBSD #include <sys/types.h>
#include <sys/socket.h> but looking at https://github.com/open62541/open62541/blob/8be32a09479401015bd8196f1c21bfdfe1849ab4/arch/eventloop_posix/eventloop_posix.h they only include
Yggdrasil/L/libssh/build_tarballs.jl Lines 15 to 18 in 4edca2e
|
I opened up an issue: open62541/open62541#6414 |
As per the issue linked above, this is indeed an upstream bug.
Option 1 would allow me to start working on the necessary changes to reflect the separate header files rather than the amalgamated version in the wrapper package (https://github.com/martinkosch/open62541.jl). |
Option 3 would be best, but we need a patch to apply to the source of v1.4.0. |
Ok, I think the patch worked (sorry, first had to actually understand how to make one - first time patching something!); the library now builds again on all platforms. @giordano, I think this is now ready for another look by you. |
Thank you for all your help on this! |
No description provided.