Walk tag chains for --include-tag options

When cloning repository with --single-branch option, tag chains are not
packed and pack file is broken in some cases.

Typical test-case:
git tag -a test_tag <commit-id>
git tag -a test_prev_tag test_tag
git tag -d test_tag

git clone --single-branch <repository>
fatal: did not receive expected object <test_tag_id>

The reason for that is missing object for original test_tag reference,
which was deleted.

Problem description:
When pack-objects is given --include-tag, it peels each tag reference
down to a commit. If the commit is prepared to be packed, we we have to
include such tag too. The problem is when the tag points to through some
chain of other tag to commit. Then, the inner tags are not added leading
to broken pack.

Fix:
When going to commit, we have to check and add any of the tags on the
way (if they were not selected, which may happen with --single-branch
option).

Change-Id: I1682d4a2c52d674f90a1b021e0f6c3524c5ce5bc
Signed-off-by: Pavel Flaška <Pavel.Flaska@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java

@@ -14,9 +14,11 @@ import static org.junit.Assert.fail;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -30,6 +32,7 @@ import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.internal.storage.dfs.DfsGarbageCollector;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.internal.storage.file.PackLock;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
@@ -65,7 +68,7 @@ public class UploadPackTest {
 
 	private TestProtocol<Object> testProtocol;
 
-	private Object ctx = new Object();
+	private final Object ctx = new Object();
 
 	private InMemoryRepository server;
 
@@ -2110,4 +2113,84 @@ public class UploadPackTest {
 			return new HashMap<>();
 		}
 	}
+
+	@Test
+	public void testSingleBranchCloneTagChain() throws Exception {
+		RevBlob blob0 = remote.blob("Initial content of first file");
+		RevBlob blob1 = remote.blob("Second file content");
+		RevCommit commit0 = remote
+				.commit(remote.tree(remote.file("prvni.txt", blob0)));
+		RevCommit commit1 = remote
+				.commit(remote.tree(remote.file("druhy.txt", blob1)), commit0);
+		remote.update("master", commit1);
+
+		RevTag heavyTag1 = remote.tag("commitTagRing", commit0);
+		remote.getRevWalk().parseHeaders(heavyTag1);
+		RevTag heavyTag2 = remote.tag("middleTagRing", heavyTag1);
+		remote.lightweightTag("refTagRing", heavyTag2);
+
+		UploadPack uploadPack = new UploadPack(remote.getRepository());
+
+		ByteArrayOutputStream cli = new ByteArrayOutputStream();
+		PacketLineOut clientWant = new PacketLineOut(cli);
+		clientWant.writeString("want " + commit1.name()
+				+ " multi_ack_detailed include-tag thin-pack ofs-delta agent=tempo/pflaska");
+		clientWant.end();
+		clientWant.writeString("done\n");
+
+		try (ByteArrayOutputStream serverResponse = new ByteArrayOutputStream()) {
+
+			uploadPack.setPreUploadHook(new PreUploadHook() {
+				@Override
+				public void onBeginNegotiateRound(UploadPack up,
+						Collection<? extends ObjectId> wants, int cntOffered)
+						throws ServiceMayNotContinueException {
+					// Do nothing.
+				}
+
+				@Override
+				public void onEndNegotiateRound(UploadPack up,
+						Collection<? extends ObjectId> wants, int cntCommon,
+						int cntNotFound, boolean ready)
+						throws ServiceMayNotContinueException {
+					// Do nothing.
+				}
+
+				@Override
+				public void onSendPack(UploadPack up,
+						Collection<? extends ObjectId> wants,
+						Collection<? extends ObjectId> haves)
+						throws ServiceMayNotContinueException {
+					// collect pack data
+					serverResponse.reset();
+				}
+			});
+			uploadPack.upload(new ByteArrayInputStream(cli.toByteArray()),
+					serverResponse, System.err);
+			InputStream packReceived = new ByteArrayInputStream(
+					serverResponse.toByteArray());
+			PackLock lock = null;
+			try (ObjectInserter ins = client.newObjectInserter()) {
+				PackParser parser = ins.newPackParser(packReceived);
+				parser.setAllowThin(true);
+				parser.setLockMessage("receive-tag-chain");
+				ProgressMonitor mlc = NullProgressMonitor.INSTANCE;
+				lock = parser.parse(mlc, mlc);
+				ins.flush();
+			} finally {
+				if (lock != null) {
+					lock.unlock();
+				}
+			}
+			InMemoryRepository.MemObjDatabase objDb = client
+					.getObjectDatabase();
+			assertTrue(objDb.has(blob0.toObjectId()));
+			assertTrue(objDb.has(blob1.toObjectId()));
+			assertTrue(objDb.has(commit0.toObjectId()));
+			assertTrue(objDb.has(commit1.toObjectId()));
+			assertTrue(objDb.has(heavyTag1.toObjectId()));
+			assertTrue(objDb.has(heavyTag2.toObjectId()));
+		}
+	}
+
 }

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -2219,8 +2219,11 @@ public class UploadPack {
 					if (peeledId == null || objectId == null)
 						continue;
 
+					objectId = ref.getObjectId();
 					if (pw.willInclude(peeledId) && !pw.willInclude(objectId)) {
-						pw.addObject(rw.parseAny(objectId));
+						RevObject o = rw.parseAny(objectId);
+						addTagChain(o, pw);
+						pw.addObject(o);
 					}
 				}
 			}
@@ -2253,6 +2256,18 @@ public class UploadPack {
 		}
 	}
 
+	private void addTagChain(
+			RevObject o, PackWriter pw) throws IOException {
+		while (Constants.OBJ_TAG == o.getType()) {
+			RevTag t = (RevTag) o;
+			o = t.getObject();
+			if (o.getType() == Constants.OBJ_TAG && !pw.willInclude(o.getId())) {
+				walk.parseBody(o);
+				pw.addObject(o);
+			}
+		}
+	}
+
 	private static class ResponseBufferedOutputStream extends OutputStream {
 		private final OutputStream rawOut;