svn: Moved "change lists by paths" filtering logic to "OneShotMergeInfoHelper"

This commit is contained in:
Konstantin Kolosovsky
2016-11-09 23:53:59 +03:00
parent 2bb920dae4
commit 8dde25bf78
2 changed files with 67 additions and 62 deletions
@@ -22,24 +22,24 @@ import com.intellij.openapi.vcs.VcsException;
import com.intellij.openapi.vcs.versionBrowser.ChangeBrowserSettings;
import com.intellij.openapi.vcs.versionBrowser.CommittedChangeList;
import com.intellij.util.Consumer;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.continuation.Where;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.idea.svn.history.*;
import org.jetbrains.idea.svn.history.LogHierarchyNode;
import org.jetbrains.idea.svn.history.SvnChangeList;
import org.jetbrains.idea.svn.history.SvnCommittedChangesProvider;
import org.jetbrains.idea.svn.history.SvnRepositoryLocation;
import org.jetbrains.idea.svn.mergeinfo.MergeChecker;
import org.jetbrains.idea.svn.mergeinfo.OneShotMergeInfoHelper;
import java.util.LinkedList;
import java.util.List;
import static com.intellij.util.containers.ContainerUtil.newArrayList;
import static org.jetbrains.idea.svn.mergeinfo.SvnMergeInfoCache.MergeCheckResult;
import static org.tmatesoft.svn.core.internal.util.SVNPathUtil.isAncestor;
public class MergeCalculatorTask extends BaseMergeTask {
@NotNull private final SvnBranchPointsCalculator.WrapperInvertor myCopyPoint;
@NotNull private final MergeChecker myMergeChecker;
@NotNull private final OneShotMergeInfoHelper myMergeChecker;
@NotNull private final List<CommittedChangeList> myNotMergedChangeLists;
@NotNull private final Consumer<MergeCalculatorTask> myCallback;
@@ -115,67 +115,11 @@ public class MergeCalculatorTask extends BaseMergeTask {
ProgressManager.getInstance().getProgressIndicator().setText2("Processing revision " + changeList.getNumber());
if (MergeCheckResult.NOT_MERGED.equals(myMergeChecker.checkList(changeList)) && !checkListForPaths(pair.getSecond())) {
if (MergeCheckResult.NOT_MERGED.equals(myMergeChecker.checkList(changeList)) && !myMergeChecker.checkListForPaths(pair.getSecond())) {
result.add(changeList);
}
}
return result;
}
// true if errors found
boolean checkListForPaths(@NotNull LogHierarchyNode node) {
// TODO: Such filtering logic is not clear enough so far (and probably not correct for all cases - for instance when we perform merge
// TODO: from branch1 to branch2 and have revision which contain merge changes from branch3 to branch1.
// TODO: In this case paths of child log entries will not contain neither urls from branch1 nor from branch2 - and checkEntry() method
// TODO: will return true => so such revision will not be used (and displayed) further.
// TODO: Why do we check entries recursively - we have a revision - set of changes in the "merge from" branch? Why do we need to check
// TODO: where they came from - we want avoid some circular merges or what? Does subversion itself perform such checks or not?
boolean isLocalChange = ContainerUtil.or(node.getChildren(), this::checkForSubtree);
return isLocalChange ||
checkForEntry(node.getMe(), myMergeContext.getRepositoryRelativeWorkingCopyPath(),
myMergeContext.getRepositoryRelativeSourcePath());
}
/**
* TODO: Why checkForEntry() from checkListForPaths() and checkForSubtree() are called with swapped parameters.
*/
// true if errors found
private boolean checkForSubtree(@NotNull LogHierarchyNode tree) {
LinkedList<LogHierarchyNode> queue = new LinkedList<>();
queue.addLast(tree);
while (!queue.isEmpty()) {
LogHierarchyNode element = queue.removeFirst();
ProgressManager.checkCanceled();
if (checkForEntry(element.getMe(), myMergeContext.getRepositoryRelativeSourcePath(),
myMergeContext.getRepositoryRelativeWorkingCopyPath())) {
return true;
}
queue.addAll(element.getChildren());
}
return false;
}
// true if errors found
// checks if either some changed path is in current branch => treat as local change
// or if no changed paths in current branch, checks if at least one path in "merge from" branch
// NOTE: this fails for "merge-source" log entries from other branches - when all changed paths are from some
// third branch - this logic treats such log entry as local.
private static boolean checkForEntry(@NotNull LogEntry entry, @NotNull String localURL, @NotNull String relativeBranch) {
boolean atLeastOneUnderBranch = false;
for (LogEntryPath path : entry.getChangedPaths().values()) {
if (isAncestor(localURL, path.getPath())) {
return true;
}
if (!atLeastOneUnderBranch && isAncestor(relativeBranch, path.getPath())) {
atLeastOneUnderBranch = true;
}
}
return !atLeastOneUnderBranch;
}
}
@@ -15,6 +15,7 @@
*/
package org.jetbrains.idea.svn.mergeinfo;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.util.SystemInfo;
import com.intellij.openapi.vcs.AreaMap;
import com.intellij.openapi.vcs.VcsException;
@@ -24,6 +25,9 @@ import org.jetbrains.annotations.Nullable;
import org.jetbrains.idea.svn.SvnPropertyKeys;
import org.jetbrains.idea.svn.api.Depth;
import org.jetbrains.idea.svn.commandLine.SvnBindException;
import org.jetbrains.idea.svn.history.LogEntry;
import org.jetbrains.idea.svn.history.LogEntryPath;
import org.jetbrains.idea.svn.history.LogHierarchyNode;
import org.jetbrains.idea.svn.history.SvnChangeList;
import org.jetbrains.idea.svn.integrate.MergeContext;
import org.jetbrains.idea.svn.properties.PropertyConsumer;
@@ -35,6 +39,7 @@ import org.tmatesoft.svn.core.wc2.SvnTarget;
import java.io.File;
import java.util.Collection;
import java.util.LinkedList;
import java.util.Map;
import java.util.Set;
@@ -206,4 +211,60 @@ public class OneShotMergeInfoHelper implements MergeChecker {
private static String toKey(@NotNull String path) {
return SystemInfo.isFileSystemCaseSensitive ? path : toUpperCase(path);
}
// true if errors found
public boolean checkListForPaths(@NotNull LogHierarchyNode node) {
// TODO: Such filtering logic is not clear enough so far (and probably not correct for all cases - for instance when we perform merge
// TODO: from branch1 to branch2 and have revision which contain merge changes from branch3 to branch1.
// TODO: In this case paths of child log entries will not contain neither urls from branch1 nor from branch2 - and checkEntry() method
// TODO: will return true => so such revision will not be used (and displayed) further.
// TODO: Why do we check entries recursively - we have a revision - set of changes in the "merge from" branch? Why do we need to check
// TODO: where they came from - we want avoid some circular merges or what? Does subversion itself perform such checks or not?
boolean isLocalChange = or(node.getChildren(), this::checkForSubtree);
return isLocalChange ||
checkForEntry(node.getMe(), myMergeContext.getRepositoryRelativeWorkingCopyPath(),
myMergeContext.getRepositoryRelativeSourcePath());
}
/**
* TODO: Why checkForEntry() from checkListForPaths() and checkForSubtree() are called with swapped parameters.
*/
// true if errors found
private boolean checkForSubtree(@NotNull LogHierarchyNode tree) {
LinkedList<LogHierarchyNode> queue = new LinkedList<>();
queue.addLast(tree);
while (!queue.isEmpty()) {
LogHierarchyNode element = queue.removeFirst();
ProgressManager.checkCanceled();
if (checkForEntry(element.getMe(), myMergeContext.getRepositoryRelativeSourcePath(),
myMergeContext.getRepositoryRelativeWorkingCopyPath())) {
return true;
}
queue.addAll(element.getChildren());
}
return false;
}
// true if errors found
// checks if either some changed path is in current branch => treat as local change
// or if no changed paths in current branch, checks if at least one path in "merge from" branch
// NOTE: this fails for "merge-source" log entries from other branches - when all changed paths are from some
// third branch - this logic treats such log entry as local.
private static boolean checkForEntry(@NotNull LogEntry entry, @NotNull String localURL, @NotNull String relativeBranch) {
boolean atLeastOneUnderBranch = false;
for (LogEntryPath path : entry.getChangedPaths().values()) {
if (isAncestor(localURL, path.getPath())) {
return true;
}
if (!atLeastOneUnderBranch && isAncestor(relativeBranch, path.getPath())) {
atLeastOneUnderBranch = true;
}
}
return !atLeastOneUnderBranch;
}
}