JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.47k stars 5.46k forks source link

SSH support in Pkg #16041

Closed davidanthoff closed 8 years ago

davidanthoff commented 8 years ago

Julia 0.5 has removed all support for cloning private repos on Windows. In previous versions one could always clone private repos via SSH remote URLs, but support for the git protocol seems entirely removed. Cloning private repos via https also doesn't work on Windows, I get the following error:

ERROR: ccall: could not find function getpass
 [inlined code] from .\c.jl:89
 in #prompt#1(::ASCIIString, ::Bool, ::Any, ::ASCIIString) at .\libgit2\utils.jl:17
 in credentials_callback(::Ptr{Ptr{Void}}, ::Cstring, ::Cstring, ::UInt32, ::Ptr{Void}) at .\libgit2\callbacks.jl:56
 [inlined code] from .\refpointer.jl:32
 in clone(::ASCIIString, ::SubString{UTF8String}, ::Base.LibGit2.CloneOptions) at .\libgit2\repository.jl:189
 in #clone#98(::ASCIIString, ::Bool, ::Ptr{Void}, ::Nullable{Base.LibGit2.AbstractPayload}, ::Any, ::ASCIIString, ::SubString{UTF8String}) at .\libgit2.jl:310
 [inlined code] from .\base.jl:111
 in clone(::ASCIIString, ::SubString{UTF8String}) at .\pkg\entry.jl:195
 in clone(::ASCIIString) at .\pkg\entry.jl:221
 [inlined code] from .\promotion.jl:229
 in (::Base.Pkg.Dir.##2#3{Array{Any,1},Base.Pkg.Entry.#clone,Tuple{ASCIIString}})() at .\pkg\dir.jl:31
 in cd(::Base.Pkg.Dir.##2#3{Array{Any,1},Base.Pkg.Entry.#clone,Tuple{ASCIIString}}, ::ASCIIString) at .\file.jl:58
 in #cd#1(::Array{Any,1}, ::Any, ::Any, ::ASCIIString, ::Vararg{ASCIIString}) at .\pkg\dir.jl:31
 in clone(::ASCIIString) at .\pkg.jl:128
 in eval(::Module, ::Any) at .\boot.jl:236

I think there are potentially two solutions: 1) Re-enable support for the git transport protocol 2) Fix the error with https connections on Windows (might be enough to fix #8228)

I believe that only having 2) would be very painful, because my understanding is that every Pkg.update would for example query for the username/password combo for private repos, i.e. as far as I know there is no support for credential storage in libgit2, right?

So I think if the git transport is not re-enabled, one would really also need integration with the git credential manager interface, so that https credentials that are stored in a registered git credential manager would be picked up by Pkg.clone and Pkg.update. My understanding is that libgit2 has no support for this, so it would probably have to be implemented manually...

I think this issue here should get the 0.5.0 milestone attached. Not being able to work with private repos is a major regression relative to 0.4 that would affect essentially anyone who has package work in private repos.

EricForgy commented 8 years ago

For what it's worth, I am in the same boat as David. I'm on Windows and have many private repos (close to 50 and counting). It took me some time to set up SSH and point all my remotes to git@github.com so that I could do Pkg.update without entering my password so many times. It was also necessary to set that up for Travis and Appveyor. Now, all my tests are failing on 0.5 because the private repos cannot be cloned. This has stopped me from doing any work on 0.5.

Like David, I would hope this gets a 0.5 label because I am REALLY looking forward to 0.5 :)

Cheers

tkelman commented 8 years ago

This particular ccall is not present on windows and evidently obsolete, so https://github.com/JuliaLang/julia/issues/8228#issuecomment-150291010 still stands. We're going to primarily support https going forward, so I'm going to close this as a duplicate of #8228.

davidanthoff commented 8 years ago

@tkelman: Once #8228 is fixed, will Pkg.update ask me for a username/password combination for every private package that I have? Or is there going to be some caching of credentials?

tkelman commented 8 years ago

Probably. I consider ssh keys far worse in usability than just typing a password, so I'm not too motivated to do anything drastic here. I believe @eschnett had looked at adding libssh to deps/Makefile but couldn't get it working, it's nontrivial to add more libraries to the Julia binary distribution.

aviks commented 8 years ago

Not everyone uses github. I have locally hosted git repositories that use ssh authentication. Is that going to be unsupported going forward? That's shame, if true.

davidanthoff commented 8 years ago

Can we leave this issue open, please? Clearly requiring username/password entering for every private package on every Pkg.update() seems like a completely unworkable solution for many users, so keeping an issue open where solutions can be sorted out seems like a good idea.

