LibGit2: allow using a void ptr for payload and add git_transfer_progress struct

[KristofferC]

I want to be able to send in an arbitrary pointer for the payload but the current constructor has too tight types.

Also adds the git_transfer_progress struct https://libgit2.github.com/libgit2/#HEAD/type/git_transfer_progress.
Useful if one wants to print some progress bar when cloning.

cc @Keno

[omus]

I believe you won't be able to use the progress feature and with our credential system at the same time. Would be worth trying this out.

[omus]

Potentially you could make GitTransferProgress a subtype of Payload

[KristofferC]

Hm, I'm not really using it as a Payload though. The relevant code is at https://github.com/JuliaLang/Pkg3.jl/pull/177/files#diff-ce43f67c143e1d23513f24ec8464985eR61 and it makes a progress bar print when cloning something. The payload is just any julia object that I want to have access to in the callback.

[omus]

Thanks for the link. I think this struct can stay as is.

[omus]

The relevant code is at https://github.com/JuliaLang/Pkg3.jl/pull/177/files#diff-ce43f67c143e1d23513f24ec8464985eR61 and it makes a progress bar print when cloning something. The payload is just any julia object that I want to have access to in the callback.

Unfortunately the way you are using LibGit2.clone you're bypassing our credential handling which means that function won't work for repos that require credentials. See

julia/stdlib/LibGit2/src/LibGit2.jl

Lines 524 to 551 in 4947e25

	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())
	# 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))
	clone_opts = CloneOptions(
	bare = Cint(isbare),
	checkout_branch = isempty(lbranch) ? Cstring(C_NULL) : Base.unsafe_convert(Cstring, lbranch),
	fetch_opts = fetch_opts,
	remote_cb = remote_cb
	)
	repo = try
	clone(repo_url, repo_path, clone_opts)
	catch err
	if isa(err, GitError) && err.code == Error.EAUTH
	reject(payload)
	end
	rethrow()
	end
	end
	approve(payload)
	return repo
	end

[KristofferC]

Ok, but this should just require some API re-organization? It seems not so nice for us to hijack very valuable slots. Couldn't the credentials and the user-passed payload be together, and the credentials callback takes care of the credentials and the user-passed callback is wrapped so that the credentials are removed before calling it... Or something like that.

[omus]

Yes, we just need to make some additional API changes. Some extra work but nothing too difficult. I would just hate to lose credential handling in Pkg3 for a progress bar.

The one trick here is both the credential callback and your progress meter are both trying to set the payload. We may need to have a special payload type which can isolate some payload data for an individual callback.

[KristofferC]

I would just hate to lose credential handling in Pkg3 for a progress bar.

I would hate to lose the pretty progress bar for boring credential handling :D

[omus]

@KristofferC Do you want to take a crack at supporting both of these? I may have some time on the weekend.

[KristofferC]

You are much more familiar with the code so if you have some time that would be great. Otherwise I will probably try get to it at some point because "I ai' losin' ma progress ba'".

[omus]

I've made some attempts to address this within LibGit2. I am not quite satisfied with the changes so far but I do know we'll have to make some minor changes to Pkg3 so that it is aware of credentials. For example Pkg3 will need to create a CachedCredentials instance to temporarily retain credentials between multiple git clones.

I'm hoping to have some changes ready for tomorrow.

[KristofferC]

Thanks for working on this. I do think that a progress bar is a significant boost to the user experience when you need to get a large repo.

[omus]

Would it make sense to include the progress bar as part of LibGit2?

[KristofferC]

I was thinking about that too. On one hand, it makes sense, anyone can take advantage of it, but on the other hand, it opens up a whole can of bikeshedding opportunities. (Exactly what should be displayed, how should the progress bar look, what should the colors be, etc etc).

[KristofferC]

@omus Is #26437 a replacement to this or to be merged together? The reason I ask is because #26437 does not include the GitTransferProgress definition.

[omus]

#26437 addresses the Payload to Ptr{Cvoid} part of this PR only.