Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Update to use modern version of LibGit2Sharp and use indent heuristic when diffing #2142

Merged
merged 26 commits into from
Mar 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e8d9527
Update to be compatible with LigGit2Sharp 0.26
jcansdale Dec 18, 2018
4658122
Fix tests that use LibGit2Sharp
jcansdale Dec 18, 2018
b0c037b
Merge branch 'master' into fixes/1469-indent-heuristic
jcansdale Dec 18, 2018
8b6a488
Remove obsolete IQueryableCommitLog.QueryBy imple
jcansdale Dec 18, 2018
32c4aac
Update to use LibGit2Sharp 0.26.0-preview-0080
jcansdale Dec 18, 2018
bbc0504
Commit[] no longer accepts \ in path
jcansdale Dec 19, 2018
128cfbe
Remove unused defaultOriginName field
jcansdale Jan 7, 2019
057844a
Enable IndentHeuristic when comparing
jcansdale Jan 8, 2019
62ccc9b
Move Compare and CompareWith to IGitService
jcansdale Jan 8, 2019
e6ebab6
Add failing Indent_Heuristic_Is_Enabled test
jcansdale Jan 8, 2019
e0eb92c
Add tests for GitService.CompareWith
jcansdale Jan 8, 2019
c222de2
Enable IndentHeuristic and make tests pass
jcansdale Jan 8, 2019
d95149d
Move IGitClient.Compare method to IGitService
jcansdale Jan 8, 2019
df677af
Add test for Compare that returns TreeChanges
jcansdale Jan 8, 2019
ca6de27
Merge branch 'master' into fixes/1469-indent-heuristic
jcansdale Jan 9, 2019
b348527
Add failing test for GitService.Compare
jcansdale Jan 9, 2019
cc43965
Change to use / separator for Git path
jcansdale Jan 9, 2019
7362f56
Merge branch 'master' into fixes/1469-indent-heuristic
jcansdale Jan 9, 2019
f685a22
Throw ArgumentException if path contains \
jcansdale Jan 9, 2019
346bfe1
Factor out Guard.ArgumentIsGitPath
jcansdale Jan 9, 2019
7d87a40
Add Guard.ArgumentIsGitPath to appropriate methods
jcansdale Jan 9, 2019
7006897
Fix issue where a Windows path is being used
jcansdale Jan 9, 2019
8c5d1f7
Add guards to HasChangesInWorkingDirectory
jcansdale Jan 9, 2019
6fa16e8
Merge branch 'master' into fixes/1469-indent-heuristic
jcansdale Mar 19, 2019
f9c9cce
Updatre to use LibGit2Sharp 0.26.0 final
jcansdale Mar 19, 2019
f14b646
Merge branch 'master' into fixes/1469-indent-heuristic
jcansdale Mar 22, 2019
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
3 changes: 1 addition & 2 deletions src/GitHub.App/GitHub.App.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="LibGit2Sharp" Version="0.23.1" />
<PackageReference Include="LibGit2Sharp.NativeBinaries" Version="1.0.164" />
<PackageReference Include="LibGit2Sharp" Version="0.26.0" />
<PackageReference Include="Madskristensen.VisualStudio.SDK" Version="14.3.75-pre" />
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.6.1" />
<PackageReference Include="Microsoft.VisualStudio.StaticReviews.Embeddable" Version="0.1.14-alpha" />
Expand Down
3 changes: 3 additions & 0 deletions src/GitHub.App/SampleData/GitServiceDesigner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@ class GitServiceDesigner : IGitService
public IRepository GetRepository(string path) => null;
public UriString GetUri(string path, string remote = "origin") => null;
public UriString GetUri(IRepository repository, string remote = "origin") => null;
public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path) => null;
public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents) => null;
public Task<TreeChanges> Compare(IRepository repository, string sha1, string sha2, bool detectRenames = false) => null;
}
}
173 changes: 42 additions & 131 deletions src/GitHub.App/Services/GitClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ namespace GitHub.Services
[PartCreationPolicy(CreationPolicy.Shared)]
public class GitClient : IGitClient
{
const string defaultOriginName = "origin";
static readonly ILogger log = LogManager.ForContext<GitClient>();
readonly IGitService gitService;
readonly PullOptions pullOptions;
Expand All @@ -44,12 +43,17 @@ public GitClient(IGitHubCredentialProvider credentialProvider, IGitService gitSe
public Task Pull(IRepository repository)
{
Guard.ArgumentNotNull(repository, nameof(repository));
return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var signature = repository.Config.BuildSignature(DateTimeOffset.UtcNow);
#pragma warning disable 0618 // TODO: Replace `Network.Pull` with `Commands.Pull`.
repository.Network.Pull(signature, pullOptions);
#pragma warning restore 0618
if (repository is Repository repo)
{
LibGit2Sharp.Commands.Pull(repo, signature, pullOptions);
}
else
{
log.Error("Couldn't pull because {Variable} isn't an instance of {Type}", nameof(repository), typeof(Repository));
}
});
}

