Support separate payloads for RemoteCallbacks (#26437)

* Deprecate LibGit2 keyword: payload

LibGit2 functions `clone`, `fetch`, and `push` all used a keyword named
`payload` which would only take credential information. The keyword was
renamed to `credentials` to be more accurate and the original keyword
is now deprecated.

* Support separate payloads for RemoteCallbacks

Make it easier to support mixing user created callbacks and default
callbacks defined by LibGit2.

* Callbacks documentation

* Add test for credential callback conflict

* Add TransferProgress struct

* Add online callback test

Author: omus

stdlib/LibGit2/src/LibGit2.jl

@@ -259,33 +259,47 @@ The keyword arguments are:
   * `remoteurl::AbstractString=""`: the URL of `remote`. If not specified,
     will be assumed based on the given name of `remote`.
   * `refspecs=AbstractString[]`: determines properties of the fetch.
-  * `payload=CredentialPayload()`: provides credentials and/or settings when authenticating
-    against a private `remote`.
+  * `credentials=nothing`: provides credentials and/or settings when authenticating against
+    a private `remote`.
+  * `callbacks=Callbacks()`: user provided callbacks and payloads.
 
 Equivalent to `git fetch [<remoteurl>|<repo>] [<refspecs>]`.
 """
 function fetch(repo::GitRepo; remote::AbstractString="origin",
                remoteurl::AbstractString="",
                refspecs::Vector{<:AbstractString}=AbstractString[],
-               payload::Union{CredentialPayload, AbstractCredential, CachedCredentials, Nothing}=CredentialPayload())
-    p = reset!(deprecate_nullable_creds(:fetch, "repo", payload), GitConfig(repo))
+               payload::Creds=nothing,
+               credentials::Creds=payload,
+               callbacks::Callbacks=Callbacks())
     rmt = if isempty(remoteurl)
         get(GitRemote, repo, remote)
     else
         GitRemoteAnon(repo, remoteurl)
     end
+
+    deprecate_payload_keyword(:fetch, "repo", payload)
+    cred_payload = reset!(CredentialPayload(credentials), GitConfig(repo))
+    if !haskey(callbacks, :credentials)
+        callbacks[:credentials] = (credentials_cb(), cred_payload)
+    elseif haskey(callbacks, :credentials) && credentials !== nothing
+        throw(ArgumentError(string(
+            "Unable to both use the provided `credentials` as a payload when the ",
+            "`callbacks` also contain a credentials payload.")))
+    end
+
     result = try
-        fo = FetchOptions(callbacks=RemoteCallbacks(credentials=credentials_cb(), payload=p))
-        fetch(rmt, refspecs, msg="from $(url(rmt))", options = fo)
+        remote_callbacks = RemoteCallbacks(callbacks)
+        fo = FetchOptions(callbacks=remote_callbacks)
+        fetch(rmt, refspecs, msg="from $(url(rmt))", options=fo)
     catch err
         if isa(err, GitError) && err.code == Error.EAUTH
-            reject(payload)
+            reject(cred_payload)
         end
         rethrow()
     finally
         close(rmt)
     end
-    approve(payload)
+    approve(cred_payload)
     return result
 end
 
@@ -300,34 +314,48 @@ The keyword arguments are:
   * `refspecs=AbstractString[]`: determines properties of the push.
   * `force::Bool=false`: determines if the push will be a force push,
      overwriting the remote branch.
-  * `payload=CredentialPayload()`: provides credentials and/or settings when authenticating
-    against a private `remote`.
+  * `credentials=nothing`: provides credentials and/or settings when authenticating against
+     a private `remote`.
+  * `callbacks=Callbacks()`: user provided callbacks and payloads.
 
 Equivalent to `git push [<remoteurl>|<repo>] [<refspecs>]`.
 """
 function push(repo::GitRepo; remote::AbstractString="origin",
               remoteurl::AbstractString="",
               refspecs::Vector{<:AbstractString}=AbstractString[],
               force::Bool=false,
-              payload::Union{CredentialPayload, AbstractCredential, CachedCredentials, Nothing}=CredentialPayload())
-    p = reset!(deprecate_nullable_creds(:push, "repo", payload), GitConfig(repo))
+              payload::Creds=nothing,
+              credentials::Creds=payload,
+              callbacks::Callbacks=Callbacks())
     rmt = if isempty(remoteurl)
         get(GitRemote, repo, remote)
     else
         GitRemoteAnon(repo, remoteurl)
     end
