From a46b0429053288de4435ac26dc22f27f4ac43978 Mon Sep 17 00:00:00 2001 From: Roberto Tyley Date: Mon, 4 Mar 2013 00:03:20 +0000 Subject: [PATCH] Fix corrupted CloneCommand bare-repo fetch-refspec (#402031) CloneCommand has been creating fetch refspecs like this on bare clones: [remote "origin"] url = ssh://example.com/my-repo.git fetch = +refs/heads/*:refs/heads//* As you can see, the destination ref pattern has a superfluous slash. It looks like this behaviour has always been the case for CloneCommand, at least since cc2197ed when code catering to bare-clone fetch refspecs was added. That was released with JGit v1.0 almost 2 years ago, so there will probably be some bare repos in the wild which will have been cloned with JGit and have these corrupted refspecs. The effect of the corrupted fetch refspec is quite interesting. Up to and including JGit 2.0, the corrupt refspec was tolerated and fetches would work as intended with no indication to the user that anything was amiss. With JGit 2.1, a change was introduced which made JGit less tolerant, and fetches now attempt to update the non-existing ref "refs/heads//master". No exception is raised, but the real ref - "refs/heads/master" - is not updated. This behaviour was noticed by a user of Agit (which does bare clones by default and recently updated from JGit v2.0 to v2.2), reported here: https://github.com/rtyley/agit/issues/92 If you run C-Git fetch on a bare-repo cloned by JGit, it flat-out rejects the refspec (checked against v1.7.10.4): fatal: Invalid refspec '+refs/heads/*:refs/heads//*' Incidentally, C-Git does not create an explicit fetch refspec at all when performing a bare clone - the full remote config generated by C-Git looks like this: [remote "origin"] url = ssh://example.com/my-repo.git Using JGit on such a repository works fine, so omitting the fetch refspec entirely is also an option. Change-Id: I14b0d359dc69b8908f68e02cea7a756ac34bf881 --- .../eclipse/jgit/api/CloneCommandTest.java | 27 ++++++++++++++++++- .../org/eclipse/jgit/api/CloneCommand.java | 9 ++++--- 2 files changed, 31 insertions(+), 5 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 99e05d4d5..742295192 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 @@ -50,6 +50,7 @@ import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; +import java.net.URISyntaxException; import java.util.Collections; import java.util.List; import java.util.Map; @@ -71,6 +72,8 @@ import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.submodule.SubmoduleStatus; import org.eclipse.jgit.submodule.SubmoduleStatusType; import org.eclipse.jgit.submodule.SubmoduleWalk; +import org.eclipse.jgit.transport.RefSpec; +import org.eclipse.jgit.transport.RemoteConfig; import org.eclipse.jgit.util.SystemReader; import org.junit.Test; @@ -105,7 +108,7 @@ public class CloneCommandTest extends RepositoryTestCase { @Test public void testCloneRepository() throws IOException, - JGitInternalException, GitAPIException { + JGitInternalException, GitAPIException, URISyntaxException { File directory = createTempDirectory("testCloneRepository"); CloneCommand command = Git.cloneRepository(); command.setDirectory(directory); @@ -130,6 +133,28 @@ public class CloneCommandTest extends RepositoryTestCase { "test", ConfigConstants.CONFIG_KEY_MERGE)); assertEquals(2, git2.branchList().setListMode(ListMode.REMOTE).call() .size()); + assertEquals(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), + fetchRefSpec(git2.getRepository())); + } + + @Test + public void testBareCloneRepository() throws IOException, + JGitInternalException, GitAPIException, URISyntaxException { + File directory = createTempDirectory("testCloneRepository_bare"); + CloneCommand command = Git.cloneRepository(); + command.setBare(true); + command.setDirectory(directory); + command.setURI("file://" + git.getRepository().getWorkTree().getPath()); + Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertEquals(new RefSpec("+refs/heads/*:refs/heads/*"), + fetchRefSpec(git2.getRepository())); + } + + public static RefSpec fetchRefSpec(Repository r) throws URISyntaxException { + RemoteConfig remoteConfig = + new RemoteConfig(r.getConfig(), Constants.DEFAULT_REMOTE_NAME); + return remoteConfig.getFetchRefSpecs().get(0); } @Test 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 22bda612f..35bf75dcc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java @@ -154,12 +154,13 @@ public class CloneCommand extends TransportCommand { RemoteConfig config = new RemoteConfig(clonedRepo.getConfig(), remote); config.addURI(u); - final String dst = bare ? Constants.R_HEADS : Constants.R_REMOTES - + config.getName(); + final String dst = (bare ? Constants.R_HEADS : Constants.R_REMOTES + + config.getName() + "/") + "*"; RefSpec refSpec = new RefSpec(); refSpec = refSpec.setForceUpdate(true); refSpec = refSpec.setSourceDestination( - Constants.R_HEADS + "*", dst + "/*"); //$NON-NLS-1$ //$NON-NLS-2$ + Constants.R_HEADS + "*", dst); //$NON-NLS-1$ //$NON-NLS-2$ + config.addFetchRefSpec(refSpec); config.update(clonedRepo.getConfig()); @@ -182,7 +183,7 @@ public class CloneCommand extends TransportCommand { private List calculateRefSpecs(final String dst) { RefSpec wcrs = new RefSpec(); wcrs = wcrs.setForceUpdate(true); - wcrs = wcrs.setSourceDestination(Constants.R_HEADS + "*", dst + "/*"); //$NON-NLS-1$ //$NON-NLS-2$ + wcrs = wcrs.setSourceDestination(Constants.R_HEADS + "*", dst); //$NON-NLS-1$ //$NON-NLS-2$ List specs = new ArrayList(); if (cloneAllBranches) specs.add(wcrs);