[file-history] track paths more precisely during history calculation

File history worked incorrectly when a file was renamed and later on a new file with the same name appeared. Commits before the first rename appeared in history for a new file, while they should not have, since they are a part of different history.

In order to fix the problem, paths tracking was rewritten. It now takes deleted flag into the account, and if file is deleted, tries to find a conflicting rename.
This commit is contained in:
Julia Beliaeva
2018-12-17 18:58:16 +03:00
parent 5e4594ce3e
commit 37b3d7b06a
2 changed files with 115 additions and 17 deletions
@@ -173,7 +173,7 @@ internal class FileHistoryRefiner(private val visibleLinearGraph: LinearGraph,
LinearGraphUtils.asLiteLinearGraph(visibleLinearGraph).walk(row, this)
pathsForCommits.forEach { commit, path ->
if (path != null && !namesData.affects(commit, path)) {
if (path != null && !namesData.affects(commit, path, true)) {
excluded.add(commit)
}
}
@@ -195,14 +195,14 @@ internal class FileHistoryRefiner(private val visibleLinearGraph: LinearGraph,
currentPath = if (down) {
val pathGetter = { parentIndex: Int ->
namesData.getPathInParentRevision(previousCommit, permanentCommitsInfo.getCommitId(parentIndex), previousPath.filePath)
namesData.getPathInParentRevision(previousCommit, permanentCommitsInfo.getCommitId(parentIndex), previousPath)
}
val path = findPathWithoutConflict(previousNodeId, pathGetter)
path ?: pathGetter(permanentLinearGraph.getCorrespondingParent(previousNodeId, currentNodeId, visibilityBuffer))
}
else {
val pathGetter = { parentIndex: Int ->
namesData.getPathInChildRevision(currentCommit, permanentCommitsInfo.getCommitId(parentIndex), previousPath.filePath)
namesData.getPathInChildRevision(currentCommit, permanentCommitsInfo.getCommitId(parentIndex), previousPath)
}
val path = findPathWithoutConflict(currentNodeId, pathGetter)
// since in reality there is no edge between the nodes, but the whole path, we need to know, which parent is affected by this path
@@ -281,27 +281,59 @@ abstract class FileNamesData(filePath: FilePath) {
return null
}
fun getPathInParentRevision(commit: Int, parent: Int, childPath: FilePath): MaybeDeletedFilePath {
val commits = UnorderedPair(commit, parent)
val otherPath = commitToRename.get(commits).firstNotNull { rename -> rename.getOtherPath(commit, childPath) }
if (otherPath != null) return MaybeDeletedFilePath(otherPath)
fun getPathInParentRevision(commit: Int, parent: Int, childPath: MaybeDeletedFilePath): MaybeDeletedFilePath {
val childFilePath = childPath.filePath
val changeKind = affectedCommits[childFilePath]?.get(commit)?.get(parent) ?: return childPath
if (changeKind == ChangeKind.NOT_CHANGED) return childPath
val changes = affectedCommits[childPath]?.get(commit) ?: return MaybeDeletedFilePath(childPath)
return MaybeDeletedFilePath(childPath, changes[parent] == ChangeKind.ADDED)
val renames = commitToRename.get(UnorderedPair(commit, parent))
if (!childPath.deleted) {
val otherPath = renames.firstNotNull { rename -> rename.getOtherPath(commit, childFilePath) }
if (otherPath != null) return MaybeDeletedFilePath(otherPath)
return MaybeDeletedFilePath(childFilePath, changeKind == ChangeKind.ADDED)
}
if (changeKind == ChangeKind.REMOVED) {
// checking if this is actually an unrelated rename
if (renames.firstNotNull { rename -> rename.getOtherPath(parent, childFilePath) } != null) return childPath
}
return MaybeDeletedFilePath(childFilePath, changeKind != ChangeKind.REMOVED)
}
fun getPathInChildRevision(commit: Int, parent: Int, parentPath: FilePath): MaybeDeletedFilePath {
val commits = UnorderedPair(commit, parent)
val otherPath = commitToRename.get(commits).firstNotNull { rename -> rename.getOtherPath(parent, parentPath) }
if (otherPath != null) return MaybeDeletedFilePath(otherPath)
fun getPathInChildRevision(commit: Int, parent: Int, parentPath: MaybeDeletedFilePath): MaybeDeletedFilePath {
val parentFilePath = parentPath.filePath
val changeKind = affectedCommits[parentFilePath]?.get(commit)?.get(parent) ?: return parentPath
if (changeKind == ChangeKind.NOT_CHANGED) return parentPath
val changes = affectedCommits[parentPath]?.get(commit) ?: return MaybeDeletedFilePath(parentPath)
return MaybeDeletedFilePath(parentPath, changes[parent] == ChangeKind.REMOVED)
val renames = commitToRename.get(UnorderedPair(commit, parent))
if (!parentPath.deleted) {
val otherPath = renames.firstNotNull { rename -> rename.getOtherPath(parent, parentFilePath) }
if (otherPath != null) return MaybeDeletedFilePath(otherPath)
return MaybeDeletedFilePath(parentFilePath, changeKind == ChangeKind.REMOVED)
}
if (changeKind == ChangeKind.ADDED) {
// checking if this is actually an unrelated rename
if (renames.firstNotNull { rename -> rename.getOtherPath(commit, parentFilePath) } != null) return parentPath
}
return MaybeDeletedFilePath(parentFilePath, changeKind != ChangeKind.ADDED)
}
fun affects(commit: Int, path: MaybeDeletedFilePath): Boolean {
fun affects(commit: Int, path: MaybeDeletedFilePath, verify: Boolean = false): Boolean {
val changes = affectedCommits[path.filePath]?.get(commit) ?: return false
if (path.deleted) return changes.containsValue(ChangeKind.REMOVED)
if (path.deleted) {
if (!changes.containsValue(ChangeKind.REMOVED)) return false
if (!verify) return true
for (parent in changes.keys()) {
if (commitToRename.get(UnorderedPair(commit, parent)).firstNotNull { rename ->
rename.getOtherPath(parent, path.filePath)
} != null) {
// this is a rename from path to something else, we should not match this commit
return false
}
}
return true
}
return !changes.containsValue(ChangeKind.REMOVED)
}
@@ -277,6 +277,72 @@ class FileHistoryTest {
7()
}
}
/*
* Two file histories: `create initialFile.txt, rename to file.txt, rename to otherFile.txt` and some time later `create file.txt`
*/
@Test
fun twoFileByTheSameName() {
val file = LocalFilePath("file.txt", false)
val otherFile = LocalFilePath("otherFile.txt", false)
val initialFile = LocalFilePath("initialFile.txt", false)
val fileNamesData = FileNamesDataBuilder(file)
.addChange(file, 0, listOf(MODIFIED), listOf(1))
.addChange(otherFile, 1, listOf(MODIFIED), listOf(2))
.addChange(file, 2, listOf(ADDED), listOf(3))
.addChange(otherFile, 3, listOf(ADDED), listOf(4))
.addChange(file, 3, listOf(REMOVED), listOf(4))
.addRename(4, 3, file, otherFile)
.addChange(file, 5, listOf(ADDED), listOf(6))
.addChange(initialFile, 5, listOf(REMOVED), listOf(6))
.addRename(6, 5, initialFile, file)
.addChange(initialFile, 6, listOf(ADDED), listOf(6))
.build()
graph {
0(1)
1(2)
2(3)
3(4)
4(5)
5(6)
6()
}.assert(0, file, fileNamesData) {
0(2.dot)
2()
}
}
@Test
fun revertedDeletion() {
val file = LocalFilePath("file.txt", false)
val renamedFile = LocalFilePath("renamedFile.txt", false)
val fileNamesData = FileNamesDataBuilder(file)
.addChange(renamedFile, 0, listOf(ADDED), listOf(1))
.addChange(file, 0, listOf(REMOVED), listOf(1))
.addRename(1, 0, file, renamedFile)
.addChange(file, 1, listOf(ADDED), listOf(2))
.addChange(file, 3, listOf(REMOVED), listOf(4))
.addChange(file, 4, listOf(MODIFIED), listOf(5))
.addChange(file, 5, listOf(ADDED), listOf(6))
.build()
graph {
0(1)
1(2)
2(3)
3(4)
4(5)
5(6)
6()
}.assert(0, renamedFile, fileNamesData) {
0(1)
1(3.dot)
3(4)
4(5)
5()
}
}
}
private class FileNamesDataBuilder(private val path: FilePath) {