Skip to content

Commit ebb6209

Browse files
barbasabarbasa
committed
Merge branch 'stable-7.6'
* stable-7.6: 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: Iff1e4cc4f13b63d3fb3bc86dec0b69a1e49cd118
2 parents 7ee50b6 + c16e266 commit ebb6209

2 files changed

Lines changed: 345 additions & 3 deletions

File tree

‎org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/TestProtocolTest.java‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,14 @@
2626
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
2727
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
2828
import org.eclipse.jgit.junit.TestRepository;
29+
import org.eclipse.jgit.lib.AnyObjectId;
30+
import org.eclipse.jgit.lib.Constants;
2931
import org.eclipse.jgit.lib.ObjectId;
32+
import org.eclipse.jgit.lib.RefUpdate;
3033
import org.eclipse.jgit.lib.Repository;
3134
import org.eclipse.jgit.revwalk.RevCommit;
35+
import org.eclipse.jgit.revwalk.RevTag;
36+
import org.eclipse.jgit.revwalk.RevWalk;
3237
import org.eclipse.jgit.storage.pack.PackStatistics;
3338
import org.eclipse.jgit.transport.BasePackFetchConnection.FetchConfig;
3439
import org.eclipse.jgit.transport.resolver.ReceivePackFactory;
@@ -112,6 +117,106 @@ public void testFetch() throws Exception {
112117
}
113118
}
114119

120+
@Test
121+
public void fetchReportsLockFailureWhenAutoFollowTagIsCreated()
122+
throws Exception {
123+
String tagName = "foo";
124+
String tagRef = "refs/tags/" + tagName;
125+
String remoteBranchName = "remotefoo";
126+
String forcedRemoteRef = "+refs/heads/" + remoteBranchName;
127+
128+
RevCommit localCommit = local.commit().add("local.txt", "new").create();
129+
RevCommit remoteCommit = remote.branch(remoteBranchName).commit()
130+
.add("remote.txt", "remote").create();
131+
RevTag remoteTag = remote.tag("foo", remoteCommit);
132+
remote.update(tagRef, remoteTag);
133+
134+
URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.lightweightTag(tagName, localCommit));
135+
136+
137+
try (Git git = new Git(local.getRepository())) {
138+
FetchResult result = git.fetch().setRemote(uri.toString())
139+
.setRefSpecs(new RefSpec(forcedRemoteRef
140+
+ ":refs/remotes/origin/" + remoteBranchName))
141+
.setTagOpt(TagOpt.AUTO_FOLLOW).call();
142+
143+
TrackingRefUpdate update = result.getTrackingRefUpdate(tagRef);
144+
assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult());
145+
// Tag hasn't been clobbered...
146+
assertEquals(localCommit,
147+
local.getRepository().exactRef(tagRef).getObjectId());
148+
// ...however the object has been downloaded
149+
assertTrue(local.getRepository().getObjectDatabase()
150+
.has(remoteCommit));
151+
}
152+
}
153+
154+
@Test
155+
public void fetchReportsLockFailureWhenDestinationRefChanges()
156+
throws Exception {
157+
String localBranchName = "localfoo";
158+
String localRef = "refs/heads/" + localBranchName;
159+
String remoteBranchName = "remotefoo";
160+
String forcedRemoteRef = "+refs/heads/" + remoteBranchName;
161+
162+
RevCommit oldLocalCommit = local.branch(localBranchName).commit()
163+
.add("local.txt", "old").create();
164+
RevCommit newLocalCommit = local.commit().parent(oldLocalCommit)
165+
.add("local.txt", "new").create();
166+
RevCommit remoteCommit = remote.branch(remoteBranchName).commit()
167+
.add("remote.txt", "remote").create();
168+
169+
URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.update(localBranchName, newLocalCommit));
170+
171+
try (Git git = new Git(local.getRepository())) {
172+
FetchResult result = git.fetch().setRemote(uri.toString())
173+
.setRefSpecs(
174+
new RefSpec(forcedRemoteRef + ":" + localRef))
175+
.call();
176+
177+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
178+
assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult());
179+
// Ref hasn't been clobbered...
180+
assertEquals(newLocalCommit,
181+
local.getRepository().exactRef(localRef).getObjectId());
182+
// ...however the object has been downloaded
183+
assertTrue(local.getRepository().getObjectDatabase()
184+
.has(remoteCommit));
185+
}
186+
}
187+
188+
@Test
189+
public void fetchReportsLockFailureWhenDestinationRefIsCreated()
190+
throws Exception {
191+
String localBranchName = "localfoo";
192+
String localRef = "refs/heads/" + localBranchName;
193+
String remoteBranchName = "remotefoo";
194+
String forcedRemoteRef = "+refs/heads/" + remoteBranchName;
195+
196+
RevCommit newLocalCommit = local.commit().add("local.txt", "new")
197+
.create();
198+
RevCommit remoteCommit = remote.branch(remoteBranchName).commit()
199+
.add("remote.txt", "remote").create();
200+
201+
URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.update(localBranchName, newLocalCommit));
202+
203+
try (Git git = new Git(local.getRepository())) {
204+
FetchResult result = git.fetch().setRemote(uri.toString())
205+
.setRefSpecs(
206+
new RefSpec(forcedRemoteRef + ":" + localRef))
207+
.call();
208+
209+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
210+
assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult());
211+
// Ref hasn't been clobbered...
212+
assertEquals(newLocalCommit,
213+
local.getRepository().exactRef(localRef).getObjectId());
214+
// ...however the object has been downloaded
215+
assertTrue(local.getRepository().getObjectDatabase()
216+
.has(remoteCommit));
217+
}
218+
}
219+
115220
@Test
116221
public void testPush() throws Exception {
117222
ObjectId master = local.branch("master").commit().create();
@@ -258,6 +363,188 @@ public void testReceivePackFactory() throws Exception {
258363
}
259364
}
260365

