Change ref parameter of CloneOperation to String

The parameter is used for specifying what branch should be checked out
after fetch. Before this change, it was of type Ref. But a Ref is not
really right here, as a Ref should point to a valid object id, and the
ids aren't generally known before a clone. This lead users of
CloneOperation to passing a Ref with only a name set, and no object id
(using a anonymous Ref subclass or passing null as object id).

Before the parameter was a Ref, it already was a String, but one could
only specify a short branch name (e.g. "master"), not a full ref name
("refs/heads/master"). With this change, it's still the full ref name,
just as a String.

Bug: 338603
Change-Id: I420059e7eda4b3bbbf335557217bc89f11c27abd
Signed-off-by: Robin Stocker <robin@nibor.org>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>

8 files changed, 24 insertions, 96 deletions

diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java
index f51587d426..a3b527dfdd 100644
--- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java
+++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java
@@ -29,7 +29,6 @@ import org.eclipse.egit.core.op.ListRemoteOperation;
 import org.eclipse.egit.core.test.DualRepositoryTestCase;
 import org.eclipse.egit.core.test.TestRepository;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.URIish;
@@ -92,9 +91,8 @@ public class ListRemoteOperationTest extends DualRepositoryTestCase {
 		// let's clone repository1 to repository2
 		URIish uri = new URIish("file:///"
 				+ repository1.getRepository().getDirectory().toString());
-		Ref master = repository1.getRepository().getRef("refs/heads/master");
 		CloneOperation clop = new CloneOperation(uri, true, null, workdir2,
-				master, "origin", 0);
+				"refs/heads/master", "origin", 0);
 		clop.run(null);
 
 		Repository existingRepo = Activator
diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/PushOperationTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/PushOperationTest.java
index 1a2cf05b3e..b4b4300d15 100644
--- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/PushOperationTest.java
+++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/PushOperationTest.java
@@ -37,7 +37,6 @@ import org.eclipse.egit.core.test.DualRepositoryTestCase;
 import org.eclipse.egit.core.test.TestRepository;
 import org.eclipse.egit.core.test.TestUtils;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