I should add that using SSH keys is working great on julia 0.4 for me, so dropping that support without replacing it with something similar would be a huge regression from my point of view.

tkelman commented 8 years ago

The windows-specific part of this issue is a duplicate of #8228.

We're using libgit2 for the package manager now, that's not up for debate. If ssh support by default is something that people want strongly enough to implement then I'm not going to say we shouldn't do it, but I don't consider it release-blocking. There's also the option of not using Pkg to manage private packages (just put them somewhere else on LOAD_PATH) since it generally isn't doing much for you with private packages in the current design.

EricForgy commented 8 years ago

Hi, I have close to 50 private repos on GitHub and there is no way I am going to enter my password 50 times every time I Pkg.update. Even 4 times ( #8228 ) is too much. If this is really the future for me with Julia, please let me know ASAP as I will need to switch to a different language and the somer I make that painful call, the better. Thanks everyone.

Keno commented 8 years ago

No, nobody wants to have to enter a password 50 times. This'll get fixed.

EricForgy commented 8 years ago

Thank you for the reassurance Keno. I was starting to panic (I do that too easily sometimes) :)

davidanthoff commented 8 years ago

Great! I think some fix should really come for 0.5, so maybe apply the 0.5 label?

At least for github and bitbucket another option would be to plug into the whole git credential storage framework. That might only work if people have git installed and configured, but as an option that would be nice, especially as it would work with https URLs.

Of course that doesn't work for people with locally hosted repos, so I think it wouldn't be a substitute for ssh support.

tkelman commented 8 years ago

We've gone without ssh support on master for 6 months now. I don't think this is release blocking.

davidanthoff commented 8 years ago

Keep in mind that the vast majority of users hasn't been on master for the last 6 months. Just because something hasn't shown up as a problem for the core dev group in the last 6 months doesn't mean that it can't be a major problem for a large user group that is only now starting to explore 0.5. This issue would keep me from upgrading to 0.5 until sorted out because it completely would break my daily workflow.

aviks commented 8 years ago

We've gone without ssh support on master for 6 months now. I don't think this is release blocking.

Oh, come on. Lots of people who use julia in "production" haven't (and shouldn't have) upgraded to 0.5 yet.

timholy commented 8 years ago

I only have a few private repos, and it's already pretty irritating.

blakejohnson commented 8 years ago

I also find this annoying. I frequently launch 0.4 just to run Pkg.update().

wildart commented 8 years ago

Password caching should not be a problem. Temporary solutions is to use ssh-agent.

tkelman commented 8 years ago

@Keno any chance enabling the EMBED_SSH_PATH option https://github.com/libgit2/libgit2/pull/2600 on additional platforms might make this a little simpler?

I don't think it's technically feasible in libgit2 just yet, but something Jake and Art and I had discussed was possibly making ssh support optional in a way that PkgDev or some other non-default package could be in charge of obtaining the binaries for libssh2.

edit: side note, even the libgit2sharp maintainers who are core developers of libgit2 choose not to support ssh in the binaries they distribute: https://github.com/libgit2/libgit2sharp/issues/255#issuecomment-212402026

tbreloff commented 8 years ago

