# HG changeset patch # User Dmitry Neverov # Date 1493118085 -7200 # Node ID 3e7fde939ccc45566d57ab720cd63690548c1247 # Parent 075f3521bd029a8824306faabd9451bb88677a14 TW-49811 prohibit agent-side checkout when 2 roots are checked out into the same dir diff -r 075f3521bd02 -r 3e7fde939ccc mercurial-agent/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialAgentSideVcsSupport.java --- a/mercurial-agent/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialAgentSideVcsSupport.java Wed Apr 05 10:36:29 2017 +0200 +++ b/mercurial-agent/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialAgentSideVcsSupport.java Tue Apr 25 13:01:25 2017 +0200 @@ -25,6 +25,7 @@ import org.jetbrains.annotations.NotNull; import java.io.File; +import java.util.*; public class MercurialAgentSideVcsSupport extends AgentVcsSupport implements UpdateByIncludeRules2 { @@ -64,16 +65,61 @@ return AgentCheckoutAbility.noVcsClientOnAgent(e.getMessage()); } + Set targetDirs = new HashSet(); try { for (IncludeRule rule : checkoutRules.getRootIncludeRules()) { MercurialIncludeRuleUpdater.checkRuleIsValid(rule); + targetDirs.add(rule.getTo()); } - return AgentCheckoutAbility.canCheckout(); } catch (VcsException e) { return AgentCheckoutAbility.notSupportedCheckoutRules(e.getMessage()); } + + List mercurialEntries = getMercurialEntries(build); + if (mercurialEntries.size() > 1) { + for (VcsRootEntry entry : mercurialEntries) { + VcsRoot otherRoot = entry.getVcsRoot(); + if (vcsRoot.equals(otherRoot)) + continue; + for (IncludeRule rule : getOtherRootRules(entry.getCheckoutRules().getRootIncludeRules())) { + if (targetDirs.contains(rule.getTo())) + return AgentCheckoutAbility.canNotCheckout("Cannot checkout VCS root '" + vcsRoot.getName() + "' into the same directory as VCS root '" + otherRoot.getName() + "'"); + } + } + } + + return AgentCheckoutAbility.canCheckout(); } + + @NotNull + private List getMercurialEntries(@NotNull AgentRunningBuild build) { + List result = new ArrayList(); + for (VcsRootEntry entry : build.getVcsRootEntries()) { + if (Constants.VCS_NAME.equals(entry.getVcsRoot().getVcsName())) + result.add(entry); + } + return result; + } + + + @NotNull + private List getOtherRootRules(@NotNull List rules) { + List result = new ArrayList(); + for (IncludeRule rule : rules) { + try { + MercurialIncludeRuleUpdater.checkRuleIsValid(rule); + } catch (VcsException e) { + //return empty root rules to skip checks; appropriate cannot checkout reason + //will be returned during otherRoot's canCheckout() call + return Collections.emptyList(); + } + result.add(rule); + } + return result; + } + + private void registerExtensions(@NotNull VcsRoot root, @NotNull CheckoutRules checkoutRules, @NotNull MercurialIncludeRuleUpdater updater) { CheckoutInfo info = new CheckoutInfo(myRepoFactory, new HgVcsRoot(root), checkoutRules); for (MercurialExtension ext :myExtentionManager.getExtensionsForCheckout(info)) { diff -r 075f3521bd02 -r 3e7fde939ccc mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/AgentSideCheckoutTest.java --- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/AgentSideCheckoutTest.java Wed Apr 05 10:36:29 2017 +0200 +++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/AgentSideCheckoutTest.java Tue Apr 25 13:01:25 2017 +0200 @@ -15,6 +15,7 @@ */ package jetbrains.buildServer.buildTriggers.vcs.mercurial; +import jetbrains.buildServer.TestInternalProperties; import jetbrains.buildServer.agent.*; import jetbrains.buildServer.agent.vcs.AgentCheckoutAbility; import jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandSettingsForRootImpl; @@ -31,6 +32,7 @@ import org.jmock.Expectations; import org.jmock.Mockery; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import java.io.File; @@ -40,8 +42,9 @@ import java.util.Map; import java.util.concurrent.*; +import static java.util.Arrays.asList; import static jetbrains.buildServer.buildTriggers.vcs.mercurial.Util.copyRepository; -import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.text.StringContains.containsString; /** @@ -63,6 +66,7 @@ @BeforeMethod protected void setUp() throws Exception { super.setUp(); + TestInternalProperties.init(); myContext = new Mockery(); @@ -118,6 +122,85 @@ assertThat(agentCheckoutAbility.getCanNotCheckoutReason().getDetails(), containsString("Invalid include rule: subdir=>subdir2")); } + + @DataProvider + public static Object[][] severalRootsSetups() throws Exception { + return new Object[][]{ + new Object[]{new Setup() + .setShouldFail(true) + }, + new Object[]{new Setup() + .setCheckoutRules1("+:.=>dir") + .setCheckoutRules2("+:.=>dir") + .setShouldFail(true) + }, + new Object[]{new Setup() + .setCheckoutRules1("+:.=>dir1") + .setCheckoutRules2("+:.=>dir2\n+:.=>dir1") + .setShouldFail(true) + }, + new Object[]{new Setup() + .setCheckoutRules2("+:.=>dir1") + .setCheckoutRules2("+:.=>dir2") + .setShouldFail(false) + } + }; + } + + + @TestFor(issues = "TW-49811") + @Test(dataProvider = "severalRootsSetups") + public void auto_checkout_many_roots(@NotNull Setup setup) throws Exception { + VcsRootImpl root1 = new VcsRootBuilder().withId(1).withUrl("http://some.org/repo1").build(); + VcsRootImpl root2 = new VcsRootBuilder().withId(2).withUrl("http://some.org/repo2").build(); + + CheckoutRules rules1 = new CheckoutRules(setup.getCheckoutRules1()); + CheckoutRules rules2 = new CheckoutRules(setup.getCheckoutRules2()); + AgentRunningBuild build = myContext.mock(AgentRunningBuild.class, "build" + myBuildCounter++); + myContext.checking(new Expectations() {{ + allowing(build).getVcsRootEntries(); + will(returnValue(asList(new VcsRootEntry(root1, rules1), new VcsRootEntry(root2, rules2)))); + }}); + + AgentCheckoutAbility canCheckout1 = myVcsSupport.canCheckout(root1, rules1, build); + AgentCheckoutAbility canCheckout2 = myVcsSupport.canCheckout(root2, rules2, build); + + if (setup.isShouldFail()) { + assertThat(canCheckout1.getCanNotCheckoutReason().getDetails(), CoreMatchers.equalTo( + "Cannot checkout VCS root '" + root1.getName() + "' into the same directory as VCS root '" + root2.getName() + "'")); + assertThat(canCheckout2.getCanNotCheckoutReason().getDetails(), CoreMatchers.equalTo( + "Cannot checkout VCS root '" + root2.getName() + "' into the same directory as VCS root '" + root1.getName() + "'")); + } else { + assertNull(canCheckout1.getCanNotCheckoutReason()); + assertNull(canCheckout2.getCanNotCheckoutReason()); + } + } + + + @TestFor(issues = "TW-49811") + public void auto_checkout_many_roots_ignore_broken() throws Exception { + //root1 and root2 have intersecting checkout dir (dir1), but + //root2 also has an unsupported checkout rule; we should ignore + //root2 during canCheckout() call for root1 + VcsRootImpl root1 = new VcsRootBuilder().withId(1).withUrl("http://some.org/repo1").build(); + VcsRootImpl root2 = new VcsRootBuilder().withId(2).withUrl("http://some.org/repo2").build(); + + CheckoutRules rules1 = new CheckoutRules("+:.=>dir1"); + CheckoutRules rules2 = new CheckoutRules("+:.=>dir1\n+:dir2=>dir3"); + AgentRunningBuild build = myContext.mock(AgentRunningBuild.class, "build" + myBuildCounter++); + myContext.checking(new Expectations() {{ + allowing(build).getVcsRootEntries(); + will(returnValue(asList(new VcsRootEntry(root1, rules1), new VcsRootEntry(root2, rules2)))); + }}); + + AgentCheckoutAbility canCheckout1 = myVcsSupport.canCheckout(root1, rules1, build); + AgentCheckoutAbility canCheckout2 = myVcsSupport.canCheckout(root2, rules2, build); + assertNull(canCheckout1.getCanNotCheckoutReason()); + assertThat(canCheckout2.getCanNotCheckoutReason().getType(), CoreMatchers.equalTo(AgentCanNotCheckoutReason.NOT_SUPPORTED_CHECKOUT_RULES)); + assertThat(canCheckout2.getCanNotCheckoutReason().getDetails(), containsString("Invalid include rule: dir2=>dir3")); + } + + public void checkout_on_agent() throws IOException, VcsException { testUpdate(createVcsRoot(simpleRepo()), "4:b06a290a363b", "cleanPatch1/after", new IncludeRule(".", ".", null)); } @@ -178,7 +261,6 @@ allowing(build).getBuildLogger(); will(returnValue(myLogger)); allowing(build).getSharedConfigParameters(); will(returnValue(sharedConfigParameters)); }}); - assertNull(myVcsSupport.canCheckout(vcsRoot, new CheckoutRules(""), build).getCanNotCheckoutReason()); myVcsSupport.getUpdater(vcsRoot, new CheckoutRules(""), version, myWorkDir, build, false).process(includeRule, actualWorkDir); File hgDir = new File(actualWorkDir, ".hg"); @@ -322,5 +404,46 @@ return "mercurial-tests/testData"; } + private static class Setup { + private String myCheckoutRules1 = CheckoutRules.DEFAULT.getAsString(); + private String myCheckoutRules2 = CheckoutRules.DEFAULT.getAsString(); + private boolean myShouldFail; + @NotNull + public String getCheckoutRules1() { + return myCheckoutRules1; + } + + @NotNull + public Setup setCheckoutRules1(@NotNull String checkoutRules1) { + myCheckoutRules1 = checkoutRules1; + return this; + } + + @NotNull + public String getCheckoutRules2() { + return myCheckoutRules2; + } + + @NotNull + public Setup setCheckoutRules2(@NotNull String checkoutRules2) { + myCheckoutRules2 = checkoutRules2; + return this; + } + + public boolean isShouldFail() { + return myShouldFail; + } + + @NotNull + public Setup setShouldFail(boolean shouldFail) { + myShouldFail = shouldFail; + return this; + } + + @Override + public String toString() { + return "rules1: '" + myCheckoutRules1 + "', rules2: '" + myCheckoutRules2 + "'"; + } + } }