Browse Source

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
stable-3.0
Roberto Tyley 12 years ago
parent
commit
a46b042905
  1. 27
      org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java
  2. 9
      org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java

27
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.File;
import java.io.IOException; import java.io.IOException;
import java.net.URISyntaxException;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map; 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.SubmoduleStatus;
import org.eclipse.jgit.submodule.SubmoduleStatusType; import org.eclipse.jgit.submodule.SubmoduleStatusType;
import org.eclipse.jgit.submodule.SubmoduleWalk; 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.eclipse.jgit.util.SystemReader;
import org.junit.Test; import org.junit.Test;
@ -105,7 +108,7 @@ public class CloneCommandTest extends RepositoryTestCase {
@Test @Test
public void testCloneRepository() throws IOException, public void testCloneRepository() throws IOException,
JGitInternalException, GitAPIException { JGitInternalException, GitAPIException, URISyntaxException {
File directory = createTempDirectory("testCloneRepository"); File directory = createTempDirectory("testCloneRepository");
CloneCommand command = Git.cloneRepository(); CloneCommand command = Git.cloneRepository();
command.setDirectory(directory); command.setDirectory(directory);
@ -130,6 +133,28 @@ public class CloneCommandTest extends RepositoryTestCase {
"test", ConfigConstants.CONFIG_KEY_MERGE)); "test", ConfigConstants.CONFIG_KEY_MERGE));
assertEquals(2, git2.branchList().setListMode(ListMode.REMOTE).call() assertEquals(2, git2.branchList().setListMode(ListMode.REMOTE).call()
.size()); .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 @Test

9
org.eclipse.jgit/src/org/eclipse/jgit/api/CloneCommand.java

@ -154,12 +154,13 @@ public class CloneCommand extends TransportCommand<CloneCommand, Git> {
RemoteConfig config = new RemoteConfig(clonedRepo.getConfig(), remote); RemoteConfig config = new RemoteConfig(clonedRepo.getConfig(), remote);
config.addURI(u); config.addURI(u);
final String dst = bare ? Constants.R_HEADS : Constants.R_REMOTES final String dst = (bare ? Constants.R_HEADS : Constants.R_REMOTES
+ config.getName(); + config.getName() + "/") + "*";
RefSpec refSpec = new RefSpec(); RefSpec refSpec = new RefSpec();
refSpec = refSpec.setForceUpdate(true); refSpec = refSpec.setForceUpdate(true);
refSpec = refSpec.setSourceDestination( 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.addFetchRefSpec(refSpec);
config.update(clonedRepo.getConfig()); config.update(clonedRepo.getConfig());
@ -182,7 +183,7 @@ public class CloneCommand extends TransportCommand<CloneCommand, Git> {
private List<RefSpec> calculateRefSpecs(final String dst) { private List<RefSpec> calculateRefSpecs(final String dst) {
RefSpec wcrs = new RefSpec(); RefSpec wcrs = new RefSpec();
wcrs = wcrs.setForceUpdate(true); 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<RefSpec> specs = new ArrayList<RefSpec>(); List<RefSpec> specs = new ArrayList<RefSpec>();
if (cloneAllBranches) if (cloneAllBranches)
specs.add(wcrs); specs.add(wcrs);

Loading…
Cancel
Save