Merge branch 'stable-6.10' into stable-7.0 * stable-6.10: TestProtocolTest: Cover additional fetch edge cases FetchProcess: Report lock failure when destination ref changes Add regression test on non-forced fetch and local updates Change-Id: I0a249b2e8df66894646c669d945509cd306bfadd
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java index 51e09e0..2a02676 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java
@@ -26,9 +26,14 @@ import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.AnyObjectId; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevTag; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.pack.PackStatistics; import org.eclipse.jgit.transport.BasePackFetchConnection.FetchConfig; import org.eclipse.jgit.transport.resolver.ReceivePackFactory; @@ -113,6 +118,106 @@ public void testFetch() throws Exception { } @Test + public void fetchReportsLockFailureWhenAutoFollowTagIsCreated() + throws Exception { + String tagName = "foo"; + String tagRef = "refs/tags/" + tagName; + String remoteBranchName = "remotefoo"; + String forcedRemoteRef = "+refs/heads/" + remoteBranchName; + + RevCommit localCommit = local.commit().add("local.txt", "new").create(); + RevCommit remoteCommit = remote.branch(remoteBranchName).commit() + .add("remote.txt", "remote").create(); + RevTag remoteTag = remote.tag("foo", remoteCommit); + remote.update(tagRef, remoteTag); + + URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.lightweightTag(tagName, localCommit)); + + + try (Git git = new Git(local.getRepository())) { + FetchResult result = git.fetch().setRemote(uri.toString()) + .setRefSpecs(new RefSpec(forcedRemoteRef + + ":refs/remotes/origin/" + remoteBranchName)) + .setTagOpt(TagOpt.AUTO_FOLLOW).call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(tagRef); + assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult()); + // Tag hasn't been clobbered... + assertEquals(localCommit, + local.getRepository().exactRef(tagRef).getObjectId()); + // ...however the object has been downloaded + assertTrue(local.getRepository().getObjectDatabase() + .has(remoteCommit)); + } + } + + @Test + public void fetchReportsLockFailureWhenDestinationRefChanges() + throws Exception { + String localBranchName = "localfoo"; + String localRef = "refs/heads/" + localBranchName; + String remoteBranchName = "remotefoo"; + String forcedRemoteRef = "+refs/heads/" + remoteBranchName; + + RevCommit oldLocalCommit = local.branch(localBranchName).commit() + .add("local.txt", "old").create(); + RevCommit newLocalCommit = local.commit().parent(oldLocalCommit) + .add("local.txt", "new").create(); + RevCommit remoteCommit = remote.branch(remoteBranchName).commit() + .add("remote.txt", "remote").create(); + + URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.update(localBranchName, newLocalCommit)); + + try (Git git = new Git(local.getRepository())) { + FetchResult result = git.fetch().setRemote(uri.toString()) + .setRefSpecs( + new RefSpec(forcedRemoteRef + ":" + localRef)) + .call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult()); + // Ref hasn't been clobbered... + assertEquals(newLocalCommit, + local.getRepository().exactRef(localRef).getObjectId()); + // ...however the object has been downloaded + assertTrue(local.getRepository().getObjectDatabase() + .has(remoteCommit)); + } + } + + @Test + public void fetchReportsLockFailureWhenDestinationRefIsCreated() + throws Exception { + String localBranchName = "localfoo"; + String localRef = "refs/heads/" + localBranchName; + String remoteBranchName = "remotefoo"; + String forcedRemoteRef = "+refs/heads/" + remoteBranchName; + + RevCommit newLocalCommit = local.commit().add("local.txt", "new") + .create(); + RevCommit remoteCommit = remote.branch(remoteBranchName).commit() + .add("remote.txt", "remote").create(); + + URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.update(localBranchName, newLocalCommit)); + + try (Git git = new Git(local.getRepository())) { + FetchResult result = git.fetch().setRemote(uri.toString()) + .setRefSpecs( + new RefSpec(forcedRemoteRef + ":" + localRef)) + .call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult()); + // Ref hasn't been clobbered... + assertEquals(newLocalCommit, + local.getRepository().exactRef(localRef).getObjectId()); + // ...however the object has been downloaded + assertTrue(local.getRepository().getObjectDatabase() + .has(remoteCommit)); + } + } + + @Test public void testPush() throws Exception { ObjectId master = local.branch("master").commit().create(); @@ -258,6 +363,188 @@ public void testReceivePackFactory() throws Exception { } } + @Test + public void nonForcedFetchRejectedWithConcurrentLocalRefUpdate() + throws Exception { + String localRef = Constants.R_HEADS + "localfoo"; + String remoteBranchName = "remotefoo"; + String nonForcedRemoteRef = Constants.R_HEADS + remoteBranchName; + + try (Repository repository = local.getRepository(); + Git git = new Git(repository); + RevWalk revWalk = local.getRevWalk()) { + RefSpec nonForcedFetchRefSpec = new RefSpec( + nonForcedRemoteRef + ":" + localRef); + RevCommit oldLocalCommit = remote.branch(remoteBranchName) + .commit().add("base.txt", "base").create(); + + git.fetch().setRemote(registerDefaultTestProtocol().toString()).setRefSpecs(nonForcedFetchRefSpec) + .call(); + assertEquals(oldLocalCommit, repository.exactRef(localRef).getObjectId()); + + RevCommit updatedLocalCommit = local.commit() + .parent(revWalk.parseCommit(oldLocalCommit)) + .add("local.txt", "new").create(); + RevCommit remoteCommit = remote.commit().parent(oldLocalCommit) + .add("remote.txt", "remote").create(); + remote.update(remoteBranchName, remoteCommit); + + URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.update(localRef, updatedLocalCommit)); + FetchResult result = git.fetch().setRemote(uri.toString()) + .setRefSpecs(nonForcedFetchRefSpec).call(); + + // Fetch has been rejected (non-ff) + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.REJECTED, update.getResult()); + + // Fetched object has been downloaded + assertTrue(repository.getObjectDatabase() + .has(remoteCommit)); + // ... but the local ref has not been altered + assertEquals(updatedLocalCommit, + repository.exactRef(localRef).getObjectId()); + } + } + + @Test + public void forcedFetchReportsLockFailureWhenDestinationRefIsDeleted() + throws Exception { + String localBranchName = "localfoo"; + String localRef = Constants.R_HEADS + localBranchName; + String remoteBranchName = "remotefoo"; + String forcedRemoteRef = "+" + Constants.R_HEADS + remoteBranchName; + + local.branch(localBranchName).commit() + .add("local.txt", "old").create(); + RevCommit remoteCommit = remote.branch(remoteBranchName).commit() + .add("remote.txt", "remote").create(); + + URIish uri = registerTestProtocolUpdatingRef((repo) -> { + RefUpdate refUpdate = repo.getRepository().updateRef(localRef); + refUpdate.setForceUpdate(true); + refUpdate.delete(); + }); + + try (Git git = new Git(local.getRepository())) { + FetchResult result = git.fetch() + .setRemote(uri.toString()) + .setRefSpecs(new RefSpec(forcedRemoteRef + ":" + localRef)) + .call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult()); + assertNull(local.getRepository().exactRef(localRef)); + assertTrue(local.getRepository().getObjectDatabase() + .has(remoteCommit)); + } + } + + @Test + public void forcedFetchReportsLockFailureWhenDestinationRefAlreadyHasNewId() + throws Exception { + String localBranchName = "localfoo"; + String localRef = Constants.R_HEADS + localBranchName; + String remoteBranchName = "remotefoo"; + String forcedRemoteRef = "+" + Constants.R_HEADS + remoteBranchName; + String tmpRef = Constants.R_HEADS + "tmp"; + + RevCommit oldLocalCommit = local.branch(localBranchName).commit() + .add("local.txt", "old").create(); + RevCommit remoteCommit = remote.branch(remoteBranchName).commit() + .add("remote.txt", "remote").create(); + + try (Git git = new Git(local.getRepository())) { + // Pre-fetch the remote object into the local object database so the + // simulated concurrent local update can point at it. + git.fetch().setRemote(registerDefaultTestProtocol().toString()) + .setRefSpecs(new RefSpec(forcedRemoteRef + ":" + tmpRef)) + .call(); + + local.update(localRef, oldLocalCommit); + + URIish uri = registerTestProtocolUpdatingRef( + (repo) -> repo.update(localRef, remoteCommit)); + + FetchResult result = git.fetch().setRemote(uri.toString()) + .setRefSpecs(new RefSpec(forcedRemoteRef + ":" + localRef)) + .call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult()); + assertEquals(remoteCommit, + local.getRepository().exactRef(localRef).getObjectId()); + } + } + + @Test + public void nonForcedFetchReportsLockFailureWhenConcurrentUpdateIsFastForward() + throws Exception { + String localBranchName = "localfoo"; + String localRef = Constants.R_HEADS + localBranchName; + String remoteBranchName = "remotefoo"; + String nonForcedRemoteRef = Constants.R_HEADS + remoteBranchName; + String tmpRef = "refs/heads/tmp"; + + RevCommit baseCommit = remote.branch(remoteBranchName).commit() + .add("base.txt", "base").create(); + RevCommit updatedLocalCommit = remote.commit().parent(baseCommit) + .add("local.txt", "local").create(); + RevCommit remoteCommit = remote.commit().parent(updatedLocalCommit) + .add("remote.txt", "remote").create(); + remote.update(remoteBranchName, remoteCommit); + + try (Git git = new Git(local.getRepository())) { + git.fetch().setRemote(registerDefaultTestProtocol().toString()) + .setRefSpecs(new RefSpec(nonForcedRemoteRef + ":" + tmpRef)) + .call(); + + local.update(localRef, baseCommit); + + URIish uri = registerTestProtocolUpdatingRef( + (repo) -> repo.update(localRef, updatedLocalCommit)); + + FetchResult result = git.fetch().setRemote(uri.toString()) + .setRefSpecs(new RefSpec(nonForcedRemoteRef + ":" + localRef)) + .call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult()); + assertEquals(updatedLocalCommit, + local.getRepository().exactRef(localRef).getObjectId()); + assertTrue(local.getRepository().getObjectDatabase() + .has(remoteCommit)); + } + } + + @Test + public void nonForcedFetchRejectedWhenCurrentRefUnchangedButNonFastForward() + throws Exception { + String localBranchName = "localfoo"; + String localRef = Constants.R_HEADS + localBranchName; + String remoteBranchName = "remotefoo"; + String nonForcedRemoteRef = Constants.R_HEADS + remoteBranchName; + + RevCommit localCommit = local.branch(localBranchName).commit() + .add("local.txt", "local").create(); + RevCommit remoteCommit = remote.branch(remoteBranchName).commit() + .add("remote.txt", "remote").create(); + + try (Git git = new Git(local.getRepository())) { + FetchResult result = git.fetch() + .setRemote(registerDefaultTestProtocol().toString()) + .setRefSpecs( + new RefSpec(nonForcedRemoteRef + ":" + localRef)) + .call(); + + TrackingRefUpdate update = result.getTrackingRefUpdate(localRef); + assertEquals(RefUpdate.Result.REJECTED, update.getResult()); + assertEquals(localCommit, + local.getRepository().exactRef(localRef).getObjectId()); + assertTrue(local.getRepository().getObjectDatabase() + .has(remoteCommit)); + } + } + private TestProtocol<User> registerDefault() { return registerProto(new DefaultUpload(), new DefaultReceive()); } @@ -269,4 +556,26 @@ private TestProtocol<User> registerProto(UploadPackFactory<User> upf, Transport.register(proto); return proto; } + + @FunctionalInterface + interface TestRepositoryUpdateFunc { + void call(TestRepository<InMemoryRepository> repository) throws Exception; + } + + private <T extends AnyObjectId> URIish registerTestProtocolUpdatingRef(TestRepositoryUpdateFunc repositoryUpdateFunc) { + TestProtocol<User> proto = registerProto((User req, Repository db) -> { + try { + repositoryUpdateFunc.call(local); + } catch (Exception e) { + throw new AssertionError("Cannot update local ref", e); + } + return new UploadPack(db); + }, new DefaultReceive()); + return proto.register(new User("user"), remote.getRepository()); + } + + private URIish registerDefaultTestProtocol() { + TestProtocol<User> setupProto = registerDefault(); + return setupProto.register(new User("user"), remote.getRepository()); + } }
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 c510194..e430442 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java
@@ -139,6 +139,18 @@ private void executeImp(final ProgressMonitor monitor, final TagOpt tagopt = transport.getTagOpt(); String getTags = (tagopt == TagOpt.NO_TAGS) ? null : Constants.R_TAGS; String getHead = null; + // Snapshot local refs before opening the fetch connection when the + // fetch has positive refspecs or may run AUTO_FOLLOW. Those are the + // paths that can later create TrackingRefUpdates from this fetch and + // need a stable expected-old-id baseline to turn concurrent local ref + // changes into LOCK_FAILURE. + // + // Skip the snapshot for the remaining cases to avoid an unnecessary + // local ref scan when this fetch cannot reach those update paths. + if (!toFetch.isEmpty() || tagopt == TagOpt.AUTO_FOLLOW) { + localRefs(); + } + try { // If we don't have a HEAD yet, we're cloning and need to get the // upstream HEAD, too. @@ -235,10 +247,23 @@ else if (tagopt == TagOpt.FETCH_TAGS) addUpdateBatchCommands(result, batch); for (ReceiveCommand cmd : batch.getCommands()) { cmd.updateType(walk); - if (cmd.getType() == UPDATE_NONFASTFORWARD - && cmd instanceof TrackingRefUpdate.Command - && !((TrackingRefUpdate.Command) cmd).canForceUpdate()) + if (!(cmd instanceof TrackingRefUpdate.Command) + || ((TrackingRefUpdate.Command) cmd).canForceUpdate()) { + continue; + } + + ObjectId currentId = currentObjectId(cmd.getRefName()); + // The initial type check used the ref value snapshotted when + // the fetch started. If the local ref moved since then, + // re-check the non-fast-forward against the current ref before + // executing the batch so non-forced rejects remain REJECTED + // instead of surfacing as LOCK_FAILURE. + ReceiveCommand refreshedCmd = new ReceiveCommand(currentId, + cmd.getNewId(), cmd.getRefName()); + refreshedCmd.updateType(walk); + if (refreshedCmd.getType() == UPDATE_NONFASTFORWARD) { cmd.setResult(REJECTED_NONFASTFORWARD); + } } if (transport.isDryRun()) { for (ReceiveCommand cmd : batch.getCommands()) { @@ -574,6 +599,14 @@ private Map<String, Ref> localRefs() throws TransportException { return localRefs; } + private ObjectId currentObjectId(String refName) throws IOException { + Ref current = transport.local.exactRef(refName); + if (current == null) { + return ObjectId.zeroId(); + } + return current.getObjectId(); + } + private void deleteStaleTrackingRefs(FetchResult result, BatchRefUpdate batch) throws IOException { Set<Ref> processed = new HashSet<>();