Skip to content

Duplicate Filepaths in a commit causes failure when performing Diff between commit tree and its parent #1004

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 1 commit into from
Mar 24, 2015
Merged

Duplicate Filepaths in a commit causes failure when performing Diff between commit tree and its parent #1004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 60 additions & 46 deletions LibGit2Sharp.Tests/DiffTreeToTreeFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void RetrievingANonExistentFileChangeReturnsNull()

var changes = repo.Diff.Compare<TreeChanges>(tree, tree);

Assert.Null(changes["batman"]);
Assert.Equal(0, changes.Count(c => c.Path == "batman"));
}
}

Expand All @@ -62,7 +62,7 @@ public void CanCompareACommitTreeAgainstItsParent()
Assert.Equal(1, changes.Count());
Assert.Equal(1, changes.Added.Count());

TreeEntryChanges treeEntryChanges = changes["1.txt"];
TreeEntryChanges treeEntryChanges = changes.Single(c => c.Path == "1.txt");

var patch = repo.Diff.Compare<Patch>(parentCommitTree, commitTree);
Assert.False(patch["1.txt"].IsBinaryComparison);
Expand Down Expand Up @@ -272,8 +272,7 @@ public void DetectsTheRenamingOfAModifiedFileByDefault()
var changes = repo.Diff.Compare<TreeChanges>(rootCommitTree, commitTreeWithRenamedFile);

Assert.Equal(1, changes.Count());
Assert.Equal("super-file.txt", changes["super-file.txt"].Path);
Assert.Equal("my-name-does-not-feel-right.txt", changes["super-file.txt"].OldPath);
Assert.Equal("my-name-does-not-feel-right.txt", changes.Single(c => c.Path == "super-file.txt").OldPath);
Assert.Equal(1, changes.Renamed.Count());
}
}
Expand Down Expand Up @@ -888,7 +887,7 @@ public void CanCompareTwoVersionsOfAFileWithADiffOfTwoHunks(int contextLines, in
Assert.Equal(1, changes.Deleted.Count());
Assert.Equal(1, changes.Added.Count());

Assert.Equal(Mode.Nonexistent, changes["my-name-does-not-feel-right.txt"].Mode);
Assert.Equal(Mode.Nonexistent, changes.Single(c => c.Path =="my-name-does-not-feel-right.txt").Mode);

var patch = repo.Diff.Compare<Patch>(rootCommitTree, mergedCommitTree, compareOptions: compareOptions);

Expand All @@ -904,17 +903,16 @@ public void CanCompareTwoVersionsOfAFileWithADiffOfTwoHunks(int contextLines, in
}
}

[Fact]
public void CanHandleTwoTreeEntryChangesWithTheSamePath()
private void CanHandleTwoTreeEntryChangesWithTheSamePath(SimilarityOptions similarity, Action<string, TreeChanges> verifier)
{
string repoPath = InitNewRepository();

using (var repo = new Repository(repoPath))
{
Blob mainContent = OdbHelper.CreateBlob(repo, "awesome content\n");
Blob mainContent = OdbHelper.CreateBlob(repo, "awesome content\n" + new string('b', 4096));
Blob linkContent = OdbHelper.CreateBlob(repo, "../../objc/Nu.h");

string path = string.Format("include{0}Nu{0}Nu.h", Path.DirectorySeparatorChar);
string path = Path.Combine("include", "Nu", "Nu.h");

var tdOld = new TreeDefinition()
.Add(path, linkContent, Mode.SymbolicLink)
Expand All @@ -930,45 +928,61 @@ public void CanHandleTwoTreeEntryChangesWithTheSamePath()
var changes = repo.Diff.Compare<TreeChanges>(treeOld, treeNew,
compareOptions: new CompareOptions
{
Similarity = SimilarityOptions.None,
Similarity = similarity,
});

/*
* $ git diff-tree -p 5c87b67 d5278d0
* diff --git a/include/Nu/Nu.h b/include/Nu/Nu.h
* deleted file mode 120000
* index 19bf568..0000000
* --- a/include/Nu/Nu.h
* +++ /dev/null
* @@ -1 +0,0 @@
* -../../objc/Nu.h
* \ No newline at end of file
* diff --git a/include/Nu/Nu.h b/include/Nu/Nu.h
* new file mode 100644
* index 0000000..f9e6561
* --- /dev/null
* +++ b/include/Nu/Nu.h
* @@ -0,0 +1 @@
* +awesome content
* diff --git a/objc/Nu.h b/objc/Nu.h
* deleted file mode 100644
* index f9e6561..0000000
* --- a/objc/Nu.h
* +++ /dev/null
* @@ -1 +0,0 @@
* -awesome content
*/
verifier(path, changes);
}
}

