Skip to content

Refactor Network Internal API #1087

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 10, 2015
Merged

Refactor Network Internal API #1087

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
342df4a
refactored `DoFetch` into three seperate methods for clarity and cons…
Jun 9, 2015
dfab508
minor beautification
Jun 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 64 additions & 42 deletions LibGit2Sharp/Network.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public virtual IEnumerable<DirectReference> ListReferences(Remote remote, Creden

using (RemoteSafeHandle remoteHandle = Proxy.git_remote_lookup(repository.Handle, remote.Name, true))
{
var gitCallbacks = new GitRemoteCallbacks {version = 1};
var gitCallbacks = new GitRemoteCallbacks { version = 1 };

if (credentialsProvider != null)
{
Expand Down Expand Up @@ -107,59 +107,81 @@ public virtual IEnumerable<DirectReference> ListReferences(string url)
}
}

static RemoteSafeHandle BuildRemoteSafeHandle(RepositorySafeHandle repoHandle, Remote remote, string url)
static RemoteSafeHandle BuildRemoteSafeHandle(RepositorySafeHandle repoHandle, Remote remote)
{
Debug.Assert((remote == null) ^ (url == null));
Debug.Assert(repoHandle != null && !repoHandle.IsClosed && !repoHandle.IsInvalid);
Debug.Assert(remote != null && remote.Name != null);

RemoteSafeHandle remoteHandle;
RemoteSafeHandle remoteHandle = Proxy.git_remote_lookup(repoHandle, remote.Name, true);
Debug.Assert(remoteHandle != null && !(remoteHandle.IsClosed || remoteHandle.IsInvalid));

if (url != null)
{
remoteHandle = Proxy.git_remote_create_anonymous(repoHandle, url);
}
else
{
remoteHandle = Proxy.git_remote_lookup(repoHandle, remote.Name, true);
}
return remoteHandle;
}

static RemoteSafeHandle BuildRemoteSafeHandle(RepositorySafeHandle repoHandle, string url)
{
Debug.Assert(repoHandle != null && !repoHandle.IsClosed && !repoHandle.IsInvalid);
Debug.Assert(url != null);

RemoteSafeHandle remoteHandle = Proxy.git_remote_create_anonymous(repoHandle, url);
Debug.Assert(remoteHandle != null && !(remoteHandle.IsClosed || remoteHandle.IsInvalid));

return remoteHandle;
}

static void DoFetch(RepositorySafeHandle repoHandle, Remote remote, string url,
FetchOptions options, string logMessage,
IEnumerable<string> refspecs)
static void DoFetch(RepositorySafeHandle repoHandle,
Remote remote,
FetchOptions options,
string logMessage,
IEnumerable<string> refspecs)
{
if (options == null)
using (RemoteSafeHandle remoteHandle = BuildRemoteSafeHandle(repoHandle, remote))
{
options = new FetchOptions();
DoFetch(options, remoteHandle, logMessage, refspecs);
}
}

using (RemoteSafeHandle remoteHandle = BuildRemoteSafeHandle(repoHandle, remote, url))
static void DoFetch(RepositorySafeHandle repoHandle,
string url,
FetchOptions options,
string logMessage,
IEnumerable<string> refspecs)
{
using (RemoteSafeHandle remoteHandle = BuildRemoteSafeHandle(repoHandle, url))
{
var callbacks = new RemoteCallbacks(options);
GitRemoteCallbacks gitCallbacks = callbacks.GenerateCallbacks();
DoFetch(options, remoteHandle, logMessage, refspecs);
}
}

// It is OK to pass the reference to the GitCallbacks directly here because libgit2 makes a copy of
// the data in the git_remote_callbacks structure. If, in the future, libgit2 changes its implementation
// to store a reference to the git_remote_callbacks structure this would introduce a subtle bug
// where the managed layer could move the git_remote_callbacks to a different location in memory,
// but libgit2 would still reference the old address.
//
// Also, if GitRemoteCallbacks were a class instead of a struct, we would need to guard against
// GC occuring in between setting the remote callbacks and actual usage in one of the functions afterwords.
var fetchOptions = new GitFetchOptions
{
RemoteCallbacks = gitCallbacks,
download_tags = Proxy.git_remote_autotag(remoteHandle),
};

if (options.TagFetchMode.HasValue)
{
fetchOptions.download_tags = options.TagFetchMode.Value;
}
private static void DoFetch(FetchOptions options, RemoteSafeHandle remoteHandle, string logMessage, IEnumerable<string> refspecs)
{
Debug.Assert(remoteHandle != null && !remoteHandle.IsClosed && !remoteHandle.IsInvalid);

options = options ?? new FetchOptions();

var callbacks = new RemoteCallbacks(options);
GitRemoteCallbacks gitCallbacks = callbacks.GenerateCallbacks();

// It is OK to pass the reference to the GitCallbacks directly here because libgit2 makes a copy of
// the data in the git_remote_callbacks structure. If, in the future, libgit2 changes its implementation
// to store a reference to the git_remote_callbacks structure this would introduce a subtle bug
// where the managed layer could move the git_remote_callbacks to a different location in memory,
// but libgit2 would still reference the old address.
//
// Also, if GitRemoteCallbacks were a class instead of a struct, we would need to guard against
// GC occuring in between setting the remote callbacks and actual usage in one of the functions afterwords.
var fetchOptions = new GitFetchOptions
{
RemoteCallbacks = gitCallbacks,
download_tags = Proxy.git_remote_autotag(remoteHandle),
};

Proxy.git_remote_fetch(remoteHandle, refspecs, fetchOptions, logMessage);
if (options.TagFetchMode.HasValue)
{
fetchOptions.download_tags = options.TagFetchMode.Value;
}

Proxy.git_remote_fetch(remoteHandle, refspecs, fetchOptions, logMessage);
}

/// <summary>
Expand Down Expand Up @@ -201,7 +223,7 @@ public virtual void Fetch(Remote remote, FetchOptions options, string logMessage
{
Ensure.ArgumentNotNull(remote, "remote");

DoFetch(repository.Handle, remote, null, options, logMessage, new string[0]);
DoFetch(repository.Handle, remote, options, logMessage, new string[0]);
}

/// <summary>
Expand Down Expand Up @@ -248,7 +270,7 @@ public virtual void Fetch(Remote remote, IEnumerable<string> refspecs, FetchOpti
Ensure.ArgumentNotNull(remote, "remote");
Ensure.ArgumentNotNull(refspecs, "refspecs");

DoFetch(repository.Handle, remote, null, options, logMessage, refspecs);
DoFetch(repository.Handle, remote, options, logMessage, refspecs);
}

/// <summary>
Expand Down Expand Up @@ -307,7 +329,7 @@ public virtual void Fetch(
Ensure.ArgumentNotNull(url, "url");
Ensure.ArgumentNotNull(refspecs, "refspecs");

DoFetch(repository.Handle, null, url, options, logMessage, refspecs);
DoFetch(repository.Handle, url, options, logMessage, refspecs);
}

/// <summary>
Expand Down