Handle the revision attribute in repo manifest.

Change-Id: I77fe073aeb13c58029551b7d6e1451a9b62dc766
Signed-off-by: Yuxuan 'fishy' Wang <fishywang@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/RepoCommandTest.java

@@ -55,11 +55,15 @@
 import org.eclipse.jgit.junit.JGitTestUtil;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.junit.Test;
 
 public class RepoCommandTest extends RepositoryTestCase {
 
+	private static final String BRANCH = "branch";
+	private static final String TAG = "release";
+
 	private Repository defaultDb;
 	private Repository notDefaultDb;
 	private Repository groupADb;
@@ -71,14 +75,22 @@ public class RepoCommandTest extends RepositoryTestCase {
 	private String groupAUri;
 	private String groupBUri;
 
+	private ObjectId oldCommitId;
+
 	public void setUp() throws Exception {
 		super.setUp();
 
 		defaultDb = createWorkRepository();
 		Git git = new Git(defaultDb);
-		JGitTestUtil.writeTrashFile(defaultDb, "hello.txt", "world");
+		JGitTestUtil.writeTrashFile(defaultDb, "hello.txt", "branch world");
 		git.add().addFilepattern("hello.txt").call();
-		git.commit().setMessage("Initial commit").call();
+		oldCommitId = git.commit().setMessage("Initial commit").call().getId();
+		git.checkout().setName(BRANCH).setCreateBranch(true).call();
+		git.checkout().setName("master").call();
+		git.tag().setName(TAG).call();
+		JGitTestUtil.writeTrashFile(defaultDb, "hello.txt", "master world");
+		git.add().addFilepattern("hello.txt").call();
+		git.commit().setMessage("Second commit").call();
 
 		notDefaultDb = createWorkRepository();
 		git = new Git(notDefaultDb);
@@ -122,7 +134,7 @@ public void testAddRepoManifest() throws Exception {
 		BufferedReader reader = new BufferedReader(new FileReader(hello));
 		String content = reader.readLine();
 		reader.close();
-		assertEquals("submodule content is as expected.", "world", content);
+		assertEquals("submodule content is as expected.", "master world", content);
 	}
 
 	@Test
@@ -205,14 +217,14 @@ public void testRepoManifestCopyfile() throws Exception {
 		BufferedReader reader = new BufferedReader(new FileReader(hello));
 		String content = reader.readLine();
 		reader.close();
-		assertEquals("The original file has expected content", "world", content);
+		assertEquals("The original file has expected content", "master world", content);
 		// The dest file should also exist
 		hello = new File(localDb.getWorkTree(), "Hello");
 		assertTrue("The destination file exists", hello.exists());
 		reader = new BufferedReader(new FileReader(hello));
 		content = reader.readLine();
 		reader.close();
-		assertEquals("The destination file has expected content", "world", content);
+		assertEquals("The destination file has expected content", "master world", content);
 	}
 
 	@Test
@@ -248,12 +260,119 @@ public void testBareRepo() throws Exception {
 		reader.close();
 		assertEquals("The first line of .gitmodules file is as expected.",
 				"[submodule \"foo\"]", content);
-		// The gitlink should be the same of remote head sha1
+		// The gitlink should be the same as remote head sha1
 		String gitlink = localDb.resolve(Constants.HEAD + ":foo").name();
 		String remote = defaultDb.resolve(Constants.HEAD).name();
 		assertEquals("The gitlink is same as remote head", remote, gitlink);
 	}
 
+	@Test
+	public void testRevision() throws Exception {
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+			.append("<manifest>")
+			.append("<remote name=\"remote1\" fetch=\".\" />")
+			.append("<default revision=\"master\" remote=\"remote1\" />")
+			.append("<project path=\"foo\" name=\"")
+			.append(defaultUri)
+			.append("\" revision=\"")
+			.append(oldCommitId.name())
+			.append("\" />")
+			.append("</manifest>");
+		writeTrashFile("manifest.xml", xmlContent.toString());
+		RepoCommand command = new RepoCommand(db);
+		command.setPath(db.getWorkTree().getAbsolutePath() + "/manifest.xml")
+			.setURI(rootUri)
+			.call();
+		File hello = new File(db.getWorkTree(), "foo/hello.txt");
+		BufferedReader reader = new BufferedReader(new FileReader(hello));
+		String content = reader.readLine();
+		reader.close();
+		assertEquals("submodule content is as expected.", "branch world", content);
+	}
+
+	@Test
+	public void testRevisionBranch() throws Exception {
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+			.append("<manifest>")
+			.append("<remote name=\"remote1\" fetch=\".\" />")
+			.append("<default revision=\"")
+			.append(BRANCH)
+			.append("\" remote=\"remote1\" />")
+			.append("<project path=\"foo\" name=\"")
+			.append(defaultUri)
+			.append("\" />")
+			.append("</manifest>");
+		writeTrashFile("manifest.xml", xmlContent.toString());
+		RepoCommand command = new RepoCommand(db);
+		command.setPath(db.getWorkTree().getAbsolutePath() + "/manifest.xml")
+			.setURI(rootUri)
+			.call();
+		File hello = new File(db.getWorkTree(), "foo/hello.txt");
+		BufferedReader reader = new BufferedReader(new FileReader(hello));
+		String content = reader.readLine();
+		reader.close();
+		assertEquals("submodule content is as expected.", "branch world", content);
+	}
+
+	@Test
+	public void testRevisionTag() throws Exception {
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+			.append("<manifest>")
+			.append("<remote name=\"remote1\" fetch=\".\" />")
+			.append("<default revision=\"master\" remote=\"remote1\" />")
+			.append("<project path=\"foo\" name=\"")
+			.append(defaultUri)
+			.append("\" revision=\"")
+			.append(TAG)
+			.append("\" />")
+			.append("</manifest>");
+		writeTrashFile("manifest.xml", xmlContent.toString());
+		RepoCommand command = new RepoCommand(db);
+		command.setPath(db.getWorkTree().getAbsolutePath() + "/manifest.xml")
+			.setURI(rootUri)
+			.call();
+		File hello = new File(db.getWorkTree(), "foo/hello.txt");
+		BufferedReader reader = new BufferedReader(new FileReader(hello));
+		String content = reader.readLine();
+		reader.close();
+		assertEquals("submodule content is as expected.", "branch world", content);
+	}
+
+	@Test
+	public void testRevisionBare() throws Exception {
+		Repository remoteDb = createBareRepository();
+		Repository tempDb = createWorkRepository();
+		StringBuilder xmlContent = new StringBuilder();
+		xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n")
+			.append("<manifest>")
+			.append("<remote name=\"remote1\" fetch=\".\" />")
+			.append("<default revision=\"")
+			.append(BRANCH)
+			.append("\" remote=\"remote1\" />")
+			.append("<project path=\"foo\" name=\"")
+			.append(defaultUri)
+			.append("\" />")
+			.append("</manifest>");
+		JGitTestUtil.writeTrashFile(tempDb, "manifest.xml", xmlContent.toString());
+		RepoCommand command = new RepoCommand(remoteDb);
+		command.setPath(tempDb.getWorkTree().getAbsolutePath() + "/manifest.xml")
+			.setURI(rootUri)
+			.call();
+		// Clone it
+		File directory = createTempDirectory("testRevisionBare");
+		CloneCommand clone = Git.cloneRepository();
+		clone.setDirectory(directory);
+		clone.setURI(remoteDb.getDirectory().toURI().toString());
+		Repository localDb = clone.call().getRepository();
+		// The gitlink should be the same as oldCommitId
+		String gitlink = localDb.resolve(Constants.HEAD + ":foo").name();
+		assertEquals("The gitlink is same as remote head",
+				oldCommitId.name(), gitlink);
+	}
+
 	private void resolveRelativeUris() {
 		// Find the longest common prefix ends with "/" as rootUri.
 		defaultUri = defaultDb.getDirectory().toURI().toString();

org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/RepoCommand.java

@@ -81,6 +81,7 @@
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.lib.Repository;
@@ -108,6 +109,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	private String path;
 	private String uri;
 	private String groups;
+	private String branch;
 	private PersonIdent author;
 	private RemoteReader callback;
 
@@ -116,7 +118,7 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	private ProgressMonitor monitor;
 
 	/**
-	 * A callback to get head sha1 of a repository from its uri.
+	 * A callback to get ref sha1 of a repository from its uri.
 	 *
 	 * We provided a default implementation {@link DefaultRemoteReader} to
 	 * use ls-remote command to read the sha1 from the repository. Callers may
@@ -124,27 +126,32 @@ public class RepoCommand extends GitCommand<RevCommit> {
 	 */
 	public interface RemoteReader {
 		/**
-		 * Read a remote repository's HEAD sha1.
+		 * Read a remote ref sha1.
 		 *
 		 * @param uri
 		 *            The URI of the remote repository
-		 * @return the sha1 of the HEAD of the remote repository
+		 * @param ref
+		 *            The ref (branch/tag/etc.) to read
+		 * @return the sha1 of the remote repository
 		 */
-		public ObjectId sha1(String uri) throws GitAPIException;
+		public ObjectId sha1(String uri, String ref) throws GitAPIException;
 	}
 
 	/** A default implementation of {@link RemoteReader} callback. */
 	public static class DefaultRemoteReader implements RemoteReader {
-		public ObjectId sha1(String uri) throws GitAPIException {
+		public ObjectId sha1(String uri, String ref) throws GitAPIException {
 			Collection<Ref> refs = Git
 					.lsRemoteRepository()
 					.setRemote(uri)
 					.call();
-			for (Ref ref : refs) {
+			// Since LsRemoteCommand.call() only returned Map.values() to us, we
-				if (Constants.HEAD.equals(ref.getName()))
+			// have to rebuild the map here.
-					return ref.getObjectId();
+			Map<String, Ref> map = new HashMap<String, Ref>(refs.size());
-			}
+			for (Ref r : refs)
-			return null;
+				map.put(r.getName(), r);
+
+			Ref r = RefDatabase.findRef(map, ref);
+			return r != null ? r.getObjectId() : null;
 		}
 	}
 
@@ -180,10 +187,12 @@ private static class Project {
 		final String path;
 		final Set<String> groups;
 		final List<CopyFile> copyfiles;
+		String revision;
 
-		Project(String name, String path, String groups) {
+		Project(String name, String path, String revision, String groups) {
 			this.name = name;
 			this.path = path;
+			this.revision = revision;
 			this.groups = new HashSet<String>();
 			if (groups != null && groups.length() > 0)
 				this.groups.addAll(Arrays.asList(groups.split(","))); //$NON-NLS-1$
@@ -204,6 +213,7 @@ private static class XmlManifest extends DefaultHandler {
 		private final Set<String> plusGroups;
 		private final Set<String> minusGroups;
 		private String defaultRemote;
+		private String defaultRevision;
 		private Project currentProject;
 
 		XmlManifest(RepoCommand command, String filename, String baseUrl, String groups) {
@@ -258,12 +268,16 @@ public void startElement(
 				currentProject = new Project( //$NON-NLS-1$
 						attributes.getValue("name"), //$NON-NLS-1$
 						attributes.getValue("path"), //$NON-NLS-1$
+						attributes.getValue("revision"), //$NON-NLS-1$
 						attributes.getValue("groups")); //$NON-NLS-1$
 			} else if ("remote".equals(qName)) { //$NON-NLS-1$
 				remotes.put(attributes.getValue("name"), //$NON-NLS-1$
 						attributes.getValue("fetch")); //$NON-NLS-1$
 			} else if ("default".equals(qName)) { //$NON-NLS-1$
 				defaultRemote = attributes.getValue("remote"); //$NON-NLS-1$
+				defaultRevision = attributes.getValue("revision"); //$NON-NLS-1$
+				if (defaultRevision == null)
+					defaultRevision = command.branch;
 			} else if ("copyfile".equals(qName)) { //$NON-NLS-1$
 				if (currentProject == null)
 					throw new SAXException(RepoText.get().invalidManifest);
@@ -301,8 +315,10 @@ public void endDocument() throws SAXException {
 			}
 			for (Project proj : projects) {
 				if (inGroups(proj)) {
+					if (proj.revision == null)
+						proj.revision = defaultRevision;
 					String url = remoteUrl + proj.name;
-					command.addSubmodule(url, proj.path);
+					command.addSubmodule(url, proj.path, proj.revision);
 					for (CopyFile copyfile : proj.copyfiles) {
 						try {
 							copyfile.copy();
@@ -349,8 +365,8 @@ private static class ManifestErrorException extends GitAPIException {
 	}
 
 	private static class RemoteUnavailableException extends GitAPIException {
-		RemoteUnavailableException(String uri, Throwable cause) {
+		RemoteUnavailableException(String uri) {
-			super(MessageFormat.format(RepoText.get().errorRemoteUnavailable, uri), cause);
+			super(MessageFormat.format(RepoText.get().errorRemoteUnavailable, uri));
 		}
 	}
 
@@ -395,6 +411,21 @@ public RepoCommand setGroups(final String groups) {
 		return this;
 	}
 
+	/**
+	 * Set default branch.
+	 *
+	 * This is generally the name of the branch the manifest file was in. If
+	 * there's no default revision (branch) specified in manifest and no
+	 * revision specified in project, this branch will be used.
+	 *
+	 * @param branch
+	 * @return this command
+	 */
+	public RepoCommand setBranch(final String branch) {
+		this.branch = branch;
+		return this;
+	}
+
 	/**
 	 * The progress monitor associated with the clone operation. By default,
 	 * this is set to <code>NullProgressMonitor</code>
@@ -474,17 +505,13 @@ public RevCommit call() throws GitAPIException {
 					// create gitlink
 					final DirCacheEntry dcEntry = new DirCacheEntry(name);
 					ObjectId objectId;
-					try {
+					if (ObjectId.isId(proj.revision))
-						objectId = callback.sha1(uri);
+						objectId = ObjectId.fromString(proj.revision);
-					} catch (GitAPIException e) {
+					else {
-						// Something wrong getting the head sha1
+						objectId = callback.sha1(uri, proj.revision);
-						throw new RemoteUnavailableException(uri, e);
-					} catch (IllegalArgumentException e) {
-						// The revision from the manifest is malformed.
-						throw new ManifestErrorException(e);
 					}
 					if (objectId == null)
-						throw new RemoteUnavailableException(uri, null);
+						throw new RemoteUnavailableException(uri);
 					dcEntry.setObjectId(objectId);
 					dcEntry.setFileMode(FileMode.GITLINK);
 					builder.add(dcEntry);
@@ -551,9 +578,9 @@ public RevCommit call() throws GitAPIException {
 		}
 	}
 
-	private void addSubmodule(String url, String name) throws SAXException {
+	private void addSubmodule(String url, String name, String revision) throws SAXException {
 		if (repo.isBare()) {
-			Project proj = new Project(url, name, null);
+			Project proj = new Project(url, name, revision, null);
 			bareProjects.add(proj);
 		} else {
 			SubmoduleAddCommand add = git
@@ -562,11 +589,30 @@ private void addSubmodule(String url, String name) throws SAXException {
 				.setURI(url);
 			if (monitor != null)
 				add.setProgressMonitor(monitor);
+
 			try {
-				add.call();
+				Repository subRepo = add.call();
+				if (revision != null) {
+					Git sub = new Git(subRepo);
+					sub.checkout().setName(findRef(revision, subRepo)).call();
+					git.add().addFilepattern(name).call();
+				}
 			} catch (GitAPIException e) {
 				throw new SAXException(e);
+			} catch (IOException e) {
+				throw new SAXException(e);
 			}
 		}
 	}
+
+	private static String findRef(String ref, Repository repo)
+			throws IOException {
+		if (!ObjectId.isId(ref)) {
+			Ref r = repo.getRef(
+					Constants.DEFAULT_REMOTE_NAME + "/" + ref);
+			if (r != null)
+				return r.getName();
+		}
+		return ref;
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java

@@ -271,4 +271,26 @@ public BatchRefUpdate newBatchUpdate() {
 	public void refresh() {
 		// nothing
 	}
+
+	/**
+	 * Try to find the specified name in the ref map using {@link #SEARCH_PATH}.
+	 *
+	 * @param map
+	 *            map of refs to search within. Names should be fully qualified,
+	 *            e.g. "refs/heads/master".
+	 * @param name
+	 *            short name of ref to find, e.g. "master" to find
+	 *            "refs/heads/master" in map.
+	 * @return The first ref matching the name, or null if not found.
+	 * @since 3.4
+	 */
+	public static Ref findRef(Map<String, Ref> map, String name) {
+		for (String prefix : SEARCH_PATH) {
+			String fullname = prefix + name;
+			Ref ref = map.get(fullname);
+			if (ref != null)
+				return ref;
+		}
+		return null;
+	}
 }