fixed errors, typos & deprecated calls

added comments and documentation

base/libgit2/callbacks.jl

@@ -35,6 +35,8 @@ In addition, if payload has field `used` it can be set to `true` to indicate tha
 Order of credential checks (if supported):
 - ssh key pair (ssh-agent if specified in `payload`)
 - plain text
+
+**Note**: Due to the specifics of `libgit2` authentication procedure, when authentication is failed, this functions is called again without any indication whether authentication was successful or not. In order not to stuck in infinite loop by repeatedly using same faulty credentials, use a call counting strategy to counteract this behaviour.
 """
 function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring,
                               username_ptr::Cstring,
@@ -62,17 +64,17 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring,
     if isset(allowed_types, Cuint(Consts.CREDTYPE_SSH_KEY))
         credid = "ssh://$host"
         # first try ssh-agent if credentials support its usage
-        if (isdefined(creds, :use_ssh_agent) ? getfield(creds, :use_ssh_agent) : 0xFF) == 0x00
+        if isdefined(creds, :use_ssh_agent) && getfield(creds, :use_ssh_agent) == 0x00
             err = ccall((:git_cred_ssh_key_from_agent, :libgit2), Cint,
                          (Ptr{Ptr{Void}}, Cstring), cred, username_ptr)
-            setfield!(creds, :use_ssh_agent, 0x01) # used ssh-agent ones
+            setfield!(creds, :use_ssh_agent, 0x01) # used ssh-agent only one time
             err == 0 && return Cint(0)
         end
 
         errcls, errmsg = Error.last_error()
         if errcls != Error.None
             # Check if we used ssh-agent
-            if (isdefined(creds, :use_ssh_agent) ? getfield(creds, :use_ssh_agent) : 0xFF) == 0x01
+            if isdefined(creds, :use_ssh_agent) && getfield(creds, :use_ssh_agent) == 0x01
                 println("ERROR: $errmsg ssh-agent")
                 setfield!(creds, :use_ssh_agent, 0x02) # reported ssh-agent error
             else
@@ -83,7 +85,7 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring,
         # if username is not provided, then prompt for it
         username = if username_ptr == Cstring(C_NULL)
             uname = creds[:user, credid] # check if credentials were already used
-            uname !== nothing && !isusedcreds ? uname : prompt("Username for `$schema$host'")
+            uname !== nothing && !isusedcreds ? uname : prompt("Username for '$schema$host'")
         else
             bytestring(username_ptr)
         end
@@ -98,7 +100,7 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring,
                 keydef # use cached value
             else
                 keydef = homedir()*"/.ssh/id_rsa.pub"
-                prompt("Public key location for `$schema$username@$host'", default=keydef)
+                prompt("Public key location for '$schema$username@$host'", default=keydef)
             end
         end
         creds[:pubkey, credid] = publickey # save credentials
@@ -111,7 +113,7 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring,
                 keydef # use cached value
             else
                 keydef = homedir()*"/.ssh/id_rsa"
-                prompt("Private key location for `$schema$username@$host'", default=keydef)
+                prompt("Private key location for '$schema$username@$host'", default=keydef)
             end
         end
         creds[:prvkey, credid] = privatekey # save credentials

base/libgit2/types.jl

@@ -58,22 +58,22 @@ abstract AbstractCredentials <: AbstractPayload
 "Returns a credentials parameter"
 function Base.getindex(p::AbstractCredentials, keys...)
     for k in keys
-        ks = symbol(k)
+        ks = Symbol(k)
         isdefined(p, ks) && return getfield(p, ks)
     end
     return nothing
 end
 "Sets credentials with `key` parameter with value"
 function Base.setindex!(p::AbstractCredentials, val, keys...)
     for k in keys
-        ks = symbol(k)
+        ks = Symbol(k)
         isdefined(p, ks) && setfield!(p, ks, val)
     end
     return nothing
 end
-"Check if credentials were used"
+"Checks if credentials were used"
 isused(p::AbstractCredentials) = true
-"Reset credentials for another usage"
+"Resets credentials for another usage"
 reset!(p::AbstractCredentials, cnt::Int=3) = nothing
 
 immutable CheckoutOptions
@@ -638,15 +638,17 @@ type EmptyCredentials <: AbstractCredentials end
 type UserPasswordCredentials <: AbstractCredentials
     user::AbstractString
     pass::AbstractString
-    count::Int
-    use_ssh_agent::UInt8
+    count::Int            # authentication failure protection count
+    use_ssh_agent::UInt8  # used for ssh-agent authentication
     UserPasswordCredentials(u::AbstractString,p::AbstractString) = new(u,p,3,zero(UInt8))
 end
+"Checks if credentials were used or failed authentication, see `LibGit2.credentials_callback`"
 function isused(p::UserPasswordCredentials)
     p.count <= 0 && return true
     p.count -= 1
     return false
 end
+"Resets authentication failure protection count"
 reset!(p::UserPasswordCredentials, cnt::Int=3) = (p.count = cnt)
 
 "Field map for cached credentials type"
@@ -655,10 +657,11 @@ const CachedCredentialsFieldMap = Dict(:user=>1, :pass=>2, :pubkey=>3, :prvkey=>
 "Credentials that support caching"
 type CachedCredentials <: AbstractCredentials
     cred::Dict{AbstractString,Vector{AbstractString}}
-    count::Int
-    use_ssh_agent::UInt8
+    count::Int            # authentication failure protection count
+    use_ssh_agent::UInt8  # used for ssh-agent authentication
     CachedCredentials() = new(Dict{AbstractString,Vector{AbstractString}}(),3,zero(UInt8))
 end
+"Returns specific credential parameter value: first index is a credential parameter name, second index is a host name (with schema)"
 function Base.getindex(p::CachedCredentials, keys...)
     length(keys) != 2 && return nothing
     key, host = keys
@@ -669,16 +672,19 @@ function Base.getindex(p::CachedCredentials, keys...)
     end
     return nothing
 end
+"Sets specific credential parameter value: first index is a credential parameter name, second index is a host name (with schema)"
 function Base.setindex!(p::CachedCredentials, val, keys...)
     length(keys) != 2 && return nothing
     key, host = keys
     !haskey(p.cred, host) && (p.cred[host] = Array(AbstractString, 4))
     haskey(CachedCredentialsFieldMap, key) && return setindex!(p.cred[host], val, CachedCredentialsFieldMap[key])
     return nothing
 end
+"Checks if credentials were used or failed authentication, see `LibGit2.credentials_callback`"
 function isused(p::CachedCredentials)
     p.count <= 0 && return true
     p.count -= 1
     return false
 end
+"Resets authentication failure protection count"
 reset!(p::CachedCredentials, cnt::Int=3) = (p.count = cnt)

base/libgit2/utils.jl

@@ -42,7 +42,8 @@ toggle(val::Integer, flag::Integer) = (val |= flag)
     return ""
 end
 
-@unix_only getpass(prompt::AbstractString) = bytestring(pointer_to_string(ccall(:getpass, Cstring, (Cstring,), prompt), true))
+@unix_only getpass(prompt::AbstractString) =
+    bytestring(pointer_to_string(ccall(:getpass, Cstring, (Cstring,), prompt), true))
 
 function prompt(msg::AbstractString; default::AbstractString="", password::Bool=false)
     msg = !isempty(default) ? msg*" [$default]:" : msg*":"

test/libgit2.jl

@@ -438,21 +438,21 @@ mktempdir() do dir
 
     #@testset "Credentials" begin
         creds = LibGit2.EmptyCredentials()
-        @test isused(creds)
-        @test reset!(creds) === nothing
+        @test LibGit2.isused(creds)
+        @test LibGit2.reset!(creds) === nothing
         @test creds[:user] === nothing
         @test creds[:pass] === nothing
         @test creds[:pubkey, "localhost"] === nothing
 
         creds_user = "USER"
         creds_pass = "PASS"
         creds = LibGit2.UserPasswordCredentials(creds_user, creds_pass)
-        @test !isused(creds)
-        @test !isused(creds)
-        @test !isused(creds)
-        @test isused(creds)
-        @test reset!(creds) == 3
-        @test !isused(creds)
+        @test !LibGit2.isused(creds)
+        @test !LibGit2.isused(creds)
+        @test !LibGit2.isused(creds)
+        @test LibGit2.isused(creds)
+        @test LibGit2.reset!(creds) == 3
+        @test !LibGit2.isused(creds)
         @test creds.count == 2
         @test creds[:user] == creds_user
         @test creds[:pass] == creds_pass
@@ -461,12 +461,12 @@ mktempdir() do dir
         @test creds[:pubkey, "localhost"] === nothing
 
         creds = LibGit2.CachedCredentials()
-        @test !isused(creds)
-        @test !isused(creds)
-        @test !isused(creds)
-        @test isused(creds)
-        @test reset!(creds) == 3
-        @test !isused(creds)
+        @test !LibGit2.isused(creds)
+        @test !LibGit2.isused(creds)
+        @test !LibGit2.isused(creds)
+        @test LibGit2.isused(creds)
+        @test LibGit2.reset!(creds) == 3
+        @test !LibGit2.isused(creds)
         @test creds.count == 2
         @test creds[:user, "localhost"] === nothing
         @test creds[:pass, "localhost"] === nothing