-
-
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
Misc fixes for LibGit2 #18066
Misc fixes for LibGit2 #18066
Conversation
An SSH URL with a path uses a colon between the host and the path: git@github.com:JuliaLang/Example.jl (correct) git@github.com/JuliaLang/Example.jl (incorrect)
@@ -160,7 +160,7 @@ function authenticate_userpass(creds::UserPasswordCredentials, libgit2credptr::P | |||
urlusername : username) | |||
userpass = prompt("Password for '$schema$username@$host'", password=true) | |||
end | |||
(creds.user != username) || (creds.pass != userpass) && reset!(creds) | |||
((creds.user != username) || (creds.pass != userpass)) && reset!(creds) |
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.
Fixes operator precedence issue. Mirrors 1d5a427
LGTM, thanks! |
(?<host>[A-Za-z0-9\-\.]+) | ||
(?:\:(?<port>\d+)?)? | ||
(?<path>.*?)$ | ||
"""x |
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.
Fixes several things with the original regex including:
- Doesn't fail or absorb password information into the username
- SSH URLs would have the colon included in the path capture. e.g. "git@github.com:JuliaLang/Example.jl" would have the path ":JuliaLang/Example.jl"
- Allows more complicated usernames including ones using hypens
- Switched to using named groups for clarity
Test was only working for environments which had GitHub SSH keys setup.
Added a new explicit SSH protocol test which generated other changes.
Travis failure is unrelated to the changes. PR should be ready to go. |
No description provided.