-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
WIP: LibGit2 - mbedTLS stream support #17246
Conversation
…to mbedtls dependent branch
@tkelman Travis fails because of old version of |
we can use a ppa or download cmake binaries, but that's going to be annoying. llvm 3.9 will require cmake 3.4.3, but until we move to that we should try to keep compatible with versions of cmake that are easily available in existing linux distros. |
As I wrote in the PR description. There is one option to avoid using custom version of |
|
It's not travis I'm concerned about, it's everyone who builds from source on linux. What do we need from new cmake, and can we live without it? Didn't libgit2 say to do the stream registration differently? libgit2/libgit2#3465 |
For For |
what do they require as the minimum version? since llvm will require 3.4.3 eventually, maybe we should just download a binary of cmake for people on linux (sorry about the spam close/reopening, stupid button placement is made worse by laggy phone) |
I cannot figure out how the build system works. It builds libraries out of order, screwing everything. |
Do you have the proper dependencies between targets? |
I need to build them incrementally: http_parser & mbedtls -> libssh2 -> libgit2 -> mbedtlsstreams |
It's a makefile script, you need to declare the appropriate dependencies. |
What are |
A library needs to be added to one of them in order to be built be default (or be the dependency of one mentioned in there). Not exactly sure why we have 3 of them. |
35c296c
to
e2d27b9
Compare
@@ -66,7 +69,7 @@ before_install: | |||
ln -s /usr/bin/gcc-5 $HOME/bin/x86_64-linux-gnu-gcc; | |||
ln -s /usr/bin/g++-5 $HOME/bin/x86_64-linux-gnu-g++; | |||
gcc --version; | |||
BUILDOPTS="-j3 VERBOSE=1 FORCE_ASSERTIONS=1 LLVM_ASSERTIONS=1"; | |||
BUILDOPTS="VERBOSE=1 FORCE_ASSERTIONS=1 LLVM_ASSERTIONS=1"; |
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.
needs to be reverted
x86_64 build gives me very strange error when Any thoughts?
Ref: https://travis-ci.org/JuliaLang/julia/jobs/143092080#L4339 |
How different would implementing the TLS stream registration in Julia look? If it would be shorter, it's likely it would be more maintainable as well. |
It looks fairly simple: two streams one for http, another for https. Very mach as HttpServer.jl with MbedTLS.jl support, using data structures of libgit2. I'll look at this during weekend. |
Ok, If I'm understanding correctly, the only problem here is the HTTPS support for generic linux binaries. I'm strongly in favor of dropping that as a release blocking item, changing this into only the things needed for the SSH support. Then, we can either ship openssl as we have been so far, or build a julia package on top of all of the existing packages to implement this. I really don't feel like maintaining a whole separate HTTPS client implementation in either C or Julia. |
SSH support as a package would make sense to me, if it's even possible. Unless there are networks where https does not work at all for public packages? |
We will need to be sure that libgit2 built against libssh built against the version of openssl that we currently use on Linux all works properly. Or build and distribute our own openssl from source. On Windows I haven't tested this but I think unmodified libgit2 built against unmodified libssh built against the OS-native crypto should work, as long as we get the build system sorted (I only care about cross-compile from cygwin because that's how our windows binaries are built, msys2 is irrelevant and not at all release-blocking). What's the situation on Mac, do we have to add openssl to our build system and only build it from source on Mac (and maybe Linux)? Or do we use the OS-native Apple crypto with the patched version of libssh, that needs a private header? As long as that doesn't interfere with our ability to build standalone distributable binaries I guess the private header thing isn't release blocking, It would be ideal to eventually get that crypto code reviewed and upstreamed to libssh eventually, but as long as it doesn't have glaring security holes (knock on wood) I guess we can live without that at first? |
I'm closing this PR in favor of #17471 |
Provide support of SSH protocol in LibGit2:
mbedtls
,libssh2
try to usegit_transport
to avoid custom version oflibgit2
mbedtls
TLS stream inlibgit2
mbedtls
Update: Proxy support will be available starting from libgit2 0.25, so I removed it from the agenda of this PR.
Update 2: Using custom transport is not a good option, because we only need TLS stream support in available http transport. Instead, custom TLS stream can be registered in libgit2. This is much less work then complete transport definition. I already done this as rejected PR libgit2/3462.
Ref: #16041