Community
Participate
Working Groups
Created attachment 98169 [details] Patch plugins implementing this feature request. For the C/C++ Tool, a better support for a flexible Project Structure is a commonly raised limitation of the current support for linked resources in the 3.3/3.4 Eclipse Platform. The support for linked resources should be improved so that: - The user can create virtual folders (i.e. "groups") that are by designed not available on the file system. Under such groups, only other groups or linked resources can be created. - The user needs to be able to create linked resources who's target is relative to a variable defined in the project itself. Currently, this is only supported at the workspace level, so it limits the usefulness of such feature. - There needs to be a away to programmatically extend the list of default variables used for linked resources path resolution so that products can specify some variables based on C/C++ source trees. - The user needs to have better support for the creation of linked resources, including drag and drop. The attached patch plugins contain an implementation (and junit tests) of those features, and fixes several existing bugs related to the deletion and moving of linked resources between projects. To use the patch plugins, one has to use the 3.3 Eclipse Platform and, for each plugin requiring to be patched, add the freescale-patch.jar at the begining of the bundle class path, along with setting the flag "Eclipse-ExtensibleAPI" to true, for example: Bundle-ClassPath: freescale-patch.jar,. Eclipse-ExtensibleAPI: true The sources are included in the patch plugins.
Thanks Serge. There is a discussion on platform-core-dev mailing list about resource model shape. I think that some parts could be used now, like more user-friendly way of creating linked resources.
Yes, thanks, I've been following the discussion on the platform-core-dev mailing list.
Created attachment 98403 [details] Additional support for Flexible Project Model (tool, property page, etc...)
The user requirements that the attached plugins implement are the following: - To be able to easily specify a project file structure that has an arbitrary hierarchy independent of the real files and directories on the file system. - To be able to use the flexible file structure transparently for all Eclipse feature set (CDT Managed builds, Team support, debugging, etc...) as well as for normal project file structures. - To be able to put Eclipse projects having a flexible file structure in a source control repository, and be retrieved on a different computer in a different workspace and having to make the minimal amount of change to be able to fully use the project in Eclipse (i.e. Not relying on absolute path specifiers). - To be able to easily build the flexible project file structure both with drag and drop and explicitly adding files to the project through a file/folder selection dialog. - To have a feature set rich enough to be able to write a plugin to import C/C++ Visual Studio projects in Eclipse and be able to build and use as a normal Eclipse project. - To be able to easily change the project hierarchy structure once created, to be able to change where one project resource points to a local file, and easily correct errors in the local resolution of the flexible project structure. - To be as backward and forward compatible as possible with the existing Eclipse IDE architecture.
Hi Serge, I've just finished porting your fragment patches to 3.4, and had a number of thoughts / questions: 1) Relative paths, which are arguably one of the most important features, doesn't appear to be supported. So you can't, for example, link to: PROJECT_LOC/../some_other_directory/ 2) I'm not entirely convinced that there needs to be additional API on IProject to getPathVariableManager(). You could imagine instead that IPathVariableMangager had additional methods getValue(String, IResource) & resolvePath(IPath, IResource) which resolve a variable / path in the context of a resource. This would allow linked resources to exist relative to any IResource in the workspace (or the IResource's location, for that matter). 3) The fragments contain changes for both the enhanced Linked resources, as well as 'groups'. The latter seems to add a whole extra layer of complexity - perhaps that platform team would be more keen if the two features were broken into independent parts? More trivial: 4) 3 insntances of static private URI createRelativePath(IPathVariableManager pvm, URI locationURI, String relativeVariable) { which to my mind doens't create a relative path :) 5) You formatted the Java code so that diff'ing against the platform is very difficult indeed... ;) Cheers, James
Created attachment 111561 [details] New version of the Core Resources additions plugins, including the PARENT variable New version of the Core Resources additions plugins, including the PARENT variable, to support relative paths.
Hi, thanks for your reply. See my comments below: 1) Relative paths, which are arguably one of the most important features, doesn't appear to be supported. So you can't, for example, link to: PROJECT_LOC/../some_other_directory/ Serge: You are right, in the source provided there was no support for relative paths. The basic problem is that it is not possible to carry parent relative characters ("../") in an IPath object, since they get striped out automatically. This is a major problem, since it would require significant API change and break a whole lot of code, since linked resources value are set by an IPath (or URL). Instead, what we did is add another path variable provider implementing a "PARENT" path variables that allows the user to iterate the parent of a path. For instance, if the user wants to have linked resource pointing to "PROJECT_LOC/../some_other_directory/foo.txt", he can create a path variable "SOME_OTHER_DIR" that contains the following value "${PARENT-1-PROJECT_LOC}/some_other_directory" (the "1" refers to how many parent directory have to be computed) then have the linked resource value be set to "SOME_OTHER_DIR/foo.txt". That may seem overly complicated for the user to setup, but this is generated programatically in our case when the user drag and drops files in the project from Windows Explorer and choose towards which path variable they should be relative to (PROJECT_LOC in the case above). The Linked Resource UI could be updated to hide the syntax "${PARENT-COUNT-VARIABLE}" to the user and simply transform them on the fly to "../", although we haven't implemented that yet. 2) I'm not entirely convinced that there needs to be additional API on IProject to getPathVariableManager(). You could imagine instead that IPathVariableMangager had additional methods getValue(String, IResource) & resolvePath(IPath, IResource) which resolve a variable / path in the context of a resource. This would allow linked resources to exist relative to any IResource in the workspace (or the IResource's location, for that matter). Serge: I think the idea is interesting, although it would require bigger API and source change, where now it only requires the initialization of the IPathVariableManager to be retrieved from the project instead of the workspace, and methods taking a IPathVariableManager passed to them don't have to change, as opposed to change each single call to getValue(String, IResource). Also, it presuppose that path variables can exist at the resource level, which seems to be a unlikely considering the burden of managing Path Variables for the user, but interesting nonetheless. 3) The fragments contain changes for both the enhanced Linked resources, as well as 'groups'. The latter seems to add a whole extra layer of complexity - perhaps that platform team would be more keen if the two features were broken into independent parts? Good point, I'll consider splitting them up. More trivial: 4) 3 insntances of static private URI createRelativePath(IPathVariableManager pvm, URI locationURI, String relativeVariable) { which to my mind doens't create a relative path :) I'll attach an updated plugin that actually transform the path to a relative path (only when explicitly requested by the user). 5) You formatted the Java code so that diff'ing against the platform is very difficult indeed... ;) Serge: Sorry about that, I don't know how it came about, I have the same struggle since we're starting to move to 3.4.
(In reply to comment #7) > Hi, thanks for your reply. See my comments below: > > > Instead, what we did is add another path variable provider implementing a > "PARENT" path variables that allows the user to iterate the parent of a path. Magic, very ingenious :) > Serge: I think the idea is interesting, although it would require bigger API > and source change, where now it only requires the initialization of the > IPathVariableManager to be retrieved from the project instead of the workspace, > and methods taking a IPathVariableManager passed to them don't have to change, > as opposed to change each single call to getValue(String, IResource). > > Also, it presuppose that path variables can exist at the resource level, which > seems to be a unlikely considering the burden of managing Path Variables for > the user, but interesting nonetheless. Well I was thinking that it seems wrong that the users of the API have to find the IResource's project, querying that variable manager (if the project exists) or querying the workspace variable manager. Instead it would be nice if the paltform just did the right thing for you when you asked for a variable to be resolved. How about a 'Workspace.getPathVariableManager(IResource)'. All the places which want the more specialised ProjectPathVariableManager need to be altered anyway. The workspace can then encapsulate the current logic - and clients can be upgraded to support the new functionality as and when it's needed. > 5) You formatted the Java code so that diff'ing against the platform is very > difficult indeed... ;) > > Serge: Sorry about that, I don't know how it came about, I have the same > struggle since we're starting to move to 3.4. On this front I think I've cleanly merged your changes into 3.4 -- it certainly diffs cleanly. I'll upload the fragments I've got in case this is any help to you.
Created attachment 111564 [details] Fragments updated for 3.4 & CDT 5 Attached are the following plugin fragments based on Serge's work ported Eclipse 3.4 & CDT 5: org.eclipse.cdt.ui.patch.tgz org.eclipse.core.resources.patch.tgz org.eclipse.core.tests.resources.patch.tgz org.eclipse.ui.ide.patch.tgz They're provided as actual Eclipse projects that can be imported into your Workspace. NB in the ui plugins, support for creating groups is commented out so the user can't create them. Though this is a trivial change. One JUnit test fails.
Hi James and Serge. Could you create a patch against org.eclipse.core.resources and org.eclipse.core.tests.resources? (In reply to comment #9) > Created an attachment (id=111564) [details] > Fragments updated for 3.4 & CDT 5 > > Attached are the following plugin fragments based on Serge's work ported > Eclipse 3.4 & CDT 5: > org.eclipse.cdt.ui.patch.tgz > org.eclipse.core.resources.patch.tgz > org.eclipse.core.tests.resources.patch.tgz > org.eclipse.ui.ide.patch.tgz > > They're provided as actual Eclipse projects that can be imported into your > Workspace. > > NB in the ui plugins, support for creating groups is commented out so the user > can't create them. Though this is a trivial change. One JUnit test fails. > Why don't create patches for org.eclipse.core.resources and org.eclipse.ui.ide etc?
(In reply to comment #10) > Why don't create patches for org.eclipse.core.resources and org.eclipse.ui.ide > etc? Will aim to do so. The main issue is that currently the fragment wraps up both features (extended links and groups) into a largeish delta. I'm not sure this is ideal for discussion / feedback from you, though maybe it's ok? Will aim to produce patches for the two features individually (if Serge doesn't get there first) when I get a few free cycles over the next couple of weeks. Cheers, James
Created attachment 111596 [details] Updated Fragments updated for 3.4 & CDT 5, including convertToRelative()
(In reply to comment #8) > (In reply to comment #7) > Well I was thinking that it seems wrong that the users of the API have to find > the IResource's project, querying that variable manager (if the project exists) > or querying the workspace variable manager. Instead it would be nice if the > paltform just did the right thing for you when you asked for a variable to be > resolved. > > How about a 'Workspace.getPathVariableManager(IResource)'. All the places > which want the more specialised ProjectPathVariableManager need to be altered > anyway. The workspace can then encapsulate the current logic - and clients can > be upgraded to support the new functionality as and when it's needed. > That sounds good to me. You are welcome to make the changes and we'll use them in our sources. With the new archive I attached, the following is added: 1) The PARENT variable is defined at the core.resource plugin level. 2) A new method IPathVariableManager.convertToRelative() is available (along with proper junit tests) that allows converting an absolute path into a path variable relative path automatically, as well as intermediate generation of PARENT variables. 3) The 3 implementations of createRelativePath() now use properly the IPathVariableManager.convertToRelative(). I'll wrap those changes in a patch shortly, although it will take some time to divide up the Independent path from the Group feature, since the drag and drop UI integrates both in one dialog.
Created attachment 111692 [details] CVS patch files for the independent path and group features. The archive contains the following files: org.eclipse.ui.ide.patch org.eclipse.cdt.ui.patch org.eclipse.core.resources.patch org.eclipse.core.tests.resources.patch and the two directories containing additional binary resources (icons): org.eclipse.cdt.ui org.eclipse.ui.ide The patch is against the HEAD of the eclipse CVS repository as of September 4th 2008. I will divide up the two features ("Independent path" and "group") latter, but this diff contains both.
Created attachment 112305 [details] CVS Patch file for the independent path feature only
I posted a blog entry about this feature a the following link: http://sergebeauchamp.blogspot.com/2008/09/improving-linked-resources-in-gandymede.html
Hi Serge, (In reply to comment #15) > Created an attachment (id=112305) [details] > CVS Patch file for the independent path feature only The patches look good :). One thing which might be a barrier to getting this into the platform (or maybe something to think about for e4) is the proliferation of environment, build, and path variables in the platform. Currently we have core.variables, cdt.core variables and core.resources variables. I think users have a tough time in knowing when they are allowed to use each kind of variable. John Arthorne made a comment in bug122945 comment 8 that it would be good to use the core.variables from core.resources. core variables support arguments too so ${workspace_loc:some_project/foo} works even if some_project is directly nested within the workspace (though using ${resource_loc:../../} doesn't appear to provide upwards relative paths...). Perhaps this is scope for another bug, but this all seems interconnected and I wonder how hard it would be to extend core.variables so that it supports the path variables use case as well as the cdt's build variables.
(In reply to comment #17) > Hi Serge, > The patches look good :). One thing which might be a barrier to getting this > into the platform (or maybe something to think about for e4) is the > proliferation of environment, build, and path variables in the platform. > > Currently we have core.variables, cdt.core variables and core.resources > variables. I think users have a tough time in knowing when they are allowed to > use each kind of variable. > > John Arthorne made a comment in bug122945 comment 8 that it would be good to > use the core.variables from core.resources. core variables support arguments > too so ${workspace_loc:some_project/foo} works even if some_project is directly > nested within the workspace (though using ${resource_loc:../../} doesn't > appear to provide upwards relative paths...). > > Perhaps this is scope for another bug, but this all seems interconnected and I > wonder how hard it would be to extend core.variables so that it supports the > path variables use case as well as the cdt's build variables. > > I think using the core.variables API instead of the core.resources.variableProviders is a great idea, and we were looking forward to do that while working on this project until we stumbled on the limitations of the core.variables API. 1) The first major limitation (which is also a requirement for the cdt variables framework) is to be able to pass a context when resolving a variable. In the case of the PathVariableManager, the context is the IProject. In the CDT, the context can be different things, including a Build Configuration, a File, or other contexes. Without a context, it is simply impossible to resolve some of the most useful variables (such as PROJECT_LOC). 2) Another problem is that in the core.resources API, the variable values are passed in the API as IPath and URL objects, and some of the core.variable syntax is invalid to build a IPath with. For example, the PathVariableManager uses the syntax "${foo-arg}" instead of "${foo:arg}"because ":" isn't stored properly. This might be worked out by dynamically translating the format but might still be problematic. 3)Another limitation is that ability for the variableProviders to supply a extension list, so the use can know through the UI which arguments are valid if the variable takes argument. Another thing that might be needed is the ability to resolve variables according to a namspace, or scope, to avoid name conflicts. For those reasons, we didn't want to start changing the core.variables plugin in any major way, and restrict our changes so API compatibility is as high as possible.
I forgot, the CDT also has types for the variables, so for instance one variable can be a directory, while another is a file, and another is a generic string. For the core.resources variableProviders, all variables are assume to be resolvable to a directory. While in the core.variables API, no types are provided at all. (In reply to comment #18) > (In reply to comment #17) > > Hi Serge, > > > The patches look good :). One thing which might be a barrier to getting this > > into the platform (or maybe something to think about for e4) is the > > proliferation of environment, build, and path variables in the platform. > > > > Currently we have core.variables, cdt.core variables and core.resources > > variables. I think users have a tough time in knowing when they are allowed to > > use each kind of variable. > > > > John Arthorne made a comment in bug122945 comment 8 that it would be good to > > use the core.variables from core.resources. core variables support arguments > > too so ${workspace_loc:some_project/foo} works even if some_project is directly > > nested within the workspace (though using ${resource_loc:../../} doesn't > > appear to provide upwards relative paths...). > > > > Perhaps this is scope for another bug, but this all seems interconnected and I > > wonder how hard it would be to extend core.variables so that it supports the > > path variables use case as well as the cdt's build variables. > > > > > > I think using the core.variables API instead of the > core.resources.variableProviders is a great idea, and we were looking forward > to do that while working on this project until we stumbled on the limitations > of the core.variables API. > > 1) The first major limitation (which is also a requirement for the cdt > variables framework) is to be able to pass a context when resolving a variable. > > In the case of the PathVariableManager, the context is the IProject. In the > CDT, the context can be different things, including a Build Configuration, a > File, or other contexes. > > Without a context, it is simply impossible to resolve some of the most useful > variables (such as PROJECT_LOC). > > 2) Another problem is that in the core.resources API, the variable values are > passed in the API as IPath and URL objects, and some of the core.variable > syntax is invalid to build a IPath with. For example, the PathVariableManager > uses the syntax "${foo-arg}" instead of "${foo:arg}"because ":" isn't stored > properly. This might be worked out by dynamically translating the format but > might still be problematic. > > 3)Another limitation is that ability for the variableProviders to supply a > extension list, so the use can know through the UI which arguments are valid if > the variable takes argument. > > Another thing that might be needed is the ability to resolve variables > according to a namspace, or scope, to avoid name conflicts. > > For those reasons, we didn't want to start changing the core.variables plugin > in any major way, and restrict our changes so API compatibility is as high as > possible. >
I think that one problem with using core.variables for Resource paths could be that core.variables are extendable. Activating the plugin(s) that contribute variable resolvers could be problematic in case those plugins depend on resources in turn -- you'd get some cyclic dependency. Another problem is that variables which can dynamically change their expansion based on some context make it very hard to track what actual resource node a path with a variable refers to. This makes it hard to compute resource deltas... or, imagine the case where path foo/${bar}/x.txt refers to resource FOO when a resource delta is computed but to a different resource BAR when a resource listener actually processes the delta. I'm not totally against using core.variables for resource path substitutions, but I guess that there would need to be some clear documentation what kinds of variables are allowed (likely only static ones, or with a context such as ${PRJ_ROOT} that cannot change), and what constraints the variable resolvers need to abide by (likely not depend on core.resources themselves). It may be hard to explain such restrictions on particular variables to extenders. Having a separate mechanism for resource path variables makes it at least easier to explain. Which doesn't necessarily mean it's the better solution.
Created attachment 115154 [details] Exception seen when opening project Sometimes we see this exception when opening a nested project (which uses linked resources to point at other resources in a container project). It seems to happen when: 1) The nested project is closed 2) A CVS update is performed on the container project 3) The nested project is reopened. In this case "/Proeject1/some/path/to/some_file" is the workspace path of the resource in the container project which is linked to from the nested project. After the exception the restore tree (as seen in Project Explorer) is empty. Also the .project file of the nested project has lost the linked resource on which the exception occurred. Furthermore the error doesn't always occur -- closing Eclipse, re-update the .project file and reopen Eclipse and the resources appear in the tree as expected. [ By Nested Proejct and container project I mean: Container project: /Proeject1 Nested Project: /nested_project (location overlaps with /Proeject1/some/path/to/nested_project) ]
Created attachment 115153 [details] Exception seen when opening project Sometimes we see this exception when opening a nested project (which uses linked resources to point at other resources in a container project). It seems to happen when: 1) The nested project is closed 2) A CVS update is performed on the container project 3) The nested project is reopened. In this case "/Proeject1/some/path/to/some_file" is the workspace path of the resource in the container project which is linked to from the nested project. After the exception the restore tree (as seen in Project Explorer) is empty. Also the .project file of the nested project has lost the linked resource on which the exception occurred. Furthermore the error doesn't always occur -- closing Eclipse, re-update the .project file and reopen Eclipse and the resources appear in the tree as expected. [ By Nested Proejct and container project I mean: Container project: /Proeject1 Nested Project: /nested_project (location overlaps with /Proeject1/some/path/to/nested_project) ]
(In reply to comment #22) > Created an attachment (id=115153) [details] > Exception seen when opening project Hmm. This looks like (another) issue with linked resource not being handled correctly by the alias manager(?). As I'm using nested projects in this case, the linked resource created in the nested project should appear in the container project too... This is probably related to bug246221 where find{Resources}ForLocation followed by getLocation() / isLinked() returns unpredictable results.
James, how are comments 21-23 related to this bug? Do you only see this problem when running with Serge's patch installed?
(In reply to comment #24) > James, how are comments 21-23 related to this bug? Do you only see this problem > when running with Serge's patch installed? Our Eclipse release does includes Serge's patches -- I thought there might be something odd happening in 'reconcileLinksAndGroups' which is different from the platform's reconcileLinks (or perhaps I had messed up the merge to 3.4...). On closer inspection it looks like the problem is a result of the overlapping projects (with linked resources between them) and not a direct consequence of the patches here. Will attempt to reproduce against a clean platform and if possible will attach another test to bug246221.
(In reply to comment #25) > (In reply to comment #24) > > James, how are comments 21-23 related to this bug? Do you only see this problem > > when running with Serge's patch installed? > > Our Eclipse release does includes Serge's patches -- I thought there might be > something odd happening in 'reconcileLinksAndGroups' which is different from > the platform's reconcileLinks (or perhaps I had messed up the merge to 3.4...). > > On closer inspection it looks like the problem is a result of the overlapping > projects (with linked resources between them) and not a direct consequence of > the patches here. Will attempt to reproduce against a clean platform and if > possible will attach another test to bug246221. > I'm looking at the issue and I'll go back to you about it shortly, but I know that having nested projects in Eclipse is not well supported with the stock 3.4, independently of my patch. For instance, if you attempt to delete both projects (the container and the nested) in the Package Explorer with the option to delete the project content on disk set, you will get an exception and be forced to abort or undo the operation .
I was investigating this bug, and I can't reproduce the problem. Can you attach a small sample of the two projects (nested and container) and give reproducible steps to get the Exception? It seems to work properly for me.
Created attachment 115486 [details] Workspace demonstrating the problem (In reply to comment #27) > I was investigating this bug, and I can't reproduce the problem. Can you > attach a small sample of the two projects (nested and container) and give > reproducible steps to get the Exception? Attached is a workspace which has the problem (or a closely related problem). I've been so far unable to create a unit test which exhibits the problem. The workspace has the outer project hello, and inner project foo. foo contains a symlink to bar in hello. There are three .project files in /hello/foo -- .project .project1 .project2 To reproduce: 1) Open the workspace in your Eclipse 2) Close the project foo 3) From a shell copy .project2/.project1 .project (copy the .project[1,2] file that is different from the existing .project file) 4) Open foo 5) Concurrent modification exception emitted I see the following concurrent modification exception which stems from this backtrace -- here AliasManager.aliases is being modified while being iterated over: AliasManager$AddToCollectionDoit.doit(IResource) line: 56 <-- modified AliasManager$LocationMap.overLappingResourcesDo(AliasManager$Doit) line: 237 AliasManager.buildAliasedProjectsSet() line: 380 AliasManager.updateStructureChanges() line: 711 AliasManager.hasNoAliases(IResource) line: 578 AliasManager.computeAliases(IResource, IFileStore) line: 425 RefreshLocalAliasVisitor.createResource(UnifiedTreeNode, Resource) line: 34 RefreshLocalAliasVisitor(RefreshLocalVisitor).synchronizeExistence(UnifiedTreeNode, Resource) line: 216 RefreshLocalAliasVisitor(RefreshLocalVisitor).visit(UnifiedTreeNode) line: 290 UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 FileSystemResourceManager.refreshResource(IResource, int, boolean, IProgressMonitor) line: 788 FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) line: 772 Folder(Resource).refreshLocal(int, IProgressMonitor) line: 1594 Folder(Resource).createLink(URI, int, IProgressMonitor) line: 695 Project.reconcileLinksAndGroups(ProjectDescription) line: 1035 Project.updateDescription() line: 1187 File.updateMetadataFiles() line: 411 RefreshLocalVisitor.visit(UnifiedTreeNode) line: 306 UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 FileSystemResourceManager.refreshResource(IResource, int, boolean, IProgressMonitor) line: 788 FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) line: 772 AliasManager.updateAliases(IResource, IFileStore, int, IProgressMonitor) line: 682 <-- Iteration over aliases Project.open(int, IProgressMonitor) line: 906 Project.open(IProgressMonitor) line: 922 OpenResourceAction.invokeOperation(IResource, IProgressMonitor) line: 153 OpenResourceAction(WorkspaceAction).execute(List, IProgressMonitor) line: 162 WorkspaceAction$2.runInWorkspace(IProgressMonitor) line: 483 WorkspaceAction$2(InternalWorkspaceJob).run(IProgressMonitor) line: 38 Worker.run() line: 55 I also noticed that you added synchronization to AliasManager.aliases. However in computeDeepAliases you do this: synchronized(aliases) { addToCollection.setCollection(aliases); } which puts a reference to the set in AddToCollectionDoIt. Access to the map through this reference doens't appear to be synchronized. It would be great if you could explain in what instances Alias Manager is accessed concurrently from multiple threads.
Thanks for the information. I can confirm that opening that workspace is problematic even for the stock 3.4 Eclipse, where I can the following exception only attempting to open the workspace: java.util.ConcurrentModificationException at java.util.HashMap$HashIterator.nextEntry(HashMap.java:841) at java.util.HashMap$KeyIterator.next(HashMap.java:877) at org.eclipse.core.internal.resources.AliasManager.updateAliases(AliasManager.java:675) at org.eclipse.core.internal.resources.Project.open(Project.java:906) at org.eclipse.core.internal.resources.Project.open(Project.java:922) at org.eclipse.ui.actions.OpenResourceAction.invokeOperation(OpenResourceAction.java:153) at org.eclipse.ui.actions.WorkspaceAction.execute(WorkspaceAction.java:162) at org.eclipse.ui.actions.WorkspaceAction$2.runInWorkspace(WorkspaceAction.java:483) at org.eclipse.core.internal.resources.InternalWorkspaceJob.run(InternalWorkspaceJob.java:38) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55) When I reproduce the steps you provided, I get the same exception as above (all with stock 3.4 release, without the patch included in this bug) We've encountered those kind of issues as well in the past, which is why we added the synchronization in the AliasManager. From what I can see, the concurrency exception revealed in your test case is caused by accessing "aliasedProjects", but I added more synchronized scopes to cover both variables in the patch attached to the bug. (In reply to comment #28) > Created an attachment (id=115486) [details] > Workspace demonstrating the problem > > (In reply to comment #27) > > I was investigating this bug, and I can't reproduce the problem. Can you > > attach a small sample of the two projects (nested and container) and give > > reproducible steps to get the Exception? > > Attached is a workspace which has the problem (or a closely related problem). > I've been so far unable to create a unit test which exhibits the problem. > > The workspace has the outer project hello, and inner project foo. foo contains > a symlink to bar in hello. There are three .project files in /hello/foo -- > .project .project1 .project2 > > To reproduce: > 1) Open the workspace in your Eclipse > 2) Close the project foo > 3) From a shell copy .project2/.project1 .project (copy the .project[1,2] file > that is different from the existing .project file) > 4) Open foo > 5) Concurrent modification exception emitted > > I see the following concurrent modification exception which stems from this > backtrace -- here AliasManager.aliases is being modified while being iterated > over: > AliasManager$AddToCollectionDoit.doit(IResource) line: 56 <-- modified > AliasManager$LocationMap.overLappingResourcesDo(AliasManager$Doit) line: 237 > AliasManager.buildAliasedProjectsSet() line: 380 > AliasManager.updateStructureChanges() line: 711 > AliasManager.hasNoAliases(IResource) line: 578 > AliasManager.computeAliases(IResource, IFileStore) line: 425 > RefreshLocalAliasVisitor.createResource(UnifiedTreeNode, Resource) line: 34 > RefreshLocalAliasVisitor(RefreshLocalVisitor).synchronizeExistence(UnifiedTreeNode, > Resource) line: 216 > RefreshLocalAliasVisitor(RefreshLocalVisitor).visit(UnifiedTreeNode) line: 290 > UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 > FileSystemResourceManager.refreshResource(IResource, int, boolean, > IProgressMonitor) line: 788 > FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) > line: 772 > Folder(Resource).refreshLocal(int, IProgressMonitor) line: 1594 > Folder(Resource).createLink(URI, int, IProgressMonitor) line: 695 > Project.reconcileLinksAndGroups(ProjectDescription) line: 1035 > Project.updateDescription() line: 1187 > File.updateMetadataFiles() line: 411 > RefreshLocalVisitor.visit(UnifiedTreeNode) line: 306 > UnifiedTree.accept(IUnifiedTreeVisitor, int) line: 99 > FileSystemResourceManager.refreshResource(IResource, int, boolean, > IProgressMonitor) line: 788 > FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) > line: 772 > AliasManager.updateAliases(IResource, IFileStore, int, IProgressMonitor) line: > 682 <-- Iteration over aliases > Project.open(int, IProgressMonitor) line: 906 > Project.open(IProgressMonitor) line: 922 > OpenResourceAction.invokeOperation(IResource, IProgressMonitor) line: 153 > OpenResourceAction(WorkspaceAction).execute(List, IProgressMonitor) line: 162 > WorkspaceAction$2.runInWorkspace(IProgressMonitor) line: 483 > WorkspaceAction$2(InternalWorkspaceJob).run(IProgressMonitor) line: 38 > Worker.run() line: 55 > > I also noticed that you added synchronization to AliasManager.aliases. However > in computeDeepAliases you do this: > synchronized(aliases) { > addToCollection.setCollection(aliases); > } > which puts a reference to the set in AddToCollectionDoIt. Access to the map > through this reference doens't appear to be synchronized. > > It would be great if you could explain in what instances Alias Manager is > accessed concurrently from multiple threads. >
Created attachment 115544 [details] A patch for the AliasManager that fixes concurrency exceptions. Note that this applies to stock 3.4 as well, not only the changes suggested by this bug.
By the way, although I'm able to confirm the patch fixes the exceptions on MacOSX for both opening the workspace and closing/opening the project "foo", on Windows, it fixes the exception only for opening the workspace. Opening "foo" fails on windows with the following exception: The project description file (.project) for 'foo' is missing. This file contains important information about the project. The project will not function properly until this file is restored. Without any stack trace. Really seems like nesting projects inside other projects isn't well supported in general.
(In reply to comment #29) Thanks for the confirmation Serge. > I can confirm that opening that workspace is problematic even for the stock 3.4 > Eclipse, where I can the following exception only attempting to open the > workspace: > > java.util.ConcurrentModificationException > at java.util.HashMap$HashIterator.nextEntry(HashMap.java:841) > at java.util.HashMap$KeyIterator.next(HashMap.java:877) > at > org.eclipse.core.internal.resources.AliasManager.updateAliases(AliasManager.java:675) > at org.eclipse.core.internal.resources.Project.open(Project.java:906) > at org.eclipse.core.internal.resources.Project.open(Project.java:922) > at I believe this is the same modification as in the stack I gave. In this case the map isn't being modified by two separate threads. What I was trying to show in that stack was that the map is modified by the refresh call in updateAliases: > AliasManager$AddToCollectionDoit.doit(IResource) line: 56 <-- modified > AliasManager$LocationMap.overLappingResourcesDo(AliasManager$Doit) line: 237 ... > FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) > line: 772 > AliasManager.updateAliases(IResource, IFileStore, int, IProgressMonitor) line: > 682 <-- Iteration over aliases So in this case additional synchronization isn't going to help as the map really is changing, in the same thread, under the iterator's feet. Given that you've seen this in stock 3.4 I've created a separate bug 251370. > We've encountered those kind of issues as well in the past, which is why we > added the synchronization in the AliasManager. Ok. I may remove the synchronization and see if I can reproduce genuine occurences of the map being changed concurrently from different threads (I have ConcurrentModificationException event set to suspend all threads...). So that we can at least document the synchronization used in the AliasManager class (I thought all of this was prevented by the use of the WorkManager lock...).
(In reply to comment #30) > Created an attachment (id=115544) [details] > A patch for the AliasManager that fixes concurrency exceptions. Note that this > applies to stock 3.4 as well, not only the changes suggested by this bug. If you set a breakpoint on ConcurrentModificationException => Suspend VM are you really seeing multi-thread concurrent modification?
(In reply to comment #32) > (In reply to comment #29) > > Thanks for the confirmation Serge. > > > > I can confirm that opening that workspace is problematic even for the stock 3.4 > > Eclipse, where I can the following exception only attempting to open the > > workspace: > > > > java.util.ConcurrentModificationException > > at java.util.HashMap$HashIterator.nextEntry(HashMap.java:841) > > at java.util.HashMap$KeyIterator.next(HashMap.java:877) > > at > > org.eclipse.core.internal.resources.AliasManager.updateAliases(AliasManager.java:675) > > at org.eclipse.core.internal.resources.Project.open(Project.java:906) > > at org.eclipse.core.internal.resources.Project.open(Project.java:922) > > at > > I believe this is the same modification as in the stack I gave. In this case > the map isn't being modified by two separate threads. What I was trying to show > in that stack was that the map is modified by the refresh call in > updateAliases: > > > AliasManager$AddToCollectionDoit.doit(IResource) line: 56 <-- modified > > AliasManager$LocationMap.overLappingResourcesDo(AliasManager$Doit) line: 237 > ... > > FileSystemResourceManager.refresh(IResource, int, boolean, IProgressMonitor) > > line: 772 > > AliasManager.updateAliases(IResource, IFileStore, int, IProgressMonitor) line: > > 682 <-- Iteration over aliases > > So in this case additional synchronization isn't going to help as the map > really is changing, in the same thread, under the iterator's feet. > I see what your saying, something strange though is that the variable change in the following code: AliasManager$AddToCollectionDoit.doit(IResource) line: 56 <-- modified AliasManager$LocationMap.overLappingResourcesDo(AliasManager$Doit) line: 237 AliasManager.buildAliasedProjectsSet() line: 380 is "aliasedProjects", not "aliases", which is iterated over by line 682. Can you confirm that the "synchronized" block changes fixes the issue for you too? Otherwise, a fix would consist of simply changing the two following lines in updateAliases(): FileSystemResourceManager localManager = workspace.getFileSystemManager(); for (Iterator it = aliases.iterator(); it.hasNext();) { by: FileSystemResourceManager localManager = workspace.getFileSystemManager(); HashSet aliasesCopy = (HashSet) aliases.clone(); for (Iterator it = aliasesCopy.iterator(); it.hasNext();) {
(In reply to comment #34) > HashSet aliasesCopy = (HashSet) aliases.clone(); > for (Iterator it = aliasesCopy.iterator(); it.hasNext();) { This seems like a more correct fix to me, but why do you clone into a HashSet? Since you're iterating over all elements anyways, I'd think that you could also add all elements into a List or Array for iterating: List aliasesCopy = new ArrayList(aliases); I'd think that this should be slightly more efficient since no re-hashing needs to take place.
... and I'd think that we should file a separate bug for the ConcurrentModificationException in the AliasManager.
(In reply to comment #33) > (In reply to comment #30) > > Created an attachment (id=115544) [details] [details] > > A patch for the AliasManager that fixes concurrency exceptions. Note that this > > applies to stock 3.4 as well, not only the changes suggested by this bug. > > If you set a breakpoint on ConcurrentModificationException => Suspend VM are > you really seeing multi-thread concurrent modification? > No, I'm not able to consistently reproduce the exception without the synchronized blocks.
(In reply to comment #35) > (In reply to comment #34) > > HashSet aliasesCopy = (HashSet) aliases.clone(); > > for (Iterator it = aliasesCopy.iterator(); it.hasNext();) { > > This seems like a more correct fix to me, but why do you clone into a HashSet? > Since you're iterating over all elements anyways, I'd think that you could also > add all elements into a List or Array for iterating: > > List aliasesCopy = new ArrayList(aliases); > > I'd think that this should be slightly more efficient since no re-hashing needs > to take place. > To be honest, I'm not sure which is more efficient, since the HashSet.clone() copies the underlying structures, without going though every item insertion and calculating their hashCode.
(In reply to comment #36) > ... and I'd think that we should file a separate bug for the > ConcurrentModificationException in the AliasManager. > I've filed bug251370 for this -- given that the problem is orthogonal to this feature, I suggest discussion move there :)
(In reply to comment #27) > I was investigating this bug, and I can't reproduce the problem. Can you > attach a small sample of the two projects (nested and container) and give > reproducible steps to get the Exception? > > It seems to work properly for me. So with your patch for the concurrent modification exception (attached bug251370) that problem has gone away. However the original problem shown in the exceptions attached here persists. I close the inner project, update the .project file from CVS. Reopen the inner .project and I get the same exception (mentioning a different folder resources) and most worryingly, the inner project's .project file is missing 3-10% of the link's when compared with the version in CVS. If I rinse and repeat, I get the same error except I lose different links each time. The missing link which produces the IllegalArgumentException is always a folder level link. The others are file level links (which live in different directories there doens't seem to be any pattern) -- in all cases the destination of the link exists in the workspace. What may be a factor is that my inner project is made of 85 linked resources which makes for a 17k .project file (apart from this linked resource loss, this really isn't going to scale to thousands of linked resources).
Created attachment 115724 [details] CVS patch files for the independent path and group features. Fixes a bug in CreateLinkedResourceGroup.java that failed to construct an URI properly from an IPath.
Created attachment 116049 [details] Project.reconcileLinksAndGroups compare link URI with raw loc This patch (appears to) fix the exception I had previously posted. It fixes the link delta check to compare raw resource uri rather than comparing resource location.
Created attachment 116054 [details] CVS patch files for the independent path and group features (including Jame's fix)
(In reply to comment #42) > Created an attachment (id=116049) [details] > Project.reconcileLinksAndGroups compare link URI with raw loc > > This patch (appears to) fix the exception I had previously posted. It fixes > the link delta check to compare raw resource uri rather than comparing resource > location. > That's a good catch, thanks James! I included your fix in the latest patch attachment.
Serge (and James), the patch spans at least the Platform Resources, and Platform IDE/UI components. It would be good to separate the patch into pieces that align with component boundaries (with appropriate depends-on information in Bugzilla) so that we can start reviewing those parts individually. (I hope this makes sense to you too.)
I notice that JDT now supports build paths relative to the project: http://download.eclipse.org/eclipse/downloads/drops/S-3.5M3-200810301917/eclipse-news-M3.html Build path supports ".." Could the way this is implemented somehow relate to the patches proposed here?
(In reply to comment #46) > I notice that JDT now supports build paths relative to the project: > > http://download.eclipse.org/eclipse/downloads/drops/S-3.5M3-200810301917/eclipse-news-M3.html > Build path supports ".." > > Could the way this is implemented somehow relate to the patches proposed here? > Yes, using this patch, a project relative linked resource could be added to the project pointing to a .jar file (equivalent of the "../external/test.jar" path). In this way, assuming the JDT would supports .jar files as normal linked resources, the user could simply drag and drop .jar files in the project to have them included in the classpath.
(In reply to comment #45) > Serge (and James), the patch spans at least the Platform Resources, and > Platform IDE/UI components. It would be good to separate the patch into pieces > that align with component boundaries (with appropriate depends-on information > in Bugzilla) so that we can start reviewing those parts individually. (I hope > this makes sense to you too.) > I'll work on spliting the patch by components and get back to you shortly about it.
*** Bug 259606 has been marked as a duplicate of this bug. ***
This stuff's already in e4, does the reassignment mean that it won't make it back into 3.x?
This was implemented by Serge and released for the e4 0.9 release. We'll open a separate bug for back-porting this work to the 3.x stream.
bug 284148 has been entered to explore back-porting the e4 resources incubator work into the Eclipse platform 3.x development stream.