Expand All @@ -59,7 +63,7 @@ public Task Push(IRepository repository, string branchName, string remoteName)
Guard.ArgumentNotEmptyString(branchName, nameof(branchName));
Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
if (repository.Head?.Commits != null && repository.Head.Commits.Any())
{
Expand All @@ -75,14 +79,11 @@ public Task Fetch(IRepository repository, string remoteName)
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
try
{
var remote = repository.Network.Remotes[remoteName];
#pragma warning disable 0618 // TODO: Replace `Network.Fetch` with `Commands.Fetch`.
repository.Network.Fetch(remote, fetchOptions);
#pragma warning restore 0618
repository.Network.Fetch(remoteName, new[] { "+refs/heads/*:refs/remotes/origin/*" }, fetchOptions);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess things have changed since this warning was disabled...

}
catch (Exception ex)
{
Expand All @@ -104,7 +105,7 @@ public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs
}
}

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
try
{
Expand All @@ -114,17 +115,15 @@ public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs
var removeRemote = false;
if (repo.Network.Remotes[remoteName] != null)
{
// If a remote with this neme already exists, use a unique name and remove remote afterwards
// If a remote with this name already exists, use a unique name and remove remote afterwards
remoteName = cloneUrl.Owner + "-" + Guid.NewGuid();
removeRemote = true;
}

var remote = repo.Network.Remotes.Add(remoteName, remoteUri.ToString());
repo.Network.Remotes.Add(remoteName, remoteUri.ToString());
try
{
#pragma warning disable 0618 // TODO: Replace `Network.Fetch` with `Commands.Fetch`.
repo.Network.Fetch(remote, refspecs, fetchOptions);
#pragma warning restore 0618
repo.Network.Fetch(remoteName, refspecs, fetchOptions);
}
finally
{
Expand All @@ -149,14 +148,11 @@ public Task Fetch(IRepository repository, string remoteName, params string[] ref
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
try
{
var remote = repository.Network.Remotes[remoteName];
#pragma warning disable 0618 // TODO: Replace `Network.Fetch` with `Commands.Fetch`.
repository.Network.Fetch(remote, refspecs, fetchOptions);
#pragma warning restore 0618
repository.Network.Fetch(remoteName, refspecs, fetchOptions);
}
catch (Exception ex)
{
Expand All @@ -173,117 +169,32 @@ public Task Checkout(IRepository repository, string branchName)
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(branchName, nameof(branchName));

return Task.Factory.StartNew(() =>
{
#pragma warning disable 0618 // TODO: Replace `IRepository.Checkout` with `Commands.Checkout`.
repository.Checkout(branchName);
#pragma warning restore 0618
});
}

public async Task<bool> CommitExists(IRepository repository, string sha)
{
return await Task.Run(() => repository.Lookup<Commit>(sha) != null).ConfigureAwait(false);
}

public Task CreateBranch(IRepository repository, string branchName)
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(branchName, nameof(branchName));

return Task.Factory.StartNew(() =>
{
repository.CreateBranch(branchName);
});
}

public Task<TreeChanges> Compare(
IRepository repository,
string sha1,
string sha2,
bool detectRenames)
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
Guard.ArgumentNotEmptyString(sha2, nameof(sha2));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var options = new CompareOptions
if (repository is Repository repo)
{
Similarity = detectRenames ? SimilarityOptions.Renames : SimilarityOptions.None
};

var commit1 = repository.Lookup<Commit>(sha1);
var commit2 = repository.Lookup<Commit>(sha2);

if (commit1 != null && commit2 != null)
{
return repository.Diff.Compare<TreeChanges>(commit1.Tree, commit2.Tree, options);
LibGit2Sharp.Commands.Checkout(repo, branchName);
}
else
{
return null;
log.Error("Couldn't checkout because {Variable} isn't an instance of {Type}", nameof(repository), typeof(Repository));
}
});
}

public Task<Patch> Compare(
IRepository repository,
string sha1,
string sha2,
string path)
public async Task<bool> CommitExists(IRepository repository, string sha)
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
Guard.ArgumentNotEmptyString(sha2, nameof(sha2));
Guard.ArgumentNotEmptyString(path, nameof(path));

return Task.Factory.StartNew(() =>
{
var commit1 = repository.Lookup<Commit>(sha1);
var commit2 = repository.Lookup<Commit>(sha2);

if (commit1 != null && commit2 != null)
{
return repository.Diff.Compare<Patch>(
commit1.Tree,
commit2.Tree,
new[] { path });
}
else
{
return null;
}
});
return await Task.Run(() => repository.Lookup<Commit>(sha) != null).ConfigureAwait(false);
}

