From 4bb46936332e9d66569810f0a77bb08bb46fc950 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 29 Apr 2022 16:45:03 +0100 Subject: [PATCH] Do not create reflog for remote tracking branches during clone When using JGit on a non-bare repository, the CloneCommand it previously created local reflogs for all branches including remote tracking ones, causing the generation of a potentially large number of files on the local filesystem. The creation of the remote-tracking branches (refs/remotes/*) during clone is not an issue for the local filesystem because all of them are stored in a single packed-refs file. However, the creation of a large number of ref logs on a local filesystem IS an issue because it may not be tuned or initialised in term of inodes to contain a very large number of files. When a user (or a CI system) performs the CloneCommand against a potentially large repository (e.g., millions of branches), it is interested in working or validating a single branch or tag and is unlikely to work with all the remote-tracking branches. The eager creation of a reflogs for all the remote-tracking branches is not just a performance issue but may also compromise the ability to use JGit for cloning a large repository. The behaviour implemented in this change is also consistent with the optimisation done in the C code-base [1]. We differentiate between clone and fetch commands using --branch option, that is only available in clone command, and is set as HEAD per default. [1] https://github.com/git/git/commit/58f233ce1ed67bbc31a429fde5c65d5050fdbd7d Bug: 579805 Change-Id: I58d0d36a8a4ce42e0f59b8bf063747c4b81bd859 Signed-off-by: Luca Milanesio --- .../eclipse/jgit/api/CloneCommandTest.java | 44 +++++++++++++++++++ .../eclipse/jgit/api/FetchCommandTest.java | 20 +++++++++ .../eclipse/jgit/transport/FetchProcess.java | 9 +++- 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index de25870bd0..c928d2ad22 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -22,6 +22,7 @@ import java.net.URISyntaxException; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.eclipse.jgit.api.ListBranchCommand.ListMode; import org.eclipse.jgit.api.errors.GitAPIException; @@ -114,6 +115,49 @@ public class CloneCommandTest extends RepositoryTestCase { assertTagOption(git2.getRepository(), TagOpt.AUTO_FOLLOW); } + @Test + public void testCloneRepository_refLogForLocalRefs() + throws IOException, JGitInternalException, GitAPIException { + File directory = createTempDirectory("testCloneRepository"); + CloneCommand command = Git.cloneRepository(); + command.setDirectory(directory); + command.setURI(fileUri()); + Git git2 = command.call(); + Repository clonedRepo = git2.getRepository(); + addRepoToClose(clonedRepo); + + List clonedRefs = clonedRepo.getRefDatabase().getRefs(); + Stream remoteRefs = clonedRefs.stream() + .filter(CloneCommandTest::isRemote); + Stream localHeadsRefs = clonedRefs.stream() + .filter(CloneCommandTest::isLocalHead); + + remoteRefs.forEach(ref -> assertFalse( + "Ref " + ref.getName() + + " is remote and should not have a reflog", + hasRefLog(clonedRepo, ref))); + localHeadsRefs.forEach(ref -> assertTrue( + "Ref " + ref.getName() + + " is local head and should have a reflog", + hasRefLog(clonedRepo, ref))); + } + + private static boolean isRemote(Ref ref) { + return ref.getName().startsWith(Constants.R_REMOTES); + } + + private static boolean isLocalHead(Ref ref) { + return !isRemote(ref) && ref.getName().startsWith(Constants.R_HEADS); + } + + private static boolean hasRefLog(Repository repo, Ref ref) { + try { + return repo.getReflogReader(ref.getName()).getLastEntry() != null; + } catch (IOException ioe) { + throw new IllegalStateException(ioe); + } + } + @Test public void testCloneRepositoryExplicitGitDir() throws IOException, JGitInternalException, GitAPIException { 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 6479d157eb..b608afa5c7 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java @@ -76,6 +76,26 @@ public class FetchCommandTest extends RepositoryTestCase { db.resolve(tagRef.getObjectId().getName())); } + @Test + public void testFetchHasRefLogForRemoteRef() throws Exception { + // create an initial commit SHA1 for the default branch + ObjectId defaultBranchSha1 = remoteGit.commit() + .setMessage("initial commit").call().getId(); + + git.fetch().setRemote("test") + .setRefSpecs("refs/heads/*:refs/remotes/origin/*").call(); + + List allFetchedRefs = git.getRepository().getRefDatabase() + .getRefs(); + assertEquals(allFetchedRefs.size(), 1); + Ref remoteRef = allFetchedRefs.get(0); + + assertTrue(remoteRef.getName().startsWith(Constants.R_REMOTES)); + assertEquals(defaultBranchSha1, remoteRef.getObjectId()); + assertNotNull(git.getRepository().getReflogReader(remoteRef.getName()) + .getLastEntry()); + } + @Test public void testForcedFetch() throws Exception { remoteGit.commit().setMessage("commit").call(); 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 34bad6e029..2cedd4b07e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java @@ -205,8 +205,13 @@ class FetchProcess { BatchRefUpdate batch = transport.local.getRefDatabase() .newBatchUpdate() - .setAllowNonFastForwards(true) - .setRefLogMessage("fetch", true); //$NON-NLS-1$ + .setAllowNonFastForwards(true); + + // Generate reflog only when fetching updates and not at the first clone + if (initialBranch == null) { + batch.setRefLogMessage("fetch", true); //$NON-NLS-1$ + } + try (RevWalk walk = new RevWalk(transport.local)) { walk.setRetainBody(false); if (monitor instanceof BatchingProgressMonitor) { -- 2.39.5