-
-
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
Libgit2 usability issues #17586
Libgit2 usability issues #17586
Conversation
@@ -59,6 +59,8 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, | |||
# parse url for schema and host | |||
urlparts = match(urlmatcher, url) | |||
schema = urlparts.captures[1] | |||
urlusername = urlparts.captures[4] | |||
urlusername == nothing && (urlusername = "") |
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.
=== nothing
@@ -59,6 +59,8 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, | |||
# parse url for schema and host | |||
urlparts = match(urlmatcher, url) | |||
schema = urlparts.captures[1] | |||
urlusername = urlparts.captures[4] |
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.
usually, username
from url is git
for git+ssh
protocol. You can use username_ptr
parameter to get it.
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.
Sorry, not sure what you're suggesting. The urlusername
only ever gets used by https. Are you saying that libgit2 is already extracting this, so we should just load it from username_ptr
?
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.
ok, for https username_ptr
is NULL
8eb5ce2
to
ff75ca4
Compare
ff75ca4
to
c652593
Compare
I've rebased out the test and will open that as a separate pull request. |
@@ -114,15 +116,23 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, | |||
ENV["SSH_KEY_PATH"] | |||
else | |||
keydefpath = creds[:prvkey, credid] # check if credentials were already used | |||
if keydefpath !== nothing && !isusedcreds | |||
keydefpath == nothing && (keydefpath = "") |
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.
=== nothing
2b7f6ca
to
b4b2b4c
Compare
Encrypted keys have `Proc-Type: 4,ENCRYPTED`, so check for that before requiring a passkey. Part of #17541.
Only prompt for a different private key. If authentication with the first key fails. This can be a little annoying if one wants to use a different private key, but the default one is encrypted, so it queries for a password, without asking for the key first. However, in that case, one probably wants to use the environment variables or the ssh agent anyway, so I'm not too concerned.
b4b2b4c
to
7ba3ebd
Compare
This is WIP, because the test fails non-deterministically and I've been unable to debug (was trying to use rr to figure out what's happening, but it crashes rr). Just putting this up so people can see what part of #17541 is already done. Will get back to fixing the test a little later.