]> source.dussan.org Git - jgit.git/commitdiff
Do not create reflog for remote tracking branches during clone 07/193007/24
authorLuca Milanesio <luca.milanesio@gmail.com>
Fri, 29 Apr 2022 15:45:03 +0000 (16:45 +0100)
committerLuca Milanesio <luca.milanesio@gmail.com>
Sat, 25 Jun 2022 11:09:01 +0000 (12:09 +0100)
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
<initialBranch> 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 <luca.milanesio@gmail.com>
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/api/FetchCommandTest.java
org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchProcess.java

index de25870bd0005d5336c3cf32af5eb497e804ed71..c928d2ad22d0e75668b8a2ae6f6e90ef6ac7a206 100644 (file)
@@ -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<Ref> clonedRefs = clonedRepo.getRefDatabase().getRefs();
+               Stream<Ref> remoteRefs = clonedRefs.stream()
+                               .filter(CloneCommandTest::isRemote);
+               Stream<Ref> 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 {
index 6479d157ebbec165b8b644e76480ffa07574b899..b608afa5c76dde39f1fe9644815328ddbd192a53 100644 (file)
@@ -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<Ref> 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();
index 34bad6e0298703932fc0acf37c3d78753e304ea6..2cedd4b07eb9de4561cd0053374a7e093874ee19 100644 (file)
@@ -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) {