public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents)
public Task CreateBranch(IRepository repository, string branchName)
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
Guard.ArgumentNotEmptyString(sha2, nameof(sha1));
Guard.ArgumentNotEmptyString(path, nameof(path));
Guard.ArgumentNotEmptyString(branchName, nameof(branchName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var commit1 = repository.Lookup<Commit>(sha1);
var commit2 = repository.Lookup<Commit>(sha2);

var treeChanges = repository.Diff.Compare<TreeChanges>(commit1.Tree, commit2.Tree);
var normalizedPath = path.Replace("/", "\\");
var renamed = treeChanges.FirstOrDefault(x => x.Path == normalizedPath);
var oldPath = renamed?.OldPath ?? path;

if (commit1 != null)
{
var contentStream = contents != null ? new MemoryStream(contents) : new MemoryStream();
var blob1 = commit1[oldPath]?.Target as Blob ?? repository.ObjectDatabase.CreateBlob(new MemoryStream());
var blob2 = repository.ObjectDatabase.CreateBlob(contentStream, path);
return repository.Diff.Compare(blob1, blob2);
}

return null;
repository.CreateBranch(branchName);
});
}

Expand All @@ -292,7 +203,7 @@ public Task<T> GetConfig<T>(IRepository repository, string key)
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(key, nameof(key));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var result = repository.Config.Get<T>(key);
return result != null ? result.Value : default(T);
Expand All @@ -305,7 +216,7 @@ public Task SetConfig(IRepository repository, string key, string value)
Guard.ArgumentNotEmptyString(key, nameof(key));
Guard.ArgumentNotEmptyString(value, nameof(value));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
repository.Config.Set(key, value);
});
Expand All @@ -316,7 +227,7 @@ public Task SetRemote(IRepository repository, string remoteName, Uri url)
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
repository.Config.Set("remote." + remoteName + ".url", url.ToString());
repository.Config.Set("remote." + remoteName + ".fetch", "+refs/heads/*:refs/remotes/" + remoteName + "/*");
Expand All @@ -329,7 +240,7 @@ public Task SetTrackingBranch(IRepository repository, string branchName, string
Guard.ArgumentNotEmptyString(branchName, nameof(branchName));
Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var remoteBranchName = IsCanonical(remoteName) ? remoteName : "refs/remotes/" + remoteName + "/" + branchName;
var remoteBranch = repository.Branches[remoteBranchName];
Expand All @@ -347,7 +258,7 @@ public Task UnsetConfig(IRepository repository, string key)
{
Guard.ArgumentNotEmptyString(key, nameof(key));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
repository.Config.Unset(key);
});
Expand All @@ -358,7 +269,7 @@ public Task<Remote> GetHttpRemote(IRepository repo, string remote)
Guard.ArgumentNotNull(repo, nameof(repo));
Guard.ArgumentNotEmptyString(remote, nameof(remote));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var uri = gitService.GetRemoteUri(repo, remote);
var remoteName = uri.IsHypertextTransferProtocol ? remote : remote + "-http";
Expand All @@ -373,9 +284,9 @@ public Task<string> ExtractFile(IRepository repository, string commitSha, string
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(commitSha, nameof(commitSha));
Guard.ArgumentNotEmptyString(fileName, nameof(fileName));
Guard.ArgumentIsGitPath(fileName, nameof(fileName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var commit = repository.Lookup<Commit>(commitSha);
if (commit == null)
Expand All @@ -392,9 +303,9 @@ public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha,
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(commitSha, nameof(commitSha));
Guard.ArgumentNotEmptyString(fileName, nameof(fileName));
Guard.ArgumentIsGitPath(fileName, nameof(fileName));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var commit = repository.Lookup<Commit>(commitSha);
if (commit == null)
Expand All @@ -421,9 +332,9 @@ public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha,
public Task<bool> IsModified(IRepository repository, string path, byte[] contents)
{
Guard.ArgumentNotNull(repository, nameof(repository));
Guard.ArgumentNotEmptyString(path, nameof(path));
Guard.ArgumentIsGitPath(path, nameof(path));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
if (repository.RetrieveStatus(path) == FileStatus.Unaltered)
{
Expand Down Expand Up @@ -491,7 +402,7 @@ public Task<bool> IsHeadPushed(IRepository repo)
{
Guard.ArgumentNotNull(repo, nameof(repo));

return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
return repo.Head.TrackingDetails.AheadBy == 0;
});
Expand All @@ -503,7 +414,7 @@ public Task<IReadOnlyList<CommitMessage>> GetMessagesForUniqueCommits(
string compareBranch,
int maxCommits)
{
return Task.Factory.StartNew(() =>
return Task.Run(() =>
{
var baseCommit = repo.Lookup<Commit>(baseBranch);
var compareCommit = repo.Lookup<Commit>(compareBranch);
Expand Down
4 changes: 4 additions & 0 deletions src/GitHub.App/Services/GitHubContextService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,10 @@ public string FindObjectishForTFSTempFile(string tempFile)
/// <inheritdoc/>
public bool HasChangesInWorkingDirectory(string repositoryDir, string commitish, string path)
{
Guard.ArgumentNotNull(path, nameof(repositoryDir));
Guard.ArgumentNotNull(path, nameof(commitish));
Guard.ArgumentIsGitPath(path, nameof(path));

using (var repo = gitService.GetRepository(repositoryDir))
{
var commit = repo.Lookup<Commit>(commitish);
Expand Down
Loading