From 96466807bef6fad22ec1567dfcdd507826984db6 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 13 Sep 2024 10:21:06 +0200 Subject: [PATCH] Compute files with conflicts in merge dry run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pushed-by: Rene Pfeuffer Co-authored-by: René Pfeuffer --- gradle/changelog/merge_conflict.yaml | 4 +++ .../repository/api/MergePreventReason.java | 15 +++++++++++ .../scm/repository/spi/GitMergeCommand.java | 26 ++++++++++++------- .../repository/spi/GitMergeCommandTest.java | 14 +++++----- scm-ui/ui-components/src/index.ts | 2 ++ .../src/repos/JumpToFileButton.tsx | 8 ++++-- 6 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 gradle/changelog/merge_conflict.yaml diff --git a/gradle/changelog/merge_conflict.yaml b/gradle/changelog/merge_conflict.yaml new file mode 100644 index 0000000000..2fd670035d --- /dev/null +++ b/gradle/changelog/merge_conflict.yaml @@ -0,0 +1,4 @@ +- type: added + description: The result of the merge dry-run now contains the names of the files that have conflicts +- type: added + description: Method to compute anchors in diff views can be used in plugins diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergePreventReason.java b/scm-core/src/main/java/sonia/scm/repository/api/MergePreventReason.java index ded3a3491f..63d444c3b5 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergePreventReason.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergePreventReason.java @@ -26,10 +26,25 @@ package sonia.scm.repository.api; import lombok.Value; +import java.util.Collection; + +import static java.util.Collections.emptyList; + /** * @since 3.3.0 */ @Value public class MergePreventReason { MergePreventReasonType type; + + Collection affectedPaths; + + public MergePreventReason(MergePreventReasonType type) { + this(type, emptyList()); + } + + public MergePreventReason(MergePreventReasonType type, Collection affectedPaths) { + this.type = type; + this.affectedPaths = affectedPaths; + } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java index f91038023e..e5025acefe 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java @@ -24,7 +24,6 @@ package sonia.scm.repository.spi; -import com.google.common.collect.ImmutableSet; import com.google.inject.assistedinject.Assisted; import jakarta.inject.Inject; import org.eclipse.jgit.api.Git; @@ -35,9 +34,9 @@ import org.eclipse.jgit.lib.IndexDiff; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.merge.ResolveMerger; import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.eclipse.jgit.treewalk.filter.PathFilter; -import sonia.scm.NotFoundException; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.GitWorkingCopyFactory; import sonia.scm.repository.InternalRepositoryException; @@ -52,8 +51,11 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.Set; +import static java.util.Optional.empty; +import static java.util.Optional.of; import static org.eclipse.jgit.merge.MergeStrategy.RECURSIVE; import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.NotFoundException.notFound; @@ -62,7 +64,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private final GitWorkingCopyFactory workingCopyFactory; private final AttributeAnalyzer attributeAnalyzer; - private static final Set STRATEGIES = ImmutableSet.of( + private static final Set STRATEGIES = Set.of( MergeStrategy.MERGE_COMMIT, MergeStrategy.FAST_FORWARD_IF_POSSIBLE, MergeStrategy.SQUASH, @@ -117,9 +119,8 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand mergePreventReasons.add(new MergePreventReason(MergePreventReasonType.EXTERNAL_MERGE_TOOL)); } - if (!isMergeableWithoutFileConflicts(repository, request.getBranchToMerge(), request.getTargetBranch())) { - mergePreventReasons.add(new MergePreventReason(MergePreventReasonType.FILE_CONFLICTS)); - } + checkMergeableWithoutFileConflicts(repository, request.getBranchToMerge(), request.getTargetBranch()) + .ifPresent(mergePreventReasons::add); return new MergeDryRunCommandResult(mergePreventReasons.isEmpty(), mergePreventReasons); } catch (IOException e) { @@ -127,11 +128,16 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand } } - private boolean isMergeableWithoutFileConflicts(Repository repository, String sourceRevision, String targetRevision) throws IOException { - return RECURSIVE.newMerger(repository, true).merge( - resolveRevisionOrThrowNotFound(repository,sourceRevision), + private Optional checkMergeableWithoutFileConflicts(Repository repository, String sourceRevision, String targetRevision) throws IOException { + ResolveMerger merger = (ResolveMerger) RECURSIVE.newMerger(repository, true); + if (!merger.merge( + resolveRevisionOrThrowNotFound(repository, sourceRevision), resolveRevisionOrThrowNotFound(repository, targetRevision) - ); + )) { + return of(new MergePreventReason(MergePreventReasonType.FILE_CONFLICTS, merger.getUnmergedPaths())); + } else { + return empty(); + } } @Override diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java index af46c03022..fb6edd1dd8 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommandTest.java @@ -110,8 +110,10 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeDryRunCommandResult result = command.dryRun(request); assertThat(result.isMergeable()).isFalse(); - assertThat(result.getReasons().size()).isEqualTo(1); - assertThat(result.getReasons().stream().toList().get(0).getType()).isEqualTo(MergePreventReasonType.FILE_CONFLICTS); + assertThat(result.getReasons()).hasSize(1); + MergePreventReason mergePreventReason = result.getReasons().stream().toList().get(0); + assertThat(mergePreventReason.getType()).isEqualTo(MergePreventReasonType.FILE_CONFLICTS); + assertThat(mergePreventReason.getAffectedPaths()).containsExactly("a.txt"); } @Test @@ -127,7 +129,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeDryRunCommandResult result = command.dryRun(request); assertThat(result.isMergeable()).isFalse(); - assertThat(result.getReasons().size()).isEqualTo(1); + assertThat(result.getReasons()).hasSize(1); assertThat(result.getReasons().stream().toList().get(0).getType()).isEqualTo(MergePreventReasonType.EXTERNAL_MERGE_TOOL); } @@ -144,7 +146,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeDryRunCommandResult result = command.dryRun(request); assertThat(result.isMergeable()).isFalse(); - assertThat(result.getReasons().size()).isEqualTo(2); + assertThat(result.getReasons()).hasSize(2); List reasons = result.getReasons().stream().toList(); assertThat(reasons.get(0).getType()).isEqualTo(MergePreventReasonType.EXTERNAL_MERGE_TOOL); assertThat(reasons.get(1).getType()).isEqualTo(MergePreventReasonType.FILE_CONFLICTS); @@ -379,7 +381,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { GitModificationsCommand modificationsCommand = new GitModificationsCommand(createContext()); List changes = modificationsCommand.getModifications("master").getAdded(); - assertThat(changes.size()).isEqualTo(3); + assertThat(changes).hasSize(3); } @@ -468,7 +470,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { } @Test(expected = NotFoundException.class) - public void shouldHandleNotExistingTargetBranchInDryRun() throws IOException { + public void shouldHandleNotExistingTargetBranchInDryRun() { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); request.setTargetBranch("not_existing"); diff --git a/scm-ui/ui-components/src/index.ts b/scm-ui/ui-components/src/index.ts index d929fb6c8e..01c68d401e 100644 --- a/scm-ui/ui-components/src/index.ts +++ b/scm-ui/ui-components/src/index.ts @@ -143,3 +143,5 @@ export { urls }; export const getPageFromMatch = urls.getPageFromMatch; export { default as useGeneratedId } from "./useGeneratedId"; + + export { getAnchorId as getDiffAnchorId } from "./repos/diff/helpers"; diff --git a/scm-ui/ui-components/src/repos/JumpToFileButton.tsx b/scm-ui/ui-components/src/repos/JumpToFileButton.tsx index 5860fc32be..d6b102d301 100644 --- a/scm-ui/ui-components/src/repos/JumpToFileButton.tsx +++ b/scm-ui/ui-components/src/repos/JumpToFileButton.tsx @@ -37,13 +37,17 @@ const Button = styled(Link)` type Props = { link: string; tooltip: string; + icon?: string; + color?: string; }; -const JumpToFileButton: FC = ({ link, tooltip }) => { +const JumpToFileButton: FC = ({ link, tooltip, icon, color }) => { + const iconToUse = icon ?? "file-code"; + const colorToUse = color ?? "inherit"; return ( );