Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2016-03-02 17:04:00 -0500
committerThomas Wolf2016-03-13 13:12:47 -0400
commite47bced605873c5e82705f9a8184f540d1dbb586 (patch)
treec9959fa5936b5c54ca91abba02f0da90f570386b
parent3f18b3527b1628a021f72939c249304a4628b4ac (diff)
downloadegit-e47bced605873c5e82705f9a8184f540d1dbb586.tar.gz
egit-e47bced605873c5e82705f9a8184f540d1dbb586.tar.xz
egit-e47bced605873c5e82705f9a8184f540d1dbb586.zip
RepositoryCache: do not prematurely remove submodules
Basically we have no way of knowing when we no longer reference a repository. In our code alone, there may be RepositoryMappings and RepositoryNodes directly referencing a Repository, but some views may also have direct references to Repository instances. We therefore cannot explicitly remove repositories from the RepositoryCache, since we have no efficient and 100% reliable way to determine whether a Repository is still in use somewhere. Therefore the RepositoryCache relies on weak reference semantics. Before https://git.eclipse.org/r/#/c/62066/ , this whole mechanism was broken anyway because the IndexDiffCache had no way to remove an IndexDiffCacheEntry instance, and those instances had hard references to the Repository. So once there was an IndexDiffCacheEntry for a repository, that Repository instance would be kept forever. https://git.eclipse.org/r/#/c/62066/ itself was a wrong approach because it neglected that some repositories might never be "configured" repositories visible in the Repositories view. Such repositories would be removed from the RepositoryCache while still in use. Submodules and nested repositories are affected by this, but so can top-level repositories. The approach taken in this change here fixes this. First, we go back to relying solely on the weak reference semantics of the RepositoryCache. Note that doing so does not give any guarantee about when exactly a no longer used and only weakly reachable Repository instance will actually be removed from the cache. Then we at least make sure that we don't keep any hard references around. That's more difficult than it may seem: * Replaced all hard references to Repository in IndexDiffCacheEntry. We now only use the repository's git directory, and use that to get the repository from the RepositoryCache, if it still is there. * The oldIndex DirCache in an IndexDiffCacheEntry also had a hard reference to the Repository. Use a DirCache.read() variant that doesn't set that link -- it's used only for writing a DirCache, which we don't do. Note that this is a bit fragile as it relies on an implementation detail of JGit, but for now I see no other way. * Even worse, some Eclipse internals do keep around hard references to some "last selection"s. Those may contain no longer used RepositoryNodes from the repository view, which still reference the Repository instance through a hard reference. We have no real way to reliably ensure that these Eclipse internals forget those nodes. Therefore we have to ensure in RemoveCommand that we actually do null out these Repository references once we're sure we have removed the node from our view. (The two places I found where Eclipse holds on to such defunct nodes are WorkbenchSourceProvider.lastShowInSelection, set when the "Shown In..." context menu was last filled, and the CommonViewer, which also remembers the last selection.) * Our own RepositorySourceProvider had a private field referencing the last selected Repository. The RepositorySourceProvider is a singleton that is instantiated very early and then kept around forever. That was resolved by using a weak reference for the repository. * The EclipseContext also managed to (indirectly) hold on to a hard reference to a Repository instance through the context variable we provided. That was solved by not passing the Repository directly as the context variable defined by RepositorySourceProvider but again only the git directory. * RebaseInteractivePlan has a static global cache of plans and each plan had a hard reference to a repository. A plan is computed when the view is opened, even if never executed. That accumulated hard references to repositories. Solved by using a weak reference. * The Eclipse resource tree's DataTreeLookup has a static cache of instances that are re-used but not cleared after use. Those may keep references to our RepositoryMapping session properties for some time after the IResource to with the property was attached has gone. The test explicitly accounts for this. In the full test run in maven, more problems showed up in a heap dump taken just before we test for no cached repositories in GitRepositoriesViewRepoDeletionTest: numerous FileRepository instances from earlier tests were still referenced. * The EclipseContext retains some handler activations of ActionHamdlers of anonymous Action subclasses of SpellcheckableMessageArea, which reference the area through this$0, which itself keeps a reference to the CommitDialog through this$0, which means we keep the CommitMessageComponent, which has a hard reference to the Repository. Solved by using static subclasses that reference only the SourceViewer. * The Eclipse NavigationHistory keeps around references to some CommitEditorInputs, which also have a hard reference to a repository. * The synchronize view references a repository through its GitSynchronizeData. Resolved in test by keeping the synchronize view closed. * The FileRepository from testCompareWithPreviousWithMerge was still referenced from the job from CompareWithPreviousActionHandler even though no such job was running anymore. Referenced in the ProgressMonitorFocusJobDialog, which was still kept around through its fontChangeListener (an inner non-static class in the ultimate ancestor class Window), which apparently somehow was still registered.. Unclear why or what happened there. Not resolved. * Same thing with testRevertFailure; referenced from RevertCommitOperation from the job in RevertHandler from ProgressMonitorFocusJobDialog. Unresolved. * Anonymous actions in SwitchToMenu still reference a long gone repository from test method selectionWithProj1. Unclear why and unresolved. * Some repositories from earlier tests were still referenced through long defunct RepositoryNode instances. Unresolved. * RepositoryPropertySourceProvider has a hard reference to its lastObject, and the RepositoryPropertySource has hard references to the configs, which may have hard references to the Repository. Resolved in test by closing the property sheet; unresolved in general. Because we can't explicitly remove items from the RepositoryCache, we also cannot explicitly remove IndexDiffCache entries. The only thing we can do is to ensure we remove IndexDiffCacheEntries when we detect that a repository in the cache no longer exists (has been garbage collected, or its git directory no longer exists.) Additionally, the resource change listener of an IndexDiffCacheEntry unregisters itself when it finds its repository has gone. I cannot really claim that this still fixes bug 483664 because there is absolutely no way to ensure that repositories vanish from the RepositoryCache in a timely manner. But it's a best-effort attempt to at least try, and at the same time not to evict repositories from the cache prematurely. The test explicitly invokes System.gc() in an attempt to make the JVM actually reclaim weakly reachable objects. This is not guaranteed, but appears to work in practice: the test thus only shows that the obvious places where we kept hard references are indeed resolved, and the repository does indeed vanish eventually. But see the "unresolved" items above: there's no guarantee that some view or action handler or Eclipse internal class doesn't somehow still manages to keep a hard reference and thus prevent reclamation. Finally, testing for an empty RepositoryCache must ensure that the RepositoryChangeScanner does not interfere; otherwise that may temporarily hold hard references to repositories. Solved using a scheduling rule. Change-Id: I3f437caccd58d6c9fb4187f66d9f53e7834a5224 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
-rw-r--r--org.eclipse.egit.core/META-INF/MANIFEST.MF3
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/Activator.java2
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java82
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java66
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java109
-rw-r--r--org.eclipse.egit.core/src/org/eclipse/egit/core/internal/rebase/RebaseInteractivePlan.java83
-rw-r--r--org.eclipse.egit.gitflow.ui/plugin.xml2
-rw-r--r--org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/decorators/GitFlowLightweightDecorator.java6
-rw-r--r--org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/properties/RepositoryPropertyTester.java14
-rw-r--r--org.eclipse.egit.ui.test/META-INF/MANIFEST.MF7
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java45
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/gitflow/AbstractGitflowHandlerTest.java6
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java46
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java143
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java2
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/RepositoryCacheRule.java50
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java56
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/rebase/RebaseInteractiveView.java5
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java54
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java14
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/selection/RepositorySourceProvider.java53
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java3
22 files changed, 627 insertions, 224 deletions
diff --git a/org.eclipse.egit.core/META-INF/MANIFEST.MF b/org.eclipse.egit.core/META-INF/MANIFEST.MF
index 0e3f07b4d..e16627710 100644
--- a/org.eclipse.egit.core/META-INF/MANIFEST.MF
+++ b/org.eclipse.egit.core/META-INF/MANIFEST.MF
@@ -17,7 +17,8 @@ Export-Package: org.eclipse.egit.core;version="4.3.0";
x-friends:="org.eclipse.egit.ui,
org.eclipse.egit.ui.test,
org.eclipse.egit.mylyn.ui,
- org.eclipse.egit.gitflow.test",
+ org.eclipse.egit.gitflow.test,
+ org.eclipse.egit.gitflow.ui",
org.eclipse.egit.core.internal;version="4.3.0";x-friends:="org.eclipse.egit.ui,org.eclipse.egit.import,org.eclipse.egit.gitflow.ui",
org.eclipse.egit.core.internal.gerrit;version="4.3.0";x-friends:="org.eclipse.egit.ui",
org.eclipse.egit.core.internal.indexdiff;version="4.3.0";x-friends:="org.eclipse.egit.ui,org.eclipse.egit.ui.test",
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/Activator.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/Activator.java
index a54cfc24a..47fedba55 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/Activator.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/Activator.java
@@ -324,7 +324,7 @@ public class Activator extends Plugin implements DebugOptionsListener {
@Override
public void stop(final BundleContext context) throws Exception {
GitProjectData.detachFromWorkspace();
- repositoryCache.dispose();
+ repositoryCache.clear();
repositoryCache = null;
indexDiffCache.dispose();
indexDiffCache = null;
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java
index 7a108e5c8..3bc13bdc6 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java
@@ -21,14 +21,11 @@ import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path;
-import org.eclipse.core.runtime.preferences.InstanceScope;
-import org.eclipse.core.runtime.preferences.IEclipsePreferences.IPreferenceChangeListener;
-import org.eclipse.core.runtime.preferences.IEclipsePreferences.PreferenceChangeEvent;
+import org.eclipse.egit.core.internal.indexdiff.IndexDiffCache;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
@@ -38,29 +35,10 @@ import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
public class RepositoryCache {
private final Map<File, Reference<Repository>> repositoryCache = new HashMap<File, Reference<Repository>>();
- private final IPreferenceChangeListener configuredRepositoriesListener = new IPreferenceChangeListener() {
-
- @Override
- public void preferenceChange(PreferenceChangeEvent event) {
- if (!RepositoryUtil.PREFS_DIRECTORIES.equals(event.getKey())) {
- return;
- }
- prune(Activator.getDefault().getRepositoryUtil().getRepositories());
- }
-
- };
-
- RepositoryCache() {
- InstanceScope.INSTANCE.getNode(Activator.getPluginId())
- .addPreferenceChangeListener(configuredRepositoriesListener);
- }
-
- void dispose() {
- InstanceScope.INSTANCE.getNode(Activator.getPluginId())
- .removePreferenceChangeListener(configuredRepositoriesListener);
- }
-
/**
+ * Looks in the cache for a {@link Repository} matching the given git
+ * directory. If there is no such Repository instance in the cache, one is
+ * created.
*
* @param gitDir
* @return an existing instance of Repository for <code>gitDir</code> or a
@@ -70,7 +48,7 @@ public class RepositoryCache {
*/
public synchronized Repository lookupRepository(final File gitDir)
throws IOException {
- prune(repositoryCache);
+ prune();
// Make sure we have a normalized path without .. segments here.
File normalizedGitDir = new Path(gitDir.getAbsolutePath()).toFile();
Reference<Repository> r = repositoryCache.get(normalizedGitDir);
@@ -84,13 +62,34 @@ public class RepositoryCache {
}
/**
+ * Looks in the cache for a {@link Repository} matching the given git
+ * directory.
+ *
+ * @param gitDir
+ * @return the cached repository, if any, or {@code null} if node found in
+ * the cache.
+ */
+ public synchronized Repository getRepository(final File gitDir) {
+ prune();
+ if (gitDir == null) {
+ return null;
+ }
+ File normalizedGitDir = new Path(gitDir.getAbsolutePath()).toFile();
+ Reference<Repository> r = repositoryCache.get(normalizedGitDir);
+ return r != null ? r.get() : null;
+ }
+
+ /**
* @return all Repository instances contained in the cache
*/
public synchronized Repository[] getAllRepositories() {
- prune(repositoryCache);
+ prune();
List<Repository> repositories = new ArrayList<Repository>();
for (Reference<Repository> reference : repositoryCache.values()) {
- repositories.add(reference.get());
+ Repository repository = reference.get();
+ if (repository != null) {
+ repositories.add(repository);
+ }
}
return repositories.toArray(new Repository[repositories.size()]);
}
@@ -143,22 +142,16 @@ public class RepositoryCache {
return repository;
}
- private static void prune(Map<File, Reference<Repository>> map) {
- for (final Iterator<Map.Entry<File, Reference<Repository>>> i = map.entrySet()
+ private void prune() {
+ for (final Iterator<Map.Entry<File, Reference<Repository>>> i = repositoryCache
+ .entrySet()
.iterator(); i.hasNext();) {
- Repository repository = i.next().getValue().get();
+ Map.Entry<File, Reference<Repository>> entry = i.next();
+ Repository repository = entry.getValue().get();
if (repository == null || !repository.getDirectory().exists()) {
i.remove();
- }
- }
- }
-
- private synchronized void prune(Set<String> configuredRepositories) {
- Iterator<File> iterator = repositoryCache.keySet().iterator();
- while (iterator.hasNext()) {
- File gitDir = iterator.next();
- if (!configuredRepositories.contains(gitDir.getAbsolutePath())) {
- iterator.remove();
+ Activator.getDefault().getIndexDiffCache()
+ .remove(entry.getKey());
}
}
}
@@ -168,6 +161,11 @@ public class RepositoryCache {
* Unit tests can use this method to get a clean beginning state
*/
public synchronized void clear() {
+ IndexDiffCache cache = Activator.getDefault().getIndexDiffCache();
+ for (Map.Entry<File, Reference<Repository>> entry : repositoryCache
+ .entrySet()) {
+ cache.remove(entry.getKey());
+ }
repositoryCache.clear();
}
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java
index 89b754880..01edce153 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java
@@ -16,7 +16,6 @@ import java.net.URI;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.Iterator;
import java.util.Map;
import java.util.Set;
@@ -31,12 +30,8 @@ import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.jobs.Job;
-import org.eclipse.core.runtime.preferences.InstanceScope;
-import org.eclipse.core.runtime.preferences.IEclipsePreferences.IPreferenceChangeListener;
-import org.eclipse.core.runtime.preferences.IEclipsePreferences.PreferenceChangeEvent;
import org.eclipse.egit.core.Activator;
import org.eclipse.egit.core.JobFamilies;
-import org.eclipse.egit.core.RepositoryUtil;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.lib.Constants;
@@ -49,7 +44,7 @@ import org.eclipse.jgit.lib.Repository;
*/
public class IndexDiffCache {
- private Map<Repository, IndexDiffCacheEntry> entries = new HashMap<Repository, IndexDiffCacheEntry>();
+ private Map<File, IndexDiffCacheEntry> entries = new HashMap<File, IndexDiffCacheEntry>();
private Set<IndexDiffChangedListener> listeners = new HashSet<IndexDiffChangedListener>();
@@ -57,18 +52,6 @@ public class IndexDiffCache {
private ExternalFileBufferListener bufferListener;
- private final IPreferenceChangeListener configuredRepositoriesListener = new IPreferenceChangeListener() {
-
- @Override
- public void preferenceChange(PreferenceChangeEvent event) {
- if (!RepositoryUtil.PREFS_DIRECTORIES.equals(event.getKey())) {
- return;
- }
- prune(Activator.getDefault().getRepositoryUtil().getRepositories());
- }
-
- };
-
/**
* Listener on buffer changes related to the workspace external files.
*/
@@ -222,12 +205,6 @@ public class IndexDiffCache {
public IndexDiffCache() {
createGlobalListener();
registerBufferListener();
- registerConfiguredRepositoriesListener();
- }
-
- private void registerConfiguredRepositoriesListener() {
- InstanceScope.INSTANCE.getNode(Activator.getPluginId())
- .addPreferenceChangeListener(configuredRepositoriesListener);
}
private void registerBufferListener() {
@@ -247,7 +224,9 @@ public class IndexDiffCache {
public IndexDiffCacheEntry getIndexDiffCacheEntry(@NonNull Repository repository) {
IndexDiffCacheEntry entry;
synchronized (entries) {
- entry = entries.get(repository);
+ File gitDir = new Path(repository.getDirectory().getAbsolutePath())
+ .toFile();
+ entry = entries.get(gitDir);
if (entry != null) {
return entry;
}
@@ -255,7 +234,7 @@ public class IndexDiffCache {
return null;
}
entry = new IndexDiffCacheEntry(repository, globalListener);
- entries.put(repository, entry);
+ entries.put(gitDir, entry);
}
return entry;
}
@@ -321,8 +300,6 @@ public class IndexDiffCache {
bufferListener = null;
}
}
- InstanceScope.INSTANCE.getNode(Activator.getPluginId())
- .removePreferenceChangeListener(configuredRepositoriesListener);
for (IndexDiffCacheEntry entry : entries.values()) {
entry.dispose();
}
@@ -334,34 +311,31 @@ public class IndexDiffCache {
}
}
- private void prune(Set<String> configuredRepositories) {
+ /**
+ * Removes the {@link IndexDiffCacheEntry} for the given repository.
+ *
+ * @param gitDir
+ * of the {@link Repository} to remove the cache entry of
+ */
+ public void remove(@NonNull File gitDir) {
synchronized (entries) {
- Iterator<Map.Entry<Repository, IndexDiffCacheEntry>> iterator = entries
- .entrySet().iterator();
- while (iterator.hasNext()) {
- Map.Entry<Repository, IndexDiffCacheEntry> cached = iterator
- .next();
- if (configuredRepositories.contains(
- cached.getKey().getDirectory().getAbsolutePath())) {
- continue;
- }
- // Repository has vanished: remove cache entry.
- IndexDiffCacheEntry cachedEntry = cached.getValue();
+ IndexDiffCacheEntry cachedEntry = entries.remove(gitDir);
+ if (cachedEntry != null) {
cachedEntry.dispose();
- iterator.remove();
}
}
}
/**
- * Retrieves the set of repositories for which there are currently entries
- * in the cache; primarily intended for use in tests.
+ * Retrieves the set of git directories of repositories for which there are
+ * currently entries in the cache; primarily intended for use in tests.
*
- * @return the set of repositories for which the cache currently has entries
+ * @return the set of git directories of repositories for which the cache
+ * currently has entries
*/
@NonNull
- public Set<Repository> currentCacheEntries() {
- Set<Repository> result = null;
+ public Set<File> currentCacheEntries() {
+ Set<File> result = null;
synchronized (entries) {
result = new HashSet<>(entries.keySet());
}
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java
index c1e7f6377..674559132 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java
@@ -74,7 +74,9 @@ public class IndexDiffCacheEntry {
private static final int RESOURCE_LIST_UPDATE_LIMIT = 1000;
- private Repository repository;
+ private final File repositoryGitDir;
+
+ private final String repositoryName;
private volatile IndexDiffData indexDiffData;
@@ -93,6 +95,7 @@ public class IndexDiffCacheEntry {
private final ListenerHandle indexChangedListenerHandle;
private final ListenerHandle refsChangedListenerHandle;
+
private IResourceChangeListener resourceChangeListener;
private static Semaphore parallelism = new Semaphore(2);
@@ -104,7 +107,9 @@ public class IndexDiffCacheEntry {
*/
public IndexDiffCacheEntry(Repository repository,
@Nullable IndexDiffChangedListener listener) {
- this.repository = repository;
+ this.repositoryGitDir = repository.getDirectory();
+ this.repositoryName = Activator.getDefault().getRepositoryUtil()
+ .getRepositoryName(repository);
if (listener != null) {
addIndexDiffChangedListener(listener);
}
@@ -128,7 +133,8 @@ public class IndexDiffCacheEntry {
createResourceChangeListener();
if (!repository.isBare()) {
try {
- lastIndex = repository.readDirCache();
+ lastIndex = DirCache.read(repository.getIndexFile(),
+ repository.getFS());
} catch (IOException ex) {
Activator
.error(MessageFormat
@@ -138,6 +144,22 @@ public class IndexDiffCacheEntry {
}
}
+ private @Nullable Repository getRepository() {
+ if (Activator.getDefault() == null) {
+ return null;
+ }
+ Repository repository = Activator.getDefault().getRepositoryCache()
+ .getRepository(repositoryGitDir);
+ if (repository == null) {
+ return null;
+ }
+ File directory = repository.getDirectory();
+ if (directory == null || !directory.exists()) {
+ return null;
+ }
+ return repository;
+ }
+
/**
* Use this method to register an {@link IndexDiffChangedListener}. The
* listener is notified when a new index diff is available.
@@ -168,8 +190,6 @@ public class IndexDiffCacheEntry {
* @return new job ready to be scheduled, never null
*/
public Job createRefreshResourcesAndIndexDiffJob() {
- final String repositoryName = Activator.getDefault().getRepositoryUtil()
- .getRepositoryName(repository);
String jobName = MessageFormat
.format(CoreText.IndexDiffCacheEntry_refreshingProjects,
repositoryName);
@@ -177,6 +197,10 @@ public class IndexDiffCacheEntry {
@Override
public IStatus runInWorkspace(IProgressMonitor monitor) {
+ Repository repository = getRepository();
+ if (repository == null) {
+ return Status.CANCEL_STATUS;
+ }
final long start = System.currentTimeMillis();
ISchedulingRule rule = RuleUtil.getRule(repository);
try {
@@ -184,6 +208,7 @@ public class IndexDiffCacheEntry {
try {
IProject[] validOpenProjects = ProjectUtil
.getValidOpenProjects(repository);
+ repository = null;
ProjectUtil.refreshResources(validOpenProjects,
monitor);
} catch (CoreException e) {
@@ -200,6 +225,7 @@ public class IndexDiffCacheEntry {
return Status.CANCEL_STATUS;
} finally {
Job.getJobManager().endRule(rule);
+ repository = null;
}
refresh();
Job next = reloadJob;
@@ -248,11 +274,13 @@ public class IndexDiffCacheEntry {
* For bare repositories this does nothing.
*/
private void refreshIndexDelta() {
- if (repository.isBare())
+ Repository repository = getRepository();
+ if (repository == null || repository.isBare()) {
return;
-
+ }
try {
- DirCache currentIndex = repository.readDirCache();
+ DirCache currentIndex = DirCache.read(repository.getIndexFile(),
+ repository.getFS());
DirCache oldIndex = lastIndex;
lastIndex = currentIndex;
@@ -313,7 +341,7 @@ public class IndexDiffCacheEntry {
updateJob.cleanupAndCancel();
}
- if (!checkRepository()) {
+ if (getRepository() == null) {
return;
}
reloadJob = new Job(getReloadJobName()) {
@@ -332,7 +360,12 @@ public class IndexDiffCacheEntry {
}
parallelism.acquire();
long startTime = System.currentTimeMillis();
- IndexDiffData result = calcIndexDiffDataFull(monitor, getName());
+ Repository repository = getRepository();
+ if (repository == null) {
+ return Status.CANCEL_STATUS;
+ }
+ IndexDiffData result = calcIndexDiffDataFull(monitor,
+ getName(), repository);
if (monitor.isCanceled() || (result == null)) {
return Status.CANCEL_STATUS;
}
@@ -346,7 +379,7 @@ public class IndexDiffCacheEntry {
message.append(indexDiffData.toString())
.toString());
}
- notifyListeners();
+ notifyListeners(repository);
return Status.OK_STATUS;
} catch (IndexReadException e) {
return Activator.error(CoreText.IndexDiffCacheEntry_cannotReadIndex, e);
@@ -368,7 +401,7 @@ public class IndexDiffCacheEntry {
return NLS
.bind("\nUpdated IndexDiffData in {0} ms\nReason: {1}\nRepository: {2}\n", //$NON-NLS-1$
new Object[] { Long.valueOf(time), trigger,
- repository.getWorkTree().getName() });
+ repositoryGitDir });
}
@Override
@@ -384,20 +417,6 @@ public class IndexDiffCacheEntry {
reloadJob.schedule();
}
- private boolean checkRepository() {
- if (Activator.getDefault() == null) {
- return false;
- }
- if (repository == null) {
- return false;
- }
- File directory = repository.getDirectory();
- if (directory == null || !directory.exists()) {
- return false;
- }
- return true;
- }
-
/**
* Jobs accessing this code should be configured as "system" jobs, to not
* interrupt autobuild jobs, see bug 474003
@@ -428,8 +447,9 @@ public class IndexDiffCacheEntry {
*/
protected void scheduleUpdateJob(final Collection<String> filesToUpdate,
final Collection<IResource> resourcesToUpdate) {
- if (!checkRepository())
+ if (getRepository() == null) {
return;
+ }
if (reloadJob != null && reloadJobIsInitializing)
return;
@@ -469,8 +489,12 @@ public class IndexDiffCacheEntry {
lock.lock();
try {
long startTime = System.currentTimeMillis();
+ Repository repository = getRepository();
+ if (repository == null) {
+ return Status.CANCEL_STATUS;
+ }
IndexDiffData result = calcIndexDiffDataIncremental(monitor,
- getName(), files, resources);
+ getName(), repository, files, resources);
if (monitor.isCanceled() || (result == null)) {
return Status.CANCEL_STATUS;
}
@@ -487,7 +511,7 @@ public class IndexDiffCacheEntry {
message.append(indexDiffData.toString())
.toString());
}
- notifyListeners();
+ notifyListeners(repository);
return Status.OK_STATUS;
} catch (IOException e) {
if (GitTraceLocation.INDEXDIFFCACHE.isActive()) {
@@ -524,12 +548,13 @@ public class IndexDiffCacheEntry {
}
private IndexDiffData calcIndexDiffDataIncremental(IProgressMonitor monitor,
- String jobName, Collection<String> filesToUpdate,
+ String jobName, Repository repository,
+ Collection<String> filesToUpdate,
Collection<IResource> resourcesToUpdate) throws IOException {
if (indexDiffData == null)
// Incremental update not possible without prior indexDiffData
// -> do full refresh instead
- return calcIndexDiffDataFull(monitor, jobName);
+ return calcIndexDiffDataFull(monitor, jobName, repository);
EclipseGitProgressTransformer jgitMonitor = new EclipseGitProgressTransformer(
monitor);
@@ -565,7 +590,7 @@ public class IndexDiffCacheEntry {
return paths;
}
- private void notifyListeners() {
+ private void notifyListeners(Repository repository) {
IndexDiffChangedListener[] tmpListeners;
synchronized (listeners) {
tmpListeners = listeners
@@ -580,7 +605,8 @@ public class IndexDiffCacheEntry {
}
}
- private IndexDiffData calcIndexDiffDataFull(IProgressMonitor monitor, String jobName)
+ private IndexDiffData calcIndexDiffDataFull(IProgressMonitor monitor,
+ String jobName, Repository repository)
throws IOException {
EclipseGitProgressTransformer jgitMonitor = new EclipseGitProgressTransformer(
monitor);
@@ -596,22 +622,27 @@ public class IndexDiffCacheEntry {
}
private String getReloadJobName() {
- String repoName = Activator.getDefault().getRepositoryUtil()
- .getRepositoryName(repository);
- return MessageFormat.format(CoreText.IndexDiffCacheEntry_reindexing, repoName);
+ return MessageFormat.format(CoreText.IndexDiffCacheEntry_reindexing,
+ repositoryName);
}
private String getUpdateJobName() {
- String repoName = Activator.getDefault().getRepositoryUtil()
- .getRepositoryName(repository);
return MessageFormat.format(
- CoreText.IndexDiffCacheEntry_reindexingIncrementally, repoName);
+ CoreText.IndexDiffCacheEntry_reindexingIncrementally,
+ repositoryName);
}
private void createResourceChangeListener() {
resourceChangeListener = new IResourceChangeListener() {
@Override
public void resourceChanged(IResourceChangeEvent event) {
+ Repository repository = getRepository();
+ if (repository == null) {
+ ResourcesPlugin.getWorkspace()
+ .removeResourceChangeListener(this);
+ resourceChangeListener = null;
+ return;
+ }
GitResourceDeltaVisitor visitor = new GitResourceDeltaVisitor(repository);
try {
event.getDelta().accept(visitor);
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/rebase/RebaseInteractivePlan.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/rebase/RebaseInteractivePlan.java
index 84f516a24..f7de27228 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/rebase/RebaseInteractivePlan.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/rebase/RebaseInteractivePlan.java
@@ -13,6 +13,7 @@ package org.eclipse.egit.core.internal.rebase;
import java.io.File;
import java.io.IOException;
+import java.lang.ref.WeakReference;
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.Collection;
@@ -112,7 +113,9 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
private JoinedList<List<PlanElement>, PlanElement> planList;
- private final Repository repository;
+ private final WeakReference<Repository> repositoryRef;
+
+ private final File myGitDir;
private ListenerHandle refsChangedListener;
@@ -144,27 +147,30 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
}
private RebaseInteractivePlan(Repository repo) {
- this.repository = repo;
- reparsePlan();
- registerIndexDiffChangeListener();
+ this.repositoryRef = new WeakReference<>(repo);
+ this.myGitDir = repo.getDirectory().getAbsoluteFile();
+ reparsePlan(repo);
+ registerIndexDiffChangeListener(repo);
registerRefChangedListener();
}
- private void registerIndexDiffChangeListener() {
- IndexDiffCacheEntry entry = org.eclipse.egit.core.Activator
- .getDefault().getIndexDiffCache()
- .getIndexDiffCacheEntry(this.repository);
+ private void registerIndexDiffChangeListener(Repository repository) {
+ IndexDiffCacheEntry entry = org.eclipse.egit.core.Activator.getDefault()
+ .getIndexDiffCache().getIndexDiffCacheEntry(repository);
if (entry != null) {
entry.addIndexDiffChangedListener(this);
}
}
private void unregisterIndexDiffChangeListener() {
- IndexDiffCacheEntry entry = org.eclipse.egit.core.Activator
- .getDefault().getIndexDiffCache()
- .getIndexDiffCacheEntry(this.repository);
- if (entry != null) {
- entry.removeIndexDiffChangedListener(this);
+ Repository repository = getRepository();
+ if (repository != null) {
+ IndexDiffCacheEntry entry = org.eclipse.egit.core.Activator
+ .getDefault().getIndexDiffCache()
+ .getIndexDiffCacheEntry(repository);
+ if (entry != null) {
+ entry.removeIndexDiffChangedListener(this);
+ }
}
}
@@ -178,8 +184,8 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
*/
@Override
public void indexDiffChanged(Repository repo, IndexDiffData indexDiffData) {
- if (RebaseInteractivePlan.this.repository == repo)
- reparsePlan();
+ if (getRepository() == repo)
+ reparsePlan(repo);
}
/**
@@ -190,8 +196,8 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
@Override
public void onRefsChanged(RefsChangedEvent event) {
Repository repo = event.getRepository();
- if (this.repository == repo)
- reparsePlan();
+ if (getRepository() == repo)
+ reparsePlan(repo);
}
/**
@@ -201,9 +207,9 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
* will create a new {@link RebaseInteractivePlan} instance.
*/
public void dispose() {
- reparsePlan();
+ reparsePlan(getRepository());
notifyPlanWasUpdatedFromRepository();
- planRegistry.remove(this.repository.getDirectory());
+ planRegistry.remove(myGitDir);
planList.clear();
planChangeListeners.clear();
unregisterIndexDiffChangeListener();
@@ -222,7 +228,7 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
* @return the repository
*/
public Repository getRepository() {
- return repository;
+ return repositoryRef.get();
}
/**
@@ -253,14 +259,14 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
private void notifyPlanElementsOrderChange(PlanElement element, int oldIndex,
int newIndex) {
- persist();
+ persist(getRepository());
for (RebaseInteractivePlanChangeListener listener : planChangeListeners)
listener.planElementsOrderChanged(this, element, oldIndex, newIndex);
}
private void notifyPlanElementActionChange(PlanElement element,
ElementAction oldType, ElementAction newType) {
- persist();
+ persist(getRepository());
for (RebaseInteractivePlanChangeListener listener : planChangeListeners)
listener.planElementTypeChanged(this, element, oldType, newType);
}
@@ -270,16 +276,18 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
listener.planWasUpdatedFromRepository(this);
}
- private void reparsePlan() {
- try (RevWalk walk = new RevWalk(repository.newObjectReader())) {
- doneList = parseDone(walk);
- todoList = parseTodo(walk);
+ private void reparsePlan(Repository repository) {
+ if (repository != null) {
+ try (RevWalk walk = new RevWalk(repository.newObjectReader())) {
+ doneList = parseDone(repository, walk);
+ todoList = parseTodo(repository, walk);
+ }
+ planList = JoinedList.wrap(doneList, todoList);
+ notifyPlanWasUpdatedFromRepository();
}
- planList = JoinedList.wrap(doneList, todoList);
- notifyPlanWasUpdatedFromRepository();
}
- private List<PlanElement> parseTodo(RevWalk walk) {
+ private List<PlanElement> parseTodo(Repository repository, RevWalk walk) {
List<RebaseTodoLine> rebaseTodoLines;
try {
rebaseTodoLines = repository.readRebaseTodo(REBASE_TODO, true);
@@ -291,7 +299,7 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
return todoElements;
}
- private List<PlanElement> parseDone(RevWalk walk) {
+ private List<PlanElement> parseDone(Repository repository, RevWalk walk) {
List<RebaseTodoLine> rebaseDoneLines;
try {
rebaseDoneLines = repository.readRebaseTodo(REBASE_DONE, false);
@@ -359,7 +367,9 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
* {@link RepositoryState#REBASING_INTERACTIVE}
*/
public boolean isRebasingInteractive() {
- return repository.getRepositoryState() == RepositoryState.REBASING_INTERACTIVE;
+ Repository repository = getRepository();
+ return repository != null && repository
+ .getRepositoryState() == RepositoryState.REBASING_INTERACTIVE;
}
/**
@@ -407,12 +417,17 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
* Only {@link PlanElement Elements} of {@link ElementType#TODO} are
* persisted.
*
+ * @param repository
+ * the plan belong to
+ *
* @return true if the todo file has been written successfully, otherwise
* false
*/
- public boolean persist() {
- if (!isRebasingInteractive())
+ private boolean persist(Repository repository) {
+ if (repository == null || repository
+ .getRepositoryState() != RepositoryState.REBASING_INTERACTIVE) {
return false;
+ }
List<RebaseTodoLine> todoLines = new LinkedList<RebaseTodoLine>();
for (PlanElement element : planList.getSecondList())
todoLines.add(element.getRebaseTodoLine());
@@ -434,7 +449,7 @@ public class RebaseInteractivePlan implements IndexDiffChangedListener,
public void parse() throws IOException {
if (!isRebasingInteractive())
return;
- reparsePlan();
+ reparsePlan(getRepository());
}
/**
diff --git a/org.eclipse.egit.gitflow.ui/plugin.xml b/org.eclipse.egit.gitflow.ui/plugin.xml
index b26fad8d6..84dd3ceef 100644
--- a/org.eclipse.egit.gitflow.ui/plugin.xml
+++ b/org.eclipse.egit.gitflow.ui/plugin.xml
@@ -477,7 +477,7 @@
id="org.eclipse.egit.gitflow.ui.propertyTester"
namespace="GitFlowRepository"
properties="isFeature,isRelease,isHotfix,isDevelop,isMaster,isInitialized,hasDefaultRemote"
- type="org.eclipse.jgit.lib.Repository">
+ type="java.lang.String">
</propertyTester>
</extension>
diff --git a/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/decorators/GitFlowLightweightDecorator.java b/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/decorators/GitFlowLightweightDecorator.java
index 5434841fa..ef31027b6 100644
--- a/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/decorators/GitFlowLightweightDecorator.java
+++ b/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/decorators/GitFlowLightweightDecorator.java
@@ -20,6 +20,7 @@ import org.eclipse.jface.resource.ImageDescriptor;
import org.eclipse.jface.viewers.IDecoration;
import org.eclipse.jface.viewers.ILightweightLabelDecorator;
import org.eclipse.jface.viewers.LabelProvider;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.swt.graphics.ImageData;
import org.eclipse.ui.PlatformUI;
@@ -79,7 +80,10 @@ public class GitFlowLightweightDecorator extends LabelProvider implements
if (element instanceof RepositoryNode) {
RepositoryNode node = (RepositoryNode) element;
- repository = new GitFlowRepository(node.getObject());
+ Repository repo = node.getObject();
+ if (repo != null) {
+ repository = new GitFlowRepository(repo);
+ }
}
return repository;
diff --git a/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/properties/RepositoryPropertyTester.java b/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/properties/RepositoryPropertyTester.java
index 18ec48de2..f62bfc8cb 100644
--- a/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/properties/RepositoryPropertyTester.java
+++ b/org.eclipse.egit.gitflow.ui/src/org/eclipse/egit/gitflow/ui/internal/properties/RepositoryPropertyTester.java
@@ -13,6 +13,7 @@ package org.eclipse.egit.gitflow.ui.internal.properties;
import static org.eclipse.egit.gitflow.ui.Activator.error;
+import java.io.File;
import java.io.IOException;
import org.eclipse.core.expressions.PropertyTester;
@@ -41,11 +42,18 @@ public class RepositoryPropertyTester extends PropertyTester {
@Override
public boolean test(Object receiver, String property, Object[] args,
Object expectedValue) {
- if (!(receiver instanceof Repository)) {
+ if (receiver == null) {
return false;
}
- Repository repository = (Repository) receiver;
- if (repository.isBare()) {
+ Repository repository = null;
+ if (receiver instanceof String) {
+ String gitDir = (String) receiver;
+ repository = org.eclipse.egit.core.Activator.getDefault()
+ .getRepositoryCache().getRepository(new File(gitDir));
+ } else if (receiver instanceof Repository) {
+ repository = (Repository) receiver;
+ }
+ if (repository == null || repository.isBare()) {
return false;
}
GitFlowRepository gitFlowRepository = new GitFlowRepository(repository);
diff --git a/org.eclipse.egit.ui.test/META-INF/MANIFEST.MF b/org.eclipse.egit.ui.test/META-INF/MANIFEST.MF
index c2eed536e..88c0eac8a 100644
--- a/org.eclipse.egit.ui.test/META-INF/MANIFEST.MF
+++ b/org.eclipse.egit.ui.test/META-INF/MANIFEST.MF
@@ -48,9 +48,10 @@ Import-Package: org.eclipse.egit.core.test;version="[4.3.0,4.4.0)",
org.eclipse.swtbot.swt.finder.utils,
org.eclipse.swtbot.swt.finder.waits,
org.eclipse.swtbot.swt.finder.widgets,
- org.junit;version="[4.3.1,5.0.0)",
- org.junit.runner;version="[4.3.1,5.0.0)",
- org.junit.runners;version="[4.3.1,5.0.0)",
+ org.junit;version="[4.7.0,5.0.0)",
+ org.junit.rules;version="[4.7.0,5.0.0)",
+ org.junit.runner;version="[4.7.0,5.0.0)",
+ org.junit.runners;version="[4.7.0,5.0.0)",
org.mockito;version="[1.8.0,1.9.0)",
org.mockito.stubbing;version="[1.8.0,1.9.0)",
org.osgi.framework;version="[1.4.0,2.0.0)"
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java
index 22c2784ef..ad7519104 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java
@@ -36,6 +36,7 @@ import org.eclipse.egit.core.GitCorePreferences;
import org.eclipse.egit.core.GitProvider;
import org.eclipse.egit.core.JobFamilies;
import org.eclipse.egit.core.RepositoryCache;
+import org.eclipse.egit.core.RepositoryUtil;
import org.eclipse.egit.core.internal.indexdiff.IndexDiffCache;
import org.eclipse.egit.core.internal.util.ResourceUtil;
import org.eclipse.egit.core.op.AddToIndexOperation;
@@ -47,7 +48,12 @@ import org.eclipse.egit.core.project.GitProjectData;
import org.eclipse.egit.core.project.RepositoryMapping;
import org.eclipse.egit.core.test.TestUtils;
import org.eclipse.egit.ui.UIPreferences;
+import org.eclipse.egit.ui.internal.dialogs.CompareTreeView;
import org.eclipse.egit.ui.internal.push.PushOperationUI;
+import org.eclipse.egit.ui.internal.rebase.RebaseInteractiveView;
+import org.eclipse.egit.ui.internal.reflog.ReflogView;
+import org.eclipse.egit.ui.internal.repository.RepositoriesView;
+import org.eclipse.egit.ui.internal.staging.StagingView;
import org.eclipse.egit.ui.test.ContextMenuHelper;
import org.eclipse.egit.ui.test.Eclipse;
import org.eclipse.egit.ui.test.TestUtil;
@@ -69,10 +75,14 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
import org.eclipse.swtbot.swt.finder.widgets.SWTBotTree;
import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem;
import org.eclipse.team.core.RepositoryProvider;
+import org.eclipse.team.ui.history.IHistoryView;
+import org.eclipse.team.ui.synchronize.ISynchronizeView;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.rules.TestName;
/**
* Base class for testing with local (file-system based) repositories
@@ -140,20 +150,39 @@ public abstract class LocalRepositoryTestCase extends EGitTestCase {
protected static final String FILE2 = "test2.txt";
-
-
protected final static TestUtils testUtils = new TestUtils();
+ private static final String[] VIEWS_TO_CLOSE = { //
+ RebaseInteractiveView.VIEW_ID, //
+ ISynchronizeView.VIEW_ID, //
+ IHistoryView.VIEW_ID, //
+ CompareTreeView.ID, //
+ ReflogView.VIEW_ID, //
+ StagingView.VIEW_ID, //
+ RepositoriesView.VIEW_ID, //
+ "org.eclipse.search.ui.views.SearchView", //
+ "org.eclipse.ui.views.PropertySheet"
+ };
+
+ @Rule
+ public TestName testName = new TestName();
+
public File getTestDirectory() {
return testDirectory;
}
+ protected static void closeGitViews() {
+ for (String viewId : VIEWS_TO_CLOSE) {
+ TestUtil.hideView(viewId);
+ }
+ }
+
@Before
public void initNewTestDirectory() throws Exception {
testMethodNumber++;
// create standalone temporary directory
testDirectory = testUtils.createTempDir("LocalRepositoriesTests"
- + testMethodNumber);
+ + testMethodNumber + '_' + testName.getMethodName());
if (testDirectory.exists())
FileUtils.delete(testDirectory, FileUtils.RECURSIVE
| FileUtils.RETRY);
@@ -174,6 +203,7 @@ public abstract class LocalRepositoryTestCase extends EGitTestCase {
TestUtil.processUIEvents();
// close all editors/dialogs
new Eclipse().reset();
+ closeGitViews();
TestUtil.processUIEvents();
// cleanup
for (IProject project : ResourcesPlugin.getWorkspace().getRoot()
@@ -201,6 +231,7 @@ public abstract class LocalRepositoryTestCase extends EGitTestCase {
.getPreferenceStore()
.setValue(UIPreferences.SHOW_DETACHED_HEAD_WARNING,
false);
+ closeGitViews();
}
@AfterClass
@@ -208,11 +239,17 @@ public abstract class LocalRepositoryTestCase extends EGitTestCase {
testUtils.deleteTempDirs();
}
- protected static void shutDownRepositories() {
+ protected static void shutDownRepositories() throws Exception {
RepositoryCache cache = Activator.getDefault().getRepositoryCache();
for(Repository repository:cache.getAllRepositories())
repository.close();
cache.clear();
+ IEclipsePreferences prefs = Activator.getDefault().getRepositoryUtil()
+ .getPreferences();
+ synchronized (prefs) {
+ prefs.put(RepositoryUtil.PREFS_DIRECTORIES, "");
+ prefs.flush();
+ }
}
protected static void deleteAllProjects() throws Exception {
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/gitflow/AbstractGitflowHandlerTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/gitflow/AbstractGitflowHandlerTest.java
index 03b5ee58e..e04897782 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/gitflow/AbstractGitflowHandlerTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/gitflow/AbstractGitflowHandlerTest.java
@@ -37,6 +37,7 @@ import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.swtbot.swt.finder.junit.SWTBotJunit4ClassRunner;
+import org.junit.After;
import org.junit.Before;
import org.junit.runner.RunWith;
@@ -59,6 +60,11 @@ public abstract class AbstractGitflowHandlerTest extends LocalRepositoryTestCase
resetPreferences();
}
+ @After
+ public void teardown() {
+ repository = null;
+ }
+
private void resetPreferences() {
IPreferenceStore prefStore = Activator.getDefault()
.getPreferenceStore();
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
index 320344a6f..7b77a7fa4 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
@@ -50,6 +50,7 @@ import org.eclipse.ui.IEditorReference;
import org.eclipse.ui.IViewPart;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -86,6 +87,8 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
parentRepositoryGitDir = createProjectAndCommitToRepository();
childRepositoryGitDir = createProjectAndCommitToRepository(CHILDREPO,
CHILDPROJECT);
+ Activator.getDefault().getRepositoryUtil()
+ .addConfiguredRepository(parentRepositoryGitDir);
parentRepository = lookupRepository(parentRepositoryGitDir);
childRepository = lookupRepository(childRepositoryGitDir);
parentProject = ResourcesPlugin.getWorkspace().getRoot()
@@ -137,6 +140,21 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
assertNotNull(subRepository);
}
+ @After
+ public void removeConfiguredRepositories() {
+ if (parentRepositoryGitDir != null) {
+ Activator.getDefault().getRepositoryUtil()
+ .removeDir(parentRepositoryGitDir);
+ }
+ if (childRepositoryGitDir != null) {
+ Activator.getDefault().getRepositoryUtil()
+ .removeDir(childRepositoryGitDir);
+ }
+ childRepository = null;
+ parentRepository = null;
+ subRepository = null;
+ }
+
@Test
public void testChildProjectMapsToSubRepo() {
RepositoryMapping mapping = RepositoryMapping.getMapping(childProject);
@@ -185,6 +203,7 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
TestUtil.joinJobs(JobFamilies.INDEX_DIFF_CACHE_UPDATE);
IndexDiffCacheEntry cache = Activator.getDefault().getIndexDiffCache()
.getIndexDiffCacheEntry(subRepository);
+ TestUtil.joinJobs(JobFamilies.INDEX_DIFF_CACHE_UPDATE);
IResourceState state = ResourceStateFactory.getInstance()
.get(cache.getIndexDiff(), file);
assertTrue("File should be staged", state.isStaged());
@@ -261,6 +280,24 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
node.getText().contains("[child"));
}
+ /**
+ * Tests that unrelated changes to the configured repositories do not
+ * prematurely remove submodules from the cache.
+ */
+ @Test
+ public void testRepoRemoval() {
+ Activator.getDefault().getRepositoryUtil()
+ .addConfiguredRepository(childRepositoryGitDir);
+ assertTrue("Should still have the subrepo in the cache",
+ containsRepo(Activator.getDefault().getRepositoryCache()
+ .getAllRepositories(), subRepository));
+ assertTrue("Should have changed the preference", Activator.getDefault()
+ .getRepositoryUtil().removeDir(childRepositoryGitDir));
+ assertTrue("Should still have the subrepo in the cache",
+ containsRepo(Activator.getDefault().getRepositoryCache()
+ .getAllRepositories(), subRepository));
+ }
+
@SuppressWarnings("restriction")
@Test
public void testHistoryFromProjectExplorerIsFromSubRepository()
@@ -283,6 +320,15 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
2);
}
+ private boolean containsRepo(Repository[] repositories, Repository needle) {
+ for (Repository repo : repositories) {
+ if (needle.equals(repo)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
private void assertRowCountInHistory(String msg, int expected)
throws Exception {
SWTBotView historyBot = TestUtil.showHistoryView();
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java
index 730b0739d..0399bed31 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/view/repositories/GitRepositoriesViewRepoDeletionTest.java
@@ -17,11 +17,22 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.io.File;
+import java.lang.management.ManagementFactory;
+import java.lang.management.MemoryMXBean;
+import java.util.Arrays;
import java.util.List;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IWorkspaceRoot;
+import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.egit.core.internal.indexdiff.IndexDiffCache;
import org.eclipse.egit.ui.Activator;
import org.eclipse.egit.ui.JobFamilies;
+import org.eclipse.egit.ui.internal.RepositoryCacheRule;
import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.egit.ui.test.ContextMenuHelper;
import org.eclipse.egit.ui.test.TestUtil;
@@ -32,6 +43,7 @@ import org.eclipse.swtbot.swt.finder.junit.SWTBotJunit4ClassRunner;
import org.eclipse.swtbot.swt.finder.widgets.SWTBotCheckBox;
import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
import org.eclipse.swtbot.swt.finder.widgets.SWTBotTree;
+import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -149,21 +161,109 @@ public class GitRepositoriesViewRepoDeletionTest extends
TestUtil.joinJobs(JobFamilies.REPOSITORY_DELETE);
refreshAndWait();
assertEmpty();
+
assertTrue(repositoryFile.exists());
assertTrue(
new File(repositoryFile.getParentFile(), PROJ1).isDirectory());
+ waitForDecorations();
+ closeGitViews();
+ TestUtil.waitForJobs(500, 5000);
+ // Session properties are stored in the Eclipse resource tree as part of
+ // the resource info. org.eclipse.core.internal.dtree.DataTreeLookup has
+ // a static LRU cache of lookup instances to avoid excessive strain on
+ // the garbage collector due to constantly allocating and then
+ // forgetting instances. These lookup objects may contain things
+ // recently queried or modified in the resource tree, such as session
+ // properties. As a result, the session properties of a deleted resource
+ // remain around a little longer than expected: to be precise, exactly
+ // 100 more queries on the Eclipse resource tree until the entry
+ // containing the session property is recycled. We use session
+ // properties to store the RepositoryMappings, which reference the
+ // repository.
+ //
+ // Make sure we clear that cache:
+ IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot();
+ IProject project = root.getProject(PROJ1);
+ for (int i = 0; i < 101; i++) {
+ // Number of iterations at least DataTreeLookup.POOL_SIZE!
+ // Use up one DataTreeLookup instance:
+ project.create(null);
+ if (i == 0) {
+ // Furthermore, the WorkbenchSourceProvider has still a
+ // reference to the last selection, which is our now long
+ // removed repository node! Arguably that's a strange thing, but
+ // strictly speaking, since there is no guarantee _when_ a
+ // weakly referenced object is removed, not even making
+ // WorkbenchSourceProvider.lastShowInSelection a WeakReference
+ // might help. Therefore, let's make sure that the last "show
+ // in" selection is no longer the RepositoryNode, which also
+ // still has a reference to the repository. That last "show in"
+ // selection is set when the "Shown in..." context menu is
+ // filled, which happens when the project explorer's context
+ // menu is activated. So we have to open that menu at least once
+ // with a different selection.
+ SWTBotTree explorerTree = TestUtil.getExplorerTree();
+ SWTBotTreeItem projectNode = TestUtil.navigateTo(explorerTree,
+ PROJ1);
+ projectNode.select();
+ ContextMenuHelper.isContextMenuItemEnabled(explorerTree, "New");
+ }
+ project.delete(true, true, null);
+ }
+ TestUtil.waitForJobs(500, 5000);
+ // And we may have the RepositoryChangeScanner running: use a job
+ // with a scheduling rule that ensures we have exclusive access.
+ final String[] results = { null, null };
+ Job verifier = new Job(testName.getMethodName()) {
+
+ @Override
+ protected IStatus run(IProgressMonitor monitor) {
+ // Wait for things to definitely quieten down. Note that
+ // waitForJobs only waits for running and waiting jobs, there
+ // may still be scheduled jobs that might wake up and run after
+ // that. TestUtil.joinJobs does really join, which also waits
+ // for scheduled jobs.
+ try {
+ TestUtil.joinJobs(
+ org.eclipse.egit.core.JobFamilies.INDEX_DIFF_CACHE_UPDATE);
+ // Is this job doing something when the view is hidden?
+ TestUtil.joinJobs(JobFamilies.REPO_VIEW_REFRESH);
+ waitForDecorations();
+ } catch (InterruptedException e) {
+ results[0] = "Interrupted";
+ Thread.currentThread().interrupt();
+ return Status.CANCEL_STATUS;
+ }
+ // Finally... Java does not give any guarantees about when
+ // exactly an only weakly reachable object is finalized and
+ // garbage collected.
+ waitForFinalization(5000);
+ // Experience shows that an explicit garbage collection run very
+ // often does reclaim only weakly reachable objects and set the
+ // weak references' referents to null, but not even that can be
+ // guaranteed! Whether or not it does may also depend on the
+ // configuration of the JVM (such as through command-line
+ // arguments).
+ Repository[] repositories = org.eclipse.egit.core.Activator
+ .getDefault().getRepositoryCache().getAllRepositories();
+ results[0] = Arrays.asList(repositories).toString();
+ IndexDiffCache indexDiffCache = org.eclipse.egit.core.Activator
+ .getDefault().getIndexDiffCache();
+ results[1] = indexDiffCache.currentCacheEntries().toString();
+ return Status.OK_STATUS;
+ }
+
+ };
+ verifier.setRule(new RepositoryCacheRule());
+ verifier.setSystem(true);
+ verifier.schedule();
+ verifier.join();
List<String> configuredRepos = org.eclipse.egit.core.Activator
.getDefault().getRepositoryUtil().getConfiguredRepositories();
assertTrue("Expected no configured repositories",
configuredRepos.isEmpty());
- Repository[] repositories = org.eclipse.egit.core.Activator.getDefault()
- .getRepositoryCache().getAllRepositories();
- assertEquals("Expected no cached repositories", 0, repositories.length);
- // A pity we can't mock the cache.
- IndexDiffCache indexDiffCache = org.eclipse.egit.core.Activator
- .getDefault().getIndexDiffCache();
- assertTrue("Expected no IndexDiffCache entries",
- indexDiffCache.currentCacheEntries().isEmpty());
+ assertEquals("Expected no cached repositories", "[]", results[0]);
+ assertEquals("Expected no IndexDiffCache entries", "[]", results[1]);
}
@Test
@@ -217,4 +317,31 @@ public class GitRepositoriesViewRepoDeletionTest extends
assertFalse(subRepo.getWorkTree().exists());
}
+ @SuppressWarnings("restriction")
+ private void waitForDecorations() throws InterruptedException {
+ TestUtil.joinJobs(
+ org.eclipse.ui.internal.decorators.DecoratorManager.FAMILY_DECORATE);
+ }
+
+ /**
+ * Best-effort attempt to get finalization to occur.
+ *
+ * @param maxMillis
+ * maximum amount of time in milliseconds to try getting the
+ * garbage collector to finalize objects
+ */
+ private void waitForFinalization(int maxMillis) {
+ long stop = System.currentTimeMillis() + maxMillis;
+ MemoryMXBean memoryBean = ManagementFactory.getMemoryMXBean();
+ do {
+ System.gc();
+ try {
+ Thread.sleep(100);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ break;
+ }
+ } while (System.currentTimeMillis() < stop
+ && memoryBean.getObjectPendingFinalizationCount() > 0);
+ }
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java
index ed3a1824a..6ccb9a48b 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/Activator.java
@@ -42,6 +42,7 @@ import org.eclipse.egit.core.RepositoryUtil;
import org.eclipse.egit.core.project.RepositoryMapping;
import org.eclipse.egit.ui.internal.ConfigurationChecker;
import org.eclipse.egit.ui.internal.KnownHosts;
+import org.eclipse.egit.ui.internal.RepositoryCacheRule;
import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.egit.ui.internal.credentials.EGitCredentialsProvider;
import org.eclipse.egit.ui.internal.trace.GitTraceLocation;
@@ -526,6 +527,7 @@ public class Activator extends AbstractUIPlugin implements DebugOptionsListener
static class RepositoryChangeScanner extends Job {
RepositoryChangeScanner() {
super(UIText.Activator_repoScanJobName);
+ setRule(new RepositoryCacheRule());
}
// FIXME, need to be more intelligent about this to avoid too much work
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/RepositoryCacheRule.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/RepositoryCacheRule.java
new file mode 100644
index 000000000..abd2e6388
--- /dev/null
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/RepositoryCacheRule.java
@@ -0,0 +1,50 @@
+/*******************************************************************************
+ * Copyright (C) 2016 Thomas Wolf <thomas.wolf@paranor.ch>
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *******************************************************************************/
+package org.eclipse.egit.ui.internal;
+
+import org.eclipse.core.runtime.jobs.ISchedulingRule;
+import org.eclipse.core.runtime.jobs.MultiRule;
+
+/**
+ * A scheduling rule that can be used by (background) jobs that access the
+ * {@link org.eclipse.egit.core.RepositoryCache}. Conflicts with other instances
+ * of this class or subclasses, and contains only other RepositoryCacheRules.
+ */
+public class RepositoryCacheRule implements ISchedulingRule {
+
+ @Override
+ public boolean contains(ISchedulingRule rule) {
+ if (rule instanceof RepositoryCacheRule) {
+ return true;
+ } else if (rule instanceof MultiRule) {
+ for (ISchedulingRule child : ((MultiRule) rule).getChildren()) {
+ if (!contains(child)) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+ }
+
+ @Override
+ public boolean isConflicting(ISchedulingRule rule) {
+ if (rule instanceof RepositoryCacheRule) {
+ return true;
+ } else if (rule instanceof MultiRule) {
+ for (ISchedulingRule child : ((MultiRule) rule).getChildren()) {
+ if (isConflicting(child)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java
index dfd6e47bd..846778836 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/SpellcheckableMessageArea.java
@@ -889,32 +889,50 @@ public class SpellcheckableMessageArea extends Composite {
return sourceViewer.getTextWidget();
}
+ private static class QuickfixAction extends Action {
+
+ private final ITextOperationTarget textOperationTarget;
+
+ public QuickfixAction(ITextOperationTarget target) {
+ textOperationTarget = target;
+ }
+
+ @Override
+ public void run() {
+ textOperationTarget.doOperation(ISourceViewer.QUICK_ASSIST);
+ }
+
+ }
+
private ActionHandler createQuickFixActionHandler(
final ITextOperationTarget textOperationTarget) {
- Action quickFixAction = new Action() {
+ Action quickFixAction = new QuickfixAction(textOperationTarget);
+ quickFixAction.setActionDefinitionId(
+ ITextEditorActionDefinitionIds.QUICK_ASSIST);
+ return new ActionHandler(quickFixAction);
+ }
- @Override
- public void run() {
- textOperationTarget.doOperation(ISourceViewer.QUICK_ASSIST);
+ private static class ContentAssistAction extends Action {
+
+ private final SourceViewer viewer;
+
+ public ContentAssistAction(SourceViewer viewer) {
+ this.viewer = viewer;
+ }
+
+ @Override
+ public void run() {
+ if (viewer.canDoOperation(ISourceViewer.CONTENTASSIST_PROPOSALS)
+ && viewer.getTextWidget().isFocusControl()) {
+ viewer.doOperation(ISourceViewer.CONTENTASSIST_PROPOSALS);
}
- };
- quickFixAction
- .setActionDefinitionId(ITextEditorActionDefinitionIds.QUICK_ASSIST);
- return new ActionHandler(quickFixAction);
+ }
+
}
private ActionHandler createContentAssistActionHandler(
- final ITextOperationTarget textOperationTarget) {
- Action proposalAction = new Action() {
- @Override
- public void run() {
- if (textOperationTarget
- .canDoOperation(ISourceViewer.CONTENTASSIST_PROPOSALS)
- && getTextWidget().isFocusControl())
- textOperationTarget
- .doOperation(ISourceViewer.CONTENTASSIST_PROPOSALS);
- }
- };
+ final SourceViewer viewer) {
+ Action proposalAction = new ContentAssistAction(viewer);
proposalAction
.setActionDefinitionId(ITextEditorActionDefinitionIds.CONTENT_ASSIST_PROPOSALS);
return new ActionHandler(proposalAction);
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/rebase/RebaseInteractiveView.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/rebase/RebaseInteractiveView.java
index 26646802c..e24684cb4 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/rebase/RebaseInteractiveView.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/rebase/RebaseInteractiveView.java
@@ -572,7 +572,10 @@ public class RebaseInteractiveView extends ViewPart implements
@Override
public void widgetSelected(SelectionEvent sEvent) {
try {
- command.execute(currentPlan.getRepository());
+ Repository repository = currentPlan.getRepository();
+ if (repository != null) {
+ command.execute(repository);
+ }
} catch (ExecutionException e) {
Activator.showError(e.getMessage(), e);
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java
index 3fe31e3c3..b54f230ca 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/RepositoryTreeNode.java
@@ -27,9 +27,9 @@ import org.eclipse.jgit.lib.Repository;
*/
public abstract class RepositoryTreeNode<T> extends PlatformObject implements Comparable<RepositoryTreeNode> {
- private final Repository myRepository;
+ private Repository myRepository;
- private final T myObject;
+ private T myObject;
private final RepositoryTreeNodeType myType;
@@ -80,10 +80,32 @@ public abstract class RepositoryTreeNode<T> extends PlatformObject implements Co
* @return the path of the file, folder or repository
*/
public IPath getPath() {
+ Repository repository = getRepository();
+ if (repository == null) {
+ return null;
+ }
return new Path(getRepository().getWorkTree().getAbsolutePath());
}
/**
+ * Removes the references to the repository and to the object. <strong>Call
+ * only after this node has been removed from the view!</strong>
+ * <p>
+ * The CommonViewer framework keeps on to a hard reference to the last
+ * selection, even if that no longer will appear in the view. Moreover, the
+ * WorkbenchSourceProvider may also hold such a reference to the
+ * RepositoryNode(s). This will preclude for some time the garbage
+ * collection and eventual removal of the Repository instance
+ * (RepositoryCache relies on WeakReference semantics). Therefore, this
+ * operation provides a means to clear the reference to the Repository in a
+ * now otherwise unreferenced RepositoryNode.
+ */
+ public void clear() {
+ myRepository = null;
+ myObject = null;
+ }
+
+ /**
* Depending on the node type, the returned type is:
*
* <table border=1>
@@ -220,16 +242,30 @@ public abstract class RepositoryTreeNode<T> extends PlatformObject implements Co
} else if (!myParent.equals(other.myParent))
return false;
if (myRepository == null) {
- if (other.myRepository != null)
+ if (other.myRepository != null) {
return false;
- } else if (!myRepository.getDirectory().equals(
- other.myRepository.getDirectory()))
- return false;
+ }
+ } else {
+ if (other.myRepository == null) {
+ return false;
+ }
+ if (!myRepository.getDirectory()
+ .equals(other.myRepository.getDirectory())) {
+ return false;
+ }
+ }
if (myObject == null) {
- if (other.myObject != null)
+ if (other.myObject != null) {
return false;
- } else if (!checkObjectsEqual(other.myObject))
- return false;
+ }
+ } else {
+ if (other.myObject == null) {
+ return false;
+ }
+ if (!checkObjectsEqual(other.myObject)) {
+ return false;
+ }
+ }
return true;
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java
index b2f0dfb5b..003de7b67 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/tree/command/RemoveCommand.java
@@ -56,8 +56,8 @@ import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
import org.eclipse.jgit.util.FileUtils;
import org.eclipse.osgi.util.NLS;
-import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.IWorkbenchSite;
+import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.handlers.HandlerUtil;
import org.eclipse.ui.progress.IWorkbenchSiteProgressService;
@@ -126,7 +126,7 @@ public class RemoveCommand extends
if (!projectsToDelete.isEmpty()) {
final boolean[] confirmedCanceled = new boolean[] { false,
false };
- Display.getDefault().syncExec(new Runnable() {
+ PlatformUI.getWorkbench().getDisplay().syncExec(new Runnable() {
@Override
public void run() {
@@ -170,6 +170,16 @@ public class RemoveCommand extends
return Activator.createErrorStatus(e.getMessage(), e);
}
}
+ PlatformUI.getWorkbench().getDisplay()
+ .asyncExec(new Runnable() {
+
+ @Override
+ public void run() {
+ for (RepositoryNode node : selectedNodes) {
+ node.clear();
+ }
+ }
+ });
return Status.OK_STATUS;
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/selection/RepositorySourceProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/selection/RepositorySourceProvider.java
index 561f78331..4beca05fd 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/selection/RepositorySourceProvider.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/selection/RepositorySourceProvider.java
@@ -8,6 +8,7 @@
*******************************************************************************/
package org.eclipse.egit.ui.internal.selection;
+import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.Map;
@@ -25,7 +26,13 @@ import org.eclipse.ui.services.IServiceLocator;
/**
* An {@link AbstractSourceProvider} that provides the current repository (based
* on the current selection) as a variable in an Eclipse
- * {@link org.eclipse.core.expressions.IEvaluationContext}.
+ * {@link org.eclipse.core.expressions.IEvaluationContext}. To avoid interfering
+ * with repository removal, which in EGit relies on {@link WeakReference}
+ * semantics, the variable provides not the {@link Repository} directly but its
+ * git directory as a string, which can then be used to obtain the
+ * {@link Repository} instance via
+ * {@link org.eclipse.egit.core.RepositoryCache#getRepository(java.io.File)}. If
+ * no repository can be determined, the variable's value is the empty string.
*/
public class RepositorySourceProvider extends AbstractSourceProvider
implements ISelectionListener, IWindowListener {
@@ -37,7 +44,10 @@ public class RepositorySourceProvider extends AbstractSourceProvider
*/
public static final String REPOSITORY_PROPERTY = "org.eclipse.egit.ui.currentRepository"; //$NON-NLS-1$
- private Repository repository;
+ /**
+ * Use a weak reference here to not preclude repository removal.
+ */
+ private WeakReference<Repository> repositoryRef;
@Override
public void initialize(IServiceLocator locator) {
@@ -48,12 +58,19 @@ public class RepositorySourceProvider extends AbstractSourceProvider
@Override
public void dispose() {
PlatformUI.getWorkbench().removeWindowListener(this);
- repository = null;
+ repositoryRef = null;
}
@Override
public Map getCurrentState() {
- return Collections.singletonMap(REPOSITORY_PROPERTY, repository);
+ @SuppressWarnings("resource")
+ Repository repository = repositoryRef == null ? null
+ : repositoryRef.get();
+ if (repository == null) {
+ return Collections.singletonMap(REPOSITORY_PROPERTY, ""); //$NON-NLS-1$
+ }
+ return Collections.singletonMap(REPOSITORY_PROPERTY,
+ repository.getDirectory().getAbsolutePath());
}
@Override
@@ -70,11 +87,29 @@ public class RepositorySourceProvider extends AbstractSourceProvider
newRepository = SelectionUtils.getRepository(
SelectionUtils.getStructuredSelection(selection));
}
- if (repository != newRepository) {
- repository = newRepository;
- fireSourceChanged(ISources.ACTIVE_WORKBENCH_WINDOW,
- REPOSITORY_PROPERTY,
- repository);
+ @SuppressWarnings("resource")
+ Repository currentRepository = repositoryRef == null ? null
+ : repositoryRef.get();
+ if (currentRepository == null && repositoryRef != null) {
+ repositoryRef = null;
+ if (newRepository == null) {
+ // Last evaluation was non-null, but that repo has since gone.
+ fireSourceChanged(ISources.ACTIVE_WORKBENCH_WINDOW,
+ REPOSITORY_PROPERTY, ""); //$NON-NLS-1$
+ return;
+ }
+ }
+ if (currentRepository != newRepository) {
+ if (newRepository != null) {
+ repositoryRef = new WeakReference<>(newRepository);
+ fireSourceChanged(ISources.ACTIVE_WORKBENCH_WINDOW,
+ REPOSITORY_PROPERTY,
+ newRepository.getDirectory().getAbsolutePath());
+ } else {
+ repositoryRef = null;
+ fireSourceChanged(ISources.ACTIVE_WORKBENCH_WINDOW,
+ REPOSITORY_PROPERTY, ""); //$NON-NLS-1$
+ }
}
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java
index 08ddc0946..011117e26 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/staging/StagingView.java
@@ -3305,9 +3305,10 @@ public class StagingView extends ViewPart implements IShowInSource {
getPreferenceStore().removePropertyChangeListener(uiPrefsListener);
-
getDialogSettings().put(STORE_SORT_STATE, sortAction.isChecked());
+ currentRepository = null;
+
disposed = true;
}

Back to the top