From bf32f53c2da4050098931e0be53ec0221e1d8393 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 15 Oct 2019 10:16:26 +0200 Subject: [PATCH 1/5] do merge commit also without diffs between branches --- .../java/sonia/scm/repository/spi/AbstractGitCommand.java | 4 ++-- .../main/java/sonia/scm/repository/spi/GitMergeCommand.java | 2 +- .../main/java/sonia/scm/repository/spi/GitModifyCommand.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) 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 bc4d939e91..396ec525a3 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 @@ -214,10 +214,10 @@ class AbstractGitCommand } } - Optional doCommit(String message, Person author) { + Optional doCommit(String message, Person author, boolean isMergeCommit) { Person authorToUse = determineAuthor(author); try { - if (!clone.status().call().isClean()) { + if (isMergeCommit || !clone.status().call().isClean()) { return of(clone.commit() .setAuthor(authorToUse.getName(), authorToUse.getMail()) .setMessage(message) 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 5643c858b5..6e30239fed 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 @@ -99,7 +99,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private void doCommit() { logger.debug("merged branch {} into {}", toMerge, target); - doCommit(MessageFormat.format(determineMessageTemplate(), toMerge, target), author); + doCommit(MessageFormat.format(determineMessageTemplate(), toMerge, target), author, true); } private String determineMessageTemplate() { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index a68be0a4da..2c48329e98 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -62,7 +62,7 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman r.execute(this); } failIfNotChanged(() -> new NoChangesMadeException(repository, ModifyWorker.this.request.getBranch())); - Optional revCommit = doCommit(request.getCommitMessage(), request.getAuthor()); + Optional revCommit = doCommit(request.getCommitMessage(), request.getAuthor(), false); push(); return revCommit.orElseThrow(() -> new NoChangesMadeException(repository, ModifyWorker.this.request.getBranch())).name(); } From ef7d958ba1a749845566a18d94dbc59be0c6cf9d Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 15 Oct 2019 13:05:26 +0200 Subject: [PATCH 2/5] fix / change unit test --- .../scm/repository/spi/GitMergeCommandTest.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) 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 674da396b5..ac51f2d983 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 @@ -6,7 +6,6 @@ import org.apache.shiro.subject.SimplePrincipalCollection; import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -83,7 +82,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { } @Test - public void shouldNotMergeTwice() throws IOException, GitAPIException { + public void shouldAllowEmptyMergeCommit() { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); request.setTargetBranch("master"); @@ -91,19 +90,10 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); MergeCommandResult mergeCommandResult = command.merge(request); - assertThat(mergeCommandResult.isSuccess()).isTrue(); - Repository repository = createContext().open(); - ObjectId firstMergeCommit = 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); } @Test From 668007e0841018f67c76b2ffb3f03a7749029ec3 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 15 Oct 2019 13:09:51 +0200 Subject: [PATCH 3/5] change unit test --- .../scm/repository/spi/GitMergeCommandTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) 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 ac51f2d983..cb1ee140d5 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 @@ -6,6 +6,7 @@ import org.apache.shiro.subject.SimplePrincipalCollection; import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -82,7 +83,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { } @Test - public void shouldAllowEmptyMergeCommit() { + public void shouldAllowEmptyMergeCommit() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); request.setTargetBranch("master"); @@ -90,10 +91,19 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); MergeCommandResult mergeCommandResult = command.merge(request); + assertThat(mergeCommandResult.isSuccess()).isTrue(); + Repository repository = createContext().open(); + ObjectId firstMergeCommit = 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(firstMergeCommit).isNotEqualTo(secondMergeCommit); } @Test From 4cb79ffe5acd62374417970f55b52326a228f4ae Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Wed, 16 Oct 2019 14:59:38 +0200 Subject: [PATCH 4/5] Detect proper merge and prevent empty commits --- .../repository/spi/AbstractGitCommand.java | 12 +++++++--- .../scm/repository/spi/GitMergeCommand.java | 2 +- .../scm/repository/spi/GitModifyCommand.java | 2 +- .../repository/spi/GitMergeCommandTest.java | 22 +++++++++++++++++- .../scm/repository/spi/scm-git-spi-test.zip | Bin 26072 -> 28443 bytes 5 files changed, 32 insertions(+), 6 deletions(-) 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 396ec525a3..2531888a8b 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 @@ -38,6 +38,7 @@ import com.google.common.base.Strings; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.Subject; import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.RefNotFoundException; import org.eclipse.jgit.lib.ObjectId; @@ -214,10 +215,11 @@ class AbstractGitCommand } } - Optional doCommit(String message, Person author, boolean isMergeCommit) { + Optional doCommit(String message, Person author) { Person authorToUse = determineAuthor(author); try { - if (isMergeCommit || !clone.status().call().isClean()) { + Status status = clone.status().call(); + if (!status.isClean() || isInMerge()) { return of(clone.commit() .setAuthor(authorToUse.getName(), authorToUse.getMail()) .setMessage(message) @@ -225,11 +227,15 @@ class AbstractGitCommand } else { return empty(); } - } catch (GitAPIException e) { + } catch (GitAPIException | IOException e) { throw new InternalRepositoryException(context.getRepository(), "could not commit changes", e); } } + private boolean isInMerge() throws IOException { + return clone.getRepository().readMergeHeads() != null && !clone.getRepository().readMergeHeads().isEmpty(); + } + void push() { try { clone.push().call(); 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 6e30239fed..5643c858b5 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 @@ -99,7 +99,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand private void doCommit() { logger.debug("merged branch {} into {}", toMerge, target); - doCommit(MessageFormat.format(determineMessageTemplate(), toMerge, target), author, true); + doCommit(MessageFormat.format(determineMessageTemplate(), toMerge, target), author); } private String determineMessageTemplate() { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 2c48329e98..a68be0a4da 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -62,7 +62,7 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman r.execute(this); } failIfNotChanged(() -> new NoChangesMadeException(repository, ModifyWorker.this.request.getBranch())); - Optional revCommit = doCommit(request.getCommitMessage(), request.getAuthor(), false); + Optional revCommit = doCommit(request.getCommitMessage(), request.getAuthor()); push(); return revCommit.orElseThrow(() -> new NoChangesMadeException(repository, ModifyWorker.this.request.getBranch())).name(); } 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 cb1ee140d5..5586a2f710 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 @@ -84,6 +84,26 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { @Test public void shouldAllowEmptyMergeCommit() throws IOException, GitAPIException { + GitMergeCommand command = createCommand(); + MergeCommandRequest request = new MergeCommandRequest(); + request.setTargetBranch("master"); + request.setBranchToMerge("empty_merge"); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + MergeCommandResult mergeCommandResult = command.merge(request); + + assertThat(mergeCommandResult.isSuccess()).isTrue(); + + Repository repository = createContext().open(); + Iterable commits = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call(); + RevCommit mergeCommit = commits.iterator().next(); + assertThat(mergeCommit.getParentCount()).isEqualTo(2); + assertThat(mergeCommit.getParent(0).name()).isEqualTo("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + assertThat(mergeCommit.getParent(1).name()).isEqualTo("d81ad6c63d7e2162308d69637b339dedd1d9201c"); + } + + @Test + public void shouldNotMergeTwice() throws IOException, GitAPIException { GitMergeCommand command = createCommand(); MergeCommandRequest request = new MergeCommandRequest(); request.setTargetBranch("master"); @@ -103,7 +123,7 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase { ObjectId secondMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId(); - assertThat(firstMergeCommit).isNotEqualTo(secondMergeCommit); + assertThat(secondMergeCommit).isEqualTo(firstMergeCommit); } @Test diff --git a/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-spi-test.zip b/scm-plugins/scm-git-plugin/src/test/resources/sonia/scm/repository/spi/scm-git-spi-test.zip index addda8609058d57eae3a0f91a0aa9e66b0a187d4..4310e637335c09568da9d3afa63ce57949cec5ab 100644 GIT binary patch delta 4143 zcmb7G3sh9q8a{K*Ac$}T#1Y5ofPld8n)jI*kf)3c3^0fc!yAS49iNhe4;`5C^fNUX{JWkM9XX4ea^rVT&5(HAKi5VZ5F0cVq5&k z^2ciy+m_9F5_hH?Kk984znOZ~7fU+UPOTr~9jRZY;2X8r;H zuobmf(XJ!+EQ8ob*QfvCeeTND^5U5qFX-OhTmSdvlMYiiF1_@EZ{_z7J@Rh`-ftj9(So)_AzV%i4T?ykGT_l$AdhZoWk=RP_m7=nA&HeW2)-^&h+& z_06u@f&N#ouaaDNXvxKh{S%wgrnR-+>oeYc`1PKHAC#X>UDX|Mv-^NXdw8$$O9OM5YAyX`oA1Dhjmy|&aWeD5FxwV9fs(Y zv_eB`Wg4qhtyJ5zYK2N=(MYw7lCfH4n)*7LuGh*G?hb>Dm=L&-86c>Y_U?JHUw!v( z_x?)53ER>4QnyBw5U`mp3@Ti&eBuH6Nu>|gZ;;N|rb4>0%UwT2uTU~73nP^&8M>}s zZK2gNno($#S_^GaGb)>0Yn9PzkANZ^g(T4P^p}ntK4viKk4lR4M{Hz>UNLPC}aGI|OrxRKgL^l?B17cwacDa?2fD zR3v&-4yiH_hGIoq;TJ5VG0jj&9l_kz4BOwT#uGvewtD z7&WbyOBtD5P1|HLnVivD)he5Xmbz!({Ux0(SB3?IEWpctO5EHa-Bey9jmnmOy!u>o zxBt}k!+rzz6n{GxcGO(>IqY2gbIz%=@AVRE2l390`9V)7hG&m_9J#HlD?Dmfwok&e zl2^N5H!dhrZOvIU_>RBP1AS0{hKiPe1FF7bti=@_K5snNo&7Z z(e}lTA9|0=d*Y^sZrCMWx*dATyPlwGE?>OY`HxGc-zSku@09ee)BUft@$j$bkPs@4 zny6Gb5Hfu3q|t$p`5atVrhE$jqFq5ztJlD-svra^U-&866%ZC|xz!{@&HNN>mF&I1 ziKZU3k4$QJLcU7r;#;S51o^b`N6>pcHy~og z)w)LRds~+rNbp_#xspCa$G~8g!1$m}jHKQlHm`mA0}!gnRv)SxI*J?2A0uIGYb8-Xs`v{qU&- zROM#CK7$bV@rLt;6d24A;zuXIzjJhGP7xBZ{usO_mK_K%k$?gb<2kP0~~2_lPj;;U$c%2@3Kfp;}Mjp%k>~XFx!n zFKh`V$q)o^4CA5o#_DPce#wi6M@q)Q-s%8Yq9@1C3B??P3s`C*xzlzlNp2iF<;EHI zDv51boT$-%0hmKFh6P~PGrsmil~^RUR(yFEGo>a69G?^5Eh*rfzF zwYZ3USS8qrWs_BqLdJT~{ncs3CC&BZT{ltOAmSq8?DiZ&-0e<}b1CtEj3h4kC|LqP zXd>~!I8a#>aI%~tqvKsdX*KXFW;nXYw>3h1Q9O*)I70^sRTLSX;38K@VNQt{E*pjT zyaXT$&Cpp#@wS7dxR4l1#DFJJ!ji}V(Q_go=S zt;FDqcqNq14TsiJiU+`y$$i){!-P+{hC)f4QDG25E5{+XumcCeBs9;Z@Xu-3I#=ef z{R9%^tHvNbks}+0uG4$#)DUZ$gZHVS)nvqjG;rMXD6Z4OdW#ICbrk-L77FXk&}*S^ zK@xmpF>?>7li{R!^a0;eA+a(UgEI<`2ewsE_?=|fTVcdgW&&AhW?hconhE(;Mm#A6 zI;zaT(o9bU-y$PXpNc_BoDwz`g>!^E(m+>Y#D7kM7fZ}8z->r(c?$J7jW|Gh&JFH>EXeSX{8y(4H$bxSWisJ>wGn?S6B298;E#Z?XBH%s z8}UD9L0h>Qs!SBVItwNvnlG}TzS@l1EroB<{9 delta 2308 zcmZ8hYfw{X8b0|>h>=SSVi7`m5(0`~VlG6I0Oq100uc@<5C|X;0!4$UrL~u-wz%x< zQm5AXxHH}Dc59a(o%K3v)Y(-W*Rl4GTgP#y+d5@I_tw>};&z?swAN0)a}LDNWHKk; z`@QFVp69*2GbQ`%s_ejCcF)nV-~Y6P2v3|MP2~Ow?2Klin^VR=)c#C1cUG4u(XKNj zA5VMK7%R$DEI&ThIs2C2_4WGwWY5vrAnEa&XtKyq8@S=l&lS@Kk6J(9y$Q^^kU@f}yaxaAEfX6=_u zS@8IBvC_SQB*od`t{0BN?O)utQD9HpcH3LXA?fC-{RVs$T6|fSp7gHVxSc}eI0A~o zzlqQigc7ehGI3;8B9K6LW>o@aqxsLcyHbl|4pmYxT=&cIrawpJvPyDBC;r}^#8bG| zJ8S2-zUsV3>}I&VS{%@&;v3IzaK1>3a!WEn^jhl4sbiuzk57qfmP{%mnU^Ht^@<-$ zSnw~xs0K+7h61)^k{1FA8T$2~;>Ca#e_Wjmshr$*)!R&#{ula^CHQ{zDusE`Uxm6| zGUeh8s~YWP&*8&$$sDlIznA7+3zIDosoS28u~p zs@}qBj~01r(y`u~C1%CH<_qle&vU!(zae(kbXf-WtxZeTwrpTAuB@$`t9kai6C$fR zV**}a^|CRbXqYQ^E~brjH1Ww0@;?mGu8^+?D1uVQ&UHm>MJBC#$=qlOT26bO#-J-r zX2bhEOCVUvB8y=wuf&ytER2-@35Q&XgnN7nJa!e(&eG!1{5ls}XX`BFcoeQ+(Hsih z0@xiii_76;upGx|iq2N6Q-qPXCQyau`m)+2dgayZ5}$`^b_zjs)qVtPEv)tU81~dw z#P0-e%W$!QBe^W58*1cx17#>aBt*J3J%v- z;aZ)BR4>5YIv)=68sbR91YbjUl7=l(F?ds>!8GqB+ttW#sG`eBLoQ`vef=6ZGv4pvg>CsqxZ~D&zkKNtsz7IV&$yJx`va zfWiNCS@=g(D@J*HBgc+VTZLYW=|Rc%^+rxM3Tsmq`1}l1Y?G|5n>h0JGQ7AcyztKQ zSsZlLK7|LHw6H`{!MAD1`$dSg@#Nn{m}m=$ye>pkDqZoDoc7xf4iw|(ofs1bRadGb3W1~-RrJi?JF6OMLSrEPMnOG64);kzy;rXxCJg*n-Ku!S3B z|HMzit+2FmB+G)~?nV-};G=Gyeb2(c?6hM;kC)uA;vNNCYJV*51-*l{u)#bNYFMn##^oVewu;ta~KX% z?!s?A{}P*YO07MI(3c5p2Nht-an4g#)j(Ich@mE z5PS+VLpcZOG*i)wGX2tsn+N)OqO%*|gF#N-LhbPBv+0m{$;W==Zwr$^6?SiHX3teI a*j$@U=OAZEih?R+TIlznj$teT;q(J{b|F~+ From 3ed64d57e5cd80b4dda0912a6f05a769259b336b Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Thu, 17 Oct 2019 06:16:28 +0000 Subject: [PATCH 5/5] Close branch bugfix/empty_merge_git