diff --git a/Jenkinsfile b/Jenkinsfile index c759de5733..37cbdb4064 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -55,7 +55,13 @@ node('docker') { if (isMainBranch()) { stage('Lifecycle') { - nexusPolicyEvaluation iqApplication: selectedApplication('scm'), iqScanPatterns: [[scanPattern: 'scm-server/target/scm-server-app.zip']], iqStage: 'build' + try { + // failBuildOnNetworkError -> so we can catch the exception and neither fail nor make our build unstable + nexusPolicyEvaluation iqApplication: selectedApplication('scm'), iqScanPatterns: [[scanPattern: 'scm-server/target/scm-server-app.zip']], iqStage: 'build', failBuildOnNetworkError: true + } catch (Exception e) { + echo "ERROR: iQ Server policy eval failed. Not marking build unstable for now." + echo "ERROR: iQ Server Exception: ${e.getMessage()}" + } } stage('Archive') { diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java index 53f712cddc..1e6d5c6447 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandResult.java @@ -12,18 +12,25 @@ import static java.util.Collections.unmodifiableCollection; * case you can use {@link #getFilesWithConflict()} to get a list of files with merge conflicts. */ public class MergeCommandResult { + private final Collection filesWithConflict; + private final String newHeadRevision; + private final String targetRevision; + private final String revisionToMerge; - private MergeCommandResult(Collection filesWithConflict) { + private MergeCommandResult(Collection filesWithConflict, String targetRevision, String revisionToMerge, String newHeadRevision) { this.filesWithConflict = filesWithConflict; + this.targetRevision = targetRevision; + this.revisionToMerge = revisionToMerge; + this.newHeadRevision = newHeadRevision; } - public static MergeCommandResult success() { - return new MergeCommandResult(emptyList()); + public static MergeCommandResult success(String targetRevision, String revisionToMerge, String newHeadRevision) { + return new MergeCommandResult(emptyList(), targetRevision, revisionToMerge, newHeadRevision); } - public static MergeCommandResult failure(Collection filesWithConflict) { - return new MergeCommandResult(new HashSet<>(filesWithConflict)); + public static MergeCommandResult failure(String targetRevision, String revisionToMerge, Collection filesWithConflict) { + return new MergeCommandResult(new HashSet<>(filesWithConflict), targetRevision, revisionToMerge, null); } /** @@ -31,7 +38,7 @@ public class MergeCommandResult { * merge conflicts. In this case you can use {@link #getFilesWithConflict()} to check what files could not be merged. */ public boolean isSuccess() { - return filesWithConflict.isEmpty(); + return filesWithConflict.isEmpty() && newHeadRevision != null; } /** @@ -41,4 +48,26 @@ public class MergeCommandResult { public Collection getFilesWithConflict() { return unmodifiableCollection(filesWithConflict); } + + /** + * Returns the revision of the new head of the target branch, if the merge was successful ({@link #isSuccess()}) + */ + public String getNewHeadRevision() { + return newHeadRevision; + } + + /** + * Returns the revision of the target branch prior to the merge. + */ + public String getTargetRevision() { + return targetRevision; + } + + /** + * Returns the revision of the branch that was merged into the target (or in case of a conflict of the revision that + * should have been merged). + */ + public String getRevisionToMerge() { + return revisionToMerge; + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java index a62e373dca..79d6b12c25 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/MergeCommand.java @@ -7,6 +7,12 @@ import sonia.scm.repository.api.MergeStrategy; import java.util.Set; public interface MergeCommand { + /** + * Executes the merge. + * @param request The parameters specifying the merge. + * @return Result holding either the new revision or a list of conflicting files. + * @throws sonia.scm.NoChangesMadeException If the merge neither had a conflict nor made any change. + */ MergeCommandResult merge(MergeCommandRequest request); MergeDryRunCommandResult dryRun(MergeCommandRequest request); diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java b/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java index 135221ea82..e4334d42b1 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java @@ -8,6 +8,14 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.UUID; +/** + * CopyOnWrite creates a copy of the target file, before it is modified. This should prevent empty or incomplete files + * on errors such as full disk. + * + * javasecurity:S2083: SonarQube thinks that the path (targetFile) is generated from an http header (HttpUtil), but + * this is not true. It looks like a false-positive, so we suppress the warning for now. + */ +@SuppressWarnings("javasecurity:S2083") public final class CopyOnWrite { private static final Logger LOG = LoggerFactory.getLogger(CopyOnWrite.class); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index adf7878221..1c807c6fc7 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -186,6 +186,10 @@ class AbstractGitCommand return context; } + sonia.scm.repository.Repository getRepository() { + return repository; + } + void checkOutBranch(String branchName) throws IOException { try { clone.checkout().setName(branchName).call(); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java index 0204ca4e3c..fdecac6314 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java @@ -2,6 +2,7 @@ package sonia.scm.repository.spi; import com.google.common.base.Strings; import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -10,6 +11,8 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.treewalk.EmptyTreeIterator; import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.treewalk.filter.PathFilter; +import sonia.scm.ContextEntry; +import sonia.scm.NotFoundException; import sonia.scm.repository.GitUtil; import sonia.scm.util.Util; @@ -35,49 +38,48 @@ final class Differ implements AutoCloseable { } private static Differ create(Repository repository, DiffCommandRequest request) throws IOException { - RevWalk walk = new RevWalk(repository); + RevWalk walk = new RevWalk(repository); - ObjectId revision = repository.resolve(request.getRevision()); - RevCommit commit = walk.parseCommit(revision); + ObjectId revision = repository.resolve(request.getRevision()); + if (revision == null) { + throw NotFoundException.notFound(ContextEntry.ContextBuilder.entity("revision not found", request.getRevision())); + } + RevCommit commit; + try { + commit = walk.parseCommit(revision); + } catch (MissingObjectException ex) { + throw NotFoundException.notFound(ContextEntry.ContextBuilder.entity("revision not found", request.getRevision())); + } - walk.markStart(commit); - commit = walk.next(); - TreeWalk treeWalk = new TreeWalk(repository); - treeWalk.reset(); - treeWalk.setRecursive(true); + walk.markStart(commit); + commit = walk.next(); + TreeWalk treeWalk = new TreeWalk(repository); + treeWalk.reset(); + treeWalk.setRecursive(true); - if (Util.isNotEmpty(request.getPath())) - { - treeWalk.setFilter(PathFilter.create(request.getPath())); - } + if (Util.isNotEmpty(request.getPath())) { + treeWalk.setFilter(PathFilter.create(request.getPath())); + } - if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) - { - ObjectId otherRevision = repository.resolve(request.getAncestorChangeset()); - ObjectId ancestorId = GitUtil.computeCommonAncestor(repository, revision, otherRevision); - RevTree tree = walk.parseCommit(ancestorId).getTree(); + if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) { + ObjectId otherRevision = repository.resolve(request.getAncestorChangeset()); + ObjectId ancestorId = GitUtil.computeCommonAncestor(repository, revision, otherRevision); + RevTree tree = walk.parseCommit(ancestorId).getTree(); + treeWalk.addTree(tree); + } else if (commit.getParentCount() > 0) { + RevTree tree = commit.getParent(0).getTree(); + + if (tree != null) { treeWalk.addTree(tree); - } - else if (commit.getParentCount() > 0) - { - RevTree tree = commit.getParent(0).getTree(); - - if (tree != null) - { - treeWalk.addTree(tree); - } - else - { - treeWalk.addTree(new EmptyTreeIterator()); - } - } - else - { + } else { treeWalk.addTree(new EmptyTreeIterator()); } + } else { + treeWalk.addTree(new EmptyTreeIterator()); + } - treeWalk.addTree(commit.getTree()); + treeWalk.addTree(commit.getTree()); return new Differ(commit, walk, treeWalk); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java index 64a20a33cb..84ea1a0bbc 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitFastForwardIfPossible.java @@ -7,6 +7,7 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.api.MergeCommandResult; import java.io.IOException; +import java.util.Collections; class GitFastForwardIfPossible extends GitMergeStrategy { @@ -22,7 +23,7 @@ class GitFastForwardIfPossible extends GitMergeStrategy { MergeResult fastForwardResult = mergeWithFastForwardOnlyMode(); if (fastForwardResult.getMergeStatus().isSuccessful()) { push(); - return MergeCommandResult.success(); + return createSuccessResult(fastForwardResult.getNewHead().name()); } else { return fallbackMerge.run(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java index 6aa68a0ea8..9a7d290d3a 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommit.java @@ -3,10 +3,16 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.MergeCommand; import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.revwalk.RevCommit; +import sonia.scm.NoChangesMadeException; import sonia.scm.repository.Repository; import sonia.scm.repository.api.MergeCommandResult; import java.io.IOException; +import java.util.Collections; +import java.util.Optional; + +import static sonia.scm.repository.spi.GitRevisionExtractor.extractRevisionFromRevCommit; class GitMergeCommit extends GitMergeStrategy { @@ -21,11 +27,12 @@ class GitMergeCommit extends GitMergeStrategy { MergeResult result = doMergeInClone(mergeCommand); if (result.getMergeStatus().isSuccessful()) { - doCommit(); + RevCommit revCommit = doCommit().orElseThrow(() -> new NoChangesMadeException(getRepository())); push(); - return MergeCommandResult.success(); + return createSuccessResult(extractRevisionFromRevCommit(revCommit)); } else { return analyseFailure(result); } } + } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java index 1d53b99c99..8e9f79d1b9 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeStrategy.java @@ -6,6 +6,7 @@ import org.eclipse.jgit.api.MergeCommand; import org.eclipse.jgit.api.MergeResult; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.revwalk.RevCommit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.InternalRepositoryException; @@ -14,6 +15,7 @@ import sonia.scm.repository.api.MergeCommandResult; import java.io.IOException; import java.text.MessageFormat; +import java.util.Optional; abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker { @@ -24,37 +26,57 @@ abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker doCommit() { + logger.debug("merged branch {} into {}", branchToMerge, targetBranch); + return doCommit(MessageFormat.format(determineMessageTemplate(), branchToMerge, targetBranch), author); + } + + MergeCommandResult createSuccessResult(String newRevision) { + return MergeCommandResult.success(targetRevision.name(), revisionToMerge.name(), newRevision); + } + + ObjectId getTargetRevision() { + return targetRevision; + } + + ObjectId getRevisionToMerge() { + return revisionToMerge; } private String determineMessageTemplate() { @@ -66,7 +88,7 @@ abstract class GitMergeStrategy extends AbstractGitCommand.GitCloneWorker new NoChangesMadeException(getRepository())); push(); - return MergeCommandResult.success(); + return MergeCommandResult.success(getTargetRevision().name(), revCommit.name(), extractRevisionFromRevCommit(revCommit)); } else { return analyseFailure(result); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRevisionExtractor.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRevisionExtractor.java new file mode 100644 index 0000000000..5055024151 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRevisionExtractor.java @@ -0,0 +1,12 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.revwalk.RevCommit; + +import java.util.Optional; + +public class GitRevisionExtractor { + + static String extractRevisionFromRevCommit(RevCommit revCommit) { + return revCommit.toString().split(" ")[1]; + } +} 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 fcd721c3a2..2616d5b6e5 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 @@ -12,6 +12,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Rule; import org.junit.Test; +import sonia.scm.NoChangesMadeException; import sonia.scm.NotFoundException; import sonia.scm.repository.Person; import sonia.scm.repository.api.MergeCommandResult; @@ -70,6 +71,8 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { MergeCommandResult mergeCommandResult = command.merge(request); assertThat(mergeCommandResult.isSuccess()).isTrue(); + assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("91b99de908fcd04772798a31c308a64aea1a5523"); + assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); Repository repository = createContext().open(); Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); @@ -106,7 +109,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommit.getParent(1).name()).isEqualTo("d81ad6c63d7e2162308d69637b339dedd1d9201c"); } - @Test + @Test(expected = NoChangesMadeException.class) public void shouldNotMergeTwice() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); @@ -120,15 +123,9 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { assertThat(mergeCommandResult.isSuccess()).isTrue(); Repository repository = createContext().open(); - ObjectId firstMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); + new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); - MergeCommandResult secondMergeCommandResult = command.merge(request); - - assertThat(secondMergeCommandResult.isSuccess()).isTrue(); - - ObjectId secondMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); - - assertThat(secondMergeCommit).isEqualTo(firstMergeCommit); + command.merge(request); } @Test @@ -234,6 +231,8 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { Repository repository = createContext().open(); assertThat(mergeCommandResult.isSuccess()).isTrue(); + assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo(mergeCommandResult.getNewHeadRevision()); + assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); RevCommit mergeCommit = commits.iterator().next(); @@ -284,6 +283,9 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); MergeCommandResult mergeCommandResult = command.merge(request); + assertThat(mergeCommandResult.getNewHeadRevision()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); + assertThat(mergeCommandResult.getRevisionToMerge()).isEqualTo("35597e9e98fe53167266583848bfef985c2adb27"); + assertThat(mergeCommandResult.getTargetRevision()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); assertThat(mergeCommandResult.isSuccess()).isTrue(); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitRevisionExtractorTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitRevisionExtractorTest.java new file mode 100644 index 0000000000..52a609b6cd --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitRevisionExtractorTest.java @@ -0,0 +1,21 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.jupiter.api.Test; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class GitRevisionExtractorTest { + + @Test + void shouldReturnRevisionFromRevCommit() { + RevCommit revCommit = mock(RevCommit.class); + when(revCommit.toString()).thenReturn("commit 123456abcdef -t 4561"); + String revision = GitRevisionExtractor.extractRevisionFromRevCommit(revCommit); + assertThat(revision).isEqualTo("123456abcdef"); + } +} diff --git a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap index 25e78abbd8..7fa1b1193f 100644 --- a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap +++ b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap @@ -336,7 +336,7 @@ exports[`Storyshots DateFromNow Default 1`] = ` exports[`Storyshots Forms|Checkbox Default 1`] = `