Assert.Equal(1, changes.Deleted.Count());
Assert.Equal(0, changes.Modified.Count());
Assert.Equal(1, changes.TypeChanged.Count());
[Fact]
public void CanHandleTwoTreeEntryChangesWithTheSamePathUsingSimilarityNone()
{
// $ git diff-tree --name-status --no-renames -r 2ccccf8 e829333
// T include/Nu/Nu.h
// D objc/Nu.h

TreeEntryChanges change = changes[path];
Assert.Equal(Mode.SymbolicLink, change.OldMode);
Assert.Equal(Mode.NonExecutableFile, change.Mode);
Assert.Equal(ChangeKind.TypeChanged, change.Status);
Assert.Equal(path, change.Path);
}
CanHandleTwoTreeEntryChangesWithTheSamePath(SimilarityOptions.None,
(path, changes) =>
{
Assert.Equal(2, changes.Count());
Assert.Equal(1, changes.Deleted.Count());
Assert.Equal(1, changes.TypeChanged.Count());

TreeEntryChanges change = changes.Single(c => c.Path== path);
Assert.Equal(Mode.SymbolicLink, change.OldMode);
Assert.Equal(Mode.NonExecutableFile, change.Mode);
Assert.Equal(ChangeKind.TypeChanged, change.Status);
Assert.Equal(path, change.Path);
});
}

[Fact]
public void CanHandleTwoTreeEntryChangesWithTheSamePathUsingSimilarityDefault()
{
// $ git diff-tree --name-status --find-renames -r 2ccccf8 e829333
// T include/Nu/Nu.h
// D objc/Nu.h
Copy link
Member Author

Choose a reason for hiding this comment

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

This result seems to not match libgit2 output. I may surely have messed my syntax somewhere.

@carlosmn @ethomson Any idea?


CanHandleTwoTreeEntryChangesWithTheSamePath(SimilarityOptions.Default,
(path, changes) =>
{
Assert.Equal(2, changes.Count());
Assert.Equal(1, changes.Deleted.Count());
Assert.Equal(1, changes.Renamed.Count());

TreeEntryChanges renamed = changes.Renamed.Single();
Assert.Equal(Mode.NonExecutableFile, renamed.OldMode);
Assert.Equal(Mode.NonExecutableFile, renamed.Mode);
Assert.Equal(ChangeKind.Renamed, renamed.Status);
Assert.Equal(path, renamed.Path);

TreeEntryChanges deleted = changes.Deleted.Single();
Assert.Equal(Mode.SymbolicLink, deleted.OldMode);
Assert.Equal(Mode.Nonexistent, deleted.Mode);
Assert.Equal(ChangeKind.Deleted, deleted.Status);
Assert.Equal(path, deleted.Path);
});
}

[Fact]
Expand Down Expand Up @@ -1104,8 +1118,8 @@ public void RetrievingDiffChangesMustAlwaysBeCaseSensitive()
{
var changes = repo.Diff.Compare<TreeChanges>(repo.Lookup<Tree>(treeOldOid), repo.Lookup<Tree>(treeNewOid));

Assert.Equal(ChangeKind.Modified, changes["a.txt"].Status);
Assert.Equal(ChangeKind.Modified, changes["A.TXT"].Status);
Assert.Equal(ChangeKind.Modified, changes.Single(c => c.Path == "a.txt").Status);
Assert.Equal(ChangeKind.Modified, changes.Single(c => c.Path == "A.TXT").Status);
}
}

Expand Down
27 changes: 3 additions & 24 deletions LibGit2Sharp/TreeChanges.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace LibGit2Sharp
[DebuggerDisplay("{DebuggerDisplay,nq}")]
public class TreeChanges : IEnumerable<TreeEntryChanges>
{
private readonly IDictionary<FilePath, TreeEntryChanges> changes = new Dictionary<FilePath, TreeEntryChanges>();
private readonly List<TreeEntryChanges> changes = new List<TreeEntryChanges>();
private readonly List<TreeEntryChanges> added = new List<TreeEntryChanges>();
private readonly List<TreeEntryChanges> deleted = new List<TreeEntryChanges>();
private readonly List<TreeEntryChanges> modified = new List<TreeEntryChanges>();
Expand Down Expand Up @@ -64,7 +64,7 @@ private void AddFileChange(GitDiffDelta delta)
var treeEntryChanges = new TreeEntryChanges(delta);

fileDispatcher[treeEntryChanges.Status](this, treeEntryChanges);
changes.Add(treeEntryChanges.Path, treeEntryChanges);
changes.Add(treeEntryChanges);
}

#region IEnumerable<TreeEntryChanges> Members
Expand All @@ -75,7 +75,7 @@ private void AddFileChange(GitDiffDelta delta)
/// <returns>An <see cref="IEnumerator{T}"/> object that can be used to iterate through the collection.</returns>
public virtual IEnumerator<TreeEntryChanges> GetEnumerator()
{
return changes.Values.GetEnumerator();
return changes.GetEnumerator();
}

/// <summary>
Expand All @@ -89,27 +89,6 @@ IEnumerator IEnumerable.GetEnumerator()

#endregion

/// <summary>
/// Gets the <see cref="TreeEntryChanges"/> corresponding to the specified <paramref name="path"/>.
/// </summary>
public virtual TreeEntryChanges this[string path]
{
get { return this[(FilePath)path]; }
}

private TreeEntryChanges this[FilePath path]
{
get
{
TreeEntryChanges treeEntryChanges;
if (changes.TryGetValue(path, out treeEntryChanges))
{
return treeEntryChanges;
}

return null;
}
}

/// <summary>
/// List of <see cref="TreeEntryChanges"/> that have been been added.
Expand Down