Fix fetching with duplicate ref updates

If packed refs are used, duplicate updates result in an exception
because JGit tries to lock the same lock file twice. With non-atomic
ref updates, this used to work, since the same ref would simply be
locked and updated twice in succession.

Let's be more lenient in this case and remove duplicates before
trying to do the ref updates. Silently skip duplicate updates
for the same ref, if they both would update the ref to the same
object ID. (If they don't, behavior is undefined anyway, and we
still throw an exception.)

Add a test that results in a duplicate ref update for a tag.

Bug: 529400
Change-Id: Ide97f20b219646ac24c22e28de0c194a29cb62a5
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java
index 0622851822ea3cb5cc3a867e1d55e54d219bf713..530fb1b2fea572bdf07aebe8f1e892eabf638d8f 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java
@@ -45,7 +45,9 @@ package org.eclipse.jgit.api;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 import org.eclipse.jgit.junit.RepositoryTestCase;
 import org.eclipse.jgit.lib.Constants;
@@ -315,4 +317,31 @@ public class FetchCommandTest extends RepositoryTestCase {
                                .setRemoveDeletedRefs(true).call();
                assertNull(db.resolve(remoteBranchName));
        }
+
+       @Test
+       public void fetchUpdateRefsWithDuplicateRefspec() throws Exception {
+               final String tagName = "foo";
+               remoteGit.commit().setMessage("commit").call();
+               Ref tagRef1 = remoteGit.tag().setName(tagName).call();
+               List<RefSpec> refSpecs = new ArrayList<>();
+               refSpecs.add(new RefSpec("+refs/heads/*:refs/remotes/origin/*"));
+               refSpecs.add(new RefSpec("+refs/tags/*:refs/tags/*"));
+               // Updating tags via the RefSpecs and setting TagOpt.FETCH_TAGS (or
+               // AUTO_FOLLOW) will result internally in *two* updates for the same
+               // ref.
+               git.fetch().setRemote("test").setRefSpecs(refSpecs)
+                               .setTagOpt(TagOpt.AUTO_FOLLOW).call();
+               assertEquals(tagRef1.getObjectId(), db.resolve(tagName));
+
+               remoteGit.commit().setMessage("commit 2").call();
+               Ref tagRef2 = remoteGit.tag().setName(tagName).setForceUpdate(true)
+                               .call();
+               FetchResult result = git.fetch().setRemote("test").setRefSpecs(refSpecs)
+                               .setTagOpt(TagOpt.FETCH_TAGS).call();
+               assertEquals(2, result.getTrackingRefUpdates().size());
+               TrackingRefUpdate update = result
+                               .getTrackingRefUpdate(Constants.R_TAGS + tagName);
+               assertEquals(RefUpdate.Result.FORCED, update.getResult());
+               assertEquals(tagRef2.getObjectId(), db.resolve(tagName));
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
index 39f0eea274aadb3bd94170e0df8d3a72185f1bfd..ed10f449dfff0675a4b552625df8704f3301bd3a 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
@@ -203,12 +203,10 @@ class FetchProcess {
                                ((BatchingProgressMonitor) monitor).setDelayStart(
                                                250, TimeUnit.MILLISECONDS);
                        }
-                       if (transport.isRemoveDeletedRefs())
+                       if (transport.isRemoveDeletedRefs()) {
                                deleteStaleTrackingRefs(result, batch);
-                       for (TrackingRefUpdate u : localUpdates) {
-                               result.add(u);
-                               batch.addCommand(u.asReceiveCommand());
                        }
+                       addUpdateBatchCommands(result, batch);
                        for (ReceiveCommand cmd : batch.getCommands()) {
                                cmd.updateType(walk);
                                if (cmd.getType() == UPDATE_NONFASTFORWARD
@@ -221,8 +219,11 @@ class FetchProcess {
                                        if (cmd.getResult() == NOT_ATTEMPTED)
                                                cmd.setResult(OK);
                                }
-                       } else
+                       } else {
                                batch.execute(walk, monitor);
+                       }
+               } catch (TransportException e) {
+                       throw e;
                } catch (IOException err) {
                        throw new TransportException(MessageFormat.format(
                                        JGitText.get().failureUpdatingTrackingRef,
@@ -239,6 +240,23 @@ class FetchProcess {
                }
        }
 
+       private void addUpdateBatchCommands(FetchResult result,
+                       BatchRefUpdate batch) throws TransportException {
+               Map<String, ObjectId> refs = new HashMap<>();
+               for (TrackingRefUpdate u : localUpdates) {
+                       // Try to skip duplicates if they'd update to the same object ID
+                       ObjectId existing = refs.get(u.getLocalName());
+                       if (existing == null) {
+                               refs.put(u.getLocalName(), u.getNewObjectId());
+                               result.add(u);
+                               batch.addCommand(u.asReceiveCommand());
+                       } else if (!existing.equals(u.getNewObjectId())) {
+                               throw new TransportException(MessageFormat
+                                               .format(JGitText.get().duplicateRef, u.getLocalName()));
+                       }
+               }
+       }
+
        private void fetchObjects(final ProgressMonitor monitor)
                        throws TransportException {
                try {