366+
@Test
367+
public void nonForcedFetchRejectedWithConcurrentLocalRefUpdate()
368+
throws Exception {
369+
String localRef = Constants.R_HEADS + "localfoo";
370+
String remoteBranchName = "remotefoo";
371+
String nonForcedRemoteRef = Constants.R_HEADS + remoteBranchName;
372+
373+
try (Repository repository = local.getRepository();
374+
Git git = new Git(repository);
375+
RevWalk revWalk = local.getRevWalk()) {
376+
RefSpec nonForcedFetchRefSpec = new RefSpec(
377+
nonForcedRemoteRef + ":" + localRef);
378+
RevCommit oldLocalCommit = remote.branch(remoteBranchName)
379+
.commit().add("base.txt", "base").create();
380+
381+
git.fetch().setRemote(registerDefaultTestProtocol().toString()).setRefSpecs(nonForcedFetchRefSpec)
382+
.call();
383+
assertEquals(oldLocalCommit, repository.exactRef(localRef).getObjectId());
384+
385+
RevCommit updatedLocalCommit = local.commit()
386+
.parent(revWalk.parseCommit(oldLocalCommit))
387+
.add("local.txt", "new").create();
388+
RevCommit remoteCommit = remote.commit().parent(oldLocalCommit)
389+
.add("remote.txt", "remote").create();
390+
remote.update(remoteBranchName, remoteCommit);
391+
392+
URIish uri = registerTestProtocolUpdatingRef((repo) -> repo.update(localRef, updatedLocalCommit));
393+
FetchResult result = git.fetch().setRemote(uri.toString())
394+
.setRefSpecs(nonForcedFetchRefSpec).call();
395+
396+
// Fetch has been rejected (non-ff)
397+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
398+
assertEquals(RefUpdate.Result.REJECTED, update.getResult());
399+
400+
// Fetched object has been downloaded
401+
assertTrue(repository.getObjectDatabase()
402+
.has(remoteCommit));
403+
// ... but the local ref has not been altered
404+
assertEquals(updatedLocalCommit,
405+
repository.exactRef(localRef).getObjectId());
406+
}
407+
}
408+
409+
@Test
410+
public void forcedFetchReportsLockFailureWhenDestinationRefIsDeleted()
411+
throws Exception {
412+
String localBranchName = "localfoo";
413+
String localRef = Constants.R_HEADS + localBranchName;
414+
String remoteBranchName = "remotefoo";
415+
String forcedRemoteRef = "+" + Constants.R_HEADS + remoteBranchName;
416+
417+
local.branch(localBranchName).commit()
418+
.add("local.txt", "old").create();
419+
RevCommit remoteCommit = remote.branch(remoteBranchName).commit()
420+
.add("remote.txt", "remote").create();
421+
422+
URIish uri = registerTestProtocolUpdatingRef((repo) -> {
423+
RefUpdate refUpdate = repo.getRepository().updateRef(localRef);
424+
refUpdate.setForceUpdate(true);
425+
refUpdate.delete();
426+
});
427+
428+
try (Git git = new Git(local.getRepository())) {
429+
FetchResult result = git.fetch()
430+
.setRemote(uri.toString())
431+
.setRefSpecs(new RefSpec(forcedRemoteRef + ":" + localRef))
432+
.call();
433+
434+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
435+
assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult());
436+
assertNull(local.getRepository().exactRef(localRef));
437+
assertTrue(local.getRepository().getObjectDatabase()
438+
.has(remoteCommit));
439+
}
440+
}
441+
442+
@Test
443+
public void forcedFetchReportsLockFailureWhenDestinationRefAlreadyHasNewId()
444+
throws Exception {
445+
String localBranchName = "localfoo";
446+
String localRef = Constants.R_HEADS + localBranchName;
447+
String remoteBranchName = "remotefoo";
448+
String forcedRemoteRef = "+" + Constants.R_HEADS + remoteBranchName;
449+
String tmpRef = Constants.R_HEADS + "tmp";
450+
451+
RevCommit oldLocalCommit = local.branch(localBranchName).commit()
452+
.add("local.txt", "old").create();
453+
RevCommit remoteCommit = remote.branch(remoteBranchName).commit()
454+
.add("remote.txt", "remote").create();
455+
456+
try (Git git = new Git(local.getRepository())) {
457+
// Pre-fetch the remote object into the local object database so the
458+
// simulated concurrent local update can point at it.
459+
git.fetch().setRemote(registerDefaultTestProtocol().toString())
460+
.setRefSpecs(new RefSpec(forcedRemoteRef + ":" + tmpRef))
461+
.call();
462+
463+
local.update(localRef, oldLocalCommit);
464+
465+
URIish uri = registerTestProtocolUpdatingRef(
466+
(repo) -> repo.update(localRef, remoteCommit));
467+
468+
FetchResult result = git.fetch().setRemote(uri.toString())
469+
.setRefSpecs(new RefSpec(forcedRemoteRef + ":" + localRef))
470+
.call();
471+
472+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
473+
assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult());
474+
assertEquals(remoteCommit,
475+
local.getRepository().exactRef(localRef).getObjectId());
476+
}
477+
}
478+
479+
@Test
480+
public void nonForcedFetchReportsLockFailureWhenConcurrentUpdateIsFastForward()
481+
throws Exception {
482+
String localBranchName = "localfoo";
483+
String localRef = Constants.R_HEADS + localBranchName;
484+
String remoteBranchName = "remotefoo";
485+
String nonForcedRemoteRef = Constants.R_HEADS + remoteBranchName;
486+
String tmpRef = "refs/heads/tmp";
487+
488+
RevCommit baseCommit = remote.branch(remoteBranchName).commit()
489+
.add("base.txt", "base").create();
490+
RevCommit updatedLocalCommit = remote.commit().parent(baseCommit)
491+
.add("local.txt", "local").create();
492+
RevCommit remoteCommit = remote.commit().parent(updatedLocalCommit)
493+
.add("remote.txt", "remote").create();
494+
remote.update(remoteBranchName, remoteCommit);
495+
496+
try (Git git = new Git(local.getRepository())) {
497+
git.fetch().setRemote(registerDefaultTestProtocol().toString())
498+
.setRefSpecs(new RefSpec(nonForcedRemoteRef + ":" + tmpRef))
499+
.call();
500+
501+
local.update(localRef, baseCommit);
502+
503+
URIish uri = registerTestProtocolUpdatingRef(
504+
(repo) -> repo.update(localRef, updatedLocalCommit));
505+
506+
FetchResult result = git.fetch().setRemote(uri.toString())
507+
.setRefSpecs(new RefSpec(nonForcedRemoteRef + ":" + localRef))
508+
.call();
509+
510+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
511+
assertEquals(RefUpdate.Result.LOCK_FAILURE, update.getResult());
512+
assertEquals(updatedLocalCommit,
513+
local.getRepository().exactRef(localRef).getObjectId());
514+
assertTrue(local.getRepository().getObjectDatabase()
515+
.has(remoteCommit));
516+
}
517+
}
518+
519+
@Test
520+
public void nonForcedFetchRejectedWhenCurrentRefUnchangedButNonFastForward()
521+
throws Exception {
522+
String localBranchName = "localfoo";
523+
String localRef = Constants.R_HEADS + localBranchName;
524+
String remoteBranchName = "remotefoo";
525+
String nonForcedRemoteRef = Constants.R_HEADS + remoteBranchName;
526+
527+
RevCommit localCommit = local.branch(localBranchName).commit()
528+
.add("local.txt", "local").create();
529+
RevCommit remoteCommit = remote.branch(remoteBranchName).commit()
530+
.add("remote.txt", "remote").create();
531+
532+
try (Git git = new Git(local.getRepository())) {
533+
FetchResult result = git.fetch()
534+
.setRemote(registerDefaultTestProtocol().toString())
535+
.setRefSpecs(
536+
new RefSpec(nonForcedRemoteRef + ":" + localRef))
537+
.call();
538+
539+
TrackingRefUpdate update = result.getTrackingRefUpdate(localRef);
540+
assertEquals(RefUpdate.Result.REJECTED, update.getResult());
541+
assertEquals(localCommit,
542+
local.getRepository().exactRef(localRef).getObjectId());
543+
assertTrue(local.getRepository().getObjectDatabase()
544+
.has(remoteCommit));
545+
}
546+
}
547+
261548
private TestProtocol<User> registerDefault() {
262549
return registerProto(new DefaultUpload(), new DefaultReceive());
263550
}
@@ -269,4 +556,26 @@ private TestProtocol<User> registerProto(UploadPackFactory<User> upf,
269556
Transport.register(proto);
270557
return proto;
271558
}
559+
560+
@FunctionalInterface
561+
interface TestRepositoryUpdateFunc {
562+
void call(TestRepository<InMemoryRepository> repository) throws Exception;
563+
}
564+
565+
private <T extends AnyObjectId> URIish registerTestProtocolUpdatingRef(TestRepositoryUpdateFunc repositoryUpdateFunc) {
566+
TestProtocol<User> proto = registerProto((User req, Repository db) -> {
567+
try {
568+
repositoryUpdateFunc.call(local);
569+
} catch (Exception e) {
570+
throw new AssertionError("Cannot update local ref", e);
571+
}
572+
return new UploadPack(db);
573+
}, new DefaultReceive());
574+
return proto.register(new User("user"), remote.getRepository());
575+
}
576+
577+
private URIish registerDefaultTestProtocol() {
578+
TestProtocol<User> setupProto = registerDefault();
579+
return setupProto.register(new User("user"), remote.getRepository());
580+
}
272581
}

