From 8dde25bf78f0651764d686265867dc6a82c366ff Mon Sep 17 00:00:00 2001 From: Konstantin Kolosovsky Date: Mon, 17 Oct 2016 18:14:10 +0300 Subject: [PATCH] svn: Moved "change lists by paths" filtering logic to "OneShotMergeInfoHelper" --- .../svn/integrate/MergeCalculatorTask.java | 68 ++----------------- .../svn/mergeinfo/OneShotMergeInfoHelper.java | 61 +++++++++++++++++ 2 files changed, 67 insertions(+), 62 deletions(-) diff --git a/plugins/svn4idea/src/org/jetbrains/idea/svn/integrate/MergeCalculatorTask.java b/plugins/svn4idea/src/org/jetbrains/idea/svn/integrate/MergeCalculatorTask.java index 86bd6322667d..9783c7143f2b 100644 --- a/plugins/svn4idea/src/org/jetbrains/idea/svn/integrate/MergeCalculatorTask.java +++ b/plugins/svn4idea/src/org/jetbrains/idea/svn/integrate/MergeCalculatorTask.java @@ -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 myNotMergedChangeLists; @NotNull private final Consumer 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 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; - } } diff --git a/plugins/svn4idea/src/org/jetbrains/idea/svn/mergeinfo/OneShotMergeInfoHelper.java b/plugins/svn4idea/src/org/jetbrains/idea/svn/mergeinfo/OneShotMergeInfoHelper.java index c27a03e26673..b1ecbbb10232 100644 --- a/plugins/svn4idea/src/org/jetbrains/idea/svn/mergeinfo/OneShotMergeInfoHelper.java +++ b/plugins/svn4idea/src/org/jetbrains/idea/svn/mergeinfo/OneShotMergeInfoHelper.java @@ -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 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; + } }