Try migration to OpenSSL #53891
Try migration to OpenSSL #53891
Conversation
inkydragon
added
domain:building
|
OK I'm learning about stdlib as I go along, feedback is very welcome. I think I've done what is needed to add OpenSSL_jl to stdlib. It allows me to build locally. There is one thing I think I missed, but I don't know how to fix it: I can't import OpenSSL_jll. I think it's built (because I had a bug in it at some point, and it was complaining which meant it tried to compile it, and now it doesn't complain anymore)… but I'm not sure. |
|
OK, somehow the OpenSSL_jll sources are not getting copied to the right directory. I see this message (not an error, but worrying): |
|
I think you need to add it to |
fxcoudert
force-pushed
the
openssl
branch
2 times, most recently
from
df3f08b
to
bbc918d
Compare
Thanks, that works. |
|
Once we finish JuliaPackaging/Yggdrasil#8377 (and JuliaPackaging/Yggdrasil#8386) and the corresponding builds are updated here, we can remove mbedTLS_jll as stdlib from here, since it isn't used for anything else, I think. |
|
Build works for me locally, but on CI it fails with: |
|
I'm not sure, but maybe that's during documentation building? |
|
Yes, it's during docs building, which I'm not doing locally (not sure what the |
|
@IanButterworth @KristofferC do I understand correctly we need to update |
|
@giordano yeah any manifests may need to be re-resolved |
May I ask how this is done? I couldn't figure it out from the doc. |
fxcoudert
force-pushed
the
openssl
branch
2 times, most recently
from
7c1c2de
to
44859e1
Compare
|
Manually updated |
|
It usually requires Pkg to resolve it to be accurate I just pushed the result of that. I also checked for other manifests in this repo that need updating and there doesn't appear to be any. Surprisingly there's only one in Pkg https://github.com/IanButterworth/Pkg.jl/blob/205309092c861d58bf787628ccbff4f5a41a0840/test/test_packages/ShouldPreserveSemver/Manifest.toml#L90 |
|
Thanks @IanButterworth That does not seem to pass CI (https://buildkite.com/julialang/julia-master/builds/35131#018e94a6-3c00-4b01-8b2a-8797e5b2ddf0), but… I can now run that command from my branch. I don't get the same output as you, here is the diff between your version and mine: Some versions and commit ids are different. But maybe more importantly, it does remove the |
|
OK, that makes the build pass on FreeBSD and macOS (both Intel and ARM). It's also failing on Windows. It's failing on My guess is that it's because in So: should I got back to the OpenSSL package and make it put libraries in On Windows, the naming convention is I'll let testing finish here, but I think Windows can be handled with: diff --git a/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl b/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl
index 26eb3d243e..bba9a0a299 100644
--- a/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl
+++ b/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl
@@ -3,7 +3,7 @@
## dummy stub for https://github.com/JuliaBinaryWrappers/OpenSSL_jll.jl
baremodule OpenSSL_jll
-using Base, Libdl
+using Base, Libdl, Base.BinaryPlatforms
const PATH_list = String[]
const LIBPATH_list = String[]
@@ -20,8 +20,13 @@ libssl_handle::Ptr{Cvoid} = C_NULL
libssl_path::String = ""
if Sys.iswindows()
- const libcrypto = "libcrypto.dll"
- const libssl = "libssl.dll"
+ if arch(HostPlatform()) == "x86_64"
+ const libcrypto = "libcrypto-3-x64.dll"
+ const libssl = "libssl-3-x64.dll"
+ else
+ const libcrypto = "libcrypto-3.dll"
+ const libssl = "libssl-3.dll"
+ end
elseif Sys.isapple()
const libcrypto = "@rpath/libcrypto.3.dylib"
const libssl = "@rpath/libssl.3.dylib" |
|
Status:
I have to dig to understand why this used to work and doesn't anymore. It is working for me on a full source build, but not with binarybuilder. Go figure… |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Yes, we stumbled upon that already: JuliaPackaging/Yggdrasil#7576 (comment) |
It works locally for me with BinaryBuilder binaries: but it's possible libgit2/openssl is picking up from my system whatever certificates it needs. Maybe we somehow need to inform openssl/libgit2 where the certificates are ( |
| # If not already done, set the environment variable `SSL_CERT_FILE` when necessary. | ||
| cert_loc = ca_roots() | ||
| if cert_loc isa String | ||
| get!(ENV, "SSL_CERT_FILE", cert_loc) | ||
| end |
There was a problem hiding this comment.
@StefanKarpinski can you please check this makes sense? I followed the same logic as in
julia/stdlib/LibGit2/src/LibGit2.jl
Lines 1027 to 1028 in 5006312
There was a problem hiding this comment.
Shouldn't this be done in NetworkOptions though, so that everyone gets it?
|
Ok, setting the environment variable doesn't work well because there are tests explicitly checking that |
|
I got some bad ideas. One is that we mangle the binary in the same way as others. The other is that we modify the source code to read the location from somewhere. |
|
Ugh, we should probably schedule a call about this to figure out what to do... |
|
I'm kind of tempted to use a simpler TLS library that is better isolated. |
|
I just read through this, and as far as I can tell, the only thing that is failing is teaching OpenSSL about the certificate location, is that correct? |
|
Since mbedtls-2.28 is EOL'd at the end of 2024, any way we could nudge Julia to OpenSSL 3.X as distros, like Fedora, would like to drop 2.28. |
|
We should modify the BinaryBuilder build of OpenSSL such that these hardcoded paths are a longer placeholder path: For example, the conda-forge builds set this as follows: conda, mamba, or pixi then replaces these with environment specific locations. For example, I'm not suggesting that Julia do the mangling directly. Rather I suggest that rather we just use an extended placeholder path so that someone could easily replace it in the BinaryBuilder openssl binaries if needed. |
The construction of the placeholder path is as follows in Rust: let placeholder_template = "_placehold";
let mut placeholder = String::new();
let placeholder_length: usize = 255;
while placeholder.len() < placeholder_length {
placeholder.push_str(placeholder_template);
}
let placeholder = placeholder
[0..placeholder_length - build_dir.join("host_env").as_os_str().len()]
.to_string();
build_dir.join(format!("host_env{}", placeholder))A similar Julia function might be as follows. julia> function gen_placehold_path(
destdir = "/workspace/destdir";
placeholder_template = "_placehold",
placeholder_length = 255
)
len = 0
iob = IOBuffer()
while len < placeholder_length
len += write(iob, placeholder_template)
end
seekstart(iob)
placeholder = String(read(iob, placeholder_length - length(destdir) - 1))
return joinpath(destdir, placeholder)
end
gen_placehold_path (generic function with 2 methods)
julia> gen_placehold_path()
"/workspace/destdir/_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place" |
This is a test of migrating the source build to OpenSSL.