‎org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,18 @@ private void executeImp(final ProgressMonitor monitor,
139139
final TagOpt tagopt = transport.getTagOpt();
140140
String getTags = (tagopt == TagOpt.NO_TAGS) ? null : Constants.R_TAGS;
141141
String getHead = null;
142+
// Snapshot local refs before opening the fetch connection when the
143+
// fetch has positive refspecs or may run AUTO_FOLLOW. Those are the
144+
// paths that can later create TrackingRefUpdates from this fetch and
145+
// need a stable expected-old-id baseline to turn concurrent local ref
146+
// changes into LOCK_FAILURE.
147+
//
148+
// Skip the snapshot for the remaining cases to avoid an unnecessary
149+
// local ref scan when this fetch cannot reach those update paths.
150+
if (!toFetch.isEmpty() || tagopt == TagOpt.AUTO_FOLLOW) {
151+
localRefs();
152+
}
153+
142154
try {
143155
// If we don't have a HEAD yet, we're cloning and need to get the
144156
// upstream HEAD, too.
@@ -235,10 +247,23 @@ else if (tagopt == TagOpt.FETCH_TAGS)
235247
addUpdateBatchCommands(result, batch);
236248
for (ReceiveCommand cmd : batch.getCommands()) {
237249
cmd.updateType(walk);
238-
if (cmd.getType() == UPDATE_NONFASTFORWARD
239-
&& cmd instanceof TrackingRefUpdate.Command
240-
&& !((TrackingRefUpdate.Command) cmd).canForceUpdate())
250+
if (!(cmd instanceof TrackingRefUpdate.Command)
251+
|| ((TrackingRefUpdate.Command) cmd).canForceUpdate()) {
252+
continue;
253+
}
254+
255+
ObjectId currentId = currentObjectId(cmd.getRefName());
256+
// The initial type check used the ref value snapshotted when
257+
// the fetch started. If the local ref moved since then,
258+
// re-check the non-fast-forward against the current ref before
259+
// executing the batch so non-forced rejects remain REJECTED
260+
// instead of surfacing as LOCK_FAILURE.
261+
ReceiveCommand refreshedCmd = new ReceiveCommand(currentId,
262+
cmd.getNewId(), cmd.getRefName());
263+
refreshedCmd.updateType(walk);
264+
if (refreshedCmd.getType() == UPDATE_NONFASTFORWARD) {
241265
cmd.setResult(REJECTED_NONFASTFORWARD);
266+
}
242267
}
243268
if (transport.isDryRun()) {
244269
for (ReceiveCommand cmd : batch.getCommands()) {
@@ -574,6 +599,14 @@ private Map<String, Ref> localRefs() throws TransportException {
574599
return localRefs;
575600
}
576601

602+
private ObjectId currentObjectId(String refName) throws IOException {
603+
Ref current = transport.local.exactRef(refName);
604+
if (current == null) {
605+
return ObjectId.zeroId();
606+
}
607+
return current.getObjectId();
608+
}
609+
577610
private void deleteStaleTrackingRefs(FetchResult result,
578611
BatchRefUpdate batch) throws IOException {
579612
Set<Ref> processed = new HashSet<>();

0 commit comments

Comments
 (0)