motiejus
/
jgit
1
Fork 0
Code Releases Activity

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 <thomas.wolf@paranor.ch>
This commit is contained in:
Thomas Wolf 2020-02-03 21:00:46 +01:00
parent 3d59d1b80c
commit 75a80c5d3c
2 changed files with 81 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java
View File

@ -441,6 +441,7 @@ public void testBareCloneRepositoryMirror() throws Exception {
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 void testCloneRepositoryOnlyOneTag() throws Exception {
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<RefSpec> 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<RefSpec> 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<Ref> refs) {
StringBuilder sb = new StringBuilder();
for (Ref f : refs) {

42
org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java
View File

@ -104,7 +104,9 @@ public class CloneCommand extends TransportCommand<CloneCommand, Git> {
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<CloneCommand, Git> {
private boolean gitDirExistsInitially;
private FETCH_TYPE fetchType;
private enum FETCH_TYPE {
MULTIPLE_BRANCHES, ALL_BRANCHES, MIRROR
}
@ -195,6 +199,7 @@ public Git call() throws GitAPIException, InvalidRemoteException,
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 Git call() throws GitAPIException, InvalidRemoteException,
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 CloneCommand setProgressMonitor(ProgressMonitor monitor) {
* @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 CloneCommand setCloneAllBranches(boolean cloneAllBranches) {
* @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 CloneCommand setCloneSubmodules(boolean cloneSubmodules) {
* Set the branches or tags to clone.
* <p>
* 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.
* </p>
*
* @param branchesToClone
@ -675,13 +691,7 @@ public CloneCommand setCloneSubmodules(boolean cloneSubmodules) {
* @return {@code this}
*/
public CloneCommand setBranchesToClone(Collection<String> 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;
}