Bugzilla – Full Text Bug Listing
| Summary: | isSynchronized ignores subfolders created automatically when refreshing a file | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Igor Fedorenko <igor> | ||||
| Component: | Resources | Assignee: | Szymon Ptaszkiewicz <sptaszkiewicz> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, sptaszkiewicz | ||||
| Version: | 3.7.1 | ||||||
| Target Milestone: | 4.4 M6 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
I came across org.eclipse.m2e.core.internal.builder.MavenBuilderImpl.workaroundBug368376(IResource, IProgressMonitor) while investigating bug 417735 and bug 428180 which are both connected to some bad interaction that involves m2e and SVN. I haven't looked at your unit test yet, but when I saw the implementation of MavenBuilderImpl.workaroundBug368376(..) the first thing that I thought was why don't you simply call refreshLocal on the project? Or find folders with the shortest path and call refreshLocal on them? Project refresh will result in a lot of unnecessary work (think of projects with 50000+ resources, for example, you really want to limit refresh to just what is necessary).
I am not sure what you mean by "folders with the shortest path". Let me illustrate what m2e does with little ascii diagram
project
|-- folder1
|-- folder2
|-- folder2-1
| |-- newFile
|-- folder2-2
|-- existingFile
m2e knows newFile was created on filesystem but does not know if folder2 or folder2-1 are known in the workspace. So m2e refreshes newFile first, then walks up folder hierarchy and refreshes all parent folders that are out of sync with DEPTH_ONE. This way refresh does not need to look at existingFile.
As a side note, m2e provides "Maven Workspace Build" view that helps troubleshooting endless workspace builds. When enabled, the view shows resource deltas that trigger invocation of MavenBuilder and also about resources changed through m2e APIs. Although not perfect, it often gives enough clues to help understand the problem.
I reviewed the attached unit test and it indeed shows a bug. If a folder is refreshed with DEPTH_ZERO, we are not aware of all its children. Since children are unknown, when we call isSynchronized(DEPTH_INFINITE) on that folder, it tries to add children from underlying file system assuming they do not exist in the workspace. But some of the children may already exist because they could have been automatically created during refreshLocal on a file nested deeper in the hierarchy (see javadoc of refreshLocal). Therefore, having no knowledge about all children doesn't mean that some of those children do not exist in workspace. Interestingly, assertion in line 50 of the unit test is not valid, because automatically created folders are in-sync with DEPTH_ZERO, and since there are no other folders, the whole project subtree is already in-sync. The return value from isSynchronized in that line should be true, but it is false because of the same bug that appears in line 67. Fixed in master: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=8a159d9afc1b7b982c072437a569730d24658c97 The new test fails on all platforms, e.g. http://download.eclipse.org/eclipse/downloads/drops4/I20140302-2000/testresults/html/org.eclipse.core.tests.resources_linux.gtk.x86_6.0.html (In reply to Dani Megert from comment #5) > The new test fails on all platforms, e.g. > > http://download.eclipse.org/eclipse/downloads/drops4/I20140302-2000/ > testresults/html/org.eclipse.core.tests.resources_linux.gtk.x86_6.0.html Yes, I'm already investigating. It failed also in all N-builds after Feb 28. The strange thing is that it fails in pre-test sanity assertion while verifying that PREF_LIGHTWEIGHT_AUTO_REFRESH is false in the default scope, but there is nothing that sets this property to true in our code. Are you aware of any pluginCustomization file used during test execution? I'm downloading the whole zip with tests to check what is inside, still about ~1,5h to go. (In reply to Szymon Ptaszkiewicz from comment #6) > Are you > aware of any pluginCustomization file used during test execution? I'm > downloading the whole zip with tests to check what is inside, still about > ~1,5h to go. Yes, our product (SDK) sets this to true. (In reply to Dani Megert from comment #7) > (In reply to Szymon Ptaszkiewicz from comment #6) > > Are you > > aware of any pluginCustomization file used during test execution? I'm > > downloading the whole zip with tests to check what is inside, still about > > ~1,5h to go. > > Yes, our product (SDK) sets this to true. I know about SDK, but this shouldn't affect tests execution, right? Or SDK is used to run tests and thus the pluginCustomization file from SDK affects the tests? Failing test is fixed in master: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=9718b22d44f53a33b9bf1d0622723ab9106e5b5a Verified in I20140303-2000. Can you please clarify expected behaviour with this bug fixed? Using the diagram from comment #2, is it sufficient for m2e to refresh folder2/folder2-1/newFile only or it still needs to walk up folder hierarchy and refresh parent folders too? If m2e does need to walk up folder hierarchy, what refresh depth should it use for folders? Thank you in advance. (In reply to Igor Fedorenko from comment #11) > Can you please clarify expected behaviour with this bug fixed? Using the > diagram from comment #2, is it sufficient for m2e to refresh > folder2/folder2-1/newFile only or it still needs to walk up folder hierarchy > and refresh parent folders too? If m2e does need to walk up folder > hierarchy, what refresh depth should it use for folders? Thank you in > advance. It should be sufficient to refresh folder2/folder2-1/newFile only. This should make all parent folders (folder2 and folder2/folder2-1) synchronized with DEPTH_ZERO. |
Created attachment 209329 [details] unit test that demonstrates the problem I have an eclipse plugin, that creates a file on filesystem nested under several levels of new directories. refreshLocal just the file does not brings enclosing workspace folders in sync with local filesystem, which is inconvenient, but probably expected. refreshLocal(IResource.DEPTH_ZERO) of the file and all enclosing folders still leaves some resources out of sync with local filesystem, which I believe is a bug. refreshLocal(IResource.DEPTH_ONE) of the file and all enclosing folders does bring everything in sync, but potentially refreshes more resources than needed. Attached unit test demonstrates the problem and was tested with Eclipse 3.7.1 and 4.2M4 on macosx x86_64 and 3.7.1 on Ubuntu linux x86.