+
+    deprecate_payload_keyword(:push, "repo", payload)
+    cred_payload = reset!(CredentialPayload(credentials), GitConfig(repo))
+    if !haskey(callbacks, :credentials)
+        callbacks[:credentials] = (credentials_cb(), cred_payload)
+    elseif haskey(callbacks, :credentials) && credentials !== nothing
+        throw(ArgumentError(string(
+            "Unable to both use the provided `credentials` as a payload when the ",
+            "`callbacks` also contain a credentials payload.")))
+    end
+
     result = try
-        push_opts = PushOptions(callbacks=RemoteCallbacks(credentials=credentials_cb(), payload=p))
+        remote_callbacks = RemoteCallbacks(callbacks)
+        push_opts = PushOptions(callbacks=remote_callbacks)
         push(rmt, refspecs, force=force, options=push_opts)
     catch err
         if isa(err, GitError) && err.code == Error.EAUTH
-            reject(payload)
+            reject(cred_payload)
         end
         rethrow()
     finally
         close(rmt)
     end
-    approve(payload)
+    approve(cred_payload)
     return result
 end
 
@@ -507,8 +535,9 @@ The keyword arguments are:
   * `remote_cb::Ptr{Cvoid}=C_NULL`: a callback which will be used to create the remote
     before it is cloned. If `C_NULL` (the default), no attempt will be made to create
     the remote - it will be assumed to already exist.
-  * `payload::CredentialPayload=CredentialPayload()`: provides credentials and/or settings
-    when authenticating against a private repository.
+  * `credentials::Creds=nothing`: provides credentials and/or settings when authenticating
+    against a private repository.
+  * `callbacks::Callbacks=Callbacks()`: user provided callbacks and payloads.
 
 Equivalent to `git clone [-b <branch>] [--bare] <repo_url> <repo_path>`.
 
@@ -525,12 +554,24 @@ function clone(repo_url::AbstractString, repo_path::AbstractString;
                branch::AbstractString="",
                isbare::Bool = false,
                remote_cb::Ptr{Cvoid} = C_NULL,
-               payload::Union{CredentialPayload, AbstractCredential, CachedCredentials, Nothing}=CredentialPayload())
+               payload::Creds=nothing,
+               credentials::Creds=payload,
+               callbacks::Callbacks=Callbacks())
+    deprecate_payload_keyword(:clone, "repo_url, repo_path", payload)
+    cred_payload = reset!(CredentialPayload(credentials))
+    if !haskey(callbacks, :credentials)
+        callbacks[:credentials] = (credentials_cb(), cred_payload)
+    elseif haskey(callbacks, :credentials) && credentials !== nothing
+        throw(ArgumentError(string(
+            "Unable to both use the provided `credentials` as a payload when the ",
+            "`callbacks` also contain a credentials payload.")))
+    end
+
     # setup clone options
     lbranch = Base.cconvert(Cstring, branch)
     GC.@preserve lbranch begin
