From 9f64d22770c04d85cea55e1b5d40fd3056a5bf1f Mon Sep 17 00:00:00 2001 From: Markus Duft Date: Tue, 9 Dec 2014 16:04:14 +0100 Subject: [PATCH] Don't let ignored resources cause index update jobs. Changes to ignored resources cause deltas fired by eclipse. These deltas will cause index update jobs even though there is nothing to do. In the worst case, a lot of resources are changed, and even though nothing happened to tracked files, a full re-index is done because the changed resource count threshold was reached. Change-Id: Ic8ab3a93cb877a7a896c9bd88a878eb35c29d046 Signed-off-by: Markus Duft Signed-off-by: Matthias Sohn --- .../internal/indexdiff/IndexDiffCacheTest.java | 100 ++++++++++++++++++++- .../indexdiff/GitResourceDeltaVisitor.java | 63 ++++++++++++- 2 files changed, 158 insertions(+), 5 deletions(-) diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java index 6dc6f6580..9fa868e6c 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java @@ -14,6 +14,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.FileOutputStream; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -146,7 +147,7 @@ public class IndexDiffCacheTest extends GitTestCase { IFile file = project.createFile("sub/ignore", new byte[] {}); testRepository.addToIndex(project.project); testRepository.createInitialCommit("testRemoveIgnoredFile\n\nfirst commit\n"); - prepareCacheEntry(); + IndexDiffCacheEntry entry = prepareCacheEntry(); IndexDiffData data1 = waitForListenerCalled(); assertThat(data1.getIgnoredNotInIndex(), hasItem("Project-1/sub/ignore")); @@ -160,11 +161,95 @@ public class IndexDiffCacheTest extends GitTestCase { file.delete(false, null); + waitForListenerNotCalled(); + entry.refresh(); // need explicit as ignored file shall not trigger. IndexDiffData data3 = waitForListenerCalled(); assertThat(data3.getIgnoredNotInIndex(), not(hasItem("Project-1/sub/ignore"))); } - private void prepareCacheEntry() { + @Test + public void testAddAndRemoveGitIgnoreFileToIgnoredDir() throws Exception { + testRepository.connect(project.project); + project.createFile(".gitignore", "ignore\n".getBytes("UTF-8")); + project.createFolder("sub"); + project.createFile("sub/ignore", new byte[] {}); + testRepository.addToIndex(project.project); + testRepository + .createInitialCommit("testRemoveIgnoredFile\n\nfirst commit\n"); + prepareCacheEntry(); + + IndexDiffData data1 = waitForListenerCalled(); + assertThat(data1.getIgnoredNotInIndex(), + hasItem("Project-1/sub/ignore")); + + project.createFile("sub/ignored", "Ignored".getBytes()); + + // adding this file will trigger a refresh, so no manual refresh must be + // required. + project.createFile("sub/.gitignore", "ignored\n".getBytes()); + + IndexDiffData data2 = waitForListenerCalled(); + assertThat(data2.getIgnoredNotInIndex(), + hasItem("Project-1/sub/ignored")); + + // removing must also trigger the listener + project.getProject().getFile("sub/.gitignore").delete(true, null); + + IndexDiffData data3 = waitForListenerCalled(); + assertThat(data3.getUntracked(), hasItem("Project-1/sub/ignored")); + } + + @Test + public void testAddAndRemoveFileToIgnoredDir() throws Exception { + testRepository.connect(project.project); + project.createFile(".gitignore", "sub\n".getBytes("UTF-8")); + project.createFolder("sub"); + project.createFile("sub/ignore", new byte[] {}); + testRepository.addToIndex(project.project); + testRepository + .createInitialCommit("testRemoveIgnoredFile\n\nfirst commit\n"); + prepareCacheEntry(); + + IndexDiffData data1 = waitForListenerCalled(); + assertThat(data1.getIgnoredNotInIndex(), hasItem("Project-1/sub")); + + // creating a file in an ignored directory will not trigger the listener + project.createFile("sub/ignored", "Ignored".getBytes()); + waitForListenerNotCalled(); + + // removing must also not trigger the listener + project.getProject().getFile("sub/ignored").delete(true, null); + waitForListenerNotCalled(); + } + + @Test + public void testModifyFileInIgnoredDir() throws Exception { + testRepository.connect(project.project); + project.createFile(".gitignore", "ignore\n".getBytes("UTF-8")); + project.createFolder("sub"); + project.createFile("sub/ignore", new byte[] {}); + testRepository.addToIndex(project.project); + testRepository + .createInitialCommit("testRemoveIgnoredFile\n\nfirst commit\n"); + prepareCacheEntry(); + + IndexDiffData data1 = waitForListenerCalled(); + assertThat(data1.getIgnoredNotInIndex(), + hasItem("Project-1/sub/ignore")); + + IFile file = project.getProject().getFile("sub/ignore"); + FileOutputStream str = new FileOutputStream(file.getLocation().toFile()); + try { + str.write("other contents".getBytes()); + } finally { + str.close(); + } + + // no job should be triggered for that change. + waitForListenerNotCalled(); + } + + private IndexDiffCacheEntry prepareCacheEntry() { IndexDiffCache indexDiffCache = Activator.getDefault() .getIndexDiffCache(); // This call should trigger an indexDiffChanged event @@ -180,6 +265,7 @@ public class IndexDiffCacheTest extends GitTestCase { indexDiffDataResult.set(indexDiffData); } }); + return cacheEntry; } private IndexDiffData waitForListenerCalled() throws InterruptedException { @@ -193,4 +279,14 @@ public class IndexDiffCacheTest extends GitTestCase { return indexDiffDataResult.get(); } + private void waitForListenerNotCalled() throws InterruptedException { + long time = 0; + while (!listenerCalled.get() && time < 1000) { + Thread.sleep(100); + time += 100; + } + assertTrue("indexDiffChanged was called where it shouldn't have been", + !listenerCalled.get()); + } + } diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/GitResourceDeltaVisitor.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/GitResourceDeltaVisitor.java index 8c9db9246..a5a2efc90 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/GitResourceDeltaVisitor.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/GitResourceDeltaVisitor.java @@ -15,6 +15,7 @@ package org.eclipse.egit.core.internal.indexdiff; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; +import java.util.Set; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IFolder; @@ -22,6 +23,7 @@ import org.eclipse.core.resources.IResource; import org.eclipse.core.resources.IResourceDelta; import org.eclipse.core.resources.IResourceDeltaVisitor; import org.eclipse.core.runtime.CoreException; +import org.eclipse.egit.core.Activator; import org.eclipse.egit.core.project.RepositoryMapping; import org.eclipse.jgit.lib.Repository; @@ -74,9 +76,20 @@ public class GitResourceDeltaVisitor implements IResourceDeltaVisitor { // Ignore the change return true; + IndexDiffCache cache = Activator.getDefault().getIndexDiffCache(); + IndexDiffCacheEntry entry = null; + + if (cache != null) + entry = cache.getIndexDiffCacheEntry(mapping.getRepository()); + if (resource instanceof IFolder && delta.getKind() == IResourceDelta.ADDED) { - filesToUpdate.add(mapping.getRepoRelativePath(resource) + "/"); //$NON-NLS-1$ + String path = mapping.getRepoRelativePath(resource) + "/"; //$NON-NLS-1$ + + if (isIgnoredInOldIndex(entry, path)) + return true; // keep going to catch .gitignore files. + + filesToUpdate.add(path); resourcesToUpdate.add(resource); return true; } @@ -98,14 +111,58 @@ public class GitResourceDeltaVisitor implements IResourceDeltaVisitor { } String repoRelativePath = mapping.getRepoRelativePath(resource); - if (repoRelativePath!= null) - filesToUpdate.add(repoRelativePath); + if (repoRelativePath == null) { + resourcesToUpdate.add(resource); + return true; + } + + if (isIgnoredInOldIndex(entry, repoRelativePath)) { + // This file is ignored in the old index, and ignore rules did not + // change: ignore the delta to avoid unnecessary index updates + return false; + } + filesToUpdate.add(repoRelativePath); resourcesToUpdate.add(resource); return true; } /** + * @param entry + * the {@link IndexDiffCacheEntry} for the repository containing + * the path. + * @param path + * the repository relative path of the resource to check + * @return whether the given path is ignored by the given + * {@link IndexDiffCacheEntry} + */ + private boolean isIgnoredInOldIndex(IndexDiffCacheEntry entry, String path) { + // fall back to processing all changes as long as there is no old index. + if (entry == null || gitIgnoreChanged) + return false; + + IndexDiffData indexDiff = entry.getIndexDiff(); + if (indexDiff == null) + return false; + + String p = path; + Set ignored = indexDiff.getIgnoredNotInIndex(); + while (p != null) { + if (ignored.contains(p)) + return true; + + p = skipLastSegment(p); + } + + return false; + } + + private String skipLastSegment(String path) { + int slashPos = path.lastIndexOf('/'); + return slashPos == -1 ? null : path.substring(0, slashPos); + } + + /** * @return collection of files to update */ public Collection getFileResourcesToUpdate() { -- 2.11.4.GIT