In case it helps... I'm very actively using 0.4, but haven't made the jump to 0.5 yet. I have lots of private repos, and I agree that package maintenance would be impossible for me (I'd just abandon it and manage the repos by hand). My password is 20 characters of gibberish, so typing it in is not an option.

Keno commented 8 years ago

You've all been heard loud and clear. There's some technical issues to figure out with libssh2, but I don't think it's insurmountable. Namely, there is a branch that uses OS X's SecureTransport as it's crypto backend, but Apple's BigNum library (needed to compute mod_exp) is a private header, so upstream may not take it. I made some attempt to use the (public) RSA encryption functions to implement modexp (this is a trick used on windows), but unfortunately Apple has an arbitrary limit on the size of the public key exponent so that doesn't work/ For our immediate purposes, we can likely just ship the version that uses the private headers, but it would be good to figure out a longer term solution.

tkelman commented 8 years ago

Another alternative we've mentioned is using mbedtls on both osx and linux, since the web stack has been using it anyway and it's more compatibly licensed than openssl is. Would that be any faster to get the build system pieces cleaned up and in a state that we can test how CI and the buildbots handle the new library dep?

quinnj commented 8 years ago

+1, it would probably have the extra benefit of improving the web stack usage/testing as well.

wildart commented 8 years ago

Mbedtls would not solve ssh problem.

Keno commented 8 years ago

It would, we can use it's cryptographic capabilities in libssh.

wildart commented 8 years ago

Do we really want to fork libssh to add mbedtls support to it?

tkelman commented 8 years ago

Keno already spent a bunch of time forking (well, continuing from an existing fork of) libssh2 and adding SecureTransport support to it but got stuck. Mbedtls would be a way of getting un-stuck, and if the code changes are small then we should see if libssh2 upstream is more receptive than libgit2 was. Or maybe libssh2 already has the flexible stream mechanisms like what libgit2 added.

wildart commented 8 years ago

The new crypto backend to libssh2 is not a small endeavor, but not impossible. I'm all for bringing mbedtls on board. Having some experience with it I could give a hand.

tkelman commented 8 years ago

We should probably inquire on the mbedtls and/or libssh2 mailing lists and find out whether others have started down this road before.

wildart commented 8 years ago

Unfortunately, nobody tried to use mbedtls.

wildart commented 8 years ago

OK, I got mbedtls crypto backend working with libssh2 (https://github.com/wildart/libssh2/tree/mbedtls). I just need to figure out how to put everything together (mbedtls, libssh2, libgit2) into julia build system.

Keno commented 8 years ago

Awesome job @wildart, have you submitted a pull request upstream?

wildart commented 8 years ago

Yep, libssh2/#106. We'll see how it will go.

EricForgy commented 8 years ago

I gave some moral support to @wildart's PR over on libssh2 and one of the maintainers asked if my support was independent claim the PR works as it is supposed to. I don't know how this stuff usually works, but if anyone else here could kick @wildart's PR around a bit and give support, it might help get it merged. Just a thought...

tkelman commented 8 years ago

A workaround people can try is generating an oauth token then using https://<access-token>:x-oauth-basic@github.com/<username>/<repo> as your remote. I haven't tested this with libgit2 yet, not sure if it'll make a difference.

wildart commented 8 years ago

@Keno Did you have a chance to look at libssh2/#106.

I created branch art/pkg-libs where I put together latest OpenSSL-less libgit2 build and all its dependencies (http_parser, mbedtls, libssh2). It's tested on Linux, Windows build is coming.

StefanKarpinski commented 8 years ago

@wildart, @keno – this is release blocking for 0.5. Any chance of getting this addressed this week?

StefanKarpinski commented 8 years ago

bump

JeffBezanson commented 8 years ago

also bump

EricForgy commented 8 years ago

It doesn't need to be @Keno , but I get the impression from https://github.com/libssh2/libssh2/pull/106 that they are willing to merge @wildart 's PR as long as an independent view is expressed saying it looks OK. I'm not qualified to give that view though.

Keno commented 8 years ago

@wildart did you get a chance to finish up the branch for julia build system changes or should I look at that?

tkelman commented 8 years ago

I independently tested that libssh PR but was unable to get it to pass libssh's tests.

wildart commented 8 years ago

@tkelman What kind of problem did you encounter? I tested it locally and travis gives a green light as well.

BTW, I'm working on windows setup right now.

tkelman commented 8 years ago

It wasn't very specific, their test suite only has like 3 components IIRC. I'll try again some time today.

wildart commented 8 years ago

@Keno art/pkg-libs branch should work fine on Linux. You can tests it. Currently, I'm fixing it for the Windows build specifics.

wildart commented 8 years ago

I fixed Windows build config and tested it for MinGW/MSYS2 build. Cygwin is coming.

   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _' |  |
  | | |_| | | | (_| |  |  Version 0.5.0-dev+4967 (2016-06-28 14:17 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit a48676c* (2 days old master)
|__/                   |  i686-w64-mingw32

julia> LibGit2.features()
3-element Array{Base.LibGit2.Consts.GIT_FEATURE,1}:
 FEATURE_THREADS
 FEATURE_HTTPS
 FEATURE_SSH

julia> LibGit2.clone("git@github.com:JuliaLang/Example.jl.git", joinpath(homedir(), "SSH"))
Base.LibGit2.GitRepo(Ptr{Void} @0x267b7478)

shell> ls ~/SSH
appveyor.yml  LICENSE.md  README.md  REQUIRE  src  test

N.B. There is an issue with the credential callback resulting in an incorrect ssh key location on Windows platform. I'll fix it shortly.

wildart commented 8 years ago

MinGW/MSYS2 & Cygwin builds are fixed. Ready to test.

@tkelman @Keno

Keno commented 8 years ago

Two issues:

  1. Repeatedly asks for credentials. Ideally it could launch an ssh-agent or similar to cache
  2. For a non-existent repository, the error message is not very good
julia> Pkg.clone("git@github.com:JuliaLang/Example.jl.git")
INFO: Cloning Example from git@github.com:JuliaLang/Example.jl.git
Private key location for 'git@github.com' [/home/kfischer/.ssh/id_rsa]:
Passphrase for /home/kfischer/.ssh/id_rsa:
INFO: Computing changes...
INFO: No packages to install, update or remove
INFO: Package database updated

julia> Pkg.clone("git@github.com:JuliaLang/IterativeSlots.jl.git")
INFO: Cloning IterativeSlots from git@github.com:JuliaLang/IterativeSlots.jl.git
Private key location for 'git@github.com' [/home/kfischer/.ssh/id_rsa]:
Passphrase for /home/kfischer/.ssh/id_rsa:
ERROR: ArgumentError: invalid value for Enum Code: -20
 in enum_argument_error(::Symbol, ::Int32) at ./Enums.jl:27
 in convert(::Type{Base.LibGit2.Error.Code}, ::Int32) at ./Enums.jl:79
 in getindex at ./array.jl:140 [inlined]
 in Base.LibGit2.Error.GitError(::Int32) at ./libgit2/error.jl:86
 in macro expansion at ./libgit2/error.jl:98 [inlined]
 in clone(::String, ::SubString{String}, ::Base.LibGit2.CloneOptions) at ./libgit2/repository.jl:189
 in #clone#103(::String, ::Bool, ::Ptr{Void}, ::Nullable{Base.LibGit2.AbstractPayload}, ::Function, ::String, ::SubString{String}) at ./libgit2/libgit2.jl:308
 in clone(::String, ::SubString{String}) at ./pkg/entry.jl:195
 in clone(::String) at ./pkg/entry.jl:221
 in (::Base.Pkg.Dir.##2#3{Array{Any,1},Base.Pkg.Entry.#clone,Tuple{String}})() at ./pkg/dir.jl:31
 in cd(::Base.Pkg.Dir.##2#3{Array{Any,1},Base.Pkg.Entry.#clone,Tuple{String}}, ::String) at ./file.jl:59
 in #cd#1(::Array{Any,1}, ::Function, ::Function, ::String, ::Vararg{String,N}) at ./pkg/dir.jl:31
 in clone(::String) at ./pkg/pkg.jl:151
 in eval(::Module, ::Any) at ./boot.jl:234
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

julia> Pkg.clone("git@github.com:JuliaLang/IterativeSolvers.jl.git")
INFO: Cloning IterativeSolvers from git@github.com:JuliaLang/IterativeSolvers.jl.git
Private key location for 'git@github.com' [/home/kfischer/.ssh/id_rsa]:
Passphrase for /home/kfischer/.ssh/id_rsa:
INFO: Computing changes...
INFO: No packages to install, update or remove
INFO: Package database updated
Keno commented 8 years ago

Hangs on Windows for me:

julia> Pkg.clone("git@github.com:JuliaLang/Example.jl.git")
INFO: Cloning Example from git@github.com:JuliaLang/Example.jl.git
Private key location for 'git@github.com' [C:\msys64\home\keno/.ssh/id_rsa]:
Passphrase for C:\msys64\home\keno/.ssh/id_rsa:
[HANGS]
Keno commented 8 years ago

Linux cross compile to windows:

kfischer2@nanosoldier1:~/julia-win64$ make
patching file CMakeLists.txt
Hunk #1 FAILED at 354.
1 out of 1 hunk FAILED -- saving rejects to file CMakeLists.txt.rej
-- Could NOT find ZLIB (missing:  ZLIB_LIBRARY ZLIB_INCLUDE_DIR)
-- zlib was not found; using bundled 3rd-party sources.
CMake Warning at CMakeLists.txt:357 (FIND_PACKAGE):
  By not providing "FindLIBSSH2.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "LIBSSH2", but
  CMake did not find one.

  Could not find a package configuration file provided by "LIBSSH2" with any
  of the following names:

    LIBSSH2Config.cmake
    libssh2-config.cmake

  Add the installation prefix of "LIBSSH2" to CMAKE_PREFIX_PATH or set
  "LIBSSH2_DIR" to a directory containing one of the above files.  If
  "LIBSSH2" provides a separate development package or SDK, be sure it has
  been installed.

CMake Error at CMakeLists.txt:358 (GET_PROPERTY):
  get_property could not find TARGET Libssh2::libssh2.  Perhaps it has not
  yet been created.

CMake Error at CMakeLists.txt:359 (GET_PROPERTY):
  get_property could not find TARGET Libssh2::libssh2.  Perhaps it has not
  yet been created.

-- LIBSSH2 not found. Set CMAKE_PREFIX_PATH if it is installed outside of the default search path.
-- Performing Test IS_WSTRICT-ALIASING=2_SUPPORTED
wildart commented 8 years ago

There are 3 env. variables that can be used to set ssh-key

Hangings are related to incorrectly provided key location. I'm investigating this issue.