Fix push to repo with non-fetched refs

When JGit uses bitmaps (which is the case after a gc), the push command
doesn't go through the code where MissingObjectExceptions are caught
for remote objects not found locally.

Fixed by removing earlier non-locally-found remote objects.

This was seen withing gerrit, see:
https://code.google.com/p/gerrit/issues/detail?id=2025

Bug: 426044
Change-Id: Ieda718a0530e3680036edfa0963ab88fdd1362c0
Signed-off-by: Jean-Jacques Lafay <jeanjacques.lafay@gmail.com>
Signed-off-by: Doug Kelly <dougk.ff7@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

2 files changed, 76 insertions, 3 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
index 7f20bb8f1e..19f074ea55 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, Chris Aniszczyk <caniszczyk@gmail.com>
+ * Copyright (C) 2010, 2014 Chris Aniszczyk <caniszczyk@gmail.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -44,13 +44,16 @@ package org.eclipse.jgit.api;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.Properties;
 
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.api.errors.JGitInternalException;
+import org.eclipse.jgit.api.errors.TransportException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.Ref;
@@ -268,4 +271,70 @@ public class PushCommandTest extends RepositoryTestCase {
 		assertEquals(null, git2.getRepository().resolve("refs/heads/master"));
 
 	}
+
+	/**
+	 * Check that missing refs don't cause errors during push
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void testPushAfterGC() throws Exception {
+		// create other repository
+		Repository db2 = createWorkRepository();
+
+		// setup the first repository
+		final StoredConfig config = db.getConfig();
+		RemoteConfig remoteConfig = new RemoteConfig(config, "test");
+		URIish uri = new URIish(db2.getDirectory().toURI().toURL());
+		remoteConfig.addURI(uri);
+		remoteConfig.update(config);
+		config.save();
+
+		Git git1 = new Git(db);
+		Git git2 = new Git(db2);
+
+		// push master (with a new commit) to the remote
+		git1.commit().setMessage("initial commit").call();
+
+		RefSpec spec = new RefSpec("refs/heads/*:refs/heads/*");
+		git1.push().setRemote("test").setRefSpecs(spec).call();
+
+		// create an unrelated ref and a commit on our remote
+		git2.branchCreate().setName("refs/heads/other").call();
+		git2.checkout().setName("refs/heads/other").call();
+
+		writeTrashFile("a", "content of a");
+		git2.add().addFilepattern("a").call();
+		RevCommit commit2 = git2.commit().setMessage("adding a").call();
+
+		// run a gc to ensure we have a bitmap index
+		Properties res = git1.gc().setExpire(null).call();
+		assertEquals(7, res.size());
+
+		// create another commit so we have something else to push
+		writeTrashFile("b", "content of b");
+		git1.add().addFilepattern("b").call();
+		RevCommit commit3 = git1.commit().setMessage("adding b").call();
+
+		try {
+			// Re-run the push.  Failure may happen here.
+			git1.push().setRemote("test").setRefSpecs(spec).call();
+		} catch (TransportException e) {
+			assertTrue("should be caused by a MissingObjectException", e
+					.getCause().getCause() instanceof MissingObjectException);
+			fail("caught MissingObjectException for a change we don't have");
+		}
+
+		// Remote will have both a and b.  Master will have only b
+		try {
+			db.resolve(commit2.getId().getName() + "^{commit}");
+			fail("id shouldn't exist locally");
+		} catch (MissingObjectException e) {
+			// we should get here
+		}
+		assertEquals(commit2.getId(),
+				db2.resolve(commit2.getId().getName() + "^{commit}"));
+		assertEquals(commit3.getId(),
+				db2.resolve(commit3.getId().getName() + "^{commit}"));
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
index def6033b8b..e367ab44c9 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java
@@ -283,8 +283,12 @@ public abstract class BasePackPushConnection extends BasePackConnection implemen
 				local.newObjectReader());
 		try {
 
-			for (final Ref r : getRefs())
-				remoteObjects.add(r.getObjectId());
+			for (final Ref r : getRefs()) {
+				// only add objects that we actually have
+				ObjectId oid = r.getObjectId();
+				if (local.hasObject(oid))
+					remoteObjects.add(oid);
+			}
 			remoteObjects.addAll(additionalHaves);
 			for (final RemoteRefUpdate r : refUpdates.values()) {
 				if (!ObjectId.zeroId().equals(r.getNewObjectId()))