Skip to main content
AgeCommit message (Collapse)AuthorFilesLines
2016-12-28Display logical line numbers in DiffEditorPageThomas Wolf1-1/+0
Give the DiffEditorPage a special LineNumberRulerColumn that can display logical line numbers of the shown hunk lines, corresponding to the line numbers in changed file versions. The ruler menu has a toggle action to switch between this logical display and the normal physical line number display. * Store the line numbers in the DiffRegions produced by the DiffRegionFormatter. Add a CONTEXT DiffRegion.Type to be able to number context lines. * Introduce a LogicalLineNumberProvider. * Add a LogicalLineNumberRulerColumn that uses such a provider to determine line numbers for a diff side. * Add a composite OldNewLogicalLineNumberRulerColumn that groups two such LogicalLineNumberRulerColumns (one for each side) together, and that contains the toggling logic for physical/logical line number display. Change-Id: I53394854523d15187c21786ca54370561d84642d Signed-off-by: Thomas Wolf <>
2016-12-27[findBugs] Fix warning "writing static from instance method"Matthias Sohn1-5/+7
Change-Id: I9952f9047bff73497d10a2efcc6164bbdc7685ca Signed-off-by: Matthias Sohn <>
2016-12-10Rename DiffStyleRangeFormatter and friendsThomas Wolf1-14/+14
Since the DiffStyleRangeFormatter no longer produces StyleRanges, let's rename this class to DiffRegionFormatter. Also rename its range classes to *Region and derive them from Region. Change-Id: I142ae0550723e819163c6c50a3c4229aa99ce855 Signed-off-by: Thomas Wolf <> Signed-off-by: Matthias Sohn <>
2016-12-06Introduce GitSynchronizer for logical model supportLaurent Delaigue3-0/+385
This allows providing distinct implementations when doing synchronizations depending on whether or not the user is interested in logical models. The default synchronizer implementation does not deal with logical models at all and thus provides the best performance. The ModelAwareGitSynchronizer consults the logical model providers to determine the set of resources involved in synchronization operations and thus can have important performance impact especially on large repositories. It uses a 'lazy' implementation of subscriber to load additional resource revisions lazily when needed. Bug: 501990 Change-Id: I9322cef70c037e0b2c270a7c2bab7d2ffd278f60 Signed-off-by: Laurent Delaigue <> Signed-off-by: Matthias Sohn <>
2016-12-02Clean up "Replace With..." actionsThomas Wolf1-26/+29
Replace deprecated <objectContribution>s by command/handler combos. Remove now unused legacy actions. "Replace with HEAD" and "Replace With Index" also work on multiple repositories. Change the resourcesAllInRepository property test to also handle repository property tests in order to be able to test the isSafe property. Restrict the "Replace with Previous" to tracked resources; on untracked resources it would only open a dialog telling the user that there was no previous revision. Which is correct, but then why allow trying to execute the command at all? Visual change: previously, "Replace with HEAD" and "Replace With Index" did not appear on a WorkingSet containing projects from several repositories. Now they do. It's unclear to me why they didn't before, and in any case they also appeared before on WorkingSets containing projects from a single repository. Both commands are and were active when projects from several repositories were selected. Bug: 495064 Change-Id: Id0fae18e042aa180310f289e48ad13ad263c9481 Signed-off-by: Thomas Wolf <>
2016-11-23Test stability fix for GitRepositoriesViewTestMatthias Sohn1-1/+4
Change-Id: I0d91487ac2b5ef2e55561181b3d66b717d483d83 Signed-off-by: Matthias Sohn <>
2016-10-26Avoid potential deadlocks in repositories viewAndrey Loskutov3-2/+11
Don't join() in UI thread on a job which triggers Display.syncExec(). Neither join not sync execution was actually needed. Bug: 506463 Change-Id: I4f11d4fa56507493a059a550108ce17c2fe96b10 Signed-off-by: Andrey Loskutov <>
2016-10-24Do not auto-fill reference in pull wizard with local branch nameMatthias Sohn1-0/+3
The reference field was auto-filled with the name of the currently checked out local branch. This doesn't make sense since there is no guarantee that a branch with the same name exists upstream. Checking if this is the case would be too slow since it involves a remote request. Hence leave the reference field empty, the user can use content assist to find candidate upstream references. Change-Id: I624cd7bd095830455910e9f212a13fb48ebea6fc Signed-off-by: Matthias Sohn <>
2016-10-20Handle rebase modes 'preserve' and 'interactive' in EGitThomas Wolf4-39/+47
EGit and JGit treated branch.<name>.rebase as a boolean config value, which is wrong: git actually also allows 'preserve' and 'interactive'. Since commit aadbb158 in JGit, JGit does recognize these extra values. Adapt the EGit UI (wizards & dialogs for branch creation, branch configuration, pulling and pushing a branch) to provide a UI to let the user select any valid BranchRebaseMode value. Introduce a new BranchRebaseModeCombo for this. Adapt UI tests. Honor the setting in all relevant operations in EGit. Note that JGit currently ignores the "interactive" setting in a PullCommand and performs a normal rebase instead. Bug: 499482 Change-Id: I0a3b639bbb9e5dd5d93134587410ba72c0549cc7 Signed-off-by: Thomas Wolf <>
2016-10-16Test stability: re-select tree itemThomas Wolf1-0/+2
Otherwise the selection may sometimes get lost, and then the menu entry cannot be found. Verified that the test still fails without the correction from commit 223247c1 and succeeds with it. Change-Id: Ia8dce4e5515a769992aa157a7e92a6a01796e55e Signed-off-by: Thomas Wolf <>
2016-10-12Fix the enablement of "Compare with->Index with HEAD"Thomas Wolf1-0/+45
This menu command makes sense only for staged files, but it was enabled wrongly also on unstaged files. Apparently that was done on purpose in commit 6d975be6 to avoid a costly index diff calculation to find out whether the file was staged. But if we are able to display "staged" decorations speedily, we certainly are also able to determine quickly whether a file is staged. Make isEnabled() properly check again, using the same mechanisms we use for decorations, which rely on the IndexDiffCache. Remove the extra check when the command is run, including the dialog. There is one quirk here: these "Compare With..." actions are contributed using the old (and deprecated) org.eclipse.ui.popupMenus extension point with <objectContribution>. They get invoked via RepositoryAction, which has some extra logic to avoid re-computing enablement when the selection didn't change as far as resources are concerned. This of course goes wrong if the enablement state of a handler depends on other state that may have changed, such as the resources git state (staged/modified/...). Add some logic to enforce always updating enablement for such handlers. This works and is even uncritical when typing in an editor because: * determining staging state via the IndexDiffCache is fast, and * the enablement is only computed when the context menu is opened. Change-Id: Ic6ec1e407f2ba1403ae52bc09b4f2546f4e221c8 Signed-off-by: Thomas Wolf <>
2016-10-12Test stability: avoid arbitrarily timed waitThomas Wolf1-4/+2
Change-Id: I391c4e5fd5e75e1cf2352bd561cf2f15b887da6f Signed-off-by: Thomas Wolf <>
2016-10-11Test stability: get node anew after having refreshed the viewThomas Wolf1-2/+6
GitRepositoriesViewBranchHandlingTest occasionally failed to select the correct node, and then could not find the "Delete Branch" menu item. Try to fix that by getting the local branches node afresh after the view has been refreshed. Change-Id: I5989776829eda9a5b000d722a941d5ad72f4d043 Signed-off-by: Thomas Wolf <>
2016-09-29Test stability: wait for background jobs to finishThomas Wolf1-0/+3
Git repository view tests that clone or add repositories should wait for background jobs before trying to assert any view state. Change-Id: I676f3c526d927cc556fed6ba54d04c2f164209b9 Signed-off-by: Thomas Wolf <>
2016-09-21Revert "Provide a remote mapping context to ModelProviders"Andrey Loskutov3-363/+0
This reverts commit 218f8cec2b7ceb5b9b6d64bb037e8f875521e27a. Bug: 501886 Change-Id: Iafff598257b9774253d95ea5f97cfcf83ae6a400 Signed-off-by: Andrey Loskutov <>
2016-09-06Test stability: use dialog's bot to find buttonThomas Wolf1-3/+9
SWTbot tests sometimes don't find the "Remove" button. Perhaps it's related to using the global bot. Use the dialog's bot instead. Change-Id: I5f7b9f8d0ea9d1faf8a0e818029330dcc881e61f Signed-off-by: Thomas Wolf <>
2016-08-30Provide a remote mapping context to ModelProvidersLaurent Delaigue3-0/+363
Allow model providers to use remote information when trying to determine whether a file is a part of a logical model or not. When a file has been deleted locally, or when a file has been created on one of the remote sides, the local side is not sufficient to compute a full model. ModelProviders will need access to the remote file data to take these locally unknown files into account. Add an interface to allow ModelProviders to be ignored in synchronizations has been added. Change-Id: I956dfb13093781accca36142cd87cf01b479fe1a Also-by: Laurent Goubet <> Also-by: Axel Richard <> Also-by: Alexandra Buzila <> Signed-off-by: Laurent Delaigue <> Signed-off-by: Matthias Sohn <>
2016-08-20Test stability: clean up after config testThomas Wolf1-3/+19
Several problems with this test: * It used the real user config. * It didn't clean up properly after tests, so that the user config contained left-overs from the test and accumulated more and more entries for the "testsection.testname" subsection with each run. Fix by cleaning once before all tests, and then after each test. Also remove the "testsection.testname" test section. And finally, do not use the real user config but a temporary file instead. Change-Id: I1b32da0cfcc109d0961073026416de163c70d7af Signed-off-by: Thomas Wolf <>
2016-08-15Test stability: eliminate one arbitrarily timed waitThomas Wolf1-4/+9
testPushToOrigin() sometimes failed with a WidgetDisposed exception. Apparently there is some refresh of the viewer occurring while the test is trying to expand a node hierarchy. It's not entirely clear why this happens here, nor why the test was waiting for jobs for at most 5 seconds. Let's try instead opening the view before renaming the branch. Renaming the branch then should indeed trigger a viewer refresh, which we can explicitly wait for. Change-Id: I5c0ee9f3be81d8ed06be13747d5d888cfa4b1037 Signed-off-by: Thomas Wolf <>
2016-08-15Auto-stage selected files on "Commit..." when staging view is usedThomas Wolf3-26/+121
* New preference UiPreferences.AUTO_STAGE_ON_COMMIT for auto-staging when the staging view is used. Default is true. * Change some private operations from CommitUI to public static utility operations, so they can be re-used. * Add auto-staging functionality in CommitActionHandler Auto-staging honors the UiPreferences.COMMIT_DIALOG_INCLUDE_UNTRACKED setting. Staging occurs in a background job. Bug: 497574 Change-Id: I5273400b602e1e4681fe0be6ed34b8fefef795a5 Signed-off-by: Thomas Wolf <>
2016-08-05Test stability: wait for push job and open result dialogThomas Wolf1-1/+4
Ensure that the dialog is always open, then close it. Just waiting for the job might leave an open dialog around, which might confuse SWTbot later on. Change-Id: Idc2c4569f927c8f66940deaddd20664f79450b28 Signed-off-by: Thomas Wolf <>
2016-07-28Fix some git actions visible in Team menu on unshared projectThomas Wolf1-1/+23
ResourceStatePropertyTester must always return false for unshared projects. Change-Id: I3a707159da561535405901a7cbfb4d4de8f6857e Signed-off-by: Thomas Wolf <>
2016-07-22Open the smart import wizard if available.tbaumann2dj1-53/+112
Bug: 494269 Change-Id: Ie221dcc8526f528e6af964c44a2a4e3135f81d95 Signed-off-by: Tobias Baumann <>
2016-07-15Test stability: force display of result dialog of background jobsThomas Wolf6-51/+32
Change-Id: Ia930bf0c9b391f6fd41869693166ed211ce1c2e1 Signed-off-by: Thomas Wolf <>
2016-07-13Try to recover after deleting opened files on NFSAndrey Loskutov1-0/+10
Fixes various LocalRepositoryTestCase based tests, which were failing if executed on NFS mount, see Bug 440182. The current workaround doesn't really checks for NFS (have no idea how to do this in Java) but allows tests running on "/home" to run "delete" cleanup twice (second "delete" round deletes unexpected nfs zombie files). The assumption is that this workaround should at least help *nix users with their NFS mounted home directories. Ideally the tests should be fixed to properly cleanup opened .pack file handles, but I've spent one day hunting the root cause on one single test without success, due the asynchronous nature of the resources creation/cleanup. Bug: 440182 Change-Id: I5e879daed954aa35de197d8e1ea43c9fbccca7d4 Signed-off-by: Andrey Loskutov <>
2016-07-02Refactor BranchOperationUIThomas Wolf1-4/+2
This class packed too many things into one, resulting in hard to understand flow of control. Also, a class with different "modes" is anathema to object-orientedness and to separation of concerns. Eliminate the different modes from BranchOperationUI: they all just defined which dialog to open initially, if any. Create those dialogs instead in the few places when the static factory methods were used, then use the BranchOperationUI to do the checkout, if appropriate. Simplify RenameBranchDialog by re-using the BranchRenameDialog :-) and by not checking for tags, which are not displayed anyway. Make both the asynchronous (start()) and the synchronous (run()) checkout operations perform the same actions, including project restoration. Factor out the checking for running launches so that it can be re-used elsewhere. Change-Id: If83e7960253d99b04e4a476329647bd009ec8898 Signed-off-by: Thomas Wolf <>
2016-07-02Avoid opening dialogs from push/fetch background jobsThomas Wolf4-31/+88
Doing so quite frequently led to UI deadlocks when modal dialogs were active when the job's result dialog popped up. Instead use the progress framework's mechanisms to give the user a possibility to open a job's result dialog at his leisure. Thus there is no danger of conflicting with any modal shells. Push and fetch UI tests adapted to the new behavior. Bug: 495512 Change-Id: I10f8df000fdfc4c91f3d5a9b75354caf2a37d876 Signed-off-by: Thomas Wolf <>
2016-06-12Test stability: try to fix three frequently failing testsThomas Wolf3-11/+38
* Wait for shells to close after having clicked finish button * Wait for add to/remove from index jobs * Open global git config preference page differently, and move opening the page into the test methods to get at least screenshots in case it fails. Change-Id: Iea78d6053fc5b554ce1b25eacfd29bdb0b473007 Signed-off-by: Thomas Wolf <>
2016-05-31Enable copying source branch in "Create Branch" dialog to clipboardMatthias Sohn1-2/+2
Bug: 494943 Change-Id: Ib7db01442c1e6ead995b3c363f43a1512f38611b Signed-off-by: Matthias Sohn <>
2016-05-30Merge "Test stability: wait for specific child node after expansion"Thomas Wolf2-3/+45
2016-05-30Test stability: wait for specific child node after expansionThomas Wolf2-3/+45
The first test in GlobalConfigurationPageTest frequently fails on Hudson because it doesn't find the child node "Configuration". It appears that child nodes in the preference page's tree are added one after another, so waiting until there is any child is not good enough: "Configuration" is the second child and might not have appeared yet. Change-Id: Ic5690b481b111fe98aa6d8da1108d5ee462c6e1a Signed-off-by: Thomas Wolf <>
2016-05-30Test stability: wait for add to index/remove from index jobsThomas Wolf1-0/+6
Staging tests need not only wait for an index diff update but also and first for the add to index or remove from index jobs. This should resolve spurious test failures in StageUnstageActionTest. Change-Id: I20efada649b0d7c92701ded7c2a471a17d600ec8 Signed-off-by: Thomas Wolf <>
2016-05-29Test stability: wait for index diff cache updateThomas Wolf1-0/+3
Apparently the DisconnectConnectTest.testDecorations() not always waits long enough for the decorations to appear. Decorations are triggered by an index diff cache update, so let's wait for that first, and only then for the decoration job. Change-Id: I5b32299006a99f3332b49e70d1fcdbfa375d362f Signed-off-by: Thomas Wolf <>
2016-05-25Test stability: after clicking finish, wait for shell to close.Thomas Wolf1-5/+14
Eliminate Thread.sleep() calls, which is a sure recipe for unstable tests. Instead wait for the shell to close, and since we then test properties of workspace resources, refresh the workspace first. Change-Id: I96fa669d655bd0118fb49e599cd3538bb555bc71 Signed-off-by: Thomas Wolf <>
2016-05-22Replace uses of deprecated Repository.getRef()Thomas Wolf10-13/+14
Use Repository.findRef() or Repository.exactRef(), as appropriate. No functional change. Change-Id: Ie481c7fa5ac69d4778cf9f70ac2b019bee6e53c9 Signed-off-by: Thomas Wolf <>
2016-05-09Set repository when opening staging view from commit actionThomas Wolf1-0/+105
Otherwise the view will come up with "No repository selected" when "link with selection" is disabled. Bug: 491907 Change-Id: Ia4c2db8201c7f2c038fb873ca78366d7d1feb67f Signed-off-by: Thomas Wolf <>
2016-05-07Test stability: one "widget disposed" message is enoughThomas Wolf3-1/+12
If a widget is disposed while we're waiting on its expansion, we would get umpteen log entries until the waiting timed out. Let's stop waiting instead, and then fail the test. Also, it appears that GitRepositoriesViewTest.testImportWizard() gets a repo view refresh at an inopportune moment. I don't quite see yet where this is coming from, but let's wait for refresh jobs after having closed the dialog the first time. Finally, FeatureFinishSquashHandlerTest expects a dialog to be opened and wants to enter text there, but didn't wait for that dialog to appear: another source of spurious test failures. Change-Id: I14f365d86dda032dbaabd593e95a7bec432c0e21 Signed-off-by: Thomas Wolf <>
2016-05-05Test stability: really shut down repositoriesThomas Wolf2-0/+2
Otherwise, repositories from previous tests may remain registered, and may at some later point found to have gone and a new refresh of the repo view may be scheduled. As a result, we may have asynchronous refreshes that occur during SWT bot tests, and tree nodes disappear at unpredictable moments. Also: in commitNonWSChangesTest, repo view is opened after a commit and thus must also wait until the repo view refresh is over before trying to find tree nodes. Change-Id: I43ff9bee19fb4983cd44486a4085640f2dac0497 Signed-off-by: Thomas Wolf <>
2016-05-04Test stability: wait for repositories view refresh after config changeThomas Wolf1-0/+1
The repositories view updates itself on a config change event. The test must wait until that refresh is over, otherwise tree nodes may disappear while we're waiting for them to be expanded, resulting in "org.eclipse.swt.SWTException: Widget is disposed". Change-Id: I0a72897bcb919b0da2bc81e124cc8c0c2cf9a171 Signed-off-by: Thomas Wolf <>
2016-05-03Test stability: after clicking finish, wait for shell to close.Thomas Wolf1-0/+2
The failure in build 8454 was apparently caused by the test switching to the repositories view before the create branch dialog had closed. Change-Id: Ic2e871f5a9da81dd68d9c43758f99118f57428d1 Signed-off-by: Thomas Wolf <>
2016-05-03Test stability: avoid asynchronous SWTBotTreeItem.expand()Thomas Wolf23-220/+287
Sometimes tests do not find children of expanded tree nodes. The root cause appears to be that expand() expands the node in an asyncExec.[1] Add a utility operation that after expanding waits until the desired node exists, and replace all calls to expand() that do expect children by that. [1] Change-Id: I23ea6a7bc519692c0e9dbe480bb2eb8dfb96adc1 Signed-off-by: Thomas Wolf <>
2016-04-03When running "Commit" action open staging view instead of commit dialogMatthias Sohn5-0/+35
Add a preference to allow switching back to the old behavior of the commit action to open the commit dialog. By default it now opens the staging view. After opening the staging view auto-select and set focus on unstaged files if there are any, otherwise set focus on the commit message. The commit dialog is still used by CleanupUncomittedChangesDialog. Bug: 490121 Change-Id: I604f31268f83ab11c3f3a869edf3c7121f0e6478 Signed-off-by: Matthias Sohn <>
2016-03-22Test stability: fix FeatureFinishSquashTestThomas Wolf1-19/+5
The test didn't wait for the shell to open in AbstractFeatureFinishHandlerTest.finishFeature(). Unfortunately, that shell didn't have a title, and was thus a bit inconvenient to wait for. * Give that shell (a TitleAreaDialog) a title. TitleAreaDialog.setTitle() does not set the shell's title! * Simplify the code: use the async clickContextMenu instead of the sync variant wrapped in an asyncExec. Change-Id: I5e26490682821888b4f62da8a323cbc1aeeaded4 Signed-off-by: Thomas Wolf <>
2016-03-21Remove session properties when disconnecting a projectThomas Wolf1-0/+20
Disconnecting a project left the session properties for the RepositoryMappings still attached to the resource info in the Eclipse resource tree. This prevents timely garbage collection of no longer used repository instances because the mappings contain hard references to repositories. Moreover, potentially stale properties might become active again when the project is re-connected. Change-Id: I845c708d3d98ba285c8db94edd2ed14deebd29fb Signed-off-by: Thomas Wolf <>
2016-03-20Fix refresh after re-connecting a project with submodules insideThomas Wolf1-5/+5
Follow-up to commit a8bcee9. When a previously connected, then disconnected project is re-connected, there is no resource change event. Thus the GitProjectData doesn't pick up inner repositories, and doesn't re-set the RepositoryMappings for folders inside the project that contain submodule or nested repository working trees. Because of another bug that I plan to fix soon (disconnecting a project does not remove the RepositoryMapping session properties), this problem is currently only visible if one quits and restarts Eclipse after having disconnected. (Otherwise, the previous session properties are still set, and everything appears fine.) Thus we need to make sure when a project is shared that we get at least a resource delta including all .git files and folders in the hierarchy. Ensure that by touching all such inner resources. Also, the SharingWizard must not suppress the refresh. Bug: 489696 Change-Id: I1bce767f54bffced241d2a0ad3c7ae8230152199 Signed-off-by: Thomas Wolf <>
2016-03-15Refresh decorations after re-connecting a projectThomas Wolf4-19/+47
Re-connecting a previously connected, then disconnected project did not refresh the decorations in the project explorer. RepositoryChangeListener was unused, and likewise GitProjectData.addRepositoryChangeListener(). Therefore, all calls to RepositoryMapping.fireRepositoryChanged() had absolutely no effect. Thus I have removed all these calls and the method. Interesting bit of EGit history: the very first version (even before EGit became an Eclipse project) of GitProjectData contained what is now known as the RepositoryCache (the one in EGit). The RepositoryChangeListener indeed was notified on changes in a repository. In that original commit, there was exactly one such listener: in the git decorator. Through various refactorings, RepositoryCache was extracted from GitProjectData, and then IndexDiffChangedListener appeared. RepositoryChangeListener became unused; GitLightweightDecorator was changed to listen on index diff changes in commit f332331. Nowadays, this RepositoryChangeListener is notified not on repository changes, but whenever a new RepositoryMapping is added to the Eclipse resource tree. And that is exactly what is needed to fix bug 489696: when a previously connected, now disconnected project is re-connected, there will be no resource change events (the project is known in Eclipse's resource tree already, and adding new RepositoryMappings as session properties doesn't trigger a resource delta). There also will be no repository or index diff related events (provided the repository is still known to EGit, for instance because it is in the Repositories view, or because there are other projects from that repository.) So the GitLightweightDecorator will not refresh decorations. Using a RepositoryChangeListener (again, after 5 years) the GitLightweightDecorator can correctly refresh the project explorer in this case. Since this listener is no longer invoked when a repository changes, but when a new RepositoryMapping appears, I have renamed and re-purposed the interface to RepositoryMappingChangeListener. Bug: 489696 Change-Id: I2b59cea1f1500cbdde554fff28b676456c8462d8 Signed-off-by: Thomas Wolf <>
2016-03-14Support copy/move of workspace if Git repository is under workspaceMatthias Sohn1-2/+4
If git repositories are located under the workspace path moving or copying the workspace broke the repository path information in .metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.egit.core.prefs since all repository paths were stored as absolute paths and paths of repositories located under the moved or copied workspace weren't matching this persisted paths anymore. Fix this by storing the repository path relative to the workspace root if a repository path is located under the workspace root. For repositories not located under the workspace root still store the absolute path since otherwise their path would break if the workspace is moved or copied. Store this in a new preference and still maintain the old preference with list of absolute repository paths in order to ensure compatibility with older EGit versions. Bug: 358285 Change-Id: Ib73b76eb1d63587a767fec59c076fbe51c75e2f1 Signed-off-by: Matthias Sohn <>
2016-03-13RepositoryCache: do not prematurely remove submodulesThomas Wolf4-12/+228
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 , 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. 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 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 <>
2016-03-02Test stability: GitRepositoriesViewRepoDeletionTestThomas Wolf1-6/+3
Must click the menu synchronously. Change-Id: I5dda8ddd48387670c36e12bd6057ac415cef2cc2 Signed-off-by: Thomas Wolf <>
2016-02-29Decorate IFolders that are submodule working directory rootsThomas Wolf1-0/+18
Introduce a new configurable submodule text decoration used for folders that are submodule working directory roots. (Or working directory roots of nested repositories.) The default shows the a dirty indicator, the branch, branch state, and the head commit's short message. The repository name is omitted since it'll always be identical to the folder name. Added a new variable for the short message of the head commit. Change-Id: I75460fd59f2e6c9bd06e82a1966e3d06f97ab709 Signed-off-by: Thomas Wolf <>

Back to the top