@@ -102,9 +101,8 @@ public class PushOperationTest extends DualRepositoryTestCase {
 		// let's clone repository1 to repository2
 		URIish uri = new URIish("file:///"
 				+ repository1.getRepository().getDirectory().toString());
-		Ref master = repository1.getRepository().getRef("refs/heads/master");
 		CloneOperation clop = new CloneOperation(uri, true, null, workdir2,
-				master, "origin", 0);
+				"refs/heads/master", "origin", 0);
 		clop.run(null);
 
 		Repository repo2 = Activator.getDefault().getRepositoryCache().lookupRepository(new File(workdir2,
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CloneOperation.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CloneOperation.java
index e0c794b290..e808d05482 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CloneOperation.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/CloneOperation.java
@@ -30,6 +30,7 @@ import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -57,7 +58,7 @@ public class CloneOperation {
 
 	private final File gitdir;
 
-	private final Ref ref;
+	private final String refName;
 
 	private final String remoteName;
 
@@ -85,22 +86,24 @@ public class CloneOperation {
 	 * @param workdir
 	 *            working directory to clone to. The directory may or may not
 	 *            already exist.
-	 * @param ref
-	 *            ref to be checked out after clone.
+	 * @param refName
+	 *            full name of ref to be checked out after clone, e.g.
+	 *            refs/heads/master, or null for no checkout
 	 * @param remoteName
 	 *            name of created remote config as source remote (typically
 	 *            named "origin").
-	 * @param timeout timeout in seconds
+	 * @param timeout
+	 *            timeout in seconds
 	 */
 	public CloneOperation(final URIish uri, final boolean allSelected,
 			final Collection<Ref> selectedBranches, final File workdir,
-			final Ref ref, final String remoteName, int timeout) {
+			final String refName, final String remoteName, int timeout) {
 		this.uri = uri;
 		this.allSelected = allSelected;
 		this.selectedBranches = selectedBranches;
 		this.workdir = workdir;
 		this.gitdir = new File(workdir, Constants.DOT_GIT);
-		this.ref = ref;
+		this.refName = refName;
 		this.remoteName = remoteName;
 		this.timeout = timeout;
 	}
@@ -175,10 +178,10 @@ public class CloneOperation {
 		local = new FileRepository(gitdir);
 		local.create();
 
-		if (ref != null && ref.getName().startsWith(Constants.R_HEADS)) {
+		if (refName != null && refName.startsWith(Constants.R_HEADS)) {
 			final RefUpdate head = local.updateRef(Constants.HEAD);
 			head.disableRefLog();
-			head.link(ref.getName());
+			head.link(refName);
 		}
 
 		FileBasedConfig config = local.getConfig();
@@ -207,12 +210,12 @@ public class CloneOperation {
 
 		// branch is like 'Constants.R_HEADS + branchName', we need only
 		// the 'branchName' part
-		if (ref != null && ref.getName().startsWith(Constants.R_HEADS)) {
-			String branchName = ref.getName().substring(Constants.R_HEADS.length());
+		if (refName != null && refName.startsWith(Constants.R_HEADS)) {
+			String branchName = Repository.shortenRefName(refName);
 
 			// setup the default remote branch for branchName
 			config.setString(ConfigConstants.CONFIG_BRANCH_SECTION, branchName, ConfigConstants.CONFIG_KEY_REMOTE, remoteName);
-			config.setString(ConfigConstants.CONFIG_BRANCH_SECTION, branchName, ConfigConstants.CONFIG_KEY_MERGE, ref.getName());
+			config.setString(ConfigConstants.CONFIG_BRANCH_SECTION, branchName, ConfigConstants.CONFIG_KEY_MERGE, refName);
 		}
 		config.save();
 	}
@@ -233,9 +236,9 @@ public class CloneOperation {
 	}
 
 	private void doCheckout(final IProgressMonitor monitor) throws IOException {
-		if (ref == null)
+		if (refName == null)
 			return;
-		final Ref head = fetchResult.getAdvertisedRef(ref.getName());
+		final Ref head = fetchResult.getAdvertisedRef(refName);
 		if (head == null || head.getObjectId() == null)
 			return;
 
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java
index 8d6b90fac3..33ea971c5b 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java
@@ -336,9 +336,8 @@ public abstract class LocalRepositoryTestCase extends EGitTestCase {
 		Repository myRepository = lookupRepository(repositoryDir);
 		URIish uri = new URIish("file:///" + myRepository.getDirectory());
 		File workdir = new File(testDirectory, CHILDREPO);
-		Ref master = myRepository.getRef("refs/heads/master");
 		CloneOperation clop = new CloneOperation(uri, true, null, workdir,
-				master, "origin", 0);
+				"refs/heads/master", "origin", 0);
 		clop.run(null);
 		return new File(workdir, Constants.DOT_GIT);
 	}
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/httpauth/PushTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/httpauth/PushTest.java
index 890a32cc9a..4bc4c8e00d 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/httpauth/PushTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/httpauth/PushTest.java
@@ -25,8 +25,6 @@ import org.eclipse.egit.ui.test.TestUtil;
 import org.eclipse.egit.ui.wizards.clone.SampleTestRepository;

 import org.eclipse.jgit.api.Git;

 import org.eclipse.jgit.lib.Constants;

-import org.eclipse.jgit.lib.ObjectId;

-import org.eclipse.jgit.lib.Ref;

 import org.eclipse.jgit.lib.Repository;

 import org.eclipse.jgit.lib.StoredConfig;

 import org.eclipse.jgit.transport.URIish;

@@ -55,9 +53,8 @@ public class PushTest extends EGitTestCase {
 		localRepoPath = new File(ResourcesPlugin.getWorkspace().getRoot()

 				.getLocation().toFile(), "test1");

 		String branch = Constants.R_HEADS + SampleTestRepository.FIX;

-		Ref ref = createRef(branch);

 		CloneOperation cloneOperation = new CloneOperation(new URIish(

-				remoteRepository.getUri()), true, null, localRepoPath, ref,

+				remoteRepository.getUri()), true, null, localRepoPath, branch,

 				"origin", 30);

 		cloneOperation

 				.setCredentialsProvider(new UsernamePasswordCredentialsProvider(

@@ -69,45 +66,6 @@ public class PushTest extends EGitTestCase {
 				.lookupRepository(new File(localRepoPath, ".git"));

 	}

 

-	private Ref createRef(final String branch) {

-		Ref ref = new Ref() {

-

-			public String getName() {

-				return branch;

-			}

-

-			public boolean isSymbolic() {

-				return false;

-			}

-

-			public Ref getLeaf() {

-				return null;

-			}

-

-			public Ref getTarget() {

-				return null;

-			}

-

-			public ObjectId getObjectId() {

-				return null;

-			}

-

-			public ObjectId getPeeledObjectId() {

-				return null;

-			}

-

-			public boolean isPeeled() {

-				return false;

-			}

-

-			public Storage getStorage() {

-				return null;

-			}

-

-		};

-		return ref;

-	}

-

 	@Test

 	public void testPush() throws Exception {

 		// change file

diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java
index 4dd542b534..5f7b08c3c3 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewBranchHandlingTest.java
@@ -15,7 +15,6 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
-import java.util.Collection;
 import java.util.List;
 
 import org.eclipse.egit.core.op.BranchOperation;
@@ -29,7 +28,6 @@ import org.eclipse.egit.ui.test.TestUtil;
 import org.eclipse.jface.dialogs.IDialogConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.URIish;
 import org.eclipse.osgi.util.NLS;
@@ -67,16 +65,8 @@ public class GitRepositoriesViewBranchHandlingTest extends
 		final URIish uri = new URIish(remoteRepositoryFile.getPath());
 		final File workdir = new File(getTestDirectory(), "Cloned");
 
-		Collection<Ref> remoteRefs = getRemoteRefs(uri);
-		Ref myref = null;
-		for (Ref ref : remoteRefs) {
-			if (ref.getName().equals("refs/heads/master")) {
-				myref = ref;
-				break;
-			}
-		}
 		CloneOperation op = new CloneOperation(uri, true, null, workdir,
-				myref, "origin", 0);
+				"refs/heads/master", "origin", 0);
 		op.run(null);
 
 		clonedRepositoryFile = new File(workdir, Constants.DOT_GIT);
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewFetchAndPushTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewFetchAndPushTest.java
index 815a3e53fa..d29d4d4d0e 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewFetchAndPushTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewFetchAndPushTest.java
@@ -14,7 +14,6 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
-import java.util.Collection;
 
 import org.eclipse.egit.core.op.CloneOperation;
 import org.eclipse.egit.ui.Activator;
@@ -24,7 +23,6 @@ import org.eclipse.egit.ui.internal.push.PushOperationUI;
 import org.eclipse.egit.ui.test.ContextMenuHelper;
 import org.eclipse.egit.ui.test.TestUtil;
 import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.storage.file.FileRepository;
 import org.eclipse.jgit.transport.RemoteConfig;
@@ -62,16 +60,8 @@ public class GitRepositoriesViewFetchAndPushTest extends
 		URIish uri = new URIish("file:///" + remoteRepositoryFile.getPath());
 		File workdir = new File(getTestDirectory(), "ClonedRepo");
 
-		Collection<Ref> remoteRefs = getRemoteRefs(uri);
-		Ref myref = null;
-		for (Ref ref : remoteRefs) {
-			if (ref.getName().equals("refs/heads/master")) {
-				myref = ref;
-				break;
-			}
-		}
 		CloneOperation op = new CloneOperation(uri, true, null, workdir,
-				myref, "origin", 0);
+				"refs/heads/master", "origin", 0);
 		op.run(null);
 
 		clonedRepositoryFile = new File(workdir, Constants.DOT_GIT);
@@ -80,15 +70,7 @@ public class GitRepositoriesViewFetchAndPushTest extends
 		uri = new URIish(remoteRepositoryFile.getPath());
 		workdir = new File(getTestDirectory(), "ClonedRepo2");
 
-		remoteRefs = getRemoteRefs(uri);
-		myref = null;
-		for (Ref ref : remoteRefs) {
-			if (ref.getName().equals("refs/heads/master")) {
-				myref = ref;
-				break;
-			}
-		}
-		op = new CloneOperation(uri, true, null, workdir, myref,
+		op = new CloneOperation(uri, true, null, workdir, "refs/heads/master",
 				"origin", 0);
 		op.run(null);
 
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java
index 6fb7cbff85..b9bd4e4d84 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/GitCloneWizard.java
@@ -189,7 +189,7 @@ public class GitCloneWizard extends Wizard {
 		int timeout = Activator.getDefault().getPreferenceStore().getInt(
 				UIPreferences.REMOTE_CONNECTION_TIMEOUT);
 		final CloneOperation op = new CloneOperation(uri, allSelected,
-				selectedBranches, workdir, ref, remoteName, timeout);
+				selectedBranches, workdir, ref.getName(), remoteName, timeout);
 		UserPasswordCredentials credentials = cloneSource.getCredentials();
 		if (credentials != null)
 			op.setCredentialsProvider(new UsernamePasswordCredentialsProvider(