-        p = reset!(deprecate_nullable_creds(:clone, "repo_url, repo_path", payload))
-        fetch_opts = FetchOptions(callbacks = RemoteCallbacks(credentials=credentials_cb(), payload=p))
+        remote_callbacks = RemoteCallbacks(callbacks)
+        fetch_opts = FetchOptions(callbacks=remote_callbacks)
         clone_opts = CloneOptions(
                     bare = Cint(isbare),
                     checkout_branch = isempty(lbranch) ? Cstring(C_NULL) : Base.unsafe_convert(Cstring, lbranch),
@@ -541,12 +582,12 @@ function clone(repo_url::AbstractString, repo_path::AbstractString;
             clone(repo_url, repo_path, clone_opts)
         catch err
             if isa(err, GitError) && err.code == Error.EAUTH
-                reject(payload)
+                reject(cred_payload)
             end
             rethrow()
         end
     end
-    approve(payload)
+    approve(cred_payload)
     return repo
 end
 

stdlib/LibGit2/src/callbacks.jl

@@ -261,13 +261,9 @@ For addition details see the LibGit2 guide on
 """
 function credentials_callback(libgit2credptr::Ptr{Ptr{Cvoid}}, url_ptr::Cstring,
                               username_ptr::Cstring,
-                              allowed_types::Cuint, payload_ptr::Ptr{Cvoid})
+                              allowed_types::Cuint, p::CredentialPayload)
     err = Cint(0)
 
-    # get `CredentialPayload` object from payload pointer
-    @assert payload_ptr != C_NULL
-    p = unsafe_pointer_to_objref(payload_ptr)::CredentialPayload
-
     # Parse URL only during the first call to this function. Future calls will use the
     # information cached inside the payload.
     if isempty(p.url)
@@ -340,6 +336,13 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Cvoid}}, url_ptr::Cstring,
     return err
 end
 
+function credentials_callback(libgit2credptr::Ptr{Ptr{Cvoid}}, url_ptr::Cstring,
+                              username_ptr::Cstring, allowed_types::Cuint,
+                              payloads::Dict)
+    p = payloads[:credentials]
+    credentials_callback(libgit2credptr, url_ptr, username_ptr, allowed_types, p)
+end
+
 function fetchhead_foreach_callback(ref_name::Cstring, remote_url::Cstring,
                         oid_ptr::Ptr{GitHash}, is_merge::Cuint, payload::Ptr{Cvoid})
     fhead_vec = unsafe_pointer_to_objref(payload)::Vector{FetchHead}
@@ -351,6 +354,6 @@ end
 "C function pointer for `mirror_callback`"
 mirror_cb() = cfunction(mirror_callback, Cint, Tuple{Ptr{Ptr{Cvoid}}, Ptr{Cvoid}, Cstring, Cstring, Ptr{Cvoid}})
 "C function pointer for `credentials_callback`"
-credentials_cb() = cfunction(credentials_callback, Cint, Tuple{Ptr{Ptr{Cvoid}}, Cstring, Cstring, Cuint, Ptr{Cvoid}})
+credentials_cb() = cfunction(credentials_callback, Cint, Tuple{Ptr{Ptr{Cvoid}}, Cstring, Cstring, Cuint, Any})
 "C function pointer for `fetchhead_foreach_callback`"
 fetchhead_foreach_cb() = cfunction(fetchhead_foreach_callback, Cint, Tuple{Cstring, Cstring, Ptr{GitHash}, Cuint, Ptr{Cvoid}})

stdlib/LibGit2/src/deprecated.jl

@@ -24,30 +24,15 @@ function prompt(msg::AbstractString; default::AbstractString="", password::Bool=
     coalesce(Base.prompt(msg, default=default, password=password), "")
 end
 
-# PR #23640
-# when this deprecation is deleted, remove all calls to it, and replace all keywords of:
-# `payload::Union{CredentialPayload, AbstractCredential, CachedCredentials, Nothing}`
-#  with `payload::CredentialPayload` from base/libgit2/libgit2.jl
-function deprecate_nullable_creds(f, sig, payload)
-    if isa(payload, Union{AbstractCredential, CachedCredentials, Nothing})
-        # Note: Be careful not to show the contents of the credentials as it could reveal a
-        # password.
-        if payload === nothing
-            msg = "`LibGit2.$f($sig; payload=nothing)` is deprecated, use "
-            msg *= "`LibGit2.$f($sig; payload=LibGit2.CredentialPayload())` instead."
-            p = CredentialPayload()
-        else
-            cred = payload
-            C = typeof(cred)
-            msg = "`LibGit2.$f($sig; payload=$C(...))` is deprecated, use "
-            msg *= "`LibGit2.$f($sig; payload=LibGit2.CredentialPayload($C(...)))` instead."
-            p = CredentialPayload(cred)
-        end
-        Base.depwarn(msg, f)
-    else
-        p = payload::CredentialPayload
+# PR #26437
+# when this deprecation is deleted, remove all calls to it, and remove the keyword of:
+# `payload` from "src/LibGit2.jl"
+function deprecate_payload_keyword(f, sig, payload)
+    if payload !== nothing
+        Base.depwarn(string(
+            "`LibGit2.$f($sig; payload=cred)` is deprecated, use ",
+            "`LibGit2.$f($sig; credentials=cred)` instead."), f)
     end
-    return p
 end
 
 @deprecate get_creds!(cache::CachedCredentials, credid, default) get!(cache, credid, default)

stdlib/LibGit2/src/types.jl

@@ -188,7 +188,21 @@ The fields represent:
     perfdata_payload::Ptr{Cvoid}
 end
 
-abstract type Payload end
+"""
+    LibGit2.TransferProgress
+
+Transfer progress information used by the `transfer_progress` remote callback.
+Matches the [`git_transfer_progress`](https://libgit2.github.com/libgit2/#HEAD/type/git_transfer_progress) struct.
+"""
+@kwdef struct TransferProgress
+    total_objects::Cuint
+    indexed_objects::Cuint
+    received_objects::Cuint
+    local_objects::Cuint
+    total_deltas::Cuint
+    indexed_deltas::Cuint
+    received_bytes::Csize_t
+end
 
 @kwdef struct RemoteCallbacksStruct
     version::Cuint                    = 1
@@ -206,6 +220,28 @@ abstract type Payload end
     payload::Ptr{Cvoid}
 end
 
+"""
+    LibGit2.Callbacks
+
+A dictionary which containing the callback name as the key and the value as a tuple of the
+callback function and payload.
+
+The `Callback` dictionary to construct `RemoteCallbacks` allows each callback to use a
+distinct payload. Each callback, when called, will receive `Dict` which will hold the
+callback's custom payload which can be accessed using the callback name.
+
+# Examples
+```julia
+julia> c = LibGit2.Callbacks(:credentials => (LibGit2.credentials_cb(), LibGit2.CredentialPayload()));
+
+julia> LibGit2.clone(url, callbacks=c);
+```
+
+See [`git_remote_callbacks`](https://libgit2.github.com/libgit2/#HEAD/type/git_remote_callbacks)
+for details on supported callbacks.
+"""
+const Callbacks = Dict{Symbol, Tuple{Ptr{Cvoid}, Any}}
+
 """
     LibGit2.RemoteCallbacks
 
@@ -215,17 +251,27 @@ Matches the [`git_remote_callbacks`](https://libgit2.github.com/libgit2/#HEAD/ty
 struct RemoteCallbacks
     cb::RemoteCallbacksStruct
     gcroot::Ref{Any}
-    function RemoteCallbacks(; payload::Union{Payload, Nothing}=nothing, kwargs...)
+
+    function RemoteCallbacks(; version::Cuint=Cuint(1), payload=C_NULL, callbacks...)
         p = Ref{Any}(payload)
-        if payload === nothing
-            pp = C_NULL
-        else
-            pp = unsafe_load(Ptr{Ptr{Cvoid}}(Base.unsafe_convert(Ptr{Any}, p)))
-        end
-        return new(RemoteCallbacksStruct(; kwargs..., payload=pp), p)
+        pp = unsafe_load(Ptr{Ptr{Cvoid}}(Base.unsafe_convert(Ptr{Any}, p)))
+        return new(RemoteCallbacksStruct(; version=version, payload=pp, callbacks...), p)
     end
 end
 
+function RemoteCallbacks(c::Callbacks)
+    callbacks = Dict{Symbol, Ptr{Cvoid}}()
+    payloads = Dict{Symbol, Any}()
+
+    for (name, (callback, payload)) in c
+        callbacks[name] = callback
+        payloads[name] = payload
+    end
+
+    RemoteCallbacks(; payload=payloads, callbacks...)
+end
+
+
 """
     LibGit2.ProxyOptions
 
@@ -1250,7 +1296,7 @@ Retains the state between multiple calls to the credential callback for the same
 A `CredentialPayload` instance is expected to be `reset!` whenever it will be used with a
 different URL.
 """
-mutable struct CredentialPayload <: Payload
+mutable struct CredentialPayload
     explicit::Union{AbstractCredential, Nothing}
     cache::Union{CachedCredentials, Nothing}
     allow_ssh_agent::Bool    # Allow the use of the SSH agent to get credentials
@@ -1293,6 +1339,9 @@ function CredentialPayload(cache::CachedCredentials; kwargs...)
     CredentialPayload(nothing, cache; kwargs...)
 end
 
+CredentialPayload(p::CredentialPayload) = p
+
+
 """
     reset!(payload, [config]) -> CredentialPayload
 
@@ -1364,3 +1413,6 @@ function reject(p::CredentialPayload; shred::Bool=true)
     shred && securezero!(cred)
     nothing
 end
+
+# Useful for functions which can handle various kinds of credentials
+const Creds = Union{CredentialPayload, AbstractCredential, CachedCredentials, Nothing}

stdlib/LibGit2/test/libgit2-helpers.jl

@@ -1,7 +1,7 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
-import LibGit2: AbstractCredential, UserPasswordCredential, SSHCredential,
-    CachedCredentials, CredentialPayload, Payload
+using LibGit2: AbstractCredential, UserPasswordCredential, SSHCredential,
+    CachedCredentials, CredentialPayload
 using Base: coalesce
 
 const DEFAULT_PAYLOAD = CredentialPayload(allow_ssh_agent=false, allow_git_helpers=false)