From 75a80c5d3c790260a35ccf21ee54329f5d160879 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Mon, 3 Feb 2020 21:00:46 +0100 Subject: [PATCH] Restore behavior of CloneCommand Commit 6216b0de changed the behavior of the setMirror(), setCloneAllBranches(), and setBranchesToClone() operations. Before that commit, these could be set and reset independently and only in call() it would be determined what exactly to do. Since that commit, the last of these calls would determine the operation. This means that the sequence cloneCommand.setCloneAllBranches(true); cloneCommand.setBranchesToClone(/* some list of refs */); would formerly do a "clone all" giving a fetch refspec with wildcards +refs/heads/*:refs/remotes/origin/* which picks up new upstream branches, whereas since commit 6216b0de individual non-wildcard fetch refspecs would be generated and new upstream branches would not be fetched anymore. Undo this behavioral change. Make the operations independently settable and resettable again, and determine the exact operation only in call(): mirror=true > cloneAll=true > specific refs, where ">" means "takes precedence over", and if none is set assume cloneAll=true. Note that mirror=true implies setBare(true). Bug: 559796 Change-Id: I7162b60e99de5e3e512bf27ff4113f554c94f5a6 Signed-off-by: Thomas Wolf --- .../eclipse/jgit/api/CloneCommandTest.java | 55 +++++++++++++++++++ .../org/eclipse/jgit/api/CloneCommand.java | 42 ++++++++------ 2 files changed, 81 insertions(+), 16 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index 3224bbb78..c42cee375 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -441,6 +441,7 @@ public class CloneCommandTest extends RepositoryTestCase { Git git2 = command.call(); addRepoToClose(git2.getRepository()); assertNotNull(git2); + assertTrue(git2.getRepository().isBare()); assertNotNull(git2.getRepository().resolve("tag-for-blob")); assertNotNull(git2.getRepository().resolve("tag-initial")); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); @@ -481,6 +482,60 @@ public class CloneCommandTest extends RepositoryTestCase { specs.get(0)); } + @Test + public void testCloneRepositoryAllBranchesTakesPreference() + throws Exception { + File directory = createTempDirectory( + "testCloneRepositoryAllBranchesTakesPreference"); + CloneCommand command = Git.cloneRepository(); + command.setCloneAllBranches(true); + command.setBranchesToClone( + Collections.singletonList("refs/heads/test")); + command.setDirectory(directory); + command.setURI(fileUri()); + Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), "refs/heads/test"); + // Expect both remote branches to exist; setCloneAllBranches(true) + // should override any setBranchesToClone(). + assertNotNull( + git2.getRepository().resolve("refs/remotes/origin/master")); + assertNotNull(git2.getRepository().resolve("refs/remotes/origin/test")); + RemoteConfig cfg = new RemoteConfig(git2.getRepository().getConfig(), + Constants.DEFAULT_REMOTE_NAME); + List specs = cfg.getFetchRefSpecs(); + assertEquals(1, specs.size()); + assertEquals(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), + specs.get(0)); + } + + @Test + public void testCloneRepositoryAllBranchesIndependent() throws Exception { + File directory = createTempDirectory( + "testCloneRepositoryAllBranchesIndependent"); + CloneCommand command = Git.cloneRepository(); + command.setCloneAllBranches(true); + command.setBranchesToClone( + Collections.singletonList("refs/heads/test")); + command.setCloneAllBranches(false); + command.setDirectory(directory); + command.setURI(fileUri()); + Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), "refs/heads/test"); + // Expect only the test branch; allBranches was re-set to false + assertNull(git2.getRepository().resolve("refs/remotes/origin/master")); + assertNotNull(git2.getRepository().resolve("refs/remotes/origin/test")); + RemoteConfig cfg = new RemoteConfig(git2.getRepository().getConfig(), + Constants.DEFAULT_REMOTE_NAME); + List specs = cfg.getFetchRefSpecs(); + assertEquals(1, specs.size()); + assertEquals(new RefSpec("+refs/heads/test:refs/remotes/origin/test"), + specs.get(0)); + } + public static String allRefNames(List refs) { StringBuilder sb = new StringBuilder(); for (Ref f : refs) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java index 809f2d111..7008cd49a 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java @@ -104,7 +104,9 @@ public class CloneCommand extends TransportCommand { private ProgressMonitor monitor = NullProgressMonitor.INSTANCE; - private FETCH_TYPE fetchType = FETCH_TYPE.ALL_BRANCHES; + private boolean cloneAllBranches; + + private boolean mirror; private boolean cloneSubmodules; @@ -118,6 +120,8 @@ public class CloneCommand extends TransportCommand { private boolean gitDirExistsInitially; + private FETCH_TYPE fetchType; + private enum FETCH_TYPE { MULTIPLE_BRANCHES, ALL_BRANCHES, MIRROR } @@ -195,6 +199,7 @@ public class CloneCommand extends TransportCommand { throw new InvalidRemoteException( MessageFormat.format(JGitText.get().invalidURL, uri)); } + setFetchType(); @SuppressWarnings("resource") // Closed by caller Repository repository = init(); FetchResult fetchResult = null; @@ -238,6 +243,20 @@ public class CloneCommand extends TransportCommand { return new Git(repository, true); } + private void setFetchType() { + if (mirror) { + fetchType = FETCH_TYPE.MIRROR; + setBare(true); + } else if (cloneAllBranches) { + fetchType = FETCH_TYPE.ALL_BRANCHES; + } else if (branchesToClone != null && !branchesToClone.isEmpty()) { + fetchType = FETCH_TYPE.MULTIPLE_BRANCHES; + } else { + // Default: neither mirror nor all nor specific refs given + fetchType = FETCH_TYPE.ALL_BRANCHES; + } + } + private static boolean isNonEmptyDirectory(File dir) { if (dir != null && dir.exists()) { File[] files = dir.listFiles(); @@ -619,8 +638,7 @@ public class CloneCommand extends TransportCommand { * @return {@code this} */ public CloneCommand setCloneAllBranches(boolean cloneAllBranches) { - this.fetchType = cloneAllBranches ? FETCH_TYPE.ALL_BRANCHES - : this.fetchType; + this.cloneAllBranches = cloneAllBranches; return this; } @@ -640,10 +658,7 @@ public class CloneCommand extends TransportCommand { * @since 5.6 */ public CloneCommand setMirror(boolean mirror) { - if (mirror) { - this.fetchType = FETCH_TYPE.MIRROR; - setBare(true); - } + this.mirror = mirror; return this; } @@ -664,8 +679,9 @@ public class CloneCommand extends TransportCommand { * Set the branches or tags to clone. *

* This is ignored if {@link #setCloneAllBranches(boolean) - * setCloneAllBranches(true)} is used. If {@code branchesToClone} is - * {@code null} or empty, it's also ignored and all branches will be cloned. + * setCloneAllBranches(true)} or {@link #setMirror(boolean) setMirror(true)} + * is used. If {@code branchesToClone} is {@code null} or empty, it's also + * ignored. *

* * @param branchesToClone @@ -675,13 +691,7 @@ public class CloneCommand extends TransportCommand { * @return {@code this} */ public CloneCommand setBranchesToClone(Collection branchesToClone) { - if (branchesToClone == null || branchesToClone.isEmpty()) { - // fallback to default - fetchType = FETCH_TYPE.ALL_BRANCHES; - } else { - this.fetchType = FETCH_TYPE.MULTIPLE_BRANCHES; - this.branchesToClone = branchesToClone; - } + this.branchesToClone = branchesToClone; return this; }