git: more correctly identify "local changes would be overwritten by"

If the "Local changes would be overwritten by merge/checkout" error
happens in one of repositories, we collect the changes which would
produce this error in subsequent repositories, to show the smart
checkout/merge dialog only once.

To identify these changes, the code used to get the diff between
the current branch and the branch to checkout:
    `git diff master..feature`
and then check for local changes among these files.
However, it is more correct to get the diff between the current working
tree and the branch to checkout:
    `git diff feature`
The difference is subtle, but new behavior is more correct.
This commit is contained in:
Kirill Likhodedov
2017-12-03 15:00:31 +03:00
parent b32322fb55
commit 7eef6bd581
3 changed files with 34 additions and 16 deletions
@@ -25,12 +25,14 @@ import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.openapi.vcs.BranchChangeListener;
import com.intellij.openapi.vcs.VcsException;
import com.intellij.openapi.vcs.FilePath;
import com.intellij.openapi.vcs.VcsNotifier;
import com.intellij.openapi.vcs.changes.Change;
import com.intellij.openapi.vcs.changes.ChangesUtil;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.util.containers.MultiMap;
import git4idea.GitUtil;
import git4idea.changes.GitChangeUtils;
import git4idea.commands.Git;
import git4idea.commands.GitMessageWithFilesDetector;
import git4idea.config.GitVcsSettings;
@@ -43,6 +45,7 @@ import java.util.*;
import static com.intellij.openapi.util.text.StringUtil.pluralize;
import static com.intellij.util.ObjectUtils.chooseNotNull;
import static git4idea.GitUtil.getRepositoryManager;
import static java.util.stream.Collectors.toList;
/**
* Common class for Git operations with branches aware of multi-root configuration,
@@ -344,27 +347,22 @@ abstract class GitBranchOperation {
}
/**
* TODO this is non-optimal and even incorrect, since such diff shows the difference between committed changes
* For each of the given repositories looks to the diff between current branch and the given branch and converts it to the list of
* local changes.
*/
@NotNull
Map<GitRepository, List<Change>> collectLocalChangesConflictingWithBranch(@NotNull Collection<GitRepository> repositories,
@NotNull String currentBranch, @NotNull String otherBranch) {
@NotNull String otherBranch) {
Map<GitRepository, List<Change>> changes = new HashMap<>();
for (GitRepository repository : repositories) {
try {
Collection<String> diff = GitUtil.getPathsDiffBetweenRefs(myGit, repository, currentBranch, otherBranch);
Collection<Change> diffWithWorkingTree = GitChangeUtils.getDiffWithWorkingTree(repository, otherBranch, false);
if (diffWithWorkingTree != null) {
List<String> diff = ChangesUtil.getPaths(diffWithWorkingTree.stream()).map(FilePath::getPath).collect(toList());
List<Change> changesInRepo = GitUtil.findLocalChangesForPaths(myProject, repository.getRoot(), diff, false);
if (!changesInRepo.isEmpty()) {
changes.put(repository, changesInRepo);
}
}
catch (VcsException e) {
// ignoring the exception: this is not fatal if we won't collect such a diff from other repositories.
// At worst, use will get double dialog proposing the smart checkout.
LOG.warn(String.format("Couldn't collect diff between %s and %s in %s", currentBranch, otherBranch, repository.getRoot()), e);
}
}
return changes;
}
@@ -392,7 +390,7 @@ abstract class GitBranchOperation {
// get all other conflicting changes
// get changes in all other repositories (except those which already have succeeded) to avoid multiple dialogs proposing smart checkout
Map<GitRepository, List<Change>> conflictingChangesInRepositories =
collectLocalChangesConflictingWithBranch(getRemainingRepositoriesExceptGiven(currentRepository), currentBranch, nextBranch);
collectLocalChangesConflictingWithBranch(getRemainingRepositoriesExceptGiven(currentRepository), nextBranch);
Set<GitRepository> otherProblematicRepositories = conflictingChangesInRepositories.keySet();
List<GitRepository> allConflictingRepositories = new ArrayList<>(otherProblematicRepositories);
@@ -405,13 +405,25 @@ class GitBranchWorkerTest : GitPlatformTest() {
fun `test local changes would be overwritten in several repositories`() {
val local1 = "local1.txt"
localChangesOverwrittenByWithoutConflict(first, "feature", listOf(local1))
val local2 = "local2.txt"
localChangesOverwrittenByWithoutConflict(second, "feature", listOf(local2))
val file1 = File(first.root.path, local1)
val file2 = File(second.root.path, local2)
val expectedLocalChanges = listOf(file1, file2).map { FileUtil.toSystemIndependentName(it.path) }
// in addition to a local change preventing checkout...
cd(second)
val local2 = second.file("local2.txt")
local2.create(LOCAL_CHANGES_OVERWRITTEN_BY.initial).addCommit("initial-local2")
git("checkout -b feature")
local2.prepend(LOCAL_CHANGES_OVERWRITTEN_BY.branchLine).addCommit("feature-local2")
// ... make another file producing diff between master and feature (but not related to the 'local change would be overwritten' error)
second.file("feature.txt").create("feature\n").addCommit("feature.txt")
git("checkout master")
local2.append(LOCAL_CHANGES_OVERWRITTEN_BY.masterLine)
cd(last)
git("branch feature")
val file1 = File(first.root.path, local1)
val file2 = local2.file
val expectedLocalChanges = listOf(file1, file2).map { FileUtil.toSystemIndependentName(it.path) }
updateChangeListManager()
var smartOperationDialogTimes = 0
@@ -209,4 +209,12 @@ internal class TestFile internal constructor(val repo: GitRepository, val file:
fun exists() = file.exists()
fun read() = FileUtil.loadFile(file)
fun cat(): String = FileUtil.loadFile(file)
fun prepend(content: String): TestFile {
val previousContent = cat()
FileUtil.writeToFile(file, content + previousContent